[PATCH 1/2] acpi: check reserved fields by fwts_acpi_reserved_zero_check

Alex Hung alex.hung at canonical.com
Sat Dec 19 03:49:17 UTC 2020


Signed-off-by: Alex Hung <alex.hung at canonical.com>
---
 src/acpi/cpep/cpep.c        | 16 +++++++--------
 src/acpi/dbg2/dbg2.c        |  2 ++
 src/acpi/dbgp/dbgp.c        | 15 +++++++++-----
 src/acpi/einj/einj.c        |  4 ++--
 src/acpi/facs/facs.c        | 16 ++++++---------
 src/acpi/fpdt/fpdt.c        |  4 ++++
 src/acpi/hest/hest.c        | 40 ++++++++++++++++++++-----------------
 src/acpi/rsdp/rsdp.c        |  9 ++++-----
 src/acpi/spmi/spmi.c        |  8 +-------
 src/lib/include/fwts_acpi.h |  2 +-
 10 files changed, 59 insertions(+), 57 deletions(-)

diff --git a/src/acpi/cpep/cpep.c b/src/acpi/cpep/cpep.c
index ebd2beb3..42b09a14 100644
--- a/src/acpi/cpep/cpep.c
+++ b/src/acpi/cpep/cpep.c
@@ -49,6 +49,7 @@ static int cpep_test1(fwts_framework *fw)
 {
 	fwts_acpi_table_cpep *cpep = (fwts_acpi_table_cpep*)table->data;
 	bool passed = true;
+	uint64_t reserved;
 	uint32_t i, n;
 
 	fwts_log_info_verbatim(fw, "CPEP Corrected Platform Error Polling Table:");
@@ -58,15 +59,12 @@ static int cpep_test1(fwts_framework *fw)
 		cpep->reserved[0], cpep->reserved[1], cpep->reserved[2], cpep->reserved[3],
 		cpep->reserved[4], cpep->reserved[4], cpep->reserved[6], cpep->reserved[7]);
 
-	if (cpep->reserved[0] | cpep->reserved[1] |
-	    cpep->reserved[2] | cpep->reserved[3] |
-	    cpep->reserved[4] | cpep->reserved[5] |
-            cpep->reserved[6] | cpep->reserved[7]) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"CPEPNonZeroReserved",
-			"CPEP: Reserved field contains a non-zero value");
-	}
+	reserved = cpep->reserved[0] + ((uint64_t) cpep->reserved[1] << 8) +
+		   ((uint64_t) cpep->reserved[2] << 16) + ((uint64_t) cpep->reserved[3] << 24) +
+		   ((uint64_t) cpep->reserved[4] << 32) + ((uint64_t) cpep->reserved[5] << 40) +
+		   ((uint64_t) cpep->reserved[6] << 48) + ((uint64_t) cpep->reserved[7] << 56);
+	fwts_acpi_reserved_zero_check(fw, "CPEP", "Reserved", reserved, sizeof(reserved), &passed);
+
 	n = (table->length - sizeof(fwts_acpi_table_cpep)) /
 		sizeof(fwts_acpi_cpep_processor_info);
 
diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
index d904620d..d0e72fe1 100644
--- a/src/acpi/dbg2/dbg2.c
+++ b/src/acpi/dbg2/dbg2.c
@@ -285,6 +285,8 @@ static int dbg2_test1(fwts_framework *fw)
 				info->port_subtype);
 		}
 
+		fwts_acpi_reserved_zero_check(fw, "DBG2", "Info Structure Reserved", info->reserved, sizeof(info->reserved), &passed);
+
 		length_ok = true;
 		dbg2_check_offset(fw, table->length, offset + info->length,
 			"DBG2 Info Structure Namespace Length", &length_ok);
