Re: [Qemu-devel] [PATCH v3 47/52] pc: acpi-build: drop remaining ssdt_misc template and use acpi_def_block()

[Igor Mammedov]

On Tue, 17 Feb 2015 17:44:31 +0100
"Michael S. Tsirkin" <address@hidden> 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 <address@hidden>
> 
> 
> 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"

 
> 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

> 
> 
> > 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 <http://www.gnu.org/licenses/>.
> > - */
> > -#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