[PATCH] hpet: hpetcheck: add ACPI HPET table check

Colin King colin.king at canonical.com
Tue Dec 11 23:35:32 UTC 2012


From: Colin Ian King <colin.king at canonical.com>

The original test has some notion of checking the HPET table
but this was #ifdef'd out from the original Linux Firmware
Developer Kit code and never implemented in fwts.  Remove the
old legacy code and fully implement a HPET table validation.

Since we want to sanity check where the kernel's view of where
the HPET is located and what the HPET table states, I've re-ordered
the sub-tests to ensure the new test runs 2nd.

Signed-off-by: Colin Ian King <colin.king at canonical.com>
---
 src/hpet/hpet_check/hpet_check.c | 224 ++++++++++++++++++++++++++++++++-------
 1 file changed, 186 insertions(+), 38 deletions(-)

diff --git a/src/hpet/hpet_check/hpet_check.c b/src/hpet/hpet_check/hpet_check.c
index 399bde2..9b8eac6 100644
--- a/src/hpet/hpet_check/hpet_check.c
+++ b/src/hpet/hpet_check/hpet_check.c
@@ -34,28 +34,6 @@ static fwts_list *klog;
 static uint64_t	hpet_base_p = 0;
 static void     *hpet_base_v = 0;
 
-#if 0
-/* check_hpet_base_hpet() -- used to parse the HPET Table for HPET base info */
-static void check_hpet_base_hpet(void)
-{
-        unsigned long address = 0;
-        unsigned long size = 0;
-	struct hpet_table *table;
-	char *table_ptr;
-
-	if (locate_acpi_table("HPET", &address, &size))
-		return;
-
-        if (address == 0 || address == -1)
-                return;
-
-        table = (struct hpet_table *) address;
-
-	hpet_base_p = table->base_address.address;
-	free((void *) address);
-}
-#endif
-
 static void hpet_parse_check_base(fwts_framework *fw,
 	const char *table, fwts_list_link *item)
 {
@@ -74,14 +52,14 @@ static void hpet_parse_check_base(fwts_framework *fw,
 				fwts_failed(fw, LOG_LEVEL_MEDIUM,
 					"HPETBaseMismatch",
 					"Mismatched HPET base between %s (%" PRIx64 ") "
-					"and the kernel (%" PRIx64 ").",
+					"and the kernel (0x%" PRIx64 ").",
 					table,
 					hpet_base_p,
 					address_base);
 			else
 				fwts_passed(fw,
 					"HPET base matches that between %s and "
-					"the kernel (%" PRIx64 ").",
+					"the kernel (0x%" PRIx64 ").",
 					table,
 					hpet_base_p);
 		}
@@ -185,7 +163,7 @@ static int hpet_check_test1(fwts_framework *fw)
 			if (str) {
 				hpet_base_p = strtoul(str+6,  NULL, 0x10);
 				fwts_passed(fw,
-					"Found HPET base %" PRIx64 " in kernel log.",
+					"Found HPET base 0x%" PRIx64 " in kernel log.",
 					hpet_base_p);
 				break;
 			}
@@ -198,7 +176,7 @@ static int hpet_check_test1(fwts_framework *fw)
 			if (str) {
 				hpet_base_p = strtoul(str+8,  NULL, 0x10);
 				fwts_passed(fw,
-					"Found HPET base %" PRIx64 " in kernel log.",
+					"Found HPET base 0x%" PRIx64 " in kernel log.",
 					hpet_base_p);
 				break;
 			}
@@ -213,8 +191,187 @@ static int hpet_check_test1(fwts_framework *fw)
 	return FWTS_OK;
 }
 
+/*
+ * Sanity check HPET table, see:
+ *    http://www.intel.co.uk/content/www/uk/en/software-developers/software-developers-hpet-spec-1-0a.html
+ */
 static int hpet_check_test2(fwts_framework *fw)
 {
+	fwts_acpi_table_info *table;
+	fwts_acpi_table_hpet *hpet;
+	bool passed = true;
+	char *page_prot;
+
+	if (fwts_acpi_find_table(fw, "HPET", 0, &table) != FWTS_OK) {
+		fwts_log_error(fw, "Cannot read ACPI table HPET.");
+		return FWTS_ERROR;
+	}
+
+	if (table == NULL) {
+		fwts_log_error(fw, "ACPI table HPET does not exist!");
+		return FWTS_ERROR;
+	}
+
+	if (table->length < sizeof(fwts_acpi_table_hpet)) {
+		fwts_failed(fw, LOG_LEVEL_HIGH, "HPETAcpiTableTooSmall",
+			"HPET ACPI table is %zd bytes long which is smaller "
+			"than the expected size of %zd bytes.",
+			table->length, sizeof(fwts_acpi_table_hpet));
+		return FWTS_ERROR;
+	}
+
+	hpet = (fwts_acpi_table_hpet *)table->data;
+
+	if (hpet->base_address.address == 0) {
+		fwts_failed(fw, LOG_LEVEL_HIGH, "HPETAcpiBaseAddressNull",
+			"HPET base address in ACPI HPET table is null.");
+		passed = false;
+	}
+
+	/*
+	 * If we've already figured out the HPET base address then
+	 * sanity check it against HPET. This should never happen.
+	 */
+	if ((hpet_base_p != 0) &&
+	    (hpet_base_p != hpet->base_address.address)) {
+		fwts_failed(fw, LOG_LEVEL_MEDIUM,
+			"HPETAcpiBaseAddressDifferFromKernel",
+			"HPET base address in ACPI HPET table "
+			"is 0x%" PRIx64 " which is different from "
+			"the kernel HPET base address of "
+			"0x%" PRIx64 ".",
+			hpet->base_address.address, hpet_base_p);
+		passed = false;
+	}
+
+#if 0
+	/*
+	 *  The Intel spec states that the address space ID
+	 *  must be memory or I/O space.
+	 */
+	if ((hpet->base_address.address_space_id != 0) &&
+	    (hpet->base_address.address_space_id != 1)) {
+		fwts_failed(fw, LOG_LEVEL_HIGH,
+			"HPETAcpiBaseAddressSpaceId",
+			"The HPET base address space ID was 0x%" PRIx8
+			", was expecting 0 (System Memory) or "
+			"1 (System I/O).",
+			hpet->base_address.address_space_id);
+		passed = false;
+	}
+#endif
+	/*
+	 *  The kernel expects the HPET address space ID
+	 *  to be memory.
+	 */
+	if (hpet->base_address.address_space_id != 0) {
+		fwts_failed(fw, LOG_LEVEL_HIGH,
+			"HPETAcpiBaseAddressSpaceIdNotMemory",
+			"The HPET base address space ID was 0x%" PRIx8
+			", however, the Kernel expects the HPET address "
+			"space ID to be 0 (System Memory). The kernel "
+			"will not parse this and the HPET configuration "
+			"will be ignored.",
+			hpet->base_address.address_space_id);
+	}
+
+	/*
+	 *  Some broken firmware advertises the HPET at
+	 *  0xfed0000000000000 instead of 0xfed00000. The kernel
+	 *  detects this and fixes it, but even so, it is wrong
+	 *  so lets check for this.
+	 */
+	if ((hpet->base_address.address >> 32) != 0) {
+		fwts_failed(fw, LOG_LEVEL_CRITICAL,
+			"HPETAcpiBaseAddressBogus",
+			"The HPET base address is bogus: 0x%" PRIx64 ".",
+			hpet->base_address.address);
+		fwts_advice(fw,
+			"Bogus HPET base address can be worked around "
+			"by using the kernel parameter 'hpet=force' if "
+			"the base addess is 0xfed0000000000000. "
+			"This will make the kernel shift the address "
+			"down 32 bits to 0xfed00000.");
+		passed = false;
+	}
+
+	/*
+	 * We don't need to check for GAS address space widths etc
+	 * since the kernel does not care and the spec doesn't
+	 * stipulate these need to be sane
+	 */
+
+	/*
+	 *   Dump out HPET
+	 */
+	fwts_log_info_verbatum(fw, "Hardware ID of Event Block:");
+	fwts_log_info_verbatum(fw, "  PCI Vendor ID              : 0x%" PRIx32,
+		(hpet->event_timer_block_id >> 16) & 0xffff);
+	fwts_log_info_verbatum(fw, "  Legacy IRQ Routing Capable : %" PRIu32,
+		(hpet->event_timer_block_id >> 15) & 1);
+	fwts_log_info_verbatum(fw, "  COUNT_SIZE_CAP counter size: %" PRIu32,
+		(hpet->event_timer_block_id >> 13) & 1);
+	fwts_log_info_verbatum(fw, "  Number of comparitors      : %" PRIu32,
+		(hpet->event_timer_block_id >> 8) & 0x1f);
+	fwts_log_info_verbatum(fw, "  Hardwre Revision ID        : 0x%" PRIx8,
+		hpet->event_timer_block_id & 0xff);
+
+	fwts_log_info_verbatum(fw, "Lower 32 bit base Address    : 0x%" PRIx64,
+		hpet->base_address.address);
+	fwts_log_info_verbatum(fw, "  Address Space ID           : 0x%" PRIx8,
+		hpet->base_address.address_space_id);
+	fwts_log_info_verbatum(fw, "  Register Bit Width         : 0x%" PRIx8,
+		hpet->base_address.register_bit_width);
+	fwts_log_info_verbatum(fw, "  Register Bit Offset        : 0x%" PRIx8,
+		hpet->base_address.register_bit_offset);
+	fwts_log_info_verbatum(fw, "  Address Width              : 0x%" PRIx8,
+		hpet->base_address.access_width);
+	fwts_log_info_verbatum(fw, "HPET sequence number         : 0x%" PRIx8,
+		hpet->hpet_number);
+	fwts_log_info_verbatum(fw, "Minimum clock tick           : 0x%" PRIx16,
+		hpet->main_counter_minimum);
+
+	switch (hpet->page_prot_and_oem_attribute & 0xf) {
+	case 0:
+		page_prot = "No guaranteed protection";
+		break;
+	case 1:
+		page_prot = "4K page protected";
+		break;
+	case 2:
+		page_prot = "64K page protected";
+		break;
+	default:
+		page_prot = "Reserved";
+		break;
+	}
+	fwts_log_info_verbatum(fw, "Page Protection              : 0x%" PRIx8 " (%s)",
+		hpet->page_prot_and_oem_attribute & 0xf,
+		page_prot);
+	fwts_log_info_verbatum(fw, "OEM attributes               : 0x%" PRIx8,
+		(hpet->page_prot_and_oem_attribute >> 4) & 0xf);
+
+	if (passed)
+		fwts_passed(fw, "HPET looks sane.");
+
+	return FWTS_OK;
+}
+
+static int hpet_check_test3(fwts_framework *fw)
+{
+	int i;
+
+	hpet_check_base_acpi_table(fw, "DSDT", 0);
+
+	for (i=0;i<11;i++) {
+		hpet_check_base_acpi_table(fw, "SSDT", i);
+	}
+	return FWTS_OK;
+}
+
+
+static int hpet_check_test4(fwts_framework *fw)
+{
 	uint64_t hpet_id;
 	uint32_t vendor_id;
 	uint32_t clk_period;
@@ -238,7 +395,7 @@ static int hpet_check_test2(fwts_framework *fw)
 			"Invalid Vendor ID: %04" PRIx32 " - this should be configured.",
 			vendor_id);
 	else
