On Thu, Nov 08, 2018 at 03:16:23PM +0100, Igor Mammedov wrote: > On Mon, 5 Nov 2018 02:40:28 +0100 > Samuel Ortiz <sa...@linux.intel.com> wrote: > > > XSDT is the 64-bit version of the legacy ACPI RSDT (Root System > > Description Table). RSDT only allow for 32-bit addressses and have thus > > been deprecated. Since ACPI version 2.0, RSDPs should point at XSDTs and > > no longer RSDTs, although RSDTs are still supported for backward > > compatibility. > > > > Since version 2.0, RSDPs should add an extended checksum, a complete table > > length and a version field to the table. > > This patch re-implements what arm/virt board already does > and fixes checksum bug in the later and at the same time > without a user (within the patch). > > I'd suggest redo it a way similar to FADT refactoring > patch 1: fix checksum bug in virt/arm > patch 2: update reference tables in test > patch 3: introduce AcpiRsdpData similar to commit 937d1b587 > (both arm and x86) wich stores all data in hos byte order > patch 4: convert arm's impl. to build_append_int_noprefix() API (commit > 5d7a334f7) > ... move out to aml-build.c > patch 5: reuse generalized arm's build_rsdp() for x86, dropping x86 > specific one > amending it to generate rev1 variant defined by revision in AcpiRsdpData > (commit dd1b2037a) > > 'make check V=1' shouldn't observe any ACPI tables changes after patch 2.
And your next suggestion is to add patch 6. I guess it's doable but this will make a single patch a 6 patch series. At this rate this series will be at 200 patches easily. Automated checks are cool but hey it's easy to see what changed in a disassembled table, and we do not update them blindly. So just note in the comment that there's a table change for ARM and expected files need to be updated and we should be fine IMHO. > > Signed-off-by: Samuel Ortiz <sa...@linux.intel.com> > > --- > > include/hw/acpi/aml-build.h | 3 +++ > > hw/acpi/aml-build.c | 37 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 40 insertions(+) > > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > index c9bcb32d81..3580d0ce90 100644 > > --- a/include/hw/acpi/aml-build.h > > +++ b/include/hw/acpi/aml-build.h > > @@ -393,6 +393,9 @@ void > > build_rsdp(GArray *table_data, > > BIOSLinker *linker, unsigned rsdt_tbl_offset); > > void > > +build_rsdp_xsdt(GArray *table_data, > > + BIOSLinker *linker, unsigned xsdt_tbl_offset); > > +void > > build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > > const char *oem_id, const char *oem_table_id); > > void > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index 51b608432f..a030d40674 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -1651,6 +1651,43 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, > > GArray *table_offsets, > > (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id); > > } > > > > +/* RSDP pointing at an XSDT */ > > +void > > +build_rsdp_xsdt(GArray *rsdp_table, > > + BIOSLinker *linker, unsigned xsdt_tbl_offset) > > +{ > > + AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp); > > + unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address); > > + unsigned xsdt_pa_offset = > > + (char *)&rsdp->xsdt_physical_address - rsdp_table->data; > > + unsigned xsdt_offset = > > + (char *)&rsdp->length - rsdp_table->data; There's a cleaner way to get at the offsets than pointer math: 1. save rsdp_table length before you push 2. add offset_of for fields If switching to build_append_int_noprefix then it's even easier - just save length before you append the int you intend to patch. > > + > > + bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16, > > + true /* fseg memory */); > > + > > + memcpy(&rsdp->signature, "RSD PTR ", 8); > > + memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6); > > + rsdp->length = cpu_to_le32(sizeof(*rsdp)); > > + /* version 2, we will use the XSDT pointer */ > > + rsdp->revision = 0x02; > > + > > + /* Address to be filled by Guest linker */ > > + bios_linker_loader_add_pointer(linker, > > + ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size, > > + ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset); > > + > > + /* Legacy checksum to be filled by Guest linker */ > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > + (char *)rsdp - rsdp_table->data, xsdt_offset, > > + (char *)&rsdp->checksum - rsdp_table->data); > > + > > + /* Extended checksum to be filled by Guest linker */ > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > + (char *)rsdp - rsdp_table->data, sizeof *rsdp, > > + (char *)&rsdp->extended_checksum - rsdp_table->data); > > +} > > + > > void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, > > uint64_t len, int node, MemoryAffinityFlags flags) > > {