Re: [PATCH v7 09/15] efi: use new page table APIs in copy_mapping

[Hongyan Xia <hx242@xxxxxxx>]

On Mon, 2020-07-27 at 13:45 +0100, Hongyan Xia wrote:
> On Tue, 2020-07-14 at 14:42 +0200, Jan Beulich wrote:
> > On 29.05.2020 13:11, Hongyan Xia wrote:
> > > From: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > 
> > > After inspection ARM doesn't have alloc_xen_pagetable so this
> > > function
> > > is x86 only, which means it is safe for us to change.
> > 
> > Well, it sits inside a "#ifndef CONFIG_ARM" section.
> > 
> > > @@ -1442,29 +1443,42 @@ static __init void copy_mapping(unsigned
> > > long mfn, unsigned long end,
> > >                                                   unsigned long
> > > emfn))
> > >  {
> > >      unsigned long next;
> > > +    l3_pgentry_t *l3src = NULL, *l3dst = NULL;
> > >  
> > >      for ( ; mfn < end; mfn = next )
> > >      {
> > >          l4_pgentry_t l4e = efi_l4_pgtable[l4_table_offset(mfn <<
> > > PAGE_SHIFT)];
> > > -        l3_pgentry_t *l3src, *l3dst;
> > >          unsigned long va = (unsigned long)mfn_to_virt(mfn);
> > >  
> > > +        if ( !((mfn << PAGE_SHIFT) & ((1UL <<
> > > L4_PAGETABLE_SHIFT)
> > > - 1)) )
> > 
> > To be in line with ...
> > 
> > > +        {
> > > +            UNMAP_DOMAIN_PAGE(l3src);
> > > +            UNMAP_DOMAIN_PAGE(l3dst);
> > > +        }
> > >          next = mfn + (1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT));
> > 
> > ... this, please avoid the left shift of mfn in the if().
> > Judgingfrom
> 
> What do you mean by "in line" here? It does not look to me that "next
> =" can be easily squashed into the if() condition.

Sorry, never mind. "in line" != "inline".

Hongyan