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 11:27:56AM +0100, Wei Liu wrote:
> On Fri, Sep 14, 2018 at 01:16:11PM +0200, Roger Pau Monne wrote:
> > Identity mapping RAM regions on the low 1MB for Dom0 is not ideal,
> > since there's data there that could be used by Xen during runtime
> > (like the AP trampoline), so instead of identity mapping the low 1MB
> > into the Dom0 physmap populate those RAM regions and copy the data.
> 
> I assume you encountered some real issues or is it just precaution?

It's simpler, and allowing Dom0 to modify the Xen AP trampoline code
is not a good idea.

> > 
> > Note that this allows to remove unshare_xen_page_with_guest since the
> > only caller was the PVH Dom0 builder.
> > 
> > Signed-off-by: Roger Pau Monné <rogewr.pau@xxxxxxxxxx>
> > ---
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> > ---
> >  xen/arch/x86/hvm/dom0_build.c | 51 +++++++++++------------------------
> >  xen/arch/x86/mm.c             | 16 -----------
> >  xen/include/xen/mm.h          |  1 -
> >  3 files changed, 16 insertions(+), 52 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> > index 5724883d8c..ad4a074391 100644
> > --- a/xen/arch/x86/hvm/dom0_build.c
> > +++ b/xen/arch/x86/hvm/dom0_build.c
> > @@ -278,33 +278,6 @@ static int __init 
> > pvh_setup_vmx_realmode_helpers(struct domain *d)
> >      return 0;
> >  }
> >  
> > -/* Assign the low 1MB to Dom0. */
> > -static void __init pvh_steal_low_ram(struct domain *d, unsigned long start,
> > -                                     unsigned long nr_pages)
> > -{
> > -    unsigned long mfn;
> > -
> > -    ASSERT(start + nr_pages <= PFN_DOWN(MB(1)));
> > -
> > -    for ( mfn = start; mfn < start + nr_pages; mfn++ )
> > -    {
> > -        struct page_info *pg = mfn_to_page(_mfn(mfn));
> > -        int rc;
> > -
> > -        rc = unshare_xen_page_with_guest(pg, dom_io);
> > -        if ( rc )
> > -        {
> > -            printk("Unable to unshare Xen mfn %#lx: %d\n", mfn, rc);
> > -            continue;
> > -        }
> > -
> > -        share_xen_page_with_guest(pg, d, SHARE_rw);
> > -        rc = guest_physmap_add_entry(d, _gfn(mfn), _mfn(mfn), 0, 
> > p2m_ram_rw);
> > -        if ( rc )
> > -            printk("Unable to add mfn %#lx to p2m: %d\n", mfn, rc);
> > -    }
> > -}
> > -
> >  static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
> >  {
> >      struct e820entry *entry, *entry_guest;
> > @@ -420,16 +393,24 @@ static int __init pvh_setup_p2m(struct domain *d)
> 
> 
> Somewhere above the hunk you modified, there is a comment saying "Memory
> below 1MB is identity mapped". Don't you need to change that as well?
> Otherwise first 1MB still 1:1 maps to 1MB machine memory in guest p2m.

Non-RAM regions are still identity mapped.

Thanks, Roger.

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