[PATCH 1/2] acpi: replace checks for reserved bits by fwts_acpi_reserved_bits_check

Alex Hung alex.hung at canonical.com
Tue Sep 26 00:15:23 UTC 2017


Signed-off-by: Alex Hung <alex.hung at canonical.com>
---
 src/acpi/bgrt/bgrt.c | 11 +++------
 src/acpi/gtdt/gtdt.c | 54 +++++++++++++++----------------------------
 src/acpi/hest/hest.c | 22 +++++++-----------
 src/acpi/iort/iort.c | 65 +++++++++++++++-------------------------------------
 src/acpi/lpit/lpit.c |  9 +-------
 src/acpi/mchi/mchi.c | 22 +++++-------------
 src/acpi/nfit/nfit.c |  9 +-------
 src/acpi/pptt/pptt.c |  9 +-------
 src/acpi/spmi/spmi.c | 19 +++------------
 9 files changed, 59 insertions(+), 161 deletions(-)

diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c
index 25030ad..5b9b413 100644
--- a/src/acpi/bgrt/bgrt.c
+++ b/src/acpi/bgrt/bgrt.c
@@ -66,14 +66,9 @@ static int bgrt_test1(fwts_framework *fw)
 			" and not the expected value of 0x01",
 			bgrt->version);
 	}
-	if (bgrt->status & ~0x7) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"BGRTStatusRersevedBits",
-			"BGRT: Status field is 0x%" PRIx8
-			", reserved bits [7:3] should be zero",
-			bgrt->status);
-	}
+
+	fwts_acpi_reserved_bits_check(fw, "BGRT", "BGRT Status", bgrt->status, sizeof(bgrt->status), 3, 7, &passed);
+
 	if (bgrt->image_type > 0x00) {
 		passed = false;
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c
index 1559d31..f3a8044 100644
--- a/src/acpi/gtdt/gtdt.c
+++ b/src/acpi/gtdt/gtdt.c
@@ -67,6 +67,7 @@ static int gtdt_test1(fwts_framework *fw)
 		fwts_acpi_table_gtdt_block *block;
 		fwts_acpi_table_gtdt_block_timer *block_timer;
 		fwts_acpi_table_gtdt_watchdog *watchdog;
+		char field[80];
 
 		switch (*ptr) {
 		case 0x00:
@@ -145,33 +146,18 @@ static int gtdt_test1(fwts_framework *fw)
 						block_timer->reserved[1],
 						block_timer->reserved[2]);
 				}
-				if (block_timer->phys_timer_flags & ~0x3) {
-					passed = false;
-					fwts_failed(fw, LOG_LEVEL_LOW,
-						"GTDTFBlockPhysTimerFlagReservedNonZero",
-						"GTDT block %" PRIu32 " physical timer "
-						"flags reserved bits 2 to 31 are "
-						"non-zero, instead got 0x%" PRIx32 ".",
-						i, block_timer->phys_timer_flags);
-				}
-				if (block_timer->virt_timer_flags & ~0x3) {
-					passed = false;
-					fwts_failed(fw, LOG_LEVEL_LOW,
-						"GTDTFBlockVirtTimerFlagReservedNonZero",
-						"GTDT block %" PRIu32 " virtual timer "
-						"flags reserved bits 2 to 31 are "
-						"non-zero, instead got 0x%" PRIx32 ".",
-						i, block_timer->virt_timer_flags);
-				}
-				if (block_timer->common_flags & ~0x3) {
-					passed = false;
-					fwts_failed(fw, LOG_LEVEL_LOW,
-						"GTDTFBlockCommonFlagReservedNonZero",
-						"GTDT block %" PRIu32 " common flags "
-						"reserved bits 2 to 31 are "
-						"non-zero, instead got 0x%" PRIx32 ".",
-						i, block_timer->common_flags);
-				}
+
+				snprintf(field, sizeof(field), "block %" PRIu32 " physical timer flags", i);
+				fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->phys_timer_flags,
+					sizeof(block_timer->phys_timer_flags), 2, 31, &passed);
+
+				snprintf(field, sizeof(field), "block %" PRIu32 " virtual timer flags", i);
+				fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->virt_timer_flags,
+					sizeof(block_timer->virt_timer_flags), 2, 31, &passed);
+
+				snprintf(field, sizeof(field), "block %" PRIu32 " common flags", i);
+				fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->common_flags,
+					sizeof(block_timer->common_flags), 2, 31, &passed);
 			}
 			ptr += block->length;
 			break;
@@ -203,15 +189,11 @@ static int gtdt_test1(fwts_framework *fw)
 					" reserved is non-zero, got 0x%" PRIx8,
 					i, watchdog->reserved);
 			}
-			if (watchdog->watchdog_timer_flags & ~0x7) {
-				passed = false;
-				fwts_failed(fw, LOG_LEVEL_LOW,
-					"GTDTWatchDogTimerFlagReservedNonZero",
-					"GTDT SBSA generic watchdog timer %" PRIu32
-					" flags reserved bits 3 to 31 are non-zero, "
-					"instead got 0x%" PRIx8 ".",
-					i, watchdog->watchdog_timer_flags);
-			}
+
+			snprintf(field, sizeof(field), "SBSA generic watchdog timer %" PRIu32 " flags", i);
+			fwts_acpi_reserved_bits_check(fw, "GTDT", field, watchdog->watchdog_timer_flags,
+				sizeof(watchdog->watchdog_timer_flags), 3, 31, &passed);
+
 			ptr += watchdog->length;
 			break;
 		default:
diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c
index 86a5312..289bd18 100644
--- a/src/acpi/hest/hest.c
+++ b/src/acpi/hest/hest.c
@@ -658,13 +658,10 @@ static void hest_check_generic_error_source(
 			"expecting value 0 to 11",
 			source->notification.type);
 	}
-	if (source->notification.configuration_write_enable & ~0x3f) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"HESTIA32ConfigWriteEnabledReservedNonZero",
-			"HEST Configuration Write Enabled Reserved bits [6:31] "
-			"are non-zero.");
-	}
+
+	fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled",
+		source->notification.configuration_write_enable,
+		sizeof(source->notification.configuration_write_enable), 6, 31, passed);
 
 	*length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source);
 	*data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source);
@@ -776,13 +773,10 @@ static void hest_check_generic_error_source_v2(
 			"expecting value 0 to 11",
 			source->notification.type);
 	}
-	if (source->notification.configuration_write_enable & ~0x3f) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"HESTIA32ConfigWriteEnabledReservedNonZero",
-			"HEST Configuration Write Enabled Reserved bits [6:31] "
-			"are non-zero.");
-	}
+
+	fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled",
+		source->notification.configuration_write_enable,
+		sizeof(source->notification.configuration_write_enable), 6, 31, passed);
 
 	*length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2);
 	*data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2);
diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c
index b7046ea..9c20610 100644
--- a/src/acpi/iort/iort.c
+++ b/src/acpi/iort/iort.c
@@ -148,28 +148,6 @@ static void iort_id_mappings_dump(
 }
 
 /*
- *  iort_id_mapping_check()
- *	sanity check ID mapping
- */
-static void iort_id_mapping_check(
-	fwts_framework *fw,
-	uint32_t i,
-	fwts_acpi_table_iort_id_mapping *id_mapping,
-	bool *passed)
-{
-	if (id_mapping->flags & ~1) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"IORTIdMappingReservedNonZero",
-			"IORT ID Mapping %" PRIu32 " Reserved Flags field reserved "
-			"bits [31:1] should be all zero, got 0x%" PRIx32
-			" instead",
-			i, id_mapping->flags);
-	}
-	/* Should also check output_reference is out of range or not */
-}
-
-/*
  *  iort_id_mappings_check()
  *	snaity check array of ID mappings
  */
@@ -182,6 +160,7 @@ static void iort_id_mappings_check(
 	uint32_t i;
 	fwts_acpi_table_iort_node *node = (fwts_acpi_table_iort_node *)data;
 	fwts_acpi_table_iort_id_mapping *id_mapping;
+	char field[80];
 
 	id_mapping = (fwts_acpi_table_iort_id_mapping *)(data + node->id_array_offset);
 
@@ -195,7 +174,9 @@ static void iort_id_mappings_check(
 				"or the IORT table size or the node is too small.", i);
 			break;
 		}
-		iort_id_mapping_check(fw, i, id_mapping, passed);
+
+		snprintf(field, sizeof(field), "ID Mapping %" PRIu32 " flags", i);
+		fwts_acpi_reserved_bits_check(fw, "IORT", field, id_mapping->flags, sizeof(id_mapping->flags), 1, 31, passed);
 	}
 }
 