diff --git a/src/acpi/dbgp/dbgp.c b/src/acpi/dbgp/dbgp.c
index fffa4762..1c38e161 100644
--- a/src/acpi/dbgp/dbgp.c
+++ b/src/acpi/dbgp/dbgp.c
@@ -49,9 +49,10 @@ static int dbgp_init(fwts_framework *fw)
  */
 static int dbgp_test1(fwts_framework *fw)
 {
-	bool passed = true;
-	char *interface_type;
 	fwts_acpi_table_dbgp *dbgp = (fwts_acpi_table_dbgp *)table->data;
+	char *interface_type;
+	bool passed = true;
+	uint32_t reserved;
 
 	if (!fwts_acpi_table_length_check(fw, "DBGP", table->length, sizeof(fwts_acpi_table_dbgp))) {
 		passed = false;
@@ -70,12 +71,13 @@ static int dbgp_test1(fwts_framework *fw)
 		break;
 	}
 
+	reserved = dbgp->reserved[0] + ((uint32_t) dbgp->reserved[1] << 8) +
+		   ((uint32_t) dbgp->reserved[2] << 16);
+
 	fwts_log_info_verbatim(fw, "DBGP Table:");
 	fwts_log_info_verbatim(fw, "  Interface Type            0x%2.2" PRIx8 " (%s)",
 		dbgp->interface_type, interface_type);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, dbgp->reserved1[0]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, dbgp->reserved1[1]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, dbgp->reserved1[2]);
+	fwts_log_info_verbatim(fw, "  Reserved:                 0x%8.8" PRIx32, reserved);
 	fwts_log_info_verbatim(fw, "  Base Address:");
 	fwts_log_info_verbatim(fw, "    Address Space ID:       0x%2.2" PRIx8, dbgp->base_address.address_space_id);
 	fwts_log_info_verbatim(fw, "    Register Bit Width      0x%2.2" PRIx8, dbgp->base_address.register_bit_width);
@@ -93,6 +95,9 @@ static int dbgp_test1(fwts_framework *fw)
 			" 0x00 (Full 16550) or 0x01 (16550 subset)",
 			dbgp->interface_type);
 	}
