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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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?

> 
> 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.

Wei.

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