@@ -381,6 +362,7 @@ static void iort_memory_access_properties_check(
 	bool *passed)
 {
 	uint8_t cca, cpm, dacs;
+	char field[80];
 
 	if (properties->cache_coherent > 1) {
 		*passed = false;
@@ -392,30 +374,19 @@ static void iort_memory_access_properties_check(
 			"not coherent).",
 			name, properties->cache_coherent);
 	}
-	if (properties->allocation_hints & ~0xf) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"IORTAllocHintsReservedNonZero",
-			"IORT %s Allocation Hints reserved bits "
-			"[7:4] are reserved and should be zero.",
-			name);
-	}
-	if (properties->reserved) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"IORTNodeReservedNonZero",
-			"IORT %s Node Reserved is 0x%" PRIx32
-			" and is reserved and should be zero.",
-			name, properties->reserved);
-	}
-	if (properties->memory_access_flags & ~0x3) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"IORTMemoryAccessFlagsReservedNonZero",
-			"IORT %s Memory Access Flags is 0x%" PRIx8
-			" and all the reserved bits [7:2] should be zero but some are not.",
-			name, properties->memory_access_flags);
-	}
+
+	snprintf(field, sizeof(field), "%s Allocation Hints", name);
+	fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->allocation_hints,
+		sizeof(properties->allocation_hints), 4, 7, passed);
+
+	snprintf(field, sizeof(field), "%s Reserved", name);
+	fwts_acpi_reserved_zero_check(fw, "IORT", field, properties->reserved,
+		sizeof(properties->reserved), passed);
+
+	snprintf(field, sizeof(field), "%s  Memory Access Flags", name);
+	fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->memory_access_flags,
+		sizeof(properties->memory_access_flags), 2, 7, passed);
+
 	cca = properties->cache_coherent & 1;
 	cpm = properties->memory_access_flags & 1;
 	dacs = (properties->memory_access_flags >> 1) & 1;
diff --git a/src/acpi/lpit/lpit.c b/src/acpi/lpit/lpit.c
index 2273ee4..eb4e3af 100644
--- a/src/acpi/lpit/lpit.c
+++ b/src/acpi/lpit/lpit.c
@@ -95,14 +95,7 @@ static void lpit_check_type_0(
 	fwts_log_nl(fw);
 
 	fwts_acpi_reserved_zero_check(fw, "LPIT", "Native C-state based LPI structure reserved", lpi->reserved, sizeof(lpi->reserved), passed);
-
-	if (lpi->flags & ~3) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"LPITNativeCStateLpitFlagsReserved",
-			"Some of the Native C-state based LPI structure flags "
-			"bits [31:2] are set, they are expected to be zero");
-	}
+	fwts_acpi_reserved_bits_check(fw, "LPIT", "LPI structure flags", lpi->flags, sizeof(lpi->flags), 2, 31, passed);
 
 	/* 2.2.1.2, if FFH, then it is a MSR, check GAS fields */
 	if (((lpi->flags & 2) == 0) &&
diff --git a/src/acpi/mchi/mchi.c b/src/acpi/mchi/mchi.c
index 6244b93..8ddff2c 100644
--- a/src/acpi/mchi/mchi.c
+++ b/src/acpi/mchi/mchi.c
@@ -116,14 +116,9 @@ static int mchi_test1(fwts_framework *fw)
 			"allowed values are 0x00 (Unspecifier), 0x01 (MCTP), 0x02 (IPMI) or "
 			"255 (OEM defined)", mchi->protocol_identifier);
 	}
-	if (mchi->interrupt_type & ~0x03) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"MCHIInterruptTypeReservedNonZero",
-			"MCHI Interrupt Type 0x%2.2" PRIx8 " has some reserved "
-			"bits [7:2] set, these should be all zero.",
-			mchi->interrupt_type);
-	}
+
+	fwts_acpi_reserved_bits_check(fw, "MCHI", "Interrupt Type", mchi->interrupt_type, sizeof(mchi->interrupt_type), 2, 7, &passed);
+
 	if (((mchi->interrupt_type & 0x01) == 0) &&
 	    (mchi->gpe != 0)) {
 		passed = false;
@@ -134,14 +129,9 @@ static int mchi_test1(fwts_framework *fw)
 			"(SCI triggered through GPE non-supported)",
 			mchi->gpe);
 	}
