From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59608) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNlGQ-0007Wy-L3 for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:44:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNlGN-0007ya-E2 for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:44:46 -0500 Received: from mail.kernel.org ([198.145.29.136]:36291) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNlGN-0007yJ-5x for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:44:43 -0500 Date: Tue, 17 Feb 2015 17:44:31 +0100 From: "Michael S. Tsirkin" Message-ID: <20150217164431.GC26775@redhat.com> References: <1423479254-15342-1-git-send-email-imammedo@redhat.com> <1423479254-15342-48-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423479254-15342-48-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 47/52] pc: acpi-build: drop remaining ssdt_misc template and use acpi_def_block() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: drjones@redhat.com, marcel.a@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, zhaoshenglong@huawei.com On Mon, Feb 09, 2015 at 10:54:09AM +0000, Igor Mammedov wrote: > It completes dynamic SSDT generation and makes it > independed of IASL binary blobs. It also hides > from user all pointer arithmetic when building > SSDT which makes resulting code a bit cleaner > and concentrating only on composing ASL construct > /i.e. a task build_ssdt() should be doing/. > > Also it makes one binary blob less stored in QEMU > source tree by removing need to keep and update > hw/i386/ssdt-misc.hex.generated file here in total > saving us ~430LOC. > > Signed-off-by: Igor Mammedov I see where we drop ssdt_misc here but I don't see acpi_def_block anywhere. Also pls don't include generated files in patches, they just make rebases painful. just say in commit log they need to be updated. > --- > hw/i386/Makefile.objs | 2 +- > hw/i386/acpi-build.c | 12 -- > hw/i386/ssdt-misc.dsl | 21 --- > hw/i386/ssdt-misc.hex.generated | 399 ---------------------------------------- > 4 files changed, 1 insertion(+), 433 deletions(-) > delete mode 100644 hw/i386/ssdt-misc.dsl > delete mode 100644 hw/i386/ssdt-misc.hex.generated > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index 6c8705d..dc8c38a 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -8,7 +8,7 @@ obj-$(CONFIG_XEN) += ../xenpv/ xen/ > obj-y += kvmvapic.o > obj-y += acpi-build.o > hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \ > - hw/i386/ssdt-misc.hex hw/i386/q35-acpi-dsdt.hex \ > + hw/i386/q35-acpi-dsdt.hex \ > hw/i386/ssdt-tpm.hex > > iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \ > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index bead77e..14c1c7d 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -466,10 +466,6 @@ build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu, > table_data->len - madt_start, 1); > } > > -#define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */ > -#define ACPI_SSDT_HEADER_LENGTH 36 > - > -#include "hw/i386/ssdt-misc.hex" > #include "hw/i386/ssdt-tpm.hex" > > /* Assign BSEL property to all buses. In the future, this can be changed > @@ -654,7 +650,6 @@ build_ssdt(Aml *table_data, > MachineState *machine = MACHINE(qdev_get_machine()); > uint32_t nr_mem = machine->ram_slots; > unsigned acpi_cpus = guest_info->apic_id_limit; > - uint8_t *ssdt_ptr; > Aml *ssdt, *sb_scope, *scope, *pkg, *dev, *method, *crs, *field, *ifctx; > int i; > > @@ -668,13 +663,6 @@ build_ssdt(Aml *table_data, > ACPI_BUILD_APPNAME4, 1, > ACPI_BUILD_APPNAME4_HEX, 1); > > - /* Copy misc variables and patch values in the S3_ / S4_ / S5_ packages */ > - acpi_data_push(ssdt->buf, sizeof(ssdp_misc_aml) - sizeof(AcpiTableHeader)); > - ssdt_ptr = (uint8_t *)ssdt->buf->data; > - memcpy(ssdt_ptr + sizeof(AcpiTableHeader), > - ssdp_misc_aml + sizeof(AcpiTableHeader), > - sizeof(ssdp_misc_aml) - sizeof(AcpiTableHeader)); > - > scope = aml_scope("\\_SB.PCI0"); > /* build PCI0._CRS */ > crs = aml_resource_template(); So before this patch, we had two headers? I don't see how this makes sense, and it does not match commit log. > diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl > deleted file mode 100644 > index 8d61f21..0000000 > --- a/hw/i386/ssdt-misc.dsl > +++ /dev/null > @@ -1,21 +0,0 @@ > -/* > - * 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 . > - */ > -#include "hw/acpi/pc-hotplug.h" > - > -ACPI_EXTRACT_ALL_CODE ssdp_misc_aml > - > -DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1) > -{ > -} > diff --git a/hw/i386/ssdt-misc.hex.generated b/hw/i386/ssdt-misc.hex.generated > deleted file mode 100644 > index cbcf61d..0000000 > --- a/hw/i386/ssdt-misc.hex.generated > +++ /dev/null > @@ -1,399 +0,0 @@ > -static unsigned char acpi_pci64_length[] = { > -0x6f > -}; > -static unsigned char acpi_s4_pkg[] = { > -0x99 > -}; > -static unsigned char ssdt_mctrl_nr_slots[] = { > -0x7d > -}; > -static unsigned char acpi_s3_name[] = { > -0x86 > -}; > -static unsigned char acpi_pci32_start[] = { > -0x2f > -}; > -static unsigned char acpi_pci64_valid[] = { > -0x43 > -}; > -static unsigned char ssdp_misc_aml[] = { > -0x53, > -0x53, > -0x44, > -0x54, > -0x6c, > -0x1, > -0x0, > -0x0, > -0x1, > -0x3, > -0x42, > -0x58, > -0x50, > -0x43, > -0x0, > -0x0, > -0x42, > -0x58, > -0x53, > -0x53, > -0x44, > -0x54, > -0x53, > -0x55, > -0x1, > -0x0, > -0x0, > -0x0, > -0x49, > -0x4e, > -0x54, > -0x4c, > -0x28, > -0x8, > -0x14, > -0x20, > -0x10, > -0x4c, > -0x5, > -0x5c, > -0x0, > -0x8, > -0x50, > -0x30, > -0x53, > -0x5f, > -0xc, > -0x78, > -0x56, > -0x34, > -0x12, > -0x8, > -0x50, > -0x30, > -0x45, > -0x5f, > -0xc, > -0x78, > -0x56, > -0x34, > -0x12, > -0x8, > -0x50, > -0x31, > -0x56, > -0x5f, > -0xa, > -0x12, > -0x8, > -0x50, > -0x31, > -0x53, > -0x5f, > -0x11, > -0xb, > -0xa, > -0x8, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x8, > -0x50, > -0x31, > -0x45, > -0x5f, > -0x11, > -0xb, > -0xa, > -0x8, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x8, > -0x50, > -0x31, > -0x4c, > -0x5f, > -0x11, > -0xb, > -0xa, > -0x8, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x0, > -0x8, > -0x4d, > -0x44, > -0x4e, > -0x52, > -0xc, > -0x78, > -0x56, > -0x34, > -0x12, > -0x10, > -0x29, > -0x5c, > -0x0, > -0x8, > -0x5f, > -0x53, > -0x33, > -0x5f, > -0x12, > -0x6, > -0x4, > -0x1, > -0x1, > -0x0, > -0x0, > -0x8, > -0x5f, > -0x53, > -0x34, > -0x5f, > -0x12, > -0x8, > -0x4, > -0xa, > -0x2, > -0xa, > -0x2, > -0x0, > -0x0, > -0x8, > -0x5f, > -0x53, > -0x35, > -0x5f, > -0x12, > -0x6, > -0x4, > -0x0, > -0x0, > -0x0, > -0x0, > -0x10, > -0x40, > -0xc, > -0x5c, > -0x2f, > -0x3, > -0x5f, > -0x53, > -0x42, > -0x5f, > -0x50, > -0x43, > -0x49, > -0x30, > -0x49, > -0x53, > -0x41, > -0x5f, > -0x5b, > -0x82, > -0x4d, > -0xa, > -0x50, > -0x45, > -0x56, > -0x54, > -0x8, > -0x5f, > -0x48, > -0x49, > -0x44, > -0xd, > -0x51, > -0x45, > -0x4d, > -0x55, > -0x30, > -0x30, > -0x30, > -0x31, > -0x0, > -0x8, > -0x50, > -0x45, > -0x53, > -0x54, > -0xb, > -0xff, > -0xff, > -0x5b, > -0x80, > -0x50, > -0x45, > -0x4f, > -0x52, > -0x1, > -0x50, > -0x45, > -0x53, > -0x54, > -0x1, > -0x5b, > -0x81, > -0xb, > -0x50, > -0x45, > -0x4f, > -0x52, > -0x1, > -0x50, > -0x45, > -0x50, > -0x54, > -0x8, > -0x14, > -0x18, > -0x5f, > -0x53, > -0x54, > -0x41, > -0x0, > -0x70, > -0x50, > -0x45, > -0x53, > -0x54, > -0x60, > -0xa0, > -0x6, > -0x93, > -0x60, > -0x0, > -0xa4, > -0x0, > -0xa1, > -0x4, > -0xa4, > -0xa, > -0xf, > -0x14, > -0xe, > -0x52, > -0x44, > -0x50, > -0x54, > -0x0, > -0x70, > -0x50, > -0x45, > -0x50, > -0x54, > -0x60, > -0xa4, > -0x60, > -0x14, > -0xc, > -0x57, > -0x52, > -0x50, > -0x54, > -0x1, > -0x70, > -0x68, > -0x50, > -0x45, > -0x50, > -0x54, > -0x8, > -0x5f, > -0x43, > -0x52, > -0x53, > -0x11, > -0xd, > -0xa, > -0xa, > -0x47, > -0x1, > -0x0, > -0x0, > -0x0, > -0x0, > -0x1, > -0x1, > -0x79, > -0x0, > -0x8b, > -0x5f, > -0x43, > -0x52, > -0x53, > -0xa, > -0x2, > -0x49, > -0x4f, > -0x4d, > -0x4e, > -0x8b, > -0x5f, > -0x43, > -0x52, > -0x53, > -0xa, > -0x4, > -0x49, > -0x4f, > -0x4d, > -0x58, > -0x14, > -0x18, > -0x5f, > -0x49, > -0x4e, > -0x49, > -0x0, > -0x70, > -0x50, > -0x45, > -0x53, > -0x54, > -0x49, > -0x4f, > -0x4d, > -0x4e, > -0x70, > -0x50, > -0x45, > -0x53, > -0x54, > -0x49, > -0x4f, > -0x4d, > -0x58 > -}; > -static unsigned char ssdt_isa_pest[] = { > -0xda > -}; > -static unsigned char acpi_s4_name[] = { > -0x92 > -}; > -static unsigned char acpi_pci64_start[] = { > -0x4d > -}; > -static unsigned char acpi_pci64_end[] = { > -0x5e > -}; > -static unsigned char acpi_pci32_end[] = { > -0x39 > -}; > -- > 1.8.3.1