Re: [Xen-devel] [PATCH v4 25/31] xen: allow HVM guests to use XENMEM_memory_map

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Aug 07, 2015 at 05:44:22PM +0200, Roger Pau Monné wrote:
> El 07/08/15 a les 14.22, Wei Liu ha escrit:
> > On Fri, Aug 07, 2015 at 12:18:02PM +0200, Roger Pau Monne wrote:
> >> Enable this hypercall for HVM guests in order to fetch the e820 memory
> >> map in the absence of an emulated BIOS. The memory map is populated and
> >> notified to Xen in arch_setup_meminit_hvm.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> >> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> >> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> >> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> >> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> ---
> >>  tools/libxc/xc_dom_x86.c | 29 ++++++++++++++++++++++++++++-
> >>  1 file changed, 28 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> >> index b587b12..87bce6e 100644
> >> --- a/tools/libxc/xc_dom_x86.c
> >> +++ b/tools/libxc/xc_dom_x86.c
> >> @@ -1205,6 +1205,7 @@ static int check_mmio_hole(uint64_t start, uint64_t 
> >> memsize,
> >>          return 1;
> >>  }
> >>  
> >> +#define MAX_E820_ENTRIES    128
> >>  static int meminit_hvm(struct xc_dom_image *dom)
> >>  {
> >>      unsigned long i, vmemid, nr_pages = dom->total_pages;
> >> @@ -1225,6 +1226,8 @@ static int meminit_hvm(struct xc_dom_image *dom)
> >>      unsigned int nr_vmemranges, nr_vnodes;
> >>      xc_interface *xch = dom->xch;
> >>      uint32_t domid = dom->guest_domid;
> >> +    struct e820entry entries[MAX_E820_ENTRIES];
> >> +    int e820_index = 0;
> >>  
> >>      if ( nr_pages > target_pages )
> >>          memflags |= XENMEMF_populate_on_demand;
> >> @@ -1275,6 +1278,13 @@ static int meminit_hvm(struct xc_dom_image *dom)
> >>          vnode_to_pnode = dom->vnode_to_pnode;
> >>      }
> >>  
> >> +    /* Add one additional memory range to account for the VGA hole */
> >> +    if ( (nr_vmemranges + (dom->emulation ? 1 : 0)) > MAX_E820_ENTRIES )
> >> +    {
> >> +        DOMPRINTF("Too many memory ranges");
> >> +        goto error_out;
> >> +    }
> >> +
> >>      total_pages = 0;
> >>      p2m_size = 0;
> >>      for ( i = 0; i < nr_vmemranges; i++ )
> >> @@ -1363,9 +1373,13 @@ static int meminit_hvm(struct xc_dom_image *dom)
> >>       * Under 2MB mode, we allocate pages in batches of no more than 8MB 
> >> to 
> >>       * ensure that we can be preempted and hence dom0 remains responsive.
> >>       */
> >> -    if ( dom->emulation )
> >> +    if ( dom->emulation ) {
> >>          rc = xc_domain_populate_physmap_exact(
> >>              xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);
> >> +        entries[e820_index].addr = 0;
> >> +        entries[e820_index].size = 0xa0 << PAGE_SHIFT;
> >> +        entries[e820_index++].type = E820_RAM;
> >> +    }
> >>  
> >>      stat_normal_pages = 0;
> >>      for ( vmemid = 0; vmemid < nr_vmemranges; vmemid++ )
> >> @@ -1392,6 +1406,12 @@ static int meminit_hvm(struct xc_dom_image *dom)
> >>          else
> >>              cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
> >>  
> >> +        /* Build an e820 map. */
> >> +        entries[e820_index].addr = cur_pages << PAGE_SHIFT;
> >> +        entries[e820_index].size = vmemranges[vmemid].end -
> >> +                                   entries[e820_index].addr;
> >> +        entries[e820_index++].type = E820_RAM;
> >> +
> >>          while ( (rc == 0) && (end_pages > cur_pages) )
> >>          {
> >>              /* Clip count to maximum 1GB extent. */
> >> @@ -1509,6 +1529,13 @@ static int meminit_hvm(struct xc_dom_image *dom)
> >>      DPRINTF("  2MB PAGES: 0x%016lx\n", stat_2mb_pages);
> >>      DPRINTF("  1GB PAGES: 0x%016lx\n", stat_1gb_pages);
> >>  
> >> +    rc = xc_domain_set_memory_map(xch, domid, entries, e820_index);
> >> +    if ( rc != 0 )
> >> +    {
> >> +        DOMPRINTF("unable to set memory map.");
> >> +        goto error_out;
> >> +    }
> >> +
> > 
> > I think in RDM series there is already a memory map generated in libxl.
> > 
> > You might want to move this to libxl, which is supposed to be the
> > arbitrator of what a guest should look like.
> > 
> > What do you think?
> 
> I would like to do that, the only problem I see is that libxl doesn't
> have any notion of the "special pages", so it doesn't know the size of
> the hole it has to made. I guess one solution would be moving the
> defines at the top of xc_dom_x86.c to a public libxc header so libxl can
> pick up the size and position of the hole.

But the code above has no notion of special pages either. Can it not
just use the E820 map generated in libxl? I think at the time you wrote
this patch libxl doesn't generate E820 for HVM guest, but it does now,
so maybe you can drop this patch?

BTW the title is not very accurate because this patch doesn't involve
allowing / disallowing hvm to use that hypercall.

Wei.

> 
> Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel