Re: [Xen-devel] [PATCH] x86/pvh: copy data from low 1MB to Dom0 physmap instead of mapping it

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Sep 17, 2018 at 07:03:27AM -0600, Jan Beulich wrote:
> >>> On 14.09.18 at 13:16, <roger.pau@xxxxxxxxxx> wrote:
> > @@ -420,16 +393,24 @@ static int __init pvh_setup_p2m(struct domain *d)
> >          addr = PFN_DOWN(d->arch.e820[i].addr);
> >          size = PFN_DOWN(d->arch.e820[i].size);
> >  
> > -        if ( addr >= MB1_PAGES )
> > -            rc = pvh_populate_memory_range(d, addr, size);
> > -        else
> > -        {
> > -            ASSERT(addr + size < MB1_PAGES);
> > -            pvh_steal_low_ram(d, addr, size);
> > -        }
> > -
> > +        rc = pvh_populate_memory_range(d, addr, size);
> >          if ( rc )
> >              return rc;
> > +
> > +        if ( addr < MB1_PAGES )
> > +        {
> > +             enum hvm_translation_result res =
> > +                 hvm_copy_to_guest_phys(mfn_to_maddr(_mfn(addr)),
> > +                                        mfn_to_virt(addr), size << 
> > PAGE_SHIFT,
> > +                                        v);
> > +
> > +            if ( res != HVMTRANS_okay )
> > +            {
> > +                printk("Failed to copy [%#lx, %#lx): %d\n",
> > +                       addr, addr + size, res);
> > +                return -EFAULT;
> > +            }
> > +        }
> >      }
> 
> Is there any guarantee (in particular on, but not limited to EFI systems)
> for E820_RAM regions to never span the 1Mb boundary? If not, you
> may end up copying memory above 1Mb here.

Right, I guess I could do something like:

end = min(MB(1), d->arch.e820[i].addr + d->arch.e820[i].size);

And calculate the size based on the 'end' value.

> Furthermore, what about RAM / non-RAM boundaries in the middle of
> a page (which is quite common a situation for the first Mb)?

There are no such RAM ranges in the guest memory map because
pvh_setup_e820 aligns the RAM regions start/end to page boundaries.
This is not ideal, so if you want I can do the following:

hvm_copy_to_guest_phys(d->arch.e820[i].addr, d->arch.e820[i].size, v);

And if pvh_setup_e820 is improved so that RAM regions are no longer
aligned to page boundaries the copy will work without issues.

> I also wonder whether it wouldn't be worthwhile to avoid calling
> modify_identity_mmio() for RAM ranges (which are now going to be
> re-mapped anyway).

I think it's easier (code-wise) to identity map the whole area and
then just populate the RAM regions as needed.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel