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

["Michael S. Tsirkin" <mst@redhat.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 <imammedo@redhat.com>


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 <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