Add in several more tests to the RSDP code that check for spec details that had not been checked for previously. This then allows us to tag the RSDP tests so that they will also be executed when running FWTS to check ACPI specification compliance (i.e., using the --acpicompliance parameter).
Signed-off-by: Al Stone al.stone@linaro.org --- src/acpi/rsdp/rsdp.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 142 insertions(+), 9 deletions(-)
diff --git a/src/acpi/rsdp/rsdp.c b/src/acpi/rsdp/rsdp.c index 5bc0215..bf830f0 100644 --- a/src/acpi/rsdp/rsdp.c +++ b/src/acpi/rsdp/rsdp.c @@ -31,9 +31,28 @@ static int rsdp_init(fwts_framework *fw) fwts_log_error(fw, "Cannot read ACPI tables."); return FWTS_ERROR; } - if (table == NULL || (table && table->length == 0)) { - fwts_log_error(fw, "ACPI RSDP table does not exist, skipping test"); - return FWTS_SKIP; + if (!table) { + /* + * Really old x86 machines may not have an RSDP, but + * ia64 and arm64 are both required to be UEFI, so + * therefore must have an RSDP. + */ + if (fw->target_arch == FWTS_ARCH_X86) + return FWTS_SKIP; + else { + fwts_log_error(fw, + "ACPI RSDP is required for the " + "%s target architecture.", + fwts_arch_get_name(fw->target_arch)); + return FWTS_ERROR; + } + } + + /* We know there is an RSDP now, so do a quick sanity check */ + if (table->length == 0) { + fwts_log_error(fw, + "ACPI RSDP table has zero length"); + return FWTS_ERROR; } return FWTS_OK; } @@ -46,6 +65,18 @@ static int rsdp_test1(fwts_framework *fw) fwts_acpi_table_rsdp *rsdp = (fwts_acpi_table_rsdp *)table->data; bool passed = true; size_t i; + int value; + uint8_t *ptr; + uint8_t checksum; + + /* verify first checksum */ + checksum = fwts_checksum(table->data, 20); + if (checksum == 0) + fwts_passed(fw, "RSDP first checksum is correct"); + else + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPBadFirstChecksum", + "RSDP first checksum is incorrect: 0x%x", checksum);
for (i = 0; i < 6; i++) { if (!isprint(rsdp->oem_id[i])) { @@ -53,7 +84,11 @@ static int rsdp_test1(fwts_framework *fw) break; } } - if (!passed) { + if (passed) + fwts_passed(fw, + "RSDP: oem_id contains only printable " + "characters."); + else { fwts_failed(fw, LOG_LEVEL_LOW, "RSDPBadOEMId", "RSDP: oem_id contains non-printable " @@ -64,16 +99,114 @@ static int rsdp_test1(fwts_framework *fw) "this should be fixed if possible to make the " "firmware ACPI complaint."); } - if (rsdp->revision > 2) { - passed = false; + + if (rsdp->revision <= 2) + fwts_passed(fw, + "RSDP: revision is %" PRIu8 ".", rsdp->revision); + else fwts_failed(fw, LOG_LEVEL_MEDIUM, "RSDPBadRevisionId", "RSDP: revision is %" PRIu8 ", expected " "value less than 2.", rsdp->revision); - }
+ /* whether RSDT or XSDT depends arch */ + if (rsdp->rsdt_address == 0 && rsdp->xsdt_address == 0) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPNoAddressesFound", + "RSDP: either the RsdtAddress must be non-zero " + "or the XsdtAddress must be non-zero. Both " + "fields are zero."); + else + fwts_passed(fw, + "RSDP: at least one of RsdtAddress or XsdtAddress " + "is non-zero."); + + if (rsdp->rsdt_address != 0 && rsdp->xsdt_address != 0) + if ((uint64_t)rsdp->rsdt_address == rsdp->xsdt_address) { + fwts_warning(fw, + "Both RSDT and XSDT addresses are set. " + "Since they are the same address, this " + "is unambiguous to the OS."); + fwts_advice(fw, + "Set only one of the 32-bit RSDT or the " + "64-bit XSDT addresses. Recent versions " + "of the spec require that only one of " + "these be used but as a practical matter, " + "many vendors do use both. If both " + "fields must be used, make sure they at " + "least contain the same value so that " + "the OS can unambiguously determine " + "which address is the correct one."); + } else { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPBothAddressesFound", + "RSDP: only one of RsdtAddress or " + "XsdtAddress should be non-zero. Both " + "fields are non-zero."); + } + else + fwts_passed(fw, + "RSDP: only one of RsdtAddress or XsdtAddress " + "is non-zero."); + + passed = false; + switch (fw->target_arch) { + case FWTS_ARCH_X86: + if (rsdp->rsdt_address != 0 || rsdp->xsdt_address != 0) + passed = true; + break; + + case FWTS_ARCH_ARM64: + if (rsdp->xsdt_address != 0) + passed = true; + break; + + default: + passed = true; + fwts_log_advice(fw, + "No clear rule for RSDT/XSDT address usage " + "is provided for the %s architecture.", + fwts_arch_get_name(fw->target_arch)); + } if (passed) - fwts_passed(fw, "No issues found in RSDP table."); + fwts_passed(fw, + "RSDP: the correct RSDT/XSDT address is being " + "used."); + else + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPBadAddresses", + "RSDP: the wrong RSDT/XSDT address is being " + "used for the %s architecture.", + fwts_arch_get_name(fw->target_arch)); + + /* check the length field */ + if (rsdp->length == 36) + fwts_passed(fw, "RSDP: the table is the correct length."); + else + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPBadLength", + "RSDP: length is %d bytes but should be 36.", + rsdp->length); + + /* verify the extended checksum */ + checksum = fwts_checksum(table->data, 36); + if (checksum == 0) + fwts_passed(fw, "RSDP second checksum is correct"); + else + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPBadSecondChecksum", + "RSDP second checksum incorrect: 0x%x", checksum); + + /* the reserved field should be zero */ + value = 0; + for (ptr = (uint8_t *)rsdp->reserved, i = 0; i < 3; i++) + value += *ptr++; + if (value) + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "RSDPReserved", + "RSDP: reserved field is non-zero."); + else + fwts_passed(fw, "RSDP: the reserved field is zero.");
return FWTS_OK; } @@ -90,4 +223,4 @@ static fwts_framework_ops rsdp_ops = { };
FWTS_REGISTER("rsdp", &rsdp_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | - FWTS_FLAG_TEST_ACPI) + FWTS_FLAG_TEST_ACPI | FWTS_FLAG_TEST_COMPLIANCE_ACPI)