+
+	fwts_acpi_reserved_zero_check(fw, "DBGP", "Reserved", reserved, sizeof(reserved), &passed);
+
 	if (dbgp->base_address.register_bit_width == 0) {
 		passed = false;
 		fwts_failed(fw, LOG_LEVEL_HIGH,
diff --git a/src/acpi/einj/einj.c b/src/acpi/einj/einj.c
index 02dd4e15..e915c110 100644
--- a/src/acpi/einj/einj.c
+++ b/src/acpi/einj/einj.c
@@ -50,8 +50,8 @@ static int einj_test1(fwts_framework *fw)
 	uint32_t reserved, i;
 	bool passed = true;
 
-	reserved = einj->reserved[0] + (einj->reserved[1] << 4) +
-		   (einj->reserved[2] << 8);
+	reserved = einj->reserved[0] + ((uint32_t) einj->reserved[1] << 8) +
+		   ((uint32_t) einj->reserved[2] << 16);
 
 	fwts_log_info_verbatim(fw, "EINJ Error Injection Table:");
 	fwts_log_info_verbatim(fw, "  Injection Header Size: 0x%8.8" PRIx32,
diff --git a/src/acpi/facs/facs.c b/src/acpi/facs/facs.c
index 7dcffc3f..340771fd 100644
--- a/src/acpi/facs/facs.c
+++ b/src/acpi/facs/facs.c
@@ -49,6 +49,7 @@ static int facs_test1(fwts_framework *fw)
 {
 	fwts_acpi_table_facs *facs = (fwts_acpi_table_facs*)table->data;
 	bool passed = true;
+	uint32_t reserved;
 	int i;
 
 	if (table->length < 64) {
@@ -61,6 +62,9 @@ static int facs_test1(fwts_framework *fw)
 		goto done;
 	}
 
+	reserved = facs->reserved[0] + ((uint32_t) facs->reserved[1] << 8) +
+		   ((uint32_t) facs->reserved[2] << 16);
+
 	fwts_log_info_verbatim(fw, "FACS Firmware ACPI Control Structure:");
 	fwts_log_info_verbatim(fw, "  Signature:                '%4.4s'", facs->signature);
 	fwts_log_info_verbatim(fw, "  Length:                   0x%8.8" PRIx32, facs->length);
@@ -70,8 +74,7 @@ static int facs_test1(fwts_framework *fw)
 	fwts_log_info_verbatim(fw, "  Flags:                    0x%8.8" PRIx32, facs->flags);
 	fwts_log_info_verbatim(fw, "  X-Firmware Waking Vector: 0x%16.16" PRIx64, facs->x_firmware_waking_vector);
 	fwts_log_info_verbatim(fw, "  Version:                  0x%2.2" PRIx8, facs->version);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8 " 0x%2.2" PRIx8 " 0x%2.2" PRIx8,
-		facs->reserved[0], facs->reserved[1], facs->reserved[2]);
+	fwts_log_info_verbatim(fw, "  Reserved:                 0x%8.8" PRIx32, reserved);
 	fwts_log_info_verbatim(fw, "  OSPM Flags:               0x%8.8" PRIx32, facs->ospm_flags);
 	for (i = 0; i < 24; i+= 4) {
 		fwts_log_info_verbatim(fw, "  Reserved:                 "
@@ -122,15 +125,8 @@ static int facs_test1(fwts_framework *fw)
 			" and should be at least 64",
 			facs->length);
 	}
-	if (facs->reserved[0] |
-	    facs->reserved[1] |
-	    facs->reserved[2]) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"FACSInvalidReserved1",
-			"FACS: 1st Reserved field is non-zero");
-	}
 
+	fwts_acpi_reserved_zero_check(fw, "FACS", "Reserved", reserved, sizeof(reserved), &passed);
 	fwts_acpi_reserved_bits_check(fw, "FACS", "Flags", facs->flags, sizeof(facs->flags), 2, 31, &passed);
 	fwts_acpi_reserved_bits_check(fw, "FACS", "OSPM Flags", facs->ospm_flags, sizeof(facs->ospm_flags), 1, 31, &passed);
 
diff --git a/src/acpi/fpdt/fpdt.c b/src/acpi/fpdt/fpdt.c
index a96341b2..f5d0457b 100644
--- a/src/acpi/fpdt/fpdt.c
+++ b/src/acpi/fpdt/fpdt.c
@@ -124,6 +124,8 @@ static int fpdt_test1(fwts_framework *fw)
 				fwts_log_info_verbatim(fw, "    Reserved:	0x%8.8" PRIx32, fbbpr->reserved);
 				fwts_log_info_verbatim(fw, "    FBPT Pointer:	0x%16.16" PRIx64, fbbpr->fbpt_addr);
 
+				fwts_acpi_reserved_zero_check(fw, "FPDT", "Reserved", fbbpr->reserved, sizeof(fbbpr->reserved), &passed);
+
 				/*
 				 * For the moment, only dump the 64-bit processor-relative physical address
 				 * of the Firmware Basic Boot Performance Table, should also get and check
@@ -155,6 +157,8 @@ static int fpdt_test1(fwts_framework *fw)
 				fwts_log_info_verbatim(fw, "    Reserved:	0x%8.8" PRIx32, s3ptpr->reserved);
 				fwts_log_info_verbatim(fw, "    S3PT Pointer:	0x%16.16" PRIx64, s3ptpr->s3pt_addr);
 
+				fwts_acpi_reserved_zero_check(fw, "FPDT", "Reserved", s3ptpr->reserved, sizeof(s3ptpr->reserved), &passed);
+
 				/*
 				 * For the moment, only dump 64-bit processor-relative physical
 				 * address of the S3 Performance Table, should also get and check
diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c
index 96314136..fdc46446 100644
--- a/src/acpi/hest/hest.c
+++ b/src/acpi/hest/hest.c
@@ -53,6 +53,7 @@ static void hest_check_ia32_arch_machine_check_exception(
 	bool *passed)
 {
 	ssize_t i, total_size;
+	uint64_t reserved2;
 
 	fwts_acpi_table_hest_ia32_machine_check_exception *exception =
 		(fwts_acpi_table_hest_ia32_machine_check_exception *)*data;
@@ -79,6 +80,11 @@ static void hest_check_ia32_arch_machine_check_exception(
 		return;
 	}
 
+	reserved2 = exception->reserved2[0] + ((uint64_t) exception->reserved2[1] << 8) +
+		   ((uint64_t) exception->reserved2[2] << 16) + ((uint64_t) exception->reserved2[3] << 24) +
+		   ((uint64_t) exception->reserved2[4] << 32) + ((uint64_t) exception->reserved2[5] << 40) +
+		   ((uint64_t) exception->reserved2[6] << 48);
+
 	fwts_log_info_verbatim(fw, "HEST IA-32 Architecture Machine Check Exception:");
 	fwts_log_info_verbatim(fw, "  Type:                     0x%2.2" PRIx8, exception->type);
 	fwts_log_info_verbatim(fw, "  Source ID:                0x%4.4" PRIx16, exception->source_id);
@@ -90,15 +96,11 @@ static void hest_check_ia32_arch_machine_check_exception(
 	fwts_log_info_verbatim(fw, "  Global Capability Data:   0x%16.16" PRIx64, exception->global_capability_init_data);
 	fwts_log_info_verbatim(fw, "  Global Control Data:      0x%16.16" PRIx64, exception->global_control_init_data);
 	fwts_log_info_verbatim(fw, "  Number of Hardware Banks: 0x%8.8" PRIx32, exception->number_of_hardware_banks);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[0]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[1]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[2]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[3]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[4]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[5]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, exception->reserved2[6]);
+	fwts_log_info_verbatim(fw, "  Reserved:                 0x%16.16" PRIx64, reserved2);
 	fwts_log_nl(fw);
 
+	fwts_acpi_reserved_zero_check(fw, "HEST", "MCE Reserved1", exception->reserved1, sizeof(exception->reserved1), passed);
+
 	if (exception->flags & ~0x5) {
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
 			"HESTIA32FlagsReserved",
@@ -115,6 +117,8 @@ static void hest_check_ia32_arch_machine_check_exception(
 		*passed = false;
 	}
 
+	fwts_acpi_reserved_zero_check(fw, "HEST", "MCE Reserved2", reserved2, sizeof(reserved2), passed);
+
 	for (i = 0; i < exception->number_of_hardware_banks; i++) {
 		fwts_acpi_table_hest_machine_check_bank *bank = &exception->bank[i];
 
@@ -165,6 +169,7 @@ static void hest_check_ia32_arch_corrected_machine_check(
 	bool *passed)
 {
 	ssize_t i, total_size;
+	uint32_t reserved2;
 
 	fwts_acpi_table_hest_ia32_corrected_machine_check *check =
 		(fwts_acpi_table_hest_ia32_corrected_machine_check *)*data;
@@ -191,6 +196,9 @@ static void hest_check_ia32_arch_corrected_machine_check(
 		return;
 	}
 
+	reserved2 = check->reserved2[0] + ((uint32_t) check->reserved2[1] << 8) +
+		   ((uint32_t) check->reserved2[2] << 16);
+
 	fwts_log_info_verbatim(fw, "HEST IA-32 Architecture Machine Check:");
 	fwts_log_info_verbatim(fw, "  Type:                     0x%2.2" PRIx8, check->type);
 	fwts_log_info_verbatim(fw, "  Source ID:                0x%4.4" PRIx16, check->source_id);
@@ -217,11 +225,11 @@ static void hest_check_ia32_arch_corrected_machine_check(
 	fwts_log_info_verbatim(fw, "    Error: Thresh. Window:  0x%4.4" PRIx16,
 		check->notification.error_threshold_window);
 	fwts_log_info_verbatim(fw, "  Number of Hardware Banks: 0x%8.8" PRIx32, check->number_of_hardware_banks);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, check->reserved2[0]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, check->reserved2[1]);
-	fwts_log_info_verbatim(fw, "  Reserved:                 0x%2.2" PRIx8, check->reserved2[2]);
+	fwts_log_info_verbatim(fw, "  Reserved:                 0x%8.8" PRIx32, reserved2);
 	fwts_log_nl(fw);
 
+	fwts_acpi_reserved_zero_check(fw, "HEST", "Machine Check Reserved1", check->reserved1, sizeof(check->reserved1), passed);
+
 	if (check->flags & ~0x5) {
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
 			"HESTIA32CorrectedMachineCheckFlagsReserved",
@@ -248,6 +256,8 @@ static void hest_check_ia32_arch_corrected_machine_check(
 			check->notification.type);
 	}
 
+	fwts_acpi_reserved_zero_check(fw, "HEST", "Machine Check Reserved2", reserved2, sizeof(reserved2), passed);
+
 	for (i = 0; i < check->number_of_hardware_banks; i++) {
 		fwts_acpi_table_hest_machine_check_bank *bank = &check->bank[i];
 
@@ -323,14 +333,8 @@ static void hest_check_acpi_table_hest_nmi_error(
 	fwts_log_info_verbatim(fw, "  Max Raw Data Length:      0x%8.8" PRIx32, err->max_raw_data_length);
 	fwts_log_nl(fw);
 
-	if (err->reserved1) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"HESTInvalidRecordsToPreallocate",
-			"HEST IA-32 Architecture NMI Reserved field "
-			"at offset 4 must be zero, instead got 0x%" PRIx16,
-			err->reserved1);
-	}
+	fwts_acpi_reserved_zero_check(fw, "HEST", "NMI Reserved", err->reserved1, sizeof(err->reserved1), passed);
+
 	if (err->number_of_records_to_preallocate < 1) {
 		*passed = false;
 		fwts_failed(fw, LOG_LEVEL_HIGH,
diff --git a/src/acpi/rsdp/rsdp.c b/src/acpi/rsdp/rsdp.c
index c7985cdf..3e1a1544 100644
--- a/src/acpi/rsdp/rsdp.c
+++ b/src/acpi/rsdp/rsdp.c
@@ -205,11 +205,10 @@ static int rsdp_test1(fwts_framework *fw)
 	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
+
+	passed = true;
+	fwts_acpi_reserved_zero_check(fw, "RSDP", "Reserved", value, sizeof(value), &passed);
+	if (passed)
 		fwts_passed(fw, "RSDP: the reserved field is zero.");
 
 	return FWTS_OK;
diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
index 2dce7e06..5db4f670 100644
--- a/src/acpi/spmi/spmi.c
+++ b/src/acpi/spmi/spmi.c
@@ -130,14 +130,8 @@ static int spmi_test1(fwts_framework *fw)
 			"Type bit 0 (SCI triggered through GPE) is set to 0",
 			spmi->gpe);
 	}
-	if (spmi->reserved2 != 0) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"SPMIReserved1NonZero",
-			"SPMI reserved field (offset 42) must be 0x00, got "
-			"0x%2.2" PRIx8 " instead", spmi->reserved2);
-	}
 
+	fwts_acpi_reserved_zero_check(fw, "SPMI", "Reserved2", spmi->reserved2, sizeof(spmi->reserved2), &passed);
 	fwts_acpi_reserved_bits_check(fw, "SPMI", "PCI device flag", spmi->pci_device_flag, sizeof(spmi->pci_device_flag), 1, 7, &passed);
 
 	if (((spmi->interrupt_type & 2) == 0) &&
diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
index 3015364c..3de50040 100644
--- a/src/lib/include/fwts_acpi.h
+++ b/src/lib/include/fwts_acpi.h
@@ -1585,7 +1585,7 @@ typedef struct {
 typedef struct {
 	fwts_acpi_table_header	header;
 	uint8_t		interface_type;
-	uint8_t		reserved1[3];
+	uint8_t		reserved[3];
 	fwts_acpi_gas	base_address;
 } __attribute__ ((packed)) fwts_acpi_table_dbgp;
 
-- 
2.25.1




More information about the fwts-devel mailing list