From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58969) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzkKF-0006RW-OX for qemu-devel@nongnu.org; Thu, 27 Oct 2016 09:02:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzkKA-0002J3-OD for qemu-devel@nongnu.org; Thu, 27 Oct 2016 09:02:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49388) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bzkKA-0002In-GI for qemu-devel@nongnu.org; Thu, 27 Oct 2016 09:02:26 -0400 Date: Thu, 27 Oct 2016 15:02:22 +0200 From: Igor Mammedov Message-ID: <20161027150222.0fa04bc5@nial.brq.redhat.com> In-Reply-To: <20161027112958.GA30231@citrix.com> References: <1477416484-29054-1-git-send-email-wei.liu2@citrix.com> <20161026170952.18c02dbb@nial.brq.redhat.com> <20161026152234.GQ30231@citrix.com> <20161027125402.63484b99@nial.brq.redhat.com> <20161027110342.GY30231@citrix.com> <20161027111058.GZ30231@citrix.com> <20161027132649.21591a9d@nial.brq.redhat.com> <20161027112958.GA30231@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Xen-devel] [PATCH RFC] acpi: don't build acpi tables for xen hvm guests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wei Liu Cc: Stefano Stabellini , qemu-devel@nongnu.org, Sander Eikelenboom , Anthony PERARD , Xen-devel On Thu, 27 Oct 2016 12:29:58 +0100 Wei Liu wrote: > On Thu, Oct 27, 2016 at 01:26:49PM +0200, Igor Mammedov wrote: > > On Thu, 27 Oct 2016 12:10:58 +0100 > > Wei Liu wrote: > > > > > On Thu, Oct 27, 2016 at 12:03:42PM +0100, Wei Liu wrote: > > > > Cc Sander > > > > > > > > On Thu, Oct 27, 2016 at 12:54:02PM +0200, Igor Mammedov wrote: > > > > > On Wed, 26 Oct 2016 16:22:34 +0100 > > > > > Wei Liu wrote: > > > > > > > > > > > On Wed, Oct 26, 2016 at 05:09:52PM +0200, Igor Mammedov wrote: > > > > > > > On Tue, 25 Oct 2016 18:28:04 +0100 > > > > > > > Wei Liu wrote: > > > > > > > > > > > > > > > Xen's toolstack is in charge of building ACPI tables. Skip acpi table > > > > > > > > building if running on Xen. > > > > > > > > > > > > > > > > This issue is discovered due to direct kernel boot on Xen doesn't boot > > > > > > > > anymore, because the new ACPI tables cause the guest to exceed its > > > > > > > > memory allocation limit. > > > > > > > > > > > > > > > > Reported-by: Sander Eikelenboom > > > > > > > > Signed-off-by: Wei Liu > > > > > > > Question is: > > > > > > > Why does xen guest get ACPI tables from QEMU instead of using > > > > > > > Xen provided ones. > > > > > > > Maybe it's firmware issue i.e. firmware side shouldn't load > > > > > > > ACPI tables from QEMU provided fwcfg file and load Xen provided instead. > > > > > > > > > > > > > > > > > > > It hasn't come to the point that the guest is booted. QEMU exits when > > > > > > trying to populate some pages for the guest, at which point the guest > > > > > > has not yet been started. In a sense, Xen guest doesn't get ACPI from > > > > > > QEMU because it never gets to that point. > > > > > > > > > > > > Direct kernel boot causes fw_cfg to be filled in. pcms->has_acpi_build > > > > > > defaults to true and acpi_enabled is also true. These make all checks in > > > > > > acpi_setup pass. QEMU proceeds to build and load ACPI tables (which are > > > > > > never going to be used by Xen guests), causing the guest to exceeds its > > > > > > limit. > > > > > > > > > > > > Wei. > > > > > Would something like this fix issue for you? > > > > > > > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > > > > index a54a468..61b6026 100644 > > > > > --- a/hw/i386/pc_piix.c > > > > > +++ b/hw/i386/pc_piix.c > > > > > @@ -1094,10 +1094,13 @@ DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa, > > > > > #ifdef CONFIG_XEN > > > > > static void xenfv_machine_options(MachineClass *m) > > > > > { > > > > > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > > > > > + > > > > > m->desc = "Xen Fully-virtualized PC"; > > > > > m->max_cpus = HVM_MAX_VCPUS; > > > > > m->default_machine_opts = "accel=xen"; > > > > > m->hot_add_cpu = pc_hot_add_cpu; > > > > > + pcmc->has_acpi_build = false; > > > > > } > > > > > > > > > > DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init, > > > > > > > > > > > > > Yes, it does. > > > > > > > > (I have a similar patch in my queue already) > > > > > > > > > > Oh, the reason I didn't send it out is because Eduardo suggested we > > > should use a new field instead of setting has_acpi_build outside of > > > PCMachineClass init function. > > > > > > I have actually gotten around to investigate this option and what is > > > needed to be done. > > static void xenfv_machine_options(MachineClass *m) > > { > > ... > > } > > > > DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init, > > xenfv_machine_options); > > > > #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \ > > static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \ > > { \ > > MachineClass *mc = MACHINE_CLASS(oc); \ > > optsfn(mc); \ > > mc->init = initfn; \ > > } \ > > ... > > > > So xenfv_machine_options() is a part of pc_machine_xenfv_class_init() > > Ah, so your (and mine) patch already fits the bill. Thanks for looking > into this. > > Are you going to submit a proper patch or do you want me to? Please submit your version of patch. > > Wei. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel