| Submitter | Colin King |
|---|---|
| Date | Dec. 11, 2012, 11:35 p.m. |
| Message ID | <1355268932-8644-1-git-send-email-colin.king@canonical.com> |
| Download | mbox | patch |
| Permalink | /patch/205340/ |
| State | Accepted |
| Headers | show |
Comments
On 12/12/2012 07:35 AM, Colin King wrote: > From: Colin Ian King <colin.king@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@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 } > }; > > Acked-by: Ivan Hu <ivan.hu@canonical.com>
On 12/12/2012 07:35 AM, Colin King wrote: > From: Colin Ian King <colin.king@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@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 } > }; > > Acked-by: Alex Hung <alex.hung@canonical.com>
Patch
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 } };