From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53648) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YO4S5-0004sV-NA for qemu-devel@nongnu.org; Wed, 18 Feb 2015 08:14:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YO4S1-00024b-KP for qemu-devel@nongnu.org; Wed, 18 Feb 2015 08:14:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YO4S1-00023e-Ay for qemu-devel@nongnu.org; Wed, 18 Feb 2015 08:14:01 -0500 Date: Wed, 18 Feb 2015 14:13:46 +0100 From: "Michael S. Tsirkin" Message-ID: <20150218131346.GI1932@redhat.com> References: <1423479254-15342-1-git-send-email-imammedo@redhat.com> <1423479254-15342-48-git-send-email-imammedo@redhat.com> <20150217164431.GC26775@redhat.com> <20150218110904.16c7af0d@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150218110904.16c7af0d@nial.brq.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 Wed, Feb 18, 2015 at 11:09:04AM +0100, Igor Mammedov wrote: > On Tue, 17 Feb 2015 17:44:31 +0100 > "Michael S. Tsirkin" wrote: > > > 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. > that's been introduced when conversion began in patch: > [6/52] pc: acpi-build: use aml_def_block() for declaring SSDT table" > OK, the description is confusing. just say dsl is now empty. > > Also pls don't include generated files in patches, > > they just make rebases painful. > > just say in commit log they need to be updated. > sure > > > > > > --- > > > 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. > nope, see > [6/52] pc: acpi-build: use aml_def_block() for declaring SSDT table" > where header from template is skipped and only content is copied Pls squash this chunk there then. > > > > > > > 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