On Sat, Apr 22, 2023 at 08:32:02PM +0000, Oliver Upton wrote:
> On Sat, Apr 22, 2023 at 01:09:26PM -0700, Ricardo Koller wrote:
> > On Mon, Apr 17, 2023 at 02:18:26PM +0800, Gavin Shan wrote:
> > > > + /* .addr (the IPA) is irrelevant for an unlinked table */
> > > > + struct kvm_pgtable_walk_data data = {
> > > > + .walker = &walker,
> > > > + .addr = 0,
> > > > + .end = kvm_granule_size(level),
> > > > + };
> > >
> > > The comment about '.addr' seems incorrect. The IPA address is still
> > > used to locate the page table entry, so I think it would be something
> > > like below:
> > >
> > > /* The IPA address (.addr) is relative to zero */
> > >
> >
> > Extended it to say this:
> >
> > * The IPA address (.addr) is relative to zero. The goal is to
> > * map "kvm_granule_size(level) - 0" worth of pages.
>
> I actually prefer the original wording, as Gavin's suggestion makes this
> comment read as though the IPA of the walk bears some degree of
> validity, which it does not.
>
> The intent of the code is to create some *ambiguous* input address
> range, so maybe:
>
> /*
> * The input address (.addr) is irrelevant for walking an
> * unlinked table. Construct an ambiguous IA range to map
> * kvm_granule_size(level) worth of memory.
> */
>
OK, this is the winner. Will go with this one in v8. Gavin, let me know
if you are not OK with this.
Thank you both,
Ricardo
> --
> Thanks,
> Oliver