-		fwts_passed(fw, "Vendor ID looks sane: %04" PRIx32 ".", vendor_id);
+		fwts_passed(fw, "Vendor ID looks sane: 0x%04" PRIx32 ".", vendor_id);
 
 	clk_period = hpet_id >> 32;
 	if ((clk_period > MAX_CLK_PERIOD) || (clk_period == 0))
@@ -253,21 +410,12 @@ static int hpet_check_test2(fwts_framework *fw)
 	return FWTS_OK;
 }
 
-static int hpet_check_test3(fwts_framework *fw)
-{
-	int i;
-
-	hpet_check_base_acpi_table(fw, "DSDT", 0);
-	for (i=0;i<11;i++) {
-		hpet_check_base_acpi_table(fw, "SSDT", i);
-	}
-	return FWTS_OK;
-}
 
 static fwts_framework_minor_test hpet_check_tests[] = {
 	{ hpet_check_test1, "Check HPET base in kernel log." },
-	{ hpet_check_test2, "Sanity check HPET configuration." },
+	{ hpet_check_test2, "Check HPET base in HPET table. "},
 	{ hpet_check_test3, "Check HPET base in DSDT and/or SSDT. "},
+	{ hpet_check_test4, "Sanity check HPET configuration." },
 	{ NULL, NULL }
 };
 
-- 
1.8.0




More information about the fwts-devel mailing list