Re: [Xen-devel] [PATCH 06/22] xen/arm: p2m: Use the typesafe MFN in mfn_to_p2m_entry

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 27/07/2016 19:25, Stefano Stabellini wrote:
> On Wed, 27 Jul 2016, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 26/07/16 23:28, Stefano Stabellini wrote:
> > > On Wed, 20 Jul 2016, Julien Grall wrote:
> > > > @@ -411,7 +411,7 @@ static int p2m_create_table(struct domain *d, lpae_t
> > > > *entry,
> > > >      if ( splitting )
> > > >      {
> > > >          p2m_type_t t = entry->p2m.type;
> > > > -        unsigned long base_pfn = entry->p2m.base;
> > > > +        mfn_t mfn = _mfn(entry->p2m.base);
> > > >          int i;
> > > >
> > > >          /*
> > > > @@ -420,8 +420,9 @@ static int p2m_create_table(struct domain *d, lpae_t
> > > > *entry,
> > > >           */
> > > >           for ( i=0 ; i < LPAE_ENTRIES; i++ )
> > > >           {
> > > > -             pte = mfn_to_p2m_entry(base_pfn +
> > > > (i<<(level_shift-LPAE_SHIFT)),
> > > > -                                    MATTR_MEM, t, p2m->default_access);
> > > > +             pte = mfn_to_p2m_entry(mfn, MATTR_MEM, t,
> > > > p2m->default_access);
> > > > +
> > > > +             mfn = mfn_add(mfn, 1UL << (level_shift - LPAE_SHIFT));
> > > Should we be incrementing mfn before calling mfn_to_p2m_entry?
> > No. The base of the superpage is mfn, after splitting the first entry will be
> > equal to the base, the second entry base + level_size...
> I understand what the patch is doing now, I confused "1" with "i" :-)
> The patch is OK. It might be more obvious as the following:
>
>
>   for ( i=0 ; i < LPAE_ENTRIES; i++ )
>   {
>      pte = mfn_to_p2m_entry(mfn_add(mfn, (i<<(level_shift-LPAE_SHIFT))),
>                             MATTR_MEM, t, p2m->default_access);
>
>
> However it's just a matter of taste, so I'll let you choose the way you
> prefer.
I wanted to avoid shifting "i" at each loop (which should save an 
instruction). However, as it seems to be confusing, I will use your 
suggestion.
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Thank you!

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel