[PATCH 1/2] acpi: add fwts_acpi_space_id_check to check GAS adrress space ids
Alex Hung
alex.hung at canonical.com
Thu Jan 7 00:34:05 UTC 2021
Signed-off-by: Alex Hung <alex.hung at canonical.com>
---
src/acpi/ecdt/ecdt.c | 25 ++-------
src/acpi/einj/einj.c | 12 +----
src/acpi/mchi/mchi.c | 16 ++----
src/acpi/pcct/pcct.c | 27 ++++------
src/acpi/spmi/spmi.c | 20 ++------
src/acpi/tcpa/tcpa.c | 26 ++++------
src/lib/include/fwts_acpi_tables.h | 1 +
src/lib/src/fwts_acpi_tables.c | 82 ++++++++++++++++++++++++++++++
8 files changed, 117 insertions(+), 92 deletions(-)
diff --git a/src/acpi/ecdt/ecdt.c b/src/acpi/ecdt/ecdt.c
index 23e2ae93..0cf81893 100644
--- a/src/acpi/ecdt/ecdt.c
+++ b/src/acpi/ecdt/ecdt.c
@@ -41,18 +41,9 @@ static int ecdt_test1(fwts_framework *fw)
uint32_t min_length;
int i;
-
/* Must be I/O Address Space or a Memory Space */
- if ((ecdt->ec_control.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) &&
- (ecdt->ec_control.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY)) {
- fwts_failed(fw, LOG_LEVEL_MEDIUM,
- "ECDTECControlInvalidAddrSpaceID",
- "ECDT EC_CONTROL address space ID is 0x%2.2" PRIx8
- "which is not a System I/O Space ID or a System "
- "Memory Space ID",
- ecdt->ec_control.address_space_id);
- passed = false;
- }
+ fwts_acpi_space_id_check(fw, "ECDT", "EC_CONTROL", &passed, ecdt->ec_control.address_space_id, 2,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY, FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO);
/* Must be correct Access Size */
if (ecdt->ec_control.access_width > 4) {
fwts_failed(fw, LOG_LEVEL_MEDIUM,
@@ -63,16 +54,8 @@ static int ecdt_test1(fwts_framework *fw)
passed = false;
}
/* Must be I/O Address Space or a Memory Space */
- if ((ecdt->ec_data.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) &&
- (ecdt->ec_data.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY)) {
- fwts_failed(fw, LOG_LEVEL_MEDIUM,
- "ECDTECControlInvalidAddrSpaceID",
- "ECDT EC_DATA address space ID is 0x%2.2" PRIx8
- "which is not a System I/O Space ID or a System "
- "Memory Space ID",
- ecdt->ec_data.address_space_id);
- passed = false;
- }
+ fwts_acpi_space_id_check(fw, "ECDT", "EC_DATA", &passed, ecdt->ec_control.address_space_id, 2,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY, FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO);
/* Must be correct Access Size */
if (ecdt->ec_data.access_width > 4) {
fwts_failed(fw, LOG_LEVEL_MEDIUM,
diff --git a/src/acpi/einj/einj.c b/src/acpi/einj/einj.c
index e6f8a7e9..de303f65 100644
--- a/src/acpi/einj/einj.c
+++ b/src/acpi/einj/einj.c
@@ -104,16 +104,8 @@ static int einj_test1(fwts_framework *fw)
}
fwts_acpi_reserved_zero_check(fw, "EINJ", "Reserved", entry->reserved, sizeof(entry->reserved), &passed);
-
- if (gas.address_space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
- gas.address_space_id != ACPI_ADR_SPACE_SYSTEM_IO) {
- fwts_failed(fw, LOG_LEVEL_MEDIUM,
- "EINJBadSpaceId",
- "EINJ Address_Space_ID must be within 0 or "
- "1, got 0x%" PRIx8 " instead",
- gas.address_space_id);
- passed = false;
- }
+ fwts_acpi_space_id_check(fw, "EINJ", "Register Region", &passed, gas.address_space_id, 2,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY, FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO);
fwts_log_nl(fw);
}
diff --git a/src/acpi/mchi/mchi.c b/src/acpi/mchi/mchi.c
index 446c5722..7fe59ded 100644
--- a/src/acpi/mchi/mchi.c
+++ b/src/acpi/mchi/mchi.c
@@ -123,19 +123,13 @@ static int mchi_test1(fwts_framework *fw)
mchi->global_system_interrupt);
}
- if ((mchi->base_address.address_space_id != 0x00) &&
- (mchi->base_address.address_space_id != 0x01) &&
- (mchi->base_address.address_space_id != 0x04)) {
- passed = false;
- fwts_failed(fw, LOG_LEVEL_HIGH,
- "MCHIAddrSpaceIdInvalid",
- "MCHI Address Space ID is 0x%2.2" PRIx8 " and should be "
- "0x00 (System Memory), 0x01 (System I/O) or 0x04 (SMBus)",
- mchi->base_address.address_space_id);
- }
+ fwts_acpi_space_id_check(fw, "MCHI", "Base Address", &passed, mchi->base_address.address_space_id, 3,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO,
+ FWTS_GAS_ADDR_SPACE_ID_SMBUS);
/* SMBus specific checks */
- if (mchi->base_address.address_space_id == 0x04) {
+ if (mchi->base_address.address_space_id == FWTS_GAS_ADDR_SPACE_ID_SMBUS) {
if ((mchi->pci_device_flag & 0x01) == 1) {
passed = false;
fwts_failed(fw, LOG_LEVEL_HIGH,
diff --git a/src/acpi/pcct/pcct.c b/src/acpi/pcct/pcct.c
index a255bb30..e2e40164 100644
--- a/src/acpi/pcct/pcct.c
+++ b/src/acpi/pcct/pcct.c
@@ -41,21 +41,19 @@ static bool subspace_length_equal(fwts_framework *fw, uint8_t type, uint8_t type
static void gas_messages(fwts_framework *fw, uint8_t type, fwts_acpi_gas *gas, bool *passed)
{
+ char label[20];
+
fwts_log_info_verbatim(fw, " Address Space ID 0x%2.2" PRIx8, gas->address_space_id);
fwts_log_info_verbatim(fw, " Register Bit Width 0x%2.2" PRIx8, gas->register_bit_width);
fwts_log_info_verbatim(fw, " Register Bit Offset 0x%2.2" PRIx8, gas->register_bit_offset);
fwts_log_info_verbatim(fw, " Access Size 0x%2.2" PRIx8, gas->access_width);
fwts_log_info_verbatim(fw, " Address 0x%16.16" PRIx64, gas->address);
- if ((gas->address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) &&
- (gas->address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY) &&
- (gas->address_space_id != FWTS_GAS_ADDR_SPACE_ID_FFH)) {
- *passed = false;
- fwts_failed(fw, LOG_LEVEL_HIGH,
- "PCCTSubspaceInvalidAddrSpaceID",
- "PCCT Subspace Type %" PRId8 " has space ID = %" PRId8
- " which is not System I/O, Memory or FFH", type, gas->address_space_id);
- }
+ snprintf(label, 20, "Subspace Type % " PRId8, type);
+ fwts_acpi_space_id_check(fw, "PCCT", label, passed, gas->address_space_id, 3,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO,
+ FWTS_GAS_ADDR_SPACE_ID_FFH);
}
static void memory_length(fwts_framework *fw, uint8_t type, uint64_t memory_range, uint64_t min_length, bool *passed)
@@ -108,15 +106,8 @@ static void generic_comm_test(fwts_framework *fw, fwts_acpi_table_pcct_subspace_
fwts_log_info_verbatim(fw, " Max Periodic Access Rate: 0x%8.8" PRIx32, entry->max_periodic_access_rate);
fwts_log_info_verbatim(fw, " Min Request Turnaround Time: 0x%8.8" PRIx32, entry->min_request_turnaround_time);
- if ((gas->address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) &&
- (gas->address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY)) {
- *passed = false;
- fwts_failed(fw, LOG_LEVEL_HIGH,
- "PCCTSubspaceInvalidAddrSpaceID",
- "PCCT Subspace Type 0 has space ID = 0x%2.2" PRIx8
- " which is not System I/O or Memory",
- gas->address_space_id);
- }
+ fwts_acpi_space_id_check(fw, "PCCT", "Subspace Type 0", passed, gas->address_space_id, 2,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY, FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO);
}
static void hw_reduced_comm_test_type1(fwts_framework *fw, fwts_acpi_table_pcct_subspace_type_1 *entry, bool *passed)
diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
index 4eed3518..8971ad21 100644
--- a/src/acpi/spmi/spmi.c
+++ b/src/acpi/spmi/spmi.c
@@ -125,21 +125,11 @@ static int spmi_test1(fwts_framework *fw)
}
/* Base address must be one of 3 types, System Memory, System I/O or SMBUS */
- if ((spmi->base_address.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY) &&
- (spmi->base_address.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) &&
- (spmi->base_address.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SMBUS)) {
- passed = false;
- fwts_failed(fw, LOG_LEVEL_MEDIUM,
- "SPMIBaseAddressSpaceID",
- "SPMI Base Address Space ID is 0x%2.2" PRIx8 " but should be one of "
- "0x%2.2" PRIx8 " (System Memory), "
- "0x%2.2" PRIx8 " (System I/O) or "
- "0x%2.2" PRIx8 " (SMBUS)",
- spmi->base_address.address_space_id,
- FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY,
- FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO,
- FWTS_GAS_ADDR_SPACE_ID_SMBUS);
- }
+ fwts_acpi_space_id_check(fw, "SPMI", "Base Address", &passed, spmi->base_address.address_space_id, 3,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO,
+ FWTS_GAS_ADDR_SPACE_ID_SMBUS);
+
/*
* For SSIF: The Address_Space_ID is SMBUS and the address field of the GAS
* holds the 7-bit slave address of the BMC on the host SMBus
diff --git a/src/acpi/tcpa/tcpa.c b/src/acpi/tcpa/tcpa.c
index 27107f09..a5b0d512 100644
--- a/src/acpi/tcpa/tcpa.c
+++ b/src/acpi/tcpa/tcpa.c
@@ -100,23 +100,15 @@ static int tcpa_server_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
}
}
- if (tcpa->server.base_addr.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY &&
- tcpa->server.base_addr.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) {
- passed = false;
- fwts_failed(fw, LOG_LEVEL_HIGH,
- "TCPABadAddressID",
- "TCPA base address ID must be 1 or zero, got "
- "0x%2.2" PRIx8 " instead", tcpa->server.base_addr.address_space_id);
- }
-
- if (tcpa->server.config_addr.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY &&
- tcpa->server.config_addr.address_space_id != FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO) {
- passed = false;
- fwts_failed(fw, LOG_LEVEL_HIGH,
- "TCPABadAddressID",
- "TCPA configuration address ID must be 1 or zero, got "
- "0x%2.2" PRIx8 " instead", tcpa->server.config_addr.address_space_id);
- }
+ fwts_acpi_space_id_check(fw, "TCPA", "Base Address", &passed,
+ tcpa->server.base_addr.address_space_id, 2,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO);
+
+ fwts_acpi_space_id_check(fw, "TCPA", "Configuration Address", &passed,
+ tcpa->server.config_addr.address_space_id, 2,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_MEMORY,
+ FWTS_GAS_ADDR_SPACE_ID_SYSTEM_IO);
return passed;
}
diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
index 602fb571..fcfb9198 100644
--- a/src/lib/include/fwts_acpi_tables.h
+++ b/src/lib/include/fwts_acpi_tables.h
@@ -70,6 +70,7 @@ void fwts_acpi_reserved_type_check(fwts_framework *fw, const char *table, uint8_
bool fwts_acpi_table_length_check(fwts_framework *fw, const char *table, uint32_t length, uint32_t size);
bool fwts_acpi_structure_length_check(fwts_framework *fw, const char *table, uint8_t subtable_type, uint32_t subtable_length, uint32_t size);
void fwts_acpi_fixed_value_check(fwts_framework *fw, fwts_log_level level, const char *table, const char *field, uint8_t actual, uint8_t must_be, bool *passed);
+void fwts_acpi_space_id_check(fwts_framework *fw, const char *table, const char *field, bool *passed, uint8_t actual, uint8_t num_type, ...);
uint32_t fwts_get_acpi_version(fwts_framework *fw);
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
index 2d0082b1..d88d31e4 100644
--- a/src/lib/src/fwts_acpi_tables.c
+++ b/src/lib/src/fwts_acpi_tables.c
@@ -1613,6 +1613,88 @@ void fwts_acpi_reserved_type_check(
}
}
+/*
+ * Space Id defined for Generic Address Structure (GAS)
+ * See Table 5.1: Generic Address Structure (GAS) in ACPI spec
+ * Also see fwts_acpi.h
+ */
+static const char *gas_space_id_names[] = {
+ "System Memory (0x0)",
+ "System I/O (0x1)",
+ "PCI Configuration (0x2)",
+ "Embedded Controller (0x3)",
+ "SMBus (0x4)",
+ "SystemCMOS (0x5)",
+ "PciBarTarget (0x6)",
+ "IPMI (0x7)",
+ "General PurposeIO (0x8)",
+ "GenericSerialBus (0x9)",
+ "Platform Communications Channel (0xa)",
+ "Functional Fixed Hardware (0x7f)",
+};
+
+static const char *get_space_id_name(uint8_t id) {
+ if (id <= 0x0a)
+ return gas_space_id_names[id];
+ else if (id == 0x7f)
+ return gas_space_id_names[0x0b];
+ else
+ return NULL;
+}
+
+/*
+ * fwts_acpi_space_id_check()
+ * check whether gas space id matches
+ */
+void fwts_acpi_space_id_check(
+ fwts_framework *fw,
+ const char *table,
+ const char *field,
+ bool *passed,
+ uint8_t actual,
+ uint8_t num_type,
+ ...)
+{
+ bool matched = false;
+ char must_be_id[255];
+ uint16_t length = 0;
+ uint8_t i, must_be;
+ char label[25];
+ va_list ap;
+
+ strncpy(label, table, 4); /* ACPI table name is 4 char long */
+ strncpy(label + 4, "BadAddressSpaceId", sizeof(label) - 4);
+ memset(must_be_id, 0, strlen(must_be_id));
+
+ va_start(ap, num_type);
+ for (i = 0; i < num_type; i++) {
+ must_be = va_arg(ap, int);
+ if (actual == must_be) {
+ matched = true;
+ break;
+ }
+
+ length += strlen(get_space_id_name(must_be));
+ if (length > 255)
+ continue;
+
+ strncat(must_be_id, get_space_id_name(must_be), strlen(get_space_id_name(must_be)));
+ if (i < num_type - 2)
+ strncat(must_be_id, ", ", 2);
+ else if (i == num_type - 2)
+ strncat(must_be_id, " or ", 4);
+ }
+
+ if (!matched) {
+ fwts_failed(fw, LOG_LEVEL_HIGH, label,
+ "%4.4s %s Space ID must be one of %s, got %s instead.",
+ table, field, must_be_id, get_space_id_name(actual));
+ *passed = false;
+ }
+
+ va_end(ap);
+}
+
/*
* fwts_acpi_table_length_check()
* verify whether table length is sane
--
2.25.1
More information about the fwts-devel
mailing list