NACK: [PATCH 2/3] acpi: replace checks for Reserved by fwts_acpi_reserved_zero_check

Alex Hung alex.hung at canonical.com
Fri Sep 8 20:17:06 UTC 2017


On 2017-09-06 05:34 PM, Alex Hung wrote:
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>   src/acpi/asf/asf.c   | 21 +++++--------
>   src/acpi/einj/einj.c | 16 ++--------
>   src/acpi/erst/erst.c |  9 ++----
>   src/acpi/hest/hest.c | 39 ++++++++++--------------
>   src/acpi/hmat/hmat.c | 85 +++++++---------------------------------------------
>   src/acpi/iort/iort.c | 11 ++-----
>   src/acpi/lpit/lpit.c |  9 +-----
>   src/acpi/mpst/mpst.c | 49 ++++--------------------------
>   src/acpi/msdm/msdm.c | 18 ++---------
>   src/acpi/nfit/nfit.c | 17 ++---------
>   src/acpi/pcct/pcct.c |  8 +----
>   src/acpi/pmtt/pmtt.c | 48 ++++-------------------------
>   src/acpi/pptt/pptt.c | 24 ++-------------
>   src/acpi/spcr/spcr.c | 32 ++++----------------
>   src/acpi/srat/srat.c |  9 +-----
>   src/acpi/tcpa/tcpa.c | 26 ++--------------
>   src/acpi/tpm2/tpm2.c |  8 +----
>   17 files changed, 71 insertions(+), 358 deletions(-)
> 
> diff --git a/src/acpi/asf/asf.c b/src/acpi/asf/asf.c
> index 0a618da..a95c8de 100644
> --- a/src/acpi/asf/asf.c
> +++ b/src/acpi/asf/asf.c
> @@ -103,13 +103,11 @@ static void asf_check_info(
>   			", however reserved bits [7:1] must be zero",
>   			info->flags);
>   	}
> -	if (info->reserved1 | info->reserved2 | info->reserved3) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"ASF!InfoReservedNonZero",
> -			"ASF! ASF_INFO Reserved fields must be zero, "
> -			"however one or more of them are non-zero");
> -	}
> +
> +	fwts_acpi_reserved_zero_check(fw, "ASF!", "ASF_INFO Reserved1", info->reserved1, sizeof(info->reserved1), passed);
> +	fwts_acpi_reserved_zero_check(fw, "ASF!", "ASF_INFO Reserved2", info->reserved2, sizeof(info->reserved2), passed);
> +	fwts_acpi_reserved_zero_check(fw, "ASF!", "ASF_INFO Reserved3", info->reserved3, sizeof(info->reserved3), passed);
> +
>   	if (*passed)
>   		fwts_passed(fw, "No issues found in ASF! ASF_INFO record.");
>   }
> @@ -466,13 +464,8 @@ static int asf_test1(fwts_framework *fw)
>   		fwts_log_info_verbatim(fw, "Length:                     0x%4.4" PRIx16, asf_hdr->length);
>   #endif
>   
> -		if (asf_hdr->reserved) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"ASF!InfoRecordReservedNonZero",
> -				"ASF! Information Record Reserved field is 0x%" PRIx8
> -				" and should be zero", asf_hdr->reserved);
> -		}
> +		fwts_acpi_reserved_zero_check(fw, "ASF!", "Information Record Reserved", asf_hdr->reserved, sizeof(asf_hdr->reserved), &passed);
> +
>   		if (asf_hdr->length > (uint32_t)length) {
>   			passed = false;
>   			fwts_failed(fw, LOG_LEVEL_HIGH,
> diff --git a/src/acpi/einj/einj.c b/src/acpi/einj/einj.c
> index 8b0f5a0..4c2ff8b 100644
> --- a/src/acpi/einj/einj.c
> +++ b/src/acpi/einj/einj.c
> @@ -71,13 +71,7 @@ static int einj_test1(fwts_framework *fw)
>   		passed = false;
>   	}
>   
> -	if (reserved) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			    "EINJReservedNonZero",
> -			    "EINJ Reserved field must be zero, got 0x%"
> -			    PRIx32 " instead", reserved);
> -		passed = false;
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "EINJ", "Reserved", reserved, sizeof(reserved), &passed);
>   
>   	fwts_log_nl(fw);
>   
> @@ -128,13 +122,7 @@ static int einj_test1(fwts_framework *fw)
>   			passed = false;
>   		}
>   
> -		if (entry->reserved != 0) {
> -			fwts_failed(fw, LOG_LEVEL_LOW,
> -				    "EINJReservedNonZero",
> -				    "EINJ Reserved field must be zero, got 0x%"
> -				    PRIx32 " instead", entry->reserved);
> -			passed = false;
> -		}
> +		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) {
> diff --git a/src/acpi/erst/erst.c b/src/acpi/erst/erst.c
> index 536a873..2304d66 100644
> --- a/src/acpi/erst/erst.c
> +++ b/src/acpi/erst/erst.c
> @@ -56,13 +56,8 @@ static int erst_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "  Reserved:                 0x%8.8" PRIx32, erst->reserved);
>   	fwts_log_info_verbatim(fw, "  Instruction Entry Count:  0x%8.8" PRIx32, erst->instruction_entry_count);
>   
> -	if (erst->reserved) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"ERSTReservedNonZero",
> -			"ERST Reserved field must be zero, got 0x%" PRIx32
> -			" instead", erst->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "ERST", "Reserved", erst->reserved, sizeof(erst->reserved), &passed);
> +
>   	if (erst->serialization_header_size > table->length) {
>   		passed = false;
>   		fwts_failed(fw, LOG_LEVEL_HIGH,
> diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c
> index fba73ba..d46a349 100644
> --- a/src/acpi/hest/hest.c
> +++ b/src/acpi/hest/hest.c
> @@ -398,6 +398,8 @@ static void hest_check_pci_express_root_port_aer(
>   	fwts_log_info_verbatim(fw, "  Root Error Command:       0x%8.8" PRIx32, aer->root_error_command);
>   	fwts_log_nl(fw);
>   
> +	fwts_acpi_reserved_zero_check(fw, "HEST", " PCI Express Root Port Reserved1", aer->reserved1, sizeof(aer->reserved1), passed);
A typo (an extra space) is between " and PCI. There are the same typo 
below as well. Let me NACK this patch as I sent out a V2 to fix the typos.

> +
>   	if (aer->flags & ~0x3) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> @@ -406,14 +408,9 @@ static void hest_check_pci_express_root_port_aer(
>   			"[2:7] must be zero, instead got 0x%" PRIx8,
>   			aer->flags);
>   	}
> -	if (aer->reserved2) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HESTPciExpressRootPortReservedNonZero",
> -			"HEST PCI Express Root Port Reserved field "
> -			"at offset 26 must be zero, instead got 0x%" PRIx16,
> -			aer->reserved2);
> -	}
> +
> +	fwts_acpi_reserved_zero_check(fw, "HEST", " PCI Express Root Port Reserved2", aer->reserved2, sizeof(aer->reserved2), passed);
> +
>   	if (aer->number_of_records_to_preallocate < 1) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_HIGH,
> @@ -478,6 +475,8 @@ static void hest_check_pci_express_device_aer(
>   	fwts_log_info_verbatim(fw, "  Advanced Capabilities:    0x%8.8" PRIx32, aer->advanced_error_capabilities_and_control);
>   	fwts_log_nl(fw);
>   
> +	fwts_acpi_reserved_zero_check(fw, "HEST", " PCI Express Device Reserved1", aer->reserved1, sizeof(aer->reserved1), passed);
> +
>   	if (aer->flags & ~0x3) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> @@ -486,14 +485,9 @@ static void hest_check_pci_express_device_aer(
>   			"[2:7] must be zero, instead got 0x%" PRIx8,
>   			aer->flags);
>   	}
> -	if (aer->reserved2) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HESTPciExpressDeviceReservedNonZero",
> -			"HEST PCI Express Device Reserved field "
> -			"at offset 26 must be zero, instead got 0x%" PRIx16,
> -			aer->reserved2);
> -	}
> +
> +	fwts_acpi_reserved_zero_check(fw, "HEST", " PCI Express Device Reserved2", aer->reserved2, sizeof(aer->reserved2), passed);
> +
>   	if (aer->number_of_records_to_preallocate < 1) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_HIGH,
> @@ -562,6 +556,8 @@ static void hest_heck_pci_express_bridge_aer(
>   	fwts_log_info_verbatim(fw, "  2nd Advanced Capabilities:0x%8.8" PRIx32, aer->secondary_advanced_error_capabilities_and_control);
>   	fwts_log_nl(fw);
>   
> +	fwts_acpi_reserved_zero_check(fw, "HEST", " PCI Express Bridge Reserved1", aer->reserved1, sizeof(aer->reserved1), passed);
> +
>   	if (aer->flags & ~0x3) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> @@ -570,14 +566,9 @@ static void hest_heck_pci_express_bridge_aer(
>   			"[2:7] must be zero, instead got 0x%" PRIx8,
>   			aer->flags);
>   	}
> -	if (aer->reserved2) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HESTPciExpressBridgeReservedNonZero",
> -			"HEST PCI Express Bridge Reserved field "
> -			"at offset 26 must be zero, instead got 0x%" PRIx16,
> -			aer->reserved2);
> -	}
> +
> +	fwts_acpi_reserved_zero_check(fw, "HEST", " PCI Express Bridge Reserved2", aer->reserved2, sizeof(aer->reserved2), passed);
> +
>   	if (aer->number_of_records_to_preallocate < 1) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_HIGH,
> diff --git a/src/acpi/hmat/hmat.c b/src/acpi/hmat/hmat.c
> index 35a1f4c..7b5c58c 100644
> --- a/src/acpi/hmat/hmat.c
> +++ b/src/acpi/hmat/hmat.c
> @@ -53,13 +53,7 @@ static void hmat_addr_range_test(fwts_framework *fw, const fwts_acpi_table_hmat_
>   	fwts_log_info_verbatim(fw, "    System Phy Addr Range Base:     0x%16.16" PRIx64, entry->phy_addr_base);
>   	fwts_log_info_verbatim(fw, "    System Phy Addr Range Length:   0x%16.16" PRIx64, entry->phy_addr_length);
>   
> -	if (entry->header.reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->header.reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->header.reserved, sizeof(entry->header.reserved), passed);
>   
>   	if (entry->flags & ~0x07) {
>   		*passed = false;
> @@ -69,21 +63,8 @@ static void hmat_addr_range_test(fwts_framework *fw, const fwts_acpi_table_hmat_
>   			"0x%4.4" PRIx16 " instead", entry->flags);
>   	}
>   
> -	if (entry->reserved1 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved1);
> -	}
> -
> -	if (entry->reserved2 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", entry->reserved2);
> -		}
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->reserved1, sizeof(entry->reserved1), passed);
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->reserved2, sizeof(entry->reserved2), passed);
>   }
>   
>   static void hmat_locality_test(fwts_framework *fw, const fwts_acpi_table_hmat_locality *entry, bool *passed)
> @@ -102,13 +83,7 @@ static void hmat_locality_test(fwts_framework *fw, const fwts_acpi_table_hmat_lo
>   	fwts_log_info_verbatim(fw, "    Reserved:                       0x%8.8" PRIx32, entry->reserved2);
>   	fwts_log_info_verbatim(fw, "    Entry Base Unit:                0x%16.16" PRIx64, entry->entry_base_unit);
>   
> -	if (entry->header.reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->header.reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->header.reserved, sizeof(entry->header.reserved), passed);
>   
>   	if (entry->flags & ~0x1f) {
>   		*passed = false;
> @@ -126,21 +101,8 @@ static void hmat_locality_test(fwts_framework *fw, const fwts_acpi_table_hmat_lo
>   			"0x%2.2" PRIx8 " instead", entry->data_type);
>   	}
>   
> -	if (entry->reserved1 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved1);
> -	}
> -
> -	if (entry->reserved2 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", entry->reserved2);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->reserved1, sizeof(entry->reserved1), passed);
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->reserved2, sizeof(entry->reserved2), passed);
>   
>   	pd_size = (entry->num_initiator + entry->num_target) * 4 +
>   	          (entry->num_initiator * entry->num_target * 2);
> @@ -165,14 +127,7 @@ static void hmat_cache_test(fwts_framework *fw, const fwts_acpi_table_hmat_cache
>   	fwts_log_info_verbatim(fw, "    Reserved:                       0x%4.4" PRIx16, entry->reserved2);
>   	fwts_log_info_verbatim(fw, "    Number of SMBIOS Handles:       0x%4.4" PRIx16, entry->num_smbios);
>   
> -	if (entry->header.reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->header.reserved);
> -	}
> -
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->header.reserved, sizeof(entry->header.reserved), passed);
>   
>   	if ((entry->cache_attr & 0xf) > 3 ||
>   	   ((entry->cache_attr >> 4) & 0xf) > 3 ||
> @@ -185,21 +140,8 @@ static void hmat_cache_test(fwts_framework *fw, const fwts_acpi_table_hmat_cache
>   			"0x%8.8" PRIx32 " instead", entry->cache_attr);
>   	}
>   
> -	if (entry->reserved1 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", entry->reserved1);
> -	}
> -
> -	if (entry->reserved2 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved2);
> -		}
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->reserved1, sizeof(entry->reserved1), passed);
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", entry->reserved2, sizeof(entry->reserved2), passed);
>   
>   	if ((entry->header.length - sizeof(fwts_acpi_table_hmat_cache)) / 2 != entry->num_smbios) {
>   		*passed = false;
> @@ -220,13 +162,8 @@ static int hmat_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "HMAT Heterogeneous Memory Attribute Table:");
>   	fwts_log_info_verbatim(fw, "  Reserved:        0x%2.2" PRIx8, hmat->reserved);
>   
> -	if (hmat->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"HMATReservedNonZero",
> -			"HMAT reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", hmat->reserved);
> -	}
> +
> +	fwts_acpi_reserved_zero_check(fw, "HMAT", "Reserved", hmat->reserved, sizeof(hmat->reserved), &passed);
>   
>   	entry = (fwts_acpi_table_hmat_header *) (table->data + sizeof(fwts_acpi_table_hmat));
>   	offset = sizeof(fwts_acpi_table_hmat);
> diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c
> index 97f096d..b131edd 100644
> --- a/src/acpi/iort/iort.c
> +++ b/src/acpi/iort/iort.c
> @@ -82,14 +82,9 @@ static void iort_node_check(
>   			" and should be zero.",
>   			node->revision);
>   	}
> -	if (node->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"IORTNodeReservedNonZero",
> -			"IORT Node Reserved field is 0x0%8.8" PRIx32
> -			" and should be zero.",
> -			node->reserved);
> -	}
> +
> +	fwts_acpi_reserved_zero_check(fw, "IORT", "Node Reserved", node->reserved, sizeof(node->reserved), passed);
> +
>   	if (no_id_mappings && node->id_mappings_count) {
>   		*passed = false;
>   		fwts_failed(fw, LOG_LEVEL_LOW,
> diff --git a/src/acpi/lpit/lpit.c b/src/acpi/lpit/lpit.c
> index e5eb566..2273ee4 100644
> --- a/src/acpi/lpit/lpit.c
> +++ b/src/acpi/lpit/lpit.c
> @@ -94,14 +94,7 @@ static void lpit_check_type_0(
>   	}
>   	fwts_log_nl(fw);
>   
> -	if (lpi->reserved) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"LPITNativeCStateLpitReservedNonZero",
> -			"Native C-state based LPI structure reserved field "
> -			"was expected to be zero, got 0x%4.4" PRIx16 " instead",
> -			lpi->reserved);
> -	}
> +	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;
> diff --git a/src/acpi/mpst/mpst.c b/src/acpi/mpst/mpst.c
> index db20df7..7a38f78 100644
> --- a/src/acpi/mpst/mpst.c
> +++ b/src/acpi/mpst/mpst.c
> @@ -57,25 +57,13 @@ static int mpst_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "  Communication Channel ID:        0x%2.2" PRIx8, mpst->channel_id);
>   	fwts_log_info_verbatim(fw, "  Reserved:                        0x%8.8" PRIx32, reserved);
>   
> -	if (reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"MPSTReservedNonZero",
> -			"MPST reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "MPST", "Reserved", reserved, sizeof(reserved), &passed);
>   
>   	node_list = (fwts_acpi_table_mpst_power_node_list *) (table->data + sizeof(fwts_acpi_table_mpst));
>   	fwts_log_info_verbatim(fw, "  Memory Power Node Count:         0x%4.4" PRIx16, node_list->count);
>   	fwts_log_info_verbatim(fw, "  Reserved:                        0x%4.4" PRIx16, node_list->reserved);
>   
> -	if (node_list->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"MPSTReservedNonZero",
> -			"MPST reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", node_list->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "MPST", "Reserved", node_list->reserved, sizeof(node_list->reserved), &passed);
>   
>   	node_offset = sizeof(fwts_acpi_table_mpst) + (sizeof(fwts_acpi_table_mpst_power_node_list));
>   	if (mpst->header.length < node_offset + sizeof(fwts_acpi_table_mpst_power_node) * node_list->count) {
> @@ -107,13 +95,7 @@ static int mpst_test1(fwts_framework *fw)
>   				"0x%2.2" PRIx8 " instead", power_node->flags);
>   		}
>   
> -		if (power_node->reserved != 0) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_LOW,
> -				"MPSTReservedNonZero",
> -				"MPST reserved field must be zero, got "
> -				"0x%2.2" PRIx8 " instead", power_node->reserved);
> -		}
> +		fwts_acpi_reserved_zero_check(fw, "MPST", "Reserved", power_node->reserved, sizeof(power_node->reserved), &passed);
>   
>   		node_length = sizeof(fwts_acpi_table_mpst_power_node) +
>   			      sizeof(fwts_acpi_table_mpst_power_state) * power_node->num_states +
> @@ -160,13 +142,7 @@ static int mpst_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "  Memory Characteristics Count:    0x%4.4" PRIx16, char_list->count);
>   	fwts_log_info_verbatim(fw, "  Reserved:                        0x%4.4" PRIx16, char_list->reserved);
>   
> -	if (char_list->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"MPSTReservedNonZero",
> -			"MPST reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", char_list->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "MPST", "Reserved", char_list->reserved, sizeof(char_list->reserved), &passed);
>   
>   	node_offset += sizeof(fwts_acpi_table_mpst_power_char_list);
>   	if (mpst->header.length < node_offset + sizeof(fwts_acpi_table_mpst_power_char) * char_list->count) {
> @@ -212,21 +188,8 @@ static int mpst_test1(fwts_framework *fw)
>   				"0x%2.2" PRIx8 " instead", power_char->flags);
>   		}
>   
> -		if (power_char->reserved1 != 0) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_LOW,
> -				"MPSTReservedNonZero",
> -				"MPST reserved field must be zero, got "
> -				"0x%4.4" PRIx16 " instead", power_char->reserved1);
> -		}
> -
> -		if (power_char->reserved2 != 0) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_LOW,
> -				"MPSTReservedNonZero",
> -				"MPST reserved field must be zero, got "
> -				"0x%16.16" PRIx64 " instead", power_char->reserved2);
> -		}
> +		fwts_acpi_reserved_zero_check(fw, "MPST", "Reserved1", power_char->reserved1, sizeof(power_char->reserved1), &passed);
> +		fwts_acpi_reserved_zero_check(fw, "MPST", "Reserved2", power_char->reserved2, sizeof(power_char->reserved2), &passed);
>   
>   		node_offset +=  sizeof(fwts_acpi_table_mpst_power_char);
>   	}
> diff --git a/src/acpi/msdm/msdm.c b/src/acpi/msdm/msdm.c
> index 049641c..f76feab 100644
> --- a/src/acpi/msdm/msdm.c
> +++ b/src/acpi/msdm/msdm.c
> @@ -69,22 +69,8 @@ static int msdm_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "  Data Reserved:            0x%8.8" PRIx32, msdm->data_reserved);
>   	fwts_log_info_verbatim(fw, "  Data Length:              0x%8.8" PRIx32, msdm->data_length);
>   
> -	if (msdm->reserved) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"MSDMReservedNonZero",
> -			"MSDM Reserved field should be zero, got 0x%8.8" PRIx32
> -			" instead",
> -			msdm->reserved);
> -	}
> -	if (msdm->data_reserved) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"MSDMDataReservedNonZero",
> -			"MSDM Data Reserved field should be zero, got 0x%8.8" PRIx32
> -			" instead",
> -			msdm->data_reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "MSDM", "Reserved", msdm->reserved, sizeof(msdm->reserved), &passed);
> +	fwts_acpi_reserved_zero_check(fw, "MSDM", "Data Reserved", msdm->data_reserved, sizeof(msdm->data_reserved), &passed);
>   
>   	/* Now check table is big enough for the data payload */
>   	if (table->length < sizeof(fwts_acpi_table_msdm) + msdm->data_length) {
> diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c
> index 37a7aad..c3b69db 100644
> --- a/src/acpi/nfit/nfit.c
> +++ b/src/acpi/nfit/nfit.c
> @@ -67,13 +67,7 @@ static int nfit_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "  Reserved:                 0x%8.8" PRIx32, nfit->reserved);
>   	fwts_log_nl(fw);
>   
> -	if (nfit->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"NFITReservedNonZero",
> -			"NFIT reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", nfit->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "NFIT", "Reserved", nfit->reserved, sizeof(nfit->reserved), &passed);
>   
>   	offset = sizeof(fwts_acpi_table_nfit);
>   	entry = (fwts_acpi_table_nfit_struct_header *) (table->data + offset);
> @@ -361,14 +355,7 @@ static int nfit_test1(fwts_framework *fw)
>   				"0x%4.4" PRIx16 " instead", entry->type);
>   		}
>   
> -		if (reserved_passed != 0) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_LOW,
> -				"NFITReservedNonZero",
> -				"NFIT reserved field must be zero, got "
> -				"0x%16.16" PRIx64 " instead", reserved_passed);
> -		}
> -
> +		fwts_acpi_reserved_zero_check(fw, "NFIT", "Reserved", reserved_passed, sizeof(reserved_passed), &passed);
>   		fwts_log_nl(fw);
>   		offset += entry->length;
>   		entry = (fwts_acpi_table_nfit_struct_header *) (table->data + offset);
> diff --git a/src/acpi/pcct/pcct.c b/src/acpi/pcct/pcct.c
> index 5753325..424cfc1 100644
> --- a/src/acpi/pcct/pcct.c
> +++ b/src/acpi/pcct/pcct.c
> @@ -232,13 +232,7 @@ static int pcct_test1(fwts_framework *fw)
>   			"0x%8.8" PRIx32 " instead", pcct->flags);
>   	}
>   
> -	if (pcct->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PCCTReservedNonZero",
> -			"PCCT reserved field must be zero, got "
> -			"0x%16.16" PRIx64 " instead", pcct->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PCCT", "Reserved", pcct->reserved, sizeof(pcct->reserved), &passed);
>   
>   	offset = sizeof(fwts_acpi_table_pcct);
>   	pcct_sub = (fwts_acpi_table_pcct_subspace_header *) (table->data + offset);
> diff --git a/src/acpi/pmtt/pmtt.c b/src/acpi/pmtt/pmtt.c
> index d8a6108..915b0c6 100644
> --- a/src/acpi/pmtt/pmtt.c
> +++ b/src/acpi/pmtt/pmtt.c
> @@ -48,13 +48,7 @@ static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_h
>   	fwts_log_info_verbatim(fw, "    Flags:                          0x%4.4" PRIx16, entry->flags);
>   	fwts_log_info_verbatim(fw, "    Reserved:                       0x%4.4" PRIx16, entry->reserved2);
>   
> -	if (entry->reserved1 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PMTTReservedNonZero",
> -			"PMTT reserved field must be zero, got "
> -			"0x%2.2" PRIx8 " instead", entry->reserved1);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PMTT", "Reserved1", entry->reserved1, sizeof(entry->reserved1), passed);
>   
>   	if (entry->flags & ~0x0F) {
>   		*passed = false;
> @@ -71,13 +65,7 @@ static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_h
>   			"PMTT Flags's Bits[3..2] must not be 11b");
>   	}
>   
> -	if (entry->reserved2 != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PMTTReservedNonZero",
> -			"PMTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved2);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PMTT", "Reserved2", entry->reserved2, sizeof(entry->reserved2), passed);
>   }
>   
>   static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmtt_physical_component *entry, bool *passed)
> @@ -88,13 +76,7 @@ static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmt
>   	fwts_log_info_verbatim(fw, "    Size of DIMM:                   0x%8.8" PRIx32, entry->memory_size);
>   	fwts_log_info_verbatim(fw, "    SMBIOS Handle:                  0x%8.8" PRIx32, entry->bios_handle);
>   
> -	if (entry->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PMTTReservedNonZero",
> -			"PMTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PMTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
>   
>   	if ((entry->bios_handle & 0xFFFF0000) != 0 && entry->bios_handle != 0xFFFFFFFF) {
>   		*passed = false;
> @@ -121,13 +103,7 @@ static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_contro
>   	fwts_log_info_verbatim(fw, "    Reserved:                       0x%4.4" PRIx16, entry->reserved);
>   	fwts_log_info_verbatim(fw, "    Number of Proximity Domains:    0x%4.4" PRIx16, entry->domain_count);
>   
> -	if (entry->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PMTTReservedNonZero",
> -			"PMTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PMTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
>   
>   	offset = sizeof(fwts_acpi_table_pmtt_controller);
>   	if (entry->header.length < offset + sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count) {
> @@ -172,13 +148,7 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en
>   	fwts_log_info_verbatim(fw, "    Socket Identifier:              0x%4.4" PRIx16, entry->socket_id);
>   	fwts_log_info_verbatim(fw, "    Reserved:                       0x%4.4" PRIx16, entry->reserved);
>   
> -	if (entry->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PMTTReservedNonZero",
> -			"PMTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PMTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
>   
>   	offset = sizeof(fwts_acpi_table_pmtt_socket);
>   	header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset);
> @@ -208,13 +178,7 @@ static int pmtt_test1(fwts_framework *fw)
>   	fwts_log_info_verbatim(fw, "PMTT Table:");
>   	fwts_log_info_verbatim(fw, "  Reserved:                         0x%8.8" PRIx32, pmtt->reserved);
>   
> -	if (pmtt->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PMTTReservedNonZero",
> -			"PMTT reserved field must be zero, got "
> -			"0x%8.8" PRIx32 " instead", pmtt->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PMTT", "Reserved", pmtt->reserved, sizeof(pmtt->reserved), &passed);
>   
>   	entry = (fwts_acpi_table_pmtt_header *) (table->data + sizeof(fwts_acpi_table_pmtt));
>   	offset = sizeof(fwts_acpi_table_pmtt);
> diff --git a/src/acpi/pptt/pptt.c b/src/acpi/pptt/pptt.c
> index 083ef02..c1d3cc0 100644
> --- a/src/acpi/pptt/pptt.c
> +++ b/src/acpi/pptt/pptt.c
> @@ -64,13 +64,7 @@ static void pptt_processor_test(fwts_framework *fw, const fwts_acpi_table_pptt_p
>   			entry->number_priv_resources);
>   	}
>   
> -	if (entry->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PPTTReservedNonZero",
> -			"PPTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
>   
>   	if (entry->flags & ~0x03) {
>   		*passed = false;
> @@ -96,13 +90,7 @@ static void pptt_cache_test(fwts_framework *fw, const fwts_acpi_table_pptt_cache
>   	fwts_log_info_verbatim(fw, "    Attributes:                     0x%2.2" PRIx8, entry->attributes);
>   	fwts_log_info_verbatim(fw, "    Line size:                      0x%4.4" PRIx16, entry->line_size);
>   
> -	if (entry->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PPTTReservedNonZero",
> -			"PPTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
>   
>   	if (entry->flags & ~0x7f) {
>   		*passed = false;
> @@ -139,13 +127,7 @@ static void pptt_id_test(fwts_framework *fw, const fwts_acpi_table_pptt_id *entr
>   	fwts_log_info_verbatim(fw, "    MINOR_REV:                      0x%4.4" PRIx16, entry->minor_rev);
>   	fwts_log_info_verbatim(fw, "    SPIN_REV:                       0x%4.4" PRIx16, entry->spin_rev);
>   
> -	if (entry->reserved != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"PPTTReservedNonZero",
> -			"PPTT reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", entry->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed);
>   }
>   
>   static int pptt_test1(fwts_framework *fw)
> diff --git a/src/acpi/spcr/spcr.c b/src/acpi/spcr/spcr.c
> index ab16d74..4f91ed6 100644
> --- a/src/acpi/spcr/spcr.c
> +++ b/src/acpi/spcr/spcr.c
> @@ -65,7 +65,6 @@ static int spcr_sbbr_revision_test(fwts_framework *fw)
>   		else
>   			fwts_failed(fw, LOG_LEVEL_CRITICAL, "spcr_revision:", "SPCR revision is outdated: %d",
>   					spcr->header.revision);
> -
>   	}
>   
>   	return FWTS_OK;
> @@ -90,6 +89,7 @@ static int spcr_sbbr_gsiv_test(fwts_framework *fw)
>   static int spcr_test1(fwts_framework *fw)
>   {
>   	char *str;
> +	uint32_t reserved1;
>   	bool reserved = false;
>   	bool pci = true;
>   	bool passed = true;
> @@ -140,18 +140,8 @@ static int spcr_test1(fwts_framework *fw)
>   			" is a reserved interface", spcr->interface_type);
>   	}
>   
> -	if ((spcr->reserved1[0] |
> -	     spcr->reserved1[1] |
> -	     spcr->reserved1[2])) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"SPCRReservedNonZero",
> -			"SPCR reserved field must be zero, got "
> -			"0x%2.2" PRIx8 "%2.2" PRIx8 "%2.2" PRIx8 " instead",
> -				spcr->reserved1[0],
> -				spcr->reserved1[1],
> -				spcr->reserved1[2]);
> -	}
> +	reserved1 = spcr->reserved1[0] + (spcr->reserved1[1] << 8) + (spcr->reserved1[2] << 16);
> +	fwts_acpi_reserved_zero_check(fw, "SPCR", "Reserved1", reserved1, sizeof(reserved1), &passed);
>   
>   	if (spcr->interrupt_type == 0) {
>   		passed = false;
> @@ -262,13 +252,7 @@ static int spcr_test1(fwts_framework *fw)
>   			" is a reserved terminal type", spcr->terminal_type);
>   	}
>   
> -	if (spcr->reserved2) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"SPCRReservedNonZero",
> -			"SPCR reserved field must be zero, got "
> -			"0x%2.2" PRIx8 " instead", spcr->reserved2);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "SPCR", "Reserved2", spcr->reserved2, sizeof(spcr->reserved2), &passed);
>   
>   	/* According to the spec, these values indicate NOT a PCI device */
>   	if ((spcr->pci_device_id == 0xffff) &&
> @@ -315,13 +299,7 @@ static int spcr_test1(fwts_framework *fw)
>   			spcr->pci_flags);
>   	}
>   
> -	if (spcr->reserved3) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"SPCRReservedNonZero",
> -			"SPCR reserved field must be zero, got "
> -			"0x%2.2" PRIx8 " instead", spcr->reserved3);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "SPCR", "Reserved3", spcr->reserved3, sizeof(spcr->reserved3), &passed);
>   
>   	if (passed)
>   		fwts_passed(fw, "No issues found in SPCR table.");
> diff --git a/src/acpi/srat/srat.c b/src/acpi/srat/srat.c
> index 9a0e5e5..50df660 100644
> --- a/src/acpi/srat/srat.c
> +++ b/src/acpi/srat/srat.c
> @@ -325,14 +325,7 @@ static void srat_check_its_affinity(
>   	fwts_log_info_verbatim(fw, "  ITS ID                    0x%8.8" PRIx32, affinity->its_id);
>   	fwts_log_nl(fw);
>   
> -	if (affinity->reserved) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"SRATITSBadReserved",
> -			"ITS Affinity Reserved field should be zero, got "
> -			"0x%" PRIx16,
> -			affinity->reserved);
> -		*passed = false;
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "SRAT", "ITS Affinity Reserved", affinity->reserved, sizeof(affinity->reserved), passed);
>   
>   done:
>   	*length -= sizeof(fwts_acpi_table_its_affinity);
> diff --git a/src/acpi/tcpa/tcpa.c b/src/acpi/tcpa/tcpa.c
> index 6fbc724..a8b6b23 100644
> --- a/src/acpi/tcpa/tcpa.c
> +++ b/src/acpi/tcpa/tcpa.c
> @@ -123,29 +123,9 @@ static int tcpa_server_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
>   	fwts_log_info_verbatim(fw, "  PCI Device:                      0x%2.2"   PRIx8, tcpa->server.pci_dev_number);
>   	fwts_log_info_verbatim(fw, "  PCI Function:                    0x%2.2"   PRIx8, tcpa->server.pci_func_number);
>   
> -	if (tcpa->server.reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"TCPAReservedNonZero",
> -			"TCPA first reserved field must be zero, got "
> -			"0x%2.2" PRIx8 " instead", tcpa->server.reserved);
> -	}
> -
> -	if (reserved2 != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"TCPAReservedNonZero",
> -			"TCPA second reserved field must be zero, got "
> -			"0x%2.2" PRIx8 " instead", reserved2);
> -	}
> -
> -	if (tcpa->server.reserved3 != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"TCPAReservedNonZero",
> -			"TCPA third reserved field must be zero, got "
> -			"0x%2.2" PRIx8 " instead", tcpa->server.reserved3);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "TCPA", "Reserved", tcpa->server.reserved, sizeof(tcpa->server.reserved), &passed);
> +	fwts_acpi_reserved_zero_check(fw, "TCPA", "Reserved2", reserved2, sizeof(reserved2), &passed);
> +	fwts_acpi_reserved_zero_check(fw, "TCPA", "Reserved3", tcpa->server.reserved3, sizeof(tcpa->server.reserved3), &passed);
>   
>   	if (tcpa->server.device_flag & 1) {
>   		if (!(tcpa->server.interrupt_flag & 2)) {
> diff --git a/src/acpi/tpm2/tpm2.c b/src/acpi/tpm2/tpm2.c
> index 2662646..dce8dd3 100644
> --- a/src/acpi/tpm2/tpm2.c
> +++ b/src/acpi/tpm2/tpm2.c
> @@ -62,13 +62,7 @@ static int tpm2_test1(fwts_framework *fw)
>   			tpm2->platform_class);
>   	}
>   
> -	if (tpm2->reserved != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"TPM2ReservedNonZero",
> -			"TPM2 reserved field must be zero, got "
> -			"0x%4.4" PRIx16 " instead", tpm2->reserved);
> -	}
> +	fwts_acpi_reserved_zero_check(fw, "TPM2", "Reserved", tpm2->reserved, sizeof(tpm2->reserved), &passed);
>   
>   	if (tpm2->start_method < 1 || tpm2->start_method >= 12) {
>   		passed = false;
> 


-- 
Cheers,
Alex Hung



More information about the fwts-devel mailing list