Hi Heinrich, On Sat, 11 Nov 2023 at 06:42, Heinrich Schuchardt <[email protected]> wrote: > > ACPI tables may comprise either RSDT, XSDT, or both. The current code fails > to check the presence of the RSDT table before accessing it. This leads to > an exception if the RSDT table is not provided. > > The XSDT table takes precedence over the RSDT table. > > Addresses in the XSDT table are 64-bit. Adjust the output accordingly. > > The ACPI specification does not require that the sequence of tables are the > same in XSDT and RSDT. Comparing entries with the same index makes no > sense.
Does it make any sense to have different sequences? Have you seen that in the wild? > > The FACS table header does not provide revision information. Correct the > description of dump_hdr(). > > Signed-off-by: Heinrich Schuchardt <[email protected]> > --- > cmd/acpi.c | 48 +++++++++++++++++++++++++++++------------------- > 1 file changed, 29 insertions(+), 19 deletions(-) > > diff --git a/cmd/acpi.c b/cmd/acpi.c > index 7e397d1a74..d02108d821 100644 > --- a/cmd/acpi.c > +++ b/cmd/acpi.c > @@ -17,7 +17,8 @@ DECLARE_GLOBAL_DATA_PTR; > /** > * dump_hdr() - Dump an ACPI header > * > - * If the header is for FACS then it shows the revision information as well > + * Except for the Firmware ACPI Control Structure (FACS) > + * additionally show the revision information. > * > * @hdr: ACPI header to dump > */ > @@ -25,7 +26,7 @@ static void dump_hdr(struct acpi_table_header *hdr) > { > bool has_hdr = memcmp(hdr->signature, "FACS", ACPI_NAME_LEN); > > - printf("%.*s %08lx %5x", ACPI_NAME_LEN, hdr->signature, > + printf("%.*s %016lx %5x", ACPI_NAME_LEN, hdr->signature, > (ulong)map_to_sysmem(hdr), hdr->length); > if (has_hdr) { > printf(" v%02d %.6s %.8s %x %.4s %x\n", hdr->revision, > @@ -43,7 +44,7 @@ static int dump_table_name(const char *sig) > hdr = acpi_find_table(sig); > if (!hdr) > return -ENOENT; > - printf("%.*s @ %08lx\n", ACPI_NAME_LEN, hdr->signature, > + printf("%.*s @ %016lx\n", ACPI_NAME_LEN, hdr->signature, We really don't want 16 hex digits of output. It just isn't readable. Should we add a way of writing a hex number with an underscore between digits 8 and 9? 12345678_3846263d Better I think (at least for now) would be to use %10lx and live with the column misalignment in the unlikely event that very large addresses are used. > (ulong)map_to_sysmem(hdr)); > print_buffer(0, hdr, 1, hdr->length, 0); > > @@ -62,26 +63,34 @@ static int list_rsdt(struct acpi_rsdt *rsdt, struct > acpi_xsdt *xsdt) > { > int len, i, count; > > - dump_hdr(&rsdt->header); > - if (xsdt) > + if (rsdt) > + dump_hdr(&rsdt->header); > + if (xsdt) { > dump_hdr(&xsdt->header); > - len = rsdt->header.length - sizeof(rsdt->header); > - count = len / sizeof(u32); > + len = xsdt->header.length - sizeof(xsdt->header); > + count = len / sizeof(u64); > + } else if (rsdt) { > + len = rsdt->header.length - sizeof(rsdt->header); > + count = len / sizeof(u32); > + } else { > + return 0; > + } > + > for (i = 0; i < count; i++) { > struct acpi_table_header *hdr; > > - if (!rsdt->entry[i]) > - break; > - hdr = map_sysmem(rsdt->entry[i], 0); > + if (xsdt) { > + if (!xsdt->entry[i]) > + break; > + hdr = map_sysmem(xsdt->entry[i], 0); > + } else { > + if (!rsdt->entry[i]) > + break; > + hdr = map_sysmem(rsdt->entry[i], 0); > + } > dump_hdr(hdr); > if (!memcmp(hdr->signature, "FACP", ACPI_NAME_LEN)) > list_fadt((struct acpi_fadt *)hdr); > - if (xsdt) { > - if (xsdt->entry[i] != rsdt->entry[i]) { > - printf(" (xsdt mismatch %llx)\n", > - xsdt->entry[i]); > - } > - } > } > > return 0; > @@ -92,10 +101,11 @@ static int list_rsdp(struct acpi_rsdp *rsdp) > struct acpi_rsdt *rsdt; > struct acpi_xsdt *xsdt; > > - printf("RSDP %08lx %5x v%02d %.6s\n", (ulong)map_to_sysmem(rsdp), > + printf("RSDP %016lx %5x v%02d %.6s\n", (ulong)map_to_sysmem(rsdp), > rsdp->length, rsdp->revision, rsdp->oem_id); > rsdt = map_sysmem(rsdp->rsdt_address, 0); > xsdt = map_sysmem(rsdp->xsdt_address, 0); > + > list_rsdt(rsdt, xsdt); > > return 0; > @@ -111,8 +121,8 @@ static int do_acpi_list(struct cmd_tbl *cmdtp, int flag, > int argc, > printf("No ACPI tables present\n"); > return 0; > } > - printf("Name Base Size Detail\n"); > - printf("---- -------- ----- ------\n"); > + printf("Name Base Size Detail\n" > + "---- ---------------- ----- > ----------------------------\n"); > list_rsdp(rsdp); > > return 0; > -- > 2.40.1 > Regards, Simon
