From patchwork Tue Dec 11 23:35:32 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: hpet: hpetcheck: add ACPI HPET table check Date: Tue, 11 Dec 2012 13:35:32 -0000 From: Colin King X-Patchwork-Id: 205340 Message-Id: <1355268932-8644-1-git-send-email-colin.king@canonical.com> To: fwts-devel@lists.ubuntu.com From: Colin Ian King 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 Acked-by: Ivan Hu Acked-by: Alex Hung --- 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 } };