From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36430) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9XS8-0001q5-Tr for qemu-devel@nongnu.org; Thu, 17 Dec 2015 07:14:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9XS3-0000QR-Gk for qemu-devel@nongnu.org; Thu, 17 Dec 2015 07:14:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46381) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9XS3-0000QC-8U for qemu-devel@nongnu.org; Thu, 17 Dec 2015 07:14:31 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id C36941392E for ; Thu, 17 Dec 2015 12:14:30 +0000 (UTC) References: <1449704528-289297-1-git-send-email-imammedo@redhat.com> <1449704528-289297-26-git-send-email-imammedo@redhat.com> <5671665A.8000603@gmail.com> <20151216152555.01a91e12@igors-macbook-pro.local> From: Marcel Apfelbaum Message-ID: <5672A724.9010800@redhat.com> Date: Thu, 17 Dec 2015 14:14:28 +0200 MIME-Version: 1.0 In-Reply-To: <20151216152555.01a91e12@igors-macbook-pro.local> Content-Type: text/plain; charset=windows-1252; format=flowed 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: Igor Mammedov Cc: qemu-devel@nongnu.org On 12/16/2015 04:25 PM, Igor Mammedov wrote: > 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. I understand there can only be one DSDT table, but can't we have a "hybrid" DSDT made from both asl files and C code? > >> >> 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. This can be a problem. I am thinking of 2 solutions: 1. Disable the iasl tests on patch 1 and re-enable it on patch 73. 2. Go for a "hybrid" DSDT. If (2) is too complicated, the implication is that all this series must be taken atomically. Of course, (1) or some other idea is needed. What do you think? Thanks, Marcel > 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 >>> >> >