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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Sep 04, 2013 at 03:23:40PM +0100, Gordan Bobic wrote:
> 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. :)

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

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

It will boot - but if you are have more than 2GB the PCI devices
will most likely not work.
> 
> Gordan

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