On Wed, 22 Sep 2021 15:33:27 +0200 Eric Auger <[email protected]> wrote:
> On 9/7/21 4:48 PM, Igor Mammedov wrote: > > Drop usage of packed structures and explicit endian > > conversions when building table and use endian agnostic > > build_append_int_noprefix() API to build it. > > > > Signed-off-by: Igor Mammedov <[email protected]> > > --- > > CC: [email protected] > > --- > > include/hw/acpi/acpi-defs.h | 14 -------------- > > include/hw/acpi/aml-build.h | 1 + > > hw/acpi/aml-build.c | 2 +- > > hw/i386/acpi-build.c | 18 ++++++++++++++---- > > 4 files changed, 16 insertions(+), 19 deletions(-) > > > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > > index 0b375e7589..1a0774edd6 100644 > > --- a/include/hw/acpi/acpi-defs.h > > +++ b/include/hw/acpi/acpi-defs.h > > @@ -117,18 +117,4 @@ typedef struct AcpiFadtData { > > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0) > > #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1) > > > > -/* > > - * ACPI 1.0 Firmware ACPI Control Structure (FACS) > > - */ > > -struct AcpiFacsDescriptorRev1 { > > - uint32_t signature; /* ACPI Signature */ > > - uint32_t length; /* Length of structure, in bytes */ > > - uint32_t hardware_signature; /* Hardware configuration signature */ > > - uint32_t firmware_waking_vector; /* ACPI OS waking vector */ > > - uint32_t global_lock; /* Global Lock */ > > - uint32_t flags; > > - uint8_t resverved3 [40]; /* Reserved - must be zero */ > > -} QEMU_PACKED; > > -typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; > > - > > #endif > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > index 6e1f42e119..6f3d1de1b1 100644 > > --- a/include/hw/acpi/aml-build.h > > +++ b/include/hw/acpi/aml-build.h > > @@ -413,6 +413,7 @@ Aml *aml_concatenate(Aml *source1, Aml *source2, Aml > > *target); > > Aml *aml_object_type(Aml *object); > > > > void build_append_int_noprefix(GArray *table, uint64_t value, int size); > > +void build_append_byte(GArray *array, uint8_t val); > what's the rationale behind changes related to build_append_byte? looks like stray remnants from previous revisions, not really needed. I'll drop it. > > > > typedef struct AcpiTable { > > const char *sig; > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index 050fdb3f37..4135b385e5 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -47,7 +47,7 @@ static void build_prepend_byte(GArray *array, uint8_t val) > > g_array_prepend_val(array, val); > > } > > > > -static void build_append_byte(GArray *array, uint8_t val) > > +void build_append_byte(GArray *array, uint8_t val) > > { > > g_array_append_val(array, val); > > } > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 9f888d5a2c..547cd4ed9d 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -345,13 +345,23 @@ static void acpi_align_size(GArray *blob, unsigned > > align) > > g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align)); > > } > > > > -/* FACS */ > > +/* > > + * ACPI spec 1.0b, > > + * 5.2.6 Firmware ACPI Control Structure > > + */ > > static void > > build_facs(GArray *table_data) > > { > > - AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof > > *facs); > > - memcpy(&facs->signature, "FACS", 4); > > - facs->length = cpu_to_le32(sizeof(*facs)); > > + const char *sig = "FACS"; > > + const uint8_t reserved[40] = {}; > > + > > + g_array_append_vals(table_data, sig, 4); /* Signature */ > > + build_append_int_noprefix(table_data, 64, 4); /* Length */ > > + build_append_int_noprefix(table_data, 0, 4); /* Hardware Signature */ > > + build_append_int_noprefix(table_data, 0, 4); /* Firmware Waking Vector > > */ > > + build_append_int_noprefix(table_data, 0, 4); /* Global Lock */ > > + build_append_int_noprefix(table_data, 0, 4); /* Flags */ > > + g_array_append_vals(table_data, reserved, 40); /* Reserved */ > > } > > > > static void build_append_pcihp_notify_entry(Aml *method, int slot) > > > Without thoses changes > > Reviewed-by: Eric Auger <[email protected]> >
