From 36e3a1e91a22a85456236e344405f795b907857d Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Date: Tue, 21 Nov 2023 15:41:28 +0100
Subject: [PATCH] cmd: acpi: fix acpi list command

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.

The return values of list_rsdt() and list_rsdp() are always zero and not
checked. Remove the return values.

Addresses in the XSDT table are 64-bit. Adjust the output accordingly.

As the RSDT table has to be ignored if the XSDT command is present there is
no need to compare the tables in a display command. Anyway the
specification does not require that the sequence of addresses in the RSDT
and XSDT table are the same.

The FACS table header does not provide revision information. Correct the
description of dump_hdr().

Adjust the ACPI test to match the changed output format of the 'acpi list'
command.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 cmd/acpi.c     | 64 +++++++++++++++++++++++++++-----------------------
 test/dm/acpi.c | 18 +++++++-------
 2 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/cmd/acpi.c b/cmd/acpi.c
index 7e397d1a74e..89970417a02 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  %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,
@@ -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 @ %16lx\n", ACPI_NAME_LEN, hdr->signature,
 	       (ulong)map_to_sysmem(hdr));
 	print_buffer(0, hdr, 1, hdr->length, 0);
 
@@ -58,47 +59,50 @@ static void list_fadt(struct acpi_fadt *fadt)
 		dump_hdr(map_sysmem(fadt->firmware_ctrl, 0));
 }
 
-static int list_rsdt(struct acpi_rsdt *rsdt, struct acpi_xsdt *xsdt)
+static void list_rsdt(struct acpi_rsdp *rsdp)
 {
 	int len, i, count;
+	struct acpi_rsdt *rsdt;
+	struct acpi_xsdt *xsdt;
 
-	dump_hdr(&rsdt->header);
-	if (xsdt)
+	if (rsdp->rsdt_address) {
+		rsdt = map_sysmem(rsdp->rsdt_address, 0);
+		dump_hdr(&rsdt->header);
+	}
+	if (rsdp->xsdt_address) {
+		xsdt = map_sysmem(rsdp->xsdt_address, 0);
 		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 (rsdp->rsdt_address) {
+		len = rsdt->header.length - sizeof(rsdt->header);
+		count = len / sizeof(u32);
+	} else {
+		return;
+	}
+
 	for (i = 0; i < count; i++) {
 		struct acpi_table_header *hdr;
+		u64 entry;
 
-		if (!rsdt->entry[i])
+		if (rsdp->xsdt_address)
+			entry = xsdt->entry[i];
+		else
+			entry = rsdt->entry[i];
+		if (!entry)
 			break;
-		hdr = map_sysmem(rsdt->entry[i], 0);
+		hdr = map_sysmem(entry, 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;
 }
 
-static int list_rsdp(struct acpi_rsdp *rsdp)
+static void 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  %16lx  %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;
+	list_rsdt(rsdp);
 }
 
 static int do_acpi_list(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -111,8 +115,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;
diff --git a/test/dm/acpi.c b/test/dm/acpi.c
index 7e661ee1166..fb071902b45 100644
--- a/test/dm/acpi.c
+++ b/test/dm/acpi.c
@@ -395,26 +395,26 @@ static int dm_test_acpi_cmd_list(struct unit_test_state *uts)
 
 	console_record_reset();
 	run_command("acpi list", 0);
-	ut_assert_nextline("Name      Base   Size  Detail");
-	ut_assert_nextline("----  --------  -----  ------");
-	ut_assert_nextline("RSDP  %08lx  %5zx  v02 U-BOOT", addr,
+	ut_assert_nextline("Name              Base   Size  Detail");
+	ut_assert_nextline("----  ----------------  -----  ----------------------------");
+	ut_assert_nextline("RSDP  %16lx  %5zx  v02 U-BOOT", addr,
 			   sizeof(struct acpi_rsdp));
 	addr = ALIGN(addr + sizeof(struct acpi_rsdp), 16);
-	ut_assert_nextline("RSDT  %08lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
+	ut_assert_nextline("RSDT  %16lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
 			   addr, sizeof(struct acpi_table_header) +
 			   3 * sizeof(u32), OEM_REVISION);
 	addr = ALIGN(addr + sizeof(struct acpi_rsdt), 16);
-	ut_assert_nextline("XSDT  %08lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
+	ut_assert_nextline("XSDT  %16lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
 			   addr, sizeof(struct acpi_table_header) +
 			   3 * sizeof(u64), OEM_REVISION);
 	addr = ALIGN(addr + sizeof(struct acpi_xsdt), 64);
-	ut_assert_nextline("DMAR  %08lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
+	ut_assert_nextline("DMAR  %16lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
 			   addr, sizeof(struct acpi_dmar), OEM_REVISION);
 	addr = ALIGN(addr + sizeof(struct acpi_dmar), 16);
-	ut_assert_nextline("DMAR  %08lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
+	ut_assert_nextline("DMAR  %16lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
 			   addr, sizeof(struct acpi_dmar), OEM_REVISION);
 	addr = ALIGN(addr + sizeof(struct acpi_dmar), 16);
-	ut_assert_nextline("DMAR  %08lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
+	ut_assert_nextline("DMAR  %16lx  %5zx  v01 U-BOOT U-BOOTBL %x INTL 0",
 			   addr, sizeof(struct acpi_dmar), OEM_REVISION);
 	ut_assert_console_end();
 
@@ -446,7 +446,7 @@ static int dm_test_acpi_cmd_dump(struct unit_test_state *uts)
 	console_record_reset();
 	run_command("acpi dump dmar", 0);
 	addr = ALIGN(map_to_sysmem(ctx.xsdt) + sizeof(struct acpi_xsdt), 64);
-	ut_assert_nextline("DMAR @ %08lx", addr);
+	ut_assert_nextline("DMAR @ %16lx", addr);
 	ut_assert_nextlines_are_dump(0x30);
 	ut_assert_console_end();
 
-- 
GitLab