Re: [Xen-devel] HVM support for e820_host (Was: Bug: Limitation of <=2GB RAM in domU persists with 4.3.0)

[Gordan Bobic <gordan@xxxxxxxxxx>]

On Wed, 4 Sep 2013 10:08:37 -0400, Konrad Rzeszutek Wilk 
<konrad.wilk@xxxxxxxxxx> wrote:
> On Wed, Sep 04, 2013 at 01:18:39AM +0100, Gordan Bobic wrote:
> > On 09/03/2013 10:30 PM, Konrad Rzeszutek Wilk wrote:
> > >On Tue, Sep 03, 2013 at 10:24:44PM +0100, Gordan Bobic wrote:
> > >>On 09/03/2013 10:10 PM, Konrad Rzeszutek Wilk wrote:
> > >>>On Tue, Sep 03, 2013 at 09:49:40PM +0100, Gordan Bobic wrote:
> > >>>>I spoke too soon - even with e820_host=0, the same error occurs.
> > >>>>What did I break? The code in question is this:
> > >>>>
> > >>>>if (libxl_defbool_val(d_config->b_info.e820_host)) {
> > >>>>     ret = libxl__e820_alloc(gc, domid, d_config);
> > >>>>     if (ret) {
> > >>>>         LIBXL__LOG_ERRNO(gc->owner, LIBXL__LOG_ERROR,
> > >>>>                 "Failed while collecting E820 with: %d 
> > (errno:%d)\n",
> > >>>>                 ret, errno);
> > >>>>     }
> > >>>>}
> > >>>>
> > >>>>With e820_host=0, that outer black should evaluate to false, 
> > should
> > >>>>it not? In libxl_create.c, if I am understanding the code 
> > correctly,
> > >>>>e820_host is defaulted to false, too. What am I missing?
> > >
> > >Does your config have 'pci' in it? The patch you sent had this:
> > >
> > >+        if (d_config->num_pcidevs)
> > >+            libxl_defbool_set(&b_info->e820_host, true);
> > >
> > >Which means that even if you did not have e820_host it will be 
> > automatically
> > >set if you have PCI devices.
> >
> > OK - that was embarrasing. Caffeine underflow error. :(
> > I backed out that block. I don't think e820_host should be implicit
> > in hvm when PCI devices are passed.
> >
> > That makes the adjusted patch fragment:
> > --- xl_cmdimpl.c.orig   2013-09-04 00:42:57.424337503 +0100
> > +++ xl_cmdimpl.c        2013-09-04 00:43:21.213886356 +0100
> > @@ -1293,7 +1293,7 @@
> >                  d_config->num_pcidevs++;
> >          }
> >          if (d_config->num_pcidevs && c_info->type == 
> > LIBXL_DOMAIN_TYPE_PV)
> I think you also want to get rid of the c_info->type check?
That would alter the current PV behaviour of implicitly
enabling e820_host with PCI devices passed, would it not?
I was hoping to maintain current behaviours intact, and
only affect what happens when e820_host=1 is set for HVMs.

> > -            libxl_defbool_set(&b_info->u.pv.e820_host, true);
> > +            libxl_defbool_set(&b_info->e820_host, true);
> >      }
> >
> >      switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
> >
> >
> > This should maintain the old behaviour for backward compatibility
> > when e820_host is not set. I just tested it and it works (with
> > e820_host=1 I get the previous error, with e820_host=0, everything
> > works fine.
> I think it might make sense to relax the PV check. That way the only
> way e820_host capability gets activated is if a the guest config
> has pci=X stanze. But perhaps that _and_ e820_host=1 is what should
> be done.
While I think these two checks should be separate in both cases,
I don't know that this won't break something for PV instances. And
I would prefer to not have to also debug that code path at this
point. :)

> Or maybe a negative check - if 'pci' stanze is there we automatically
> turn on e820_host=1 (right now that is how it works). If the user
> has thought 'e820_host=0' and 'pci=xxx' then we would turn the E820
> off? That way if something is odd we can turn this off?
I am not disagreeing at all - I just really don't want to change
the current PV behaviour since that will potentially require
extra debugging. Current PV behaviour seems to be that that if
PCI devices are passed, e820_host=1 is always set regardless
of whether it is explicitly enabled or disabled in the config.

And I have no idea what will happen with a PV domain with
PCI devices if e820_host=1 is disabled.

Gordan

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