On 21.03.25 11:49, Heinrich Schuchardt wrote:
On 15.03.25 15:26, Simon Glass wrote:When the ACPI tables come from an earlier bootloader it is helpful to see whether the checksums are correct or not. Add a -c flag to the 'acpi list' command to support that.Signed-off-by: Simon Glass <s...@chromium.org> --- (no changes since v3) Changes in v3: - Add new patch to support checking checksums with ACPI command cmd/acpi.c | 57 +++++++++++++++++++++++++++--------------- doc/usage/cmd/acpi.rst | 20 ++++++++++++++- test/dm/acpi.c | 46 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/cmd/acpi.c b/cmd/acpi.c index 2273176f106..bb243202009 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -7,6 +7,7 @@ #include <display_options.h> #include <log.h> #include <mapmem.h> +#include <tables_csum.h> #include <acpi/acpi_table.h> #include <asm/acpi_table.h> #include <asm/global_data.h> @@ -15,6 +16,17 @@ DECLARE_GLOBAL_DATA_PTR; +static const char *show_checksum(void *ptr, uint size, bool chksums) +{ + uint checksum; + + if (!chksums) + return ""; + checksum = table_compute_checksum(ptr, size); + + return checksum ? " bad" : " OK";Please, move the leading spaces to the printf statements below.+} + /** * dump_hdr() - Dump an ACPI header * @@ -23,16 +35,17 @@ DECLARE_GLOBAL_DATA_PTR; * * @hdr: ACPI header to dump */ -static void dump_hdr(struct acpi_table_header *hdr) +static void dump_hdr(struct acpi_table_header *hdr, bool chksums) { bool has_hdr = memcmp(hdr->signature, "FACS", ACPI_NAME_LEN); printf("%.*s %16lx %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, + printf(" v%02d %.6s %.8s %x %.4s %x%s\n", hdr->revision,%x results in variable length output. To avoid ragged layout, please, define the output length. Best regards Heinrichhdr->oem_id, hdr->oem_table_id, hdr->oem_revision, - hdr->creator_id, hdr->creator_revision); + hdr->creator_id, hdr->creator_revision, + show_checksum(hdr, hdr->length, chksums)); } else { printf("\n"); } @@ -52,22 +65,22 @@ static int dump_table_name(const char *sig) return 0; } -static void list_fadt(struct acpi_fadt *fadt) +static void list_fadt(struct acpi_fadt *fadt, bool chksums) { if (fadt->header.revision >= 3 && fadt->x_dsdt) - dump_hdr(nomap_sysmem(fadt->x_dsdt, 0)); + dump_hdr(nomap_sysmem(fadt->x_dsdt, 0), chksums); else if (fadt->dsdt) - dump_hdr(nomap_sysmem(fadt->dsdt, 0)); + dump_hdr(nomap_sysmem(fadt->dsdt, 0), chksums); if (!IS_ENABLED(CONFIG_X86) && !IS_ENABLED(CONFIG_SANDBOX) && !(fadt->flags & ACPI_FADT_HW_REDUCED_ACPI)) log_err("FADT not ACPI-hardware-reduced-compliant\n"); if (fadt->header.revision >= 3 && fadt->x_firmware_ctrl) - dump_hdr(nomap_sysmem(fadt->x_firmware_ctrl, 0)); + dump_hdr(nomap_sysmem(fadt->x_firmware_ctrl, 0), chksums); else if (fadt->firmware_ctrl) - dump_hdr(nomap_sysmem(fadt->firmware_ctrl, 0)); + dump_hdr(nomap_sysmem(fadt->firmware_ctrl, 0), chksums); } -static void list_rsdt(struct acpi_rsdp *rsdp) +static void list_rsdt(struct acpi_rsdp *rsdp, bool chksums) { int len, i, count; struct acpi_rsdt *rsdt; @@ -75,11 +88,11 @@ static void list_rsdt(struct acpi_rsdp *rsdp) if (rsdp->rsdt_address) { rsdt = nomap_sysmem(rsdp->rsdt_address, 0); - dump_hdr(&rsdt->header); + dump_hdr(&rsdt->header, chksums); } if (rsdp->xsdt_address) { xsdt = nomap_sysmem(rsdp->xsdt_address, 0); - dump_hdr(&xsdt->header); + dump_hdr(&xsdt->header, chksums); len = xsdt->header.length - sizeof(xsdt->header); count = len / sizeof(u64); } else if (rsdp->rsdt_address) { @@ -100,24 +113,28 @@ static void list_rsdt(struct acpi_rsdp *rsdp) if (!entry) break; hdr = nomap_sysmem(entry, 0); - dump_hdr(hdr); + dump_hdr(hdr, chksums); if (!memcmp(hdr->signature, "FACP", ACPI_NAME_LEN)) - list_fadt((struct acpi_fadt *)hdr); + list_fadt((struct acpi_fadt *)hdr, chksums); } } -static void list_rsdp(struct acpi_rsdp *rsdp) +static void list_rsdp(struct acpi_rsdp *rsdp, bool chksums) { - printf("RSDP %16lx %5x v%02d %.6s\n", (ulong)map_to_sysmem(rsdp), - rsdp->length, rsdp->revision, rsdp->oem_id); - list_rsdt(rsdp); + printf("RSDP %16lx %5x v%02d %.6s%s%s\n", + (ulong)map_to_sysmem(rsdp), rsdp->length, rsdp->revision, + rsdp->oem_id, show_checksum(rsdp, 0x14, chksums), + show_checksum(rsdp, rsdp->length, chksums)); + list_rsdt(rsdp, chksums); } static int do_acpi_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct acpi_rsdp *rsdp; + bool chksums; + chksums = argc >= 2 && !strcmp("-c", argv[1]); rsdp = map_sysmem(gd_acpi_start(), 0); if (!rsdp) { printf("No ACPI tables present\n");@@ -125,7 +142,7 @@ static int do_acpi_list(struct cmd_tbl *cmdtp, int flag, int argc,} printf("Name Base Size Detail\n""---- ---------------- ----- ----------------------------\n");- list_rsdp(rsdp); + list_rsdp(rsdp, chksums); return 0; }@@ -187,13 +204,13 @@ static int do_acpi_dump(struct cmd_tbl *cmdtp, int flag, int argc,} U_BOOT_LONGHELP(acpi, - "list - list ACPI tables\n" + "list [-c] - list ACPI tables [check checksums]\n"
Let's keep things simple for the user: Always check the checksums. Only print a notice if a checksum is wrong. Best regards Heinrich
"acpi items [-d] - List/dump each piece of ACPI data from devices\n""acpi set [<addr>] - Set or show address of ACPI tables\n" "acpi dump <name> - Dump ACPI table"); U_BOOT_CMD_WITH_SUBCMDS(acpi, "ACPI tables", acpi_help_text, - U_BOOT_SUBCMD_MKENT(list, 1, 1, do_acpi_list), + U_BOOT_SUBCMD_MKENT(list, 2, 1, do_acpi_list), U_BOOT_SUBCMD_MKENT(items, 2, 1, do_acpi_items), U_BOOT_SUBCMD_MKENT(set, 2, 1, do_acpi_set), U_BOOT_SUBCMD_MKENT(dump, 2, 1, do_acpi_dump)); diff --git a/doc/usage/cmd/acpi.rst b/doc/usage/cmd/acpi.rst index 9f30972fe53..e652968d584 100644 --- a/doc/usage/cmd/acpi.rst +++ b/doc/usage/cmd/acpi.rst @@ -11,7 +11,7 @@ Synopsis :: - acpi list + acpi list [-c] acpi items [-d] acpi dump <name> acpi set <address>@@ -38,6 +38,9 @@ List the ACPI tables that have been generated. Each table has a 4-charactertable name (e.g. SSDT, FACS) and has a format defined by the `ACPI specification`_.+The `-c` flag tells U-Boot to verify the checksums and print 'OK' or 'BAD' next+to each table. +U-Boot does not currently support decoding the tables. Unlike devicetree, ACPI tables have no regular schema and also some include bytecode, so decoding thetables requires a lot of code. @@ -259,5 +262,20 @@ pointer:: WAET bff76a3b 28 v01 BOCHS BXPC 1 BXPC 1 SSDT bff95040 c5 v02 COREv4 COREBOOT 2a CORE 20221020 +This shows checking that the checksums are correct for each table:: + + => acpi list -c + Name Base Size Detail + ---- ---------------- ----- ---------------------------- + RSDP bec9a000 24 v00 BOCHS OK OK + RSDT bec9bd4a 38 v01 BOCHS BXPC 1 BXPC 1 OK + FACP bec9bb46 74 v01 BOCHS BXPC 1 BXPC 1 OK + DSDT bec9a080 1ac6 v01 BOCHS BXPC 1 BXPC 1 OK + FACS bec9a040 40 + APIC bec9bbba 78 v03 BOCHS BXPC 1 BXPC 1 OK + HPET bec9bc32 38 v01 BOCHS BXPC 1 BXPC 1 OK + SRAT bec9bc6a b8 v01 BOCHS BXPC 1 BXPC 1 OK + WAET bec9bd22 28 v01 BOCHS BXPC 1 BXPC 1 OK +.. _`ACPI specification`: https://uefi.org/sites/default/files/ resources/ACPI_6_3_final_Jan30.pdfdiff --git a/test/dm/acpi.c b/test/dm/acpi.c index f98f9d1e74b..db012b6d2f1 100644 --- a/test/dm/acpi.c +++ b/test/dm/acpi.c@@ -439,6 +439,52 @@ static int dm_test_acpi_cmd_list(struct unit_test_state *uts)}DM_TEST(dm_test_acpi_cmd_list, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE);+/* Test 'acpi list -c' command */ +static int dm_test_acpi_cmd_list_chksum(struct unit_test_state *uts) +{ + struct acpi_ctx ctx; + ulong addr; + void *buf; + + buf = memalign(16, BUF_SIZE); + ut_assertnonnull(buf); + addr = map_to_sysmem(buf); + ut_assertok(setup_ctx_and_base_tables(uts, &ctx, addr)); + + ut_assertok(acpi_write_dev_tables(&ctx)); + + run_command("acpi list -c", 0); + ut_assert_nextline("Name Base Size Detail");+ ut_assert_nextline("---- ---------------- ----- ----------------------------");+ ut_assert_nextline("RSDP %16lx %5zx v02 U-BOOT OK OK", addr, + sizeof(struct acpi_rsdp)); + addr = ALIGN(addr + sizeof(struct acpi_rsdp), 16);+ ut_assert_nextline("RSDT %16lx %5zx v01 U-BOOT U-BOOTBL %x INTL 0 OK",+ addr, sizeof(struct acpi_table_header) + + 3 * sizeof(u32), OEM_REVISION); + addr = ALIGN(addr + sizeof(struct acpi_rsdt), 16);+ ut_assert_nextline("XSDT %16lx %5zx v01 U-BOOT U-BOOTBL %x INTL 0 OK",+ addr, sizeof(struct acpi_table_header) + + 3 * sizeof(u64), OEM_REVISION); + addr = ALIGN(addr + sizeof(struct acpi_xsdt), 64);+ ut_assert_nextline("DMAR %16lx %5zx v01 U-BOOT U-BOOTBL %x INTL 0 OK",+ addr, sizeof(struct acpi_dmar), OEM_REVISION); + addr = ALIGN(addr + sizeof(struct acpi_dmar), 16);+ ut_assert_nextline("DMAR %16lx %5zx v01 U-BOOT U-BOOTBL %x INTL 0 OK",+ addr, sizeof(struct acpi_dmar), OEM_REVISION); + addr = ALIGN(addr + sizeof(struct acpi_dmar), 16);+ ut_assert_nextline("DMAR %16lx %5zx v01 U-BOOT U-BOOTBL %x INTL 0 OK",+ addr, sizeof(struct acpi_dmar), OEM_REVISION); + ut_assert_console_end(); + ut_assert_console_end(); + unmap_sysmem(buf); + free(buf); + + return 0; +} +DM_TEST(dm_test_acpi_cmd_list_chksum, + UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE); + /* Test 'acpi dump' command */ static int dm_test_acpi_cmd_dump(struct unit_test_state *uts) {
