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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.07.2020 14:45, Hongyan Xia wrote:
> On Tue, 2020-07-14 at 14:42 +0200, Jan Beulich wrote:
> > On 29.05.2020 13:11, Hongyan Xia wrote:
> > > @@ -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.
I'm not thinking about squashing anything into an if(). I've talked
about avoiding the left shift of mfn, as this last quoted line does
(by instead subtracting PAGE_SHIFT from the left-shift count.

Jan

Jan