Re: Claim mode and HVM PoD interact badly

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Jan 20, 2014 at 04:29:43PM +0000, Wei Liu wrote:
> On Fri, Jan 10, 2014 at 09:58:07AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jan 10, 2014 at 11:59:42AM +0000, Ian Campbell wrote:
> > > create ^
> > > owner Wei Liu <wei.liu2@citrix.com>
> > > thanks
> > > 
> > > On Fri, 2014-01-10 at 11:56 +0000, Wei Liu wrote:
> > > > When I have following configuration in HVM config file:
> > > >   memory=128
> > > >   maxmem=256
> > > > and have claim_mode=1 in /etc/xen/xl.conf, xl create fails with
> > > > 
> > > > xc: error: Could not allocate memory for HVM guest as we cannot claim memory! (22 = Invalid argument): Internal error
> > > > libxl: error: libxl_dom.c:647:libxl__build_hvm: hvm building failed
> > > > libxl: error: libxl_create.c:1000:domcreate_rebuild_done: cannot (re-)build domain: -3
> > > > libxl: error: libxl_dm.c:1467:kill_device_model: unable to find device model pid in /local/domain/82/image/device-model-pid
> > > > libxl: error: libxl.c:1425:libxl__destroy_domid: libxl__destroy_device_model failed for 82
> > > > 
> > > > With claim_mode=0, I can sucessfuly create HVM guest.
> > > 
> > > Is it trying to claim 256M instead of 128M? (although the likelyhood
> > 
> > No. 128MB actually.
> > 
> > > that you only have 128-255M free is quite low, or are you
> > > autoballooning?)
> > 
> > This patch fixes it for me. It basically sets the amount of pages
> > claimed to be 'maxmem' instead of 'memory' for PoD.
> > 
> > I don't know PoD very well, and this claim is only valid during the
> > allocation of the guests memory - so the 'target_pages' value might be
> > the wrong one. However looking at the hypervisor's
> > 'p2m_pod_set_mem_target' I see this comment:
> > 
> >  316  *     B <T': Set the PoD cache size equal to the number of outstanding PoD
> >  317  *   entries.  The balloon driver will deflate the balloon to give back
> >  318  *   the remainder of the ram to the guest OS.
> > 
> > Which implies to me that we _need_ the 'maxmem' amount of memory at boot time.
> > And then it is the responsibility of the balloon driver to give the memory
> > back (and this is where the 'static-max' et al come in play to tell the
> > balloon driver to balloon out).
> > 
> > 
> > diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
> > index 77bd365..65e9577 100644
> > --- a/tools/libxc/xc_hvm_build_x86.c
> > +++ b/tools/libxc/xc_hvm_build_x86.c
> > @@ -335,7 +335,12 @@ static int setup_guest(xc_interface *xch,
> >  
> >      /* try to claim pages for early warning of insufficient memory available */
> >      if ( claim_enabled ) {
> > -        rc = xc_domain_claim_pages(xch, dom, nr_pages - cur_pages);
> > +        unsigned long nr = nr_pages - cur_pages;
> > +
> > +        if ( pod_mode )
> > +            nr = target_pages - 0x20;
> > +
> 
> I'm a bit confused, did this work for you? At this point d->tot_pages
> should be (target_pages - 0x20). However in the hypervisor logic if you
> try to claim the exact amount of pages as d->tot_pages it should return
> EINVAL.
> 
> Furthur more, the original logic doesn't look right. In PV guest
> creation, xc tries to claim "memory=" pages, while in HVM guest creation
> it tries to claim "maxmem=" pages. I think HVM code is wrong.
> 
> And George shed me some light on PoD this morning, "cache" in PoD should
> be the pool of pages that used to populate into guest physical memory.
> In that sense it should be the size of mem_target ("memory=").
> 
> So I come up with a fix like this. Any idea?

The patch I came up didn't work. It did work the first time but I think
that is because I had 'claim=0' in the xl.conf and forgot about it (sigh).

After a bit of rebooting I realized that the patch didn't do the right
job. The one way it _did_ work was to claim the amount of pages
that the guest had allocated at this moment. That is for the PoD
path the 'xc_domain_set_pod_target' (which is called before the
claim call) ends up allocating memory.

So the claim 'clamp' was done past that moment (which is not good).

I will try out your patch later this week and report. Thanks!

> 
> Wei.
> 
> ---8<---
> diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
> index 77bd365..472f1df 100644
> --- a/tools/libxc/xc_hvm_build_x86.c
> +++ b/tools/libxc/xc_hvm_build_x86.c
> @@ -49,6 +49,8 @@
>  #define NR_SPECIAL_PAGES     8
>  #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
>  
> +#define POD_VGA_HOLE_SIZE (0x20)
> +
>  static int modules_init(struct xc_hvm_build_args *args,
>                          uint64_t vend, struct elf_binary *elf,
>                          uint64_t *mstart_out, uint64_t *mend_out)
> @@ -305,11 +307,13 @@ static int setup_guest(xc_interface *xch,
>      if ( pod_mode )
>      {
>          /*
> -         * Subtract 0x20 from target_pages for the VGA "hole".  Xen will
> -         * adjust the PoD cache size so that domain tot_pages will be
> -         * target_pages - 0x20 after this call.
> +         * Subtract POD_VGA_HOLE_SIZE from target_pages for the VGA
> +         * "hole".  Xen will adjust the PoD cache size so that domain
> +         * tot_pages will be target_pages - POD_VGA_HOLE_SIZE after
> +         * this call.
>           */
> -        rc = xc_domain_set_pod_target(xch, dom, target_pages - 0x20,
> +        rc = xc_domain_set_pod_target(xch, dom,
> +                                      target_pages - POD_VGA_HOLE_SIZE,
>                                        NULL, NULL, NULL);
>          if ( rc != 0 )
>          {
> @@ -335,7 +339,12 @@ static int setup_guest(xc_interface *xch,
>  
>      /* try to claim pages for early warning of insufficient memory available */
>      if ( claim_enabled ) {
> -        rc = xc_domain_claim_pages(xch, dom, nr_pages - cur_pages);
> +        unsigned long nr = target_pages;
> +
> +        if ( pod_mode )
> +            nr -= POD_VGA_HOLE_SIZE;
> +
> +        rc = xc_domain_claim_pages(xch, dom, nr);
>          if ( rc != 0 )
>          {
>              PERROR("Could not allocate memory for HVM guest as we cannot claim memory!");
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 5f484a2..1e44ba3 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -339,8 +339,8 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
>          goto out;
>      }
>  
> -    /* disallow a claim not exceeding current tot_pages or above max_pages */
> -    if ( (pages <= d->tot_pages) || (pages > d->max_pages) )
> +    /* disallow a claim below current tot_pages or above max_pages */
> +    if ( (pages < d->tot_pages) || (pages > d->max_pages) )
>      {
>          ret = -EINVAL;
>          goto out;
> 
> 
> 
> > +        rc = xc_domain_claim_pages(xch, dom, nr);
> >          if ( rc != 0 )
> >          {
> >              PERROR("Could not allocate memory for HVM guest as we cannot claim memory!");