From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: HVM support for e820_host (Was: Bug: Limitation of <=2GB RAM in domU persists with 4.3.0) Date: Wed, 4 Sep 2013 10:08:37 -0400 Message-ID: <20130904140837.GB3188@phenom.dumpdata.com> References: <20130729180431.GQ5848@phenom.dumpdata.com> <20130903145934.GC1487@konrad-lan.dumpdata.com> <52263CBD.1090402@bobich.net> <52264826.3010402@bobich.net> <52264B64.1080302@bobich.net> <20130903211058.GA14214@phenom.dumpdata.com> <5226539C.4000906@bobich.net> <20130903213021.GA14453@phenom.dumpdata.com> <52267C5F.6000309@bobich.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <52267C5F.6000309@bobich.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Gordan Bobic Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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? > - 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. 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 will have a play with the other two patches tomorrow. > > Gordan