-	if (mchi->pci_device_flag & ~0x01) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"MCHIPciDeviceFlagReservedNonZero",
-			"MCHI PCI Device Flag 0x%2.2" PRIx8 " has some reserved "
-			"bits [7:1] set, these should be all zero.",
-			mchi->pci_device_flag);
-	}
+
+	fwts_acpi_reserved_bits_check(fw, "MCHI", "PCI Device Flag", mchi->pci_device_flag, sizeof(mchi->pci_device_flag), 1, 7, &passed);
+
 	if (((mchi->interrupt_type & 0x02) == 0) &&
 	    (mchi->global_system_interrupt != 0)) {
 		passed = false;
diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
index fc69798..e1a4815 100644
--- a/src/acpi/nfit/nfit.c
+++ b/src/acpi/nfit/nfit.c
@@ -284,14 +284,7 @@ static int nfit_test1(fwts_framework *fw)
 			if (nfit_struct->reserved != 0)
 				reserved_passed = nfit_struct->reserved;
 
-			if (nfit_struct->valid_fields & ~0x01) {
-				passed = false;
-				fwts_failed(fw, LOG_LEVEL_HIGH,
-					"NFITBadValidField",
-					"NFIT Valid Field's Bits[7..1] must be zero, got "
-					"0x%2.2" PRIx8 " instead", nfit_struct->valid_fields);
-			}
-
+			fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed);
 			fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed);
 
 		} else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) {
diff --git a/src/acpi/pptt/pptt.c b/src/acpi/pptt/pptt.c
index 50b7491..25f5d9a 100644
--- a/src/acpi/pptt/pptt.c
+++ b/src/acpi/pptt/pptt.c
@@ -85,14 +85,7 @@ static void pptt_cache_test(fwts_framework *fw, const fwts_acpi_table_pptt_cache
 
 	fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
 	fwts_acpi_reserved_bits_check(fw, "PPTT", "Flags", entry->flags, sizeof(entry->flags), 7, 31, passed);
-
-	if (entry->attributes & ~0x1f) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"PPTTBadAttributes",
-			"PPTT attributes's Bits[7..5] must be zero, got "
-			"0x%2.2" PRIx8 " instead", entry->attributes);
-	}
+	fwts_acpi_reserved_bits_check(fw, "PPTT", "Attributes", entry->attributes, sizeof(entry->attributes), 5, 7, passed);
 }
 
 static void pptt_id_test(fwts_framework *fw, const fwts_acpi_table_pptt_id *entry, bool *passed)
diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
index 23b9b22..86acb11 100644
--- a/src/acpi/spmi/spmi.c
+++ b/src/acpi/spmi/spmi.c
@@ -123,14 +123,7 @@ static int spmi_test1(fwts_framework *fw)
 			"0x%2.2" PRIx8 " instead", spmi->reserved1);
 	}
 
-	/* Only bottom 2 bits should be set */
-	if (spmi->interrupt_type & ~0x03) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"SPMIInterruptTypeReservedNonZero",
-			"SPMI Interrupt Type reserved bits [7:2] must be 0, got "
-			"0x%2.2" PRIx8 " instead", spmi->interrupt_type);
-	}
+	fwts_acpi_reserved_bits_check(fw, "SPMI", "Interrupt type", spmi->interrupt_type, sizeof(spmi->interrupt_type), 2, 7, &passed);
 
 	/* Check for zero GPE on specific condition of interrupt type */
 	if (((spmi->interrupt_type & 1) == 0) &&
@@ -149,14 +142,8 @@ static int spmi_test1(fwts_framework *fw)
 			"SPMI reserved field (offset 42) must be 0x00, got "
 			"0x%2.2" PRIx8 " instead", spmi->reserved2);
 	}
-	/* Only bottom 1 bit should be set */
-	if (spmi->pci_device_flag & ~0x01) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"SPMIPciDeviceFlag",
-			"SPMI PCI Device Flag reserved bits [7:1] must be 0, got "
-			"0x%2.2" PRIx8 " instead", spmi->pci_device_flag);
-	}
+
+	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) &&
 	    (spmi->global_system_interrupt != 0)) {
-- 
2.7.4




More information about the fwts-devel mailing list