From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60431) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9D1q-0007eB-PB for qemu-devel@nongnu.org; Wed, 16 Dec 2015 09:26:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9D1m-0003T2-Mo for qemu-devel@nongnu.org; Wed, 16 Dec 2015 09:26:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53421) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9D1m-0003SX-Ep for qemu-devel@nongnu.org; Wed, 16 Dec 2015 09:26:02 -0500 Date: Wed, 16 Dec 2015 15:25:55 +0100 From: Igor Mammedov Message-ID: <20151216152555.01a91e12@igors-macbook-pro.local> In-Reply-To: <5671665A.8000603@gmail.com> References: <1449704528-289297-1-git-send-email-imammedo@redhat.com> <1449704528-289297-26-git-send-email-imammedo@redhat.com> <5671665A.8000603@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 25/74] pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: qemu-devel@nongnu.org On Wed, 16 Dec 2015 15:25:46 +0200 Marcel Apfelbaum wrote: > On 12/10/2015 01:41 AM, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov > > --- > > hw/acpi/Makefile.objs | 2 +- > > hw/acpi/memory_hotplug_acpi_table.c | 40 > > +++++++++++++++++++++++++++++++++++++ > > hw/i386/acpi-build.c | 3 +++ > > include/hw/acpi/memory_hotplug.h | 4 ++++ 4 files changed, 48 > > insertions(+), 1 deletion(-) create mode 100644 > > hw/acpi/memory_hotplug_acpi_table.c > > > > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs > > index 7d3230c..c04064e 100644 > > --- a/hw/acpi/Makefile.objs > > +++ b/hw/acpi/Makefile.objs > > @@ -1,7 +1,7 @@ > > common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o > > common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o > > common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o > > -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o > > +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o > > memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) += > > acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o > > common-obj-$(CONFIG_ACPI) += aml-build.o > > diff --git a/hw/acpi/memory_hotplug_acpi_table.c > > b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644 > > index 0000000..25bbf5e > > --- /dev/null > > +++ b/hw/acpi/memory_hotplug_acpi_table.c > > @@ -0,0 +1,40 @@ > > +/* > > + * Memory hotplug AML code of DSDT ACPI table > > You are advertising as part of the DSDT code, but you are putting in > SSDT, right? By the way, maybe the answer is clear, but why are you > moving it to SSDT? There could be only one instance of DSDT and I temporarily put converted DSDT bits in SSDT until conversion is complete. > > Last thing, from this patch forward (until maybe the last one) make > check fails, right? (Specifically the acpi tests) yes, starting from this patch and upto 72nd acpi test fails since patches move DSDT bits into SSDT. But patch 73, when precompiled DSDT is empty and provides only header, moves converted DSDT content out of SSDT into QEMU generated DSDT, and drops ASL template. > > > > + * > > + * Copyright (C) 2013 Red Hat Inc > > + * > > + * Author: Igor Mammedov > > + * > > + * This program is free software; you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as > > published by > > + * the Free Software Foundation; either version 2 of the License, > > or > > + * (at your option) any later version. > > + > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + > > + * You should have received a copy of the GNU General Public > > License along > > + * with this program; if not, see . > > + */ > > Regarding the license, I was advised to just add a pointer to the > main license instead of copying it in each file, something like: > > This work is licensed under the terms of the GNU GPL, version 2 > or later. See the COPYING file in the top-level directory. > > Thanks, > Marcel > > > + > > +#include > > +#include "hw/acpi/memory_hotplug.h" > > +#include "include/hw/acpi/pc-hotplug.h" > > +#include "hw/boards.h" > > + > > +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem, > > + uint16_t io_base, uint16_t io_len) > > +{ > > + Aml *pci_scope; > > + Aml *ctrl_dev; > > + > > + /* scope for memory hotplug controller device node */ > > + pci_scope = aml_scope("_SB.PCI0"); > > + ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE)); > > + { > > + } > > + aml_append(pci_scope, ctrl_dev); > > + aml_append(ctx, pci_scope); > > +} > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index bbd37e9..1b609e6 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker, > > /* Reserve space for header */ > > acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader)); > > > > + build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base, > > + pm->mem_hp_io_len); > > + > > /* Extra PCI root buses are implemented only for i440fx */ > > bus = find_i440fx(); > > if (bus) { > > diff --git a/include/hw/acpi/memory_hotplug.h > > b/include/hw/acpi/memory_hotplug.h index 1342adb..b6e9f50 100644 > > --- a/include/hw/acpi/memory_hotplug.h > > +++ b/include/hw/acpi/memory_hotplug.h > > @@ -4,6 +4,7 @@ > > #include "hw/qdev-core.h" > > #include "hw/acpi/acpi.h" > > #include "migration/vmstate.h" > > +#include "hw/acpi/aml-build.h" > > > > /** > > * MemStatus: > > @@ -45,4 +46,7 @@ extern const VMStateDescription > > vmstate_memory_hotplug; vmstate_memory_hotplug, MemHotplugState) > > > > void acpi_memory_ospm_status(MemHotplugState *mem_st, > > ACPIOSTInfoList ***list); + > > +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem, > > + uint16_t io_base, uint16_t io_len); > > #endif > > >