Message ID | 20210907144814.741785-22-imammedo@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | acpi: refactor error prone build_header() and packed structures usage in ACPI tables | expand |
On 9/7/21 4:48 PM, Igor Mammedov wrote: > it replaces error-prone pointer arithmetic for build_header() API, > with 2 calls to start and finish table creation, > which hides offsets magic from API user. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v3: > * s/acpi_init_table|acpi_table_composed/acpi_table_begin|acpi_table_end/ > > CC: marcel.apfelbaum@gmail.com > CC: shannon.zhaosl@gmail.com > CC: peter.maydell@linaro.org > CC: qemu-arm@nongnu.org > CC: drjones@redhat.com > CC: eauger@redhat.com > --- > include/hw/acpi/acpi-defs.h | 9 --------- > hw/arm/virt-acpi-build.c | 19 +++++++++++-------- > hw/i386/acpi-common.c | 19 +++++++++++-------- > 3 files changed, 22 insertions(+), 25 deletions(-) > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > index c4f0a202e8..c7fa5caa06 100644 > --- a/include/hw/acpi/acpi-defs.h > +++ b/include/hw/acpi/acpi-defs.h > @@ -176,15 +176,6 @@ typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; > #define ACPI_DUAL_PIC 0 > #define ACPI_MULTIPLE_APIC 1 > > -/* Master MADT */ > - > -struct AcpiMultipleApicTable { > - ACPI_TABLE_HEADER_DEF /* ACPI common table header */ > - uint32_t local_apic_address; /* Physical address of local APIC */ > - uint32_t flags; > -} QEMU_PACKED; > -typedef struct AcpiMultipleApicTable AcpiMultipleApicTable; > - > /* Values for Type in APIC sub-headers */ > > #define ACPI_APIC_PROCESSOR 0 > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6ba02cf281..e3bdcd44e8 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -567,19 +567,26 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > vms->oem_table_id); > } > > -/* MADT */ > +/* > + * ACPI spec, Revision 5.0 > + * 5.2.12 Multiple APIC Description Table (MADT) > + */ > static void > build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > { > VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); > - int madt_start = table_data->len; > const MemMapEntry *memmap = vms->memmap; > const int *irqmap = vms->irqmap; > AcpiMadtGenericDistributor *gicd; > AcpiMadtGenericMsiFrame *gic_msi; > int i; > + AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id, > + .oem_table_id = vms->oem_table_id }; > > - acpi_data_push(table_data, sizeof(AcpiMultipleApicTable)); > + acpi_table_begin(&table, table_data); > + /* Local Interrupt Controller Address */ > + build_append_int_noprefix(table_data, 0, 4); > + build_append_int_noprefix(table_data, 0, 4); /* Flags */ > > gicd = acpi_data_push(table_data, sizeof *gicd); > gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR; > @@ -650,11 +657,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS); > gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE); > } > - > - build_header(linker, table_data, > - (void *)(table_data->data + madt_start), "APIC", > - table_data->len - madt_start, 3, vms->oem_id, > - vms->oem_table_id); > + acpi_table_end(linker, &table); > } > > /* FADT */ > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c > index 1f5947fcf9..a0cde1d874 100644 > --- a/hw/i386/acpi-common.c > +++ b/hw/i386/acpi-common.c > @@ -71,24 +71,29 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > } > } > > +/* > + * ACPI spec, Revision 1.0b > + * 5.2.8 Multiple APIC Description Table > + */ > void acpi_build_madt(GArray *table_data, BIOSLinker *linker, > X86MachineState *x86ms, AcpiDeviceIf *adev, > const char *oem_id, const char *oem_table_id) > { > MachineClass *mc = MACHINE_GET_CLASS(x86ms); > const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms)); > - int madt_start = table_data->len; > AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev); > bool x2apic_mode = false; > > - AcpiMultipleApicTable *madt; > AcpiMadtIoApic *io_apic; > AcpiMadtIntsrcovr *intsrcovr; > int i; > + AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id, > + .oem_table_id = oem_table_id }; > > - madt = acpi_data_push(table_data, sizeof *madt); > - madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS); > - madt->flags = cpu_to_le32(1); > + acpi_table_begin(&table, table_data); > + /* Local APIC Address */ > + build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4); > + build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ > > for (i = 0; i < apic_ids->len; i++) { > adevc->madt_cpu(adev, i, apic_ids, table_data); > @@ -156,8 +161,6 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, > local_nmi->lint = 1; /* ACPI_LINT1 */ > } > > - build_header(linker, table_data, > - (void *)(table_data->data + madt_start), "APIC", > - table_data->len - madt_start, 1, oem_id, oem_table_id); > + acpi_table_end(linker, &table); > } > > Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index c4f0a202e8..c7fa5caa06 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -176,15 +176,6 @@ typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; #define ACPI_DUAL_PIC 0 #define ACPI_MULTIPLE_APIC 1 -/* Master MADT */ - -struct AcpiMultipleApicTable { - ACPI_TABLE_HEADER_DEF /* ACPI common table header */ - uint32_t local_apic_address; /* Physical address of local APIC */ - uint32_t flags; -} QEMU_PACKED; -typedef struct AcpiMultipleApicTable AcpiMultipleApicTable; - /* Values for Type in APIC sub-headers */ #define ACPI_APIC_PROCESSOR 0 diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 6ba02cf281..e3bdcd44e8 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -567,19 +567,26 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) vms->oem_table_id); } -/* MADT */ +/* + * ACPI spec, Revision 5.0 + * 5.2.12 Multiple APIC Description Table (MADT) + */ static void build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); - int madt_start = table_data->len; const MemMapEntry *memmap = vms->memmap; const int *irqmap = vms->irqmap; AcpiMadtGenericDistributor *gicd; AcpiMadtGenericMsiFrame *gic_msi; int i; + AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id, + .oem_table_id = vms->oem_table_id }; - acpi_data_push(table_data, sizeof(AcpiMultipleApicTable)); + acpi_table_begin(&table, table_data); + /* Local Interrupt Controller Address */ + build_append_int_noprefix(table_data, 0, 4); + build_append_int_noprefix(table_data, 0, 4); /* Flags */ gicd = acpi_data_push(table_data, sizeof *gicd); gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR; @@ -650,11 +657,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS); gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE); } - - build_header(linker, table_data, - (void *)(table_data->data + madt_start), "APIC", - table_data->len - madt_start, 3, vms->oem_id, - vms->oem_table_id); + acpi_table_end(linker, &table); } /* FADT */ diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c index 1f5947fcf9..a0cde1d874 100644 --- a/hw/i386/acpi-common.c +++ b/hw/i386/acpi-common.c @@ -71,24 +71,29 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, } } +/* + * ACPI spec, Revision 1.0b + * 5.2.8 Multiple APIC Description Table + */ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, X86MachineState *x86ms, AcpiDeviceIf *adev, const char *oem_id, const char *oem_table_id) { MachineClass *mc = MACHINE_GET_CLASS(x86ms); const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms)); - int madt_start = table_data->len; AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev); bool x2apic_mode = false; - AcpiMultipleApicTable *madt; AcpiMadtIoApic *io_apic; AcpiMadtIntsrcovr *intsrcovr; int i; + AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id, + .oem_table_id = oem_table_id }; - madt = acpi_data_push(table_data, sizeof *madt); - madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS); - madt->flags = cpu_to_le32(1); + acpi_table_begin(&table, table_data); + /* Local APIC Address */ + build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4); + build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ for (i = 0; i < apic_ids->len; i++) { adevc->madt_cpu(adev, i, apic_ids, table_data); @@ -156,8 +161,6 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, local_nmi->lint = 1; /* ACPI_LINT1 */ } - build_header(linker, table_data, - (void *)(table_data->data + madt_start), "APIC", - table_data->len - madt_start, 1, oem_id, oem_table_id); + acpi_table_end(linker, &table); }
it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v3: * s/acpi_init_table|acpi_table_composed/acpi_table_begin|acpi_table_end/ CC: marcel.apfelbaum@gmail.com CC: shannon.zhaosl@gmail.com CC: peter.maydell@linaro.org CC: qemu-arm@nongnu.org CC: drjones@redhat.com CC: eauger@redhat.com --- include/hw/acpi/acpi-defs.h | 9 --------- hw/arm/virt-acpi-build.c | 19 +++++++++++-------- hw/i386/acpi-common.c | 19 +++++++++++-------- 3 files changed, 22 insertions(+), 25 deletions(-)