diff mbox series

[2/3] acpi: refactor checks for fixed values by fwts_acpi_fixed_value_check

Message ID 20210105041952.161864-2-alex.hung@canonical.com
State Accepted
Headers show
Series [1/3] lib: add fwts_acpi_fixed_value_check function to check fixed values | expand

Commit Message

Alex Hung Jan. 5, 2021, 4:19 a.m. UTC
Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 src/acpi/bgrt/bgrt.c | 20 ++------------------
 src/acpi/cpep/cpep.c | 26 ++++++++------------------
 src/acpi/dbg2/dbg2.c | 10 ++--------
 src/acpi/fpdt/fpdt.c | 29 +++++------------------------
 src/acpi/iort/iort.c | 10 ++--------
 src/acpi/msdm/msdm.c | 11 +++--------
 src/acpi/spcr/spcr.c | 18 ++----------------
 src/acpi/spmi/spmi.c |  9 +--------
 src/acpi/srat/srat.c |  8 +-------
 src/acpi/tcpa/tcpa.c | 38 ++++----------------------------------
 src/acpi/wpbt/wpbt.c | 19 ++++---------------
 src/acpi/xenv/xenv.c |  9 +--------
 12 files changed, 35 insertions(+), 172 deletions(-)

Comments

Colin Ian King Jan. 5, 2021, 8:49 a.m. UTC | #1
On 05/01/2021 04:19, Alex Hung wrote:
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  src/acpi/bgrt/bgrt.c | 20 ++------------------
>  src/acpi/cpep/cpep.c | 26 ++++++++------------------
>  src/acpi/dbg2/dbg2.c | 10 ++--------
>  src/acpi/fpdt/fpdt.c | 29 +++++------------------------
>  src/acpi/iort/iort.c | 10 ++--------
>  src/acpi/msdm/msdm.c | 11 +++--------
>  src/acpi/spcr/spcr.c | 18 ++----------------
>  src/acpi/spmi/spmi.c |  9 +--------
>  src/acpi/srat/srat.c |  8 +-------
>  src/acpi/tcpa/tcpa.c | 38 ++++----------------------------------
>  src/acpi/wpbt/wpbt.c | 19 ++++---------------
>  src/acpi/xenv/xenv.c |  9 +--------
>  12 files changed, 35 insertions(+), 172 deletions(-)
> 
> diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c
> index e15044ce..0efee7dc 100644
> --- a/src/acpi/bgrt/bgrt.c
> +++ b/src/acpi/bgrt/bgrt.c
> @@ -46,25 +46,9 @@ static int bgrt_test1(fwts_framework *fw)
>  	fwts_log_info_verbatim(fw, "  Image Offset X:           0x%8.8" PRIx32, bgrt->image_offset_x);
>  	fwts_log_info_verbatim(fw, "  Image Offset Y:           0x%8.8" PRIx32, bgrt->image_offset_y);
>  
> -	if (bgrt->version != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"BGRTInvalidVersion",
> -			"BGRT: Version field is 0x%" PRIx8
> -			" and not the expected value of 0x01",
> -			bgrt->version);
> -	}
> -
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "BGRT", "Version", bgrt->version, 1, &passed);
>  	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,
> -			"BGRTInvalidImageType",
> -			"BGRT: Image Type field is 0x%" PRIx8
> -			", the only allowed type is 0x00",
> -			bgrt->image_type);
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "BGRT", "Image Type", bgrt->image_type, 0, &passed);
>  
>  	if (passed)
>  		fwts_passed(fw, "No issues found in BGRT table.");
> diff --git a/src/acpi/cpep/cpep.c b/src/acpi/cpep/cpep.c
> index e0b0772f..2e0f4eda 100644
> --- a/src/acpi/cpep/cpep.c
> +++ b/src/acpi/cpep/cpep.c
> @@ -58,26 +58,16 @@ static int cpep_test1(fwts_framework *fw)
>  
>  	for (i = 0; i < n; i++) {
>  		bool cpep_passed = true;
> +		char label[40];
> +
>  		fwts_acpi_cpep_processor_info *info = &cpep->cpep_info[i];
>  
> -		if (info->type != 0) {
> -			cpep_passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"CPEPInvalidProcStructureType",
> -				"CPEP: Processor Structure %" PRIu32
> -				" Type field is 0x%" PRIx8
> -				" and was expected to be 0x00",
> -				i, info->type);
> -		}
> -		if (info->length != 8) {
> -			cpep_passed = false;
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"CPEPInvalidProcStructureType",
> -				"CPEP: Processor Structure %" PRIu32
> -				" Length field is 0x%" PRIx8
> -				" and was expected to be 0x08",
> -				i, info->length);
> -		}
> +		snprintf(label, 40, "Processor Structure %d Type", i);
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "CPEP", label, info->type, 0, &cpep_passed);
> +
> +		snprintf(label, 40, "Processor Structure %d Length", i);
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "CPEP", label, info->length, 8, &cpep_passed);
> +
>  		/* Should probably sanity check processor UID, EIDs at a later date */
>  
>  		/* Some feedback if polling interval is very short */
> diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
> index d0e72fe1..2b4eb73a 100644
> --- a/src/acpi/dbg2/dbg2.c
> +++ b/src/acpi/dbg2/dbg2.c
> @@ -260,14 +260,8 @@ static int dbg2_test1(fwts_framework *fw)
>  		fwts_log_info_verbatim(fw, "  Address Size Offset:      0x%4.4" PRIx16, info->address_size_offset);
>  		fwts_log_nl(fw);
>  
> -		if (info->revision != 0) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"DBG2NonZeroRevision",
> -				"DBG2 Info Structure Revision is 0x%2.2" PRIx8
> -				" and was expecting 0x00",
> -				info->revision);
> -		}
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "DBG2", "Info Structure Revision", info->revision, 0, &passed);
> +
>  		if (port == NULL) {
>  			passed = false;
>  			fwts_failed(fw, LOG_LEVEL_HIGH,
> diff --git a/src/acpi/fpdt/fpdt.c b/src/acpi/fpdt/fpdt.c
> index 085477bf..88cd55e9 100644
> --- a/src/acpi/fpdt/fpdt.c
> +++ b/src/acpi/fpdt/fpdt.c
> @@ -71,14 +71,7 @@ static int fpdt_test1(fwts_framework *fw)
>  		goto done;
>  	}
>  
> -	if (header->revision != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"FPDTBadRevision",
> -			"FPDT revision is incorrect, expecting 0x01,"
> -			" instead got 0x%2.2" PRIx8,
> -			header->revision);
> -	}
> +	fwts_acpi_revision_check("FPDT", header->revision, 1, &passed);
>  
>  	ptr += sizeof(fwts_acpi_table_header);
>  	while (ptr < data + table->length) {
> @@ -120,14 +113,8 @@ static int fpdt_test1(fwts_framework *fw)
>  				fwts_log_info(fw, "Note: currently fwts does not check FBPT validity and the associated data");
>  			}
>  
> -			if (fbbpr->fpdt.revision != 1) {
> -				passed = false;
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					"FPDTInvalidRevision",
> -					"FPDT: Revision field is 0x%" PRIx8
> -					" and not the expected value of 0x01",
> -					fbbpr->fpdt.revision);
> -			}
> +			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "FPDT", "FBPT Revision", fbbpr->fpdt.revision, 1, &passed);
> +
>  			/* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
>  			break;
>  		case 0x0001:	/* S3 Performance Table Pointer Record */
> @@ -153,14 +140,8 @@ static int fpdt_test1(fwts_framework *fw)
>  				fwts_log_info(fw, "Note: currently fwts does not check S3PT validity and the associated data");
>  			}
>  
> -			if (s3ptpr->fpdt.revision != 1) {
> -				passed = false;
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					"FPDTInvalidRevision",
> -					"FPDT: Revision field is 0x%" PRIx8
> -					" and not the expected value of 0x01",
> -					s3ptpr->fpdt.revision);
> -			}
> +			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "FPDT", "S3PT Revision", s3ptpr->fpdt.revision, 1, &passed);
> +
>  			/* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
>  			break;
>  		case 0x0002 ... 0x0fff:
> diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c
> index e3857c32..2f2d7838 100644
> --- a/src/acpi/iort/iort.c
> +++ b/src/acpi/iort/iort.c
> @@ -70,14 +70,8 @@ static void iort_node_check(
>  				node->revision);
>  		}
>  
> -	} else if (node->revision != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"IORTNodeRevisionNonZero",
> -			"IORT Node Revision field is 0x%2.2" PRIx8
> -			" and should be zero.",
> -			node->revision);
> -	}
> +	} else
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "IORT", "IORT Node Revision", node->revision, 0, passed);
>  
>  	fwts_acpi_reserved_zero_check(fw, "IORT", "Node Reserved", node->reserved, sizeof(node->reserved), passed);
>  
> diff --git a/src/acpi/msdm/msdm.c b/src/acpi/msdm/msdm.c
> index 9f9d0dc7..bffc8699 100644
> --- a/src/acpi/msdm/msdm.c
> +++ b/src/acpi/msdm/msdm.c
> @@ -71,14 +71,9 @@ static int msdm_test1(fwts_framework *fw)
>  	switch (msdm->data_type) {
>  	case 0x0001:
>  		/* Check license key size */
> -		if (msdm->data_length != 0x1d) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"MSDMDataLengthInvalid",
> -				"MSDM Data Length field should be 0x0000001d, got 0x8.8%" PRIx32
> -				" instead",
> -				msdm->data_length);
> -		} else {
> +		if (msdm->data_length != 0x1d)
> +			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "MSDM", "Data Length", msdm->data_length, 29, &passed);
> +		else {
>  			size_t i;
>  			bool invalid = false;
>  			/* Note, no checks to see if this is a valid key! */
> diff --git a/src/acpi/spcr/spcr.c b/src/acpi/spcr/spcr.c
> index bde4ec9c..c05da580 100644
> --- a/src/acpi/spcr/spcr.c
> +++ b/src/acpi/spcr/spcr.c
> @@ -202,22 +202,8 @@ static int spcr_test1(fwts_framework *fw)
>  			" is a reserved baud rate", spcr->baud_rate);
>  	}
>  
> -	if (spcr->parity != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"SPCRReservedValueUsed",
> -			"SPCR Parity field must be zero, got 0x%2.2" PRIx8 " instead",
> -			spcr->parity);
> -	}
> -
> -	if (spcr->stop_bits != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"SPCRReservedValueUsed",
> -			"SPCR Stop field must be 1, got 0x%2.2" PRIx8 " instead",
> -			spcr->stop_bits);
> -	}
> -
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "SPCR", "Parity", spcr->parity, 0, &passed);
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "SPCR", "Stop", spcr->stop_bits, 1, &passed);
>  	fwts_acpi_reserved_bits_check(fw, "SPCR", "Flow control", spcr->flow_control, sizeof(spcr->flow_control), 3, 7, &passed);
>  
>  	reserved = false;
> diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
> index ed7886dd..edabb494 100644
> --- a/src/acpi/spmi/spmi.c
> +++ b/src/acpi/spmi/spmi.c
> @@ -96,14 +96,7 @@ static int spmi_test1(fwts_framework *fw)
>  			spmi->interface_type);
>  	}
>  
> -	if (spmi->reserved1 != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"SPMIReserved1Not1",
> -			"SPMI reserved field (offset 37) must be 0x01, got "
> -			"0x%2.2" PRIx8 " instead", spmi->reserved1);
> -	}
> -
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "SPMI", "Reserved1", spmi->reserved1, 1, &passed);
>  	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 */
> diff --git a/src/acpi/srat/srat.c b/src/acpi/srat/srat.c
> index 1dd19eae..56ee5e3d 100644
> --- a/src/acpi/srat/srat.c
> +++ b/src/acpi/srat/srat.c
> @@ -347,13 +347,7 @@ static int srat_test1(fwts_framework *fw)
>  	bool passed = true;
>  	ssize_t length = (ssize_t)srat->header.length;
>  
> -	if (srat->reserved1 != 1) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"SRATInterfaceReserved",
> -			"SRAT reserved field 1 should be 1, instead was "
> -			"0x%" PRIx32, srat->reserved1);
> -		passed = false;
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "SRAT", "Revision1", srat->reserved1, 1, &passed);
>  
>  	data += sizeof(fwts_acpi_table_srat);
>  	length -= sizeof(fwts_acpi_table_srat);
> diff --git a/src/acpi/tcpa/tcpa.c b/src/acpi/tcpa/tcpa.c
> index 5904858e..817f7273 100644
> --- a/src/acpi/tcpa/tcpa.c
> +++ b/src/acpi/tcpa/tcpa.c
> @@ -30,23 +30,8 @@ static int tcpa_client_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
>  {
>  	bool passed = true;
>  
> -	if (tcpa->header.length != 50) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadSize",
> -			"TCPA size is incorrect, expecting 0x32 bytes, "
> -			"instead got 0x%8.8" PRIx32 " bytes",
> -			tcpa->header.length);
> -	}
> -
> -	if (tcpa->header.revision != 2) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadRevision",
> -			"TCPA revision is incorrect, expecting 0x02, "
> -			"instead got 0x%2.2" PRIx8,
> -			tcpa->header.revision);
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "TCPA", "Length", tcpa->header.length, 50, &passed);
> +	fwts_acpi_revision_check("TCPA", tcpa->header.revision, 2, &passed);
>  
>  	fwts_log_info_verbatim(fw, "TCPA Table:");
>  	fwts_log_info_verbatim(fw, "  Platform Class:                  0x%4.4"   PRIx16, tcpa->platform_class);
> @@ -61,23 +46,8 @@ static int tcpa_server_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
>  	bool passed = true;
>  	uint32_t reserved2;
>  
> -	if (tcpa->header.length != 100) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadSize",
> -			"TCPA size is incorrect, expecting 0x64 bytes, "
> -			"instead got 0x%8.8" PRIx32 " bytes",
> -			tcpa->header.length);
> -	}
> -
> -	if (tcpa->header.revision != 2) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadRevision",
> -			"TCPA revision is incorrect, expecting 0x02, "
> -			"instead got 0x%2.2" PRIx8,
> -			tcpa->header.revision);
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "TCPA", "Length", tcpa->header.length, 100, &passed);
> +	fwts_acpi_revision_check("TCPA", tcpa->header.revision, 2, &passed);
>  
>  	reserved2 = tcpa->server.reserved2[0] + (tcpa->server.reserved2[1] << 4) + (tcpa->server.reserved2[2] << 8);
>  
> diff --git a/src/acpi/wpbt/wpbt.c b/src/acpi/wpbt/wpbt.c
> index 64eebf91..7082cff7 100644
> --- a/src/acpi/wpbt/wpbt.c
> +++ b/src/acpi/wpbt/wpbt.c
> @@ -44,22 +44,11 @@ static int wpbt_test1(fwts_framework *fw)
>  	fwts_log_info_verbatim(fw, "  Content Layout:           0x%2.2" PRIx8, wpbt->layout);
>  	fwts_log_info_verbatim(fw, "  Content Type:             0x%2.2" PRIx8, wpbt->type);
>  
> -	if (wpbt->layout != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"WPBTBadLayout",
> -			"WPBT supports Content Layout 1 only, got "
> -			"0x%2.2" PRIx8 " instead", wpbt->layout);
> -	}
> -
> -	if (wpbt->type != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"WPBTBadType",
> -			"WPBT supports Content Type 1 only, got "
> -			"0x%2.2" PRIx8 " instead", wpbt->type);
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "WPBT", "Layout", wpbt->layout, 1, &passed);
>  
> -	} else {
> +	if (wpbt->type != 1)
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "WPBT", "Type", wpbt->type, 1, &passed);
> +	else {
>  		fwts_acpi_table_wpbt_type1 *type = (fwts_acpi_table_wpbt_type1 *) (table->data + sizeof(fwts_acpi_table_wpbt));
>  		fwts_log_info_verbatim(fw, "  Arguments Length:         0x%4.4" PRIx16, type->arguments_length);
>  
> diff --git a/src/acpi/xenv/xenv.c b/src/acpi/xenv/xenv.c
> index 012faac6..4199e934 100644
> --- a/src/acpi/xenv/xenv.c
> +++ b/src/acpi/xenv/xenv.c
> @@ -43,14 +43,7 @@ static int xenv_test1(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	}
>  
> -	if (xenv->header.revision != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"XENVBadRevision",
> -			"XENV revision is incorrect, expecting 0x01, "
> -			"instead got 0x%2.2" PRIx8,
> -			xenv->header.revision);
> -	}
> +	fwts_acpi_revision_check("XENV", xenv->header.revision, 1, &passed);
>  
>  	fwts_log_info_verbatim(fw, "XENV Table:");
>  	fwts_log_info_verbatim(fw, "  GNT Start Address:               0x%16.16" PRIx64, xenv->gnt_start);
> 
Acked-by: Colin Ian King <colin.king@canonical.com>
Ivan Hu Jan. 6, 2021, 6:07 a.m. UTC | #2
On 1/5/21 12:19 PM, Alex Hung wrote:
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  src/acpi/bgrt/bgrt.c | 20 ++------------------
>  src/acpi/cpep/cpep.c | 26 ++++++++------------------
>  src/acpi/dbg2/dbg2.c | 10 ++--------
>  src/acpi/fpdt/fpdt.c | 29 +++++------------------------
>  src/acpi/iort/iort.c | 10 ++--------
>  src/acpi/msdm/msdm.c | 11 +++--------
>  src/acpi/spcr/spcr.c | 18 ++----------------
>  src/acpi/spmi/spmi.c |  9 +--------
>  src/acpi/srat/srat.c |  8 +-------
>  src/acpi/tcpa/tcpa.c | 38 ++++----------------------------------
>  src/acpi/wpbt/wpbt.c | 19 ++++---------------
>  src/acpi/xenv/xenv.c |  9 +--------
>  12 files changed, 35 insertions(+), 172 deletions(-)
> 
> diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c
> index e15044ce..0efee7dc 100644
> --- a/src/acpi/bgrt/bgrt.c
> +++ b/src/acpi/bgrt/bgrt.c
> @@ -46,25 +46,9 @@ static int bgrt_test1(fwts_framework *fw)
>  	fwts_log_info_verbatim(fw, "  Image Offset X:           0x%8.8" PRIx32, bgrt->image_offset_x);
>  	fwts_log_info_verbatim(fw, "  Image Offset Y:           0x%8.8" PRIx32, bgrt->image_offset_y);
>  
> -	if (bgrt->version != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"BGRTInvalidVersion",
> -			"BGRT: Version field is 0x%" PRIx8
> -			" and not the expected value of 0x01",
> -			bgrt->version);
> -	}
> -
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "BGRT", "Version", bgrt->version, 1, &passed);
>  	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,
> -			"BGRTInvalidImageType",
> -			"BGRT: Image Type field is 0x%" PRIx8
> -			", the only allowed type is 0x00",
> -			bgrt->image_type);
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "BGRT", "Image Type", bgrt->image_type, 0, &passed);
>  
>  	if (passed)
>  		fwts_passed(fw, "No issues found in BGRT table.");
> diff --git a/src/acpi/cpep/cpep.c b/src/acpi/cpep/cpep.c
> index e0b0772f..2e0f4eda 100644
> --- a/src/acpi/cpep/cpep.c
> +++ b/src/acpi/cpep/cpep.c
> @@ -58,26 +58,16 @@ static int cpep_test1(fwts_framework *fw)
>  
>  	for (i = 0; i < n; i++) {
>  		bool cpep_passed = true;
> +		char label[40];
> +
>  		fwts_acpi_cpep_processor_info *info = &cpep->cpep_info[i];
>  
> -		if (info->type != 0) {
> -			cpep_passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"CPEPInvalidProcStructureType",
> -				"CPEP: Processor Structure %" PRIu32
> -				" Type field is 0x%" PRIx8
> -				" and was expected to be 0x00",
> -				i, info->type);
> -		}
> -		if (info->length != 8) {
> -			cpep_passed = false;
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"CPEPInvalidProcStructureType",
> -				"CPEP: Processor Structure %" PRIu32
> -				" Length field is 0x%" PRIx8
> -				" and was expected to be 0x08",
> -				i, info->length);
> -		}
> +		snprintf(label, 40, "Processor Structure %d Type", i);
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "CPEP", label, info->type, 0, &cpep_passed);
> +
> +		snprintf(label, 40, "Processor Structure %d Length", i);
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "CPEP", label, info->length, 8, &cpep_passed);
> +
>  		/* Should probably sanity check processor UID, EIDs at a later date */
>  
>  		/* Some feedback if polling interval is very short */
> diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
> index d0e72fe1..2b4eb73a 100644
> --- a/src/acpi/dbg2/dbg2.c
> +++ b/src/acpi/dbg2/dbg2.c
> @@ -260,14 +260,8 @@ static int dbg2_test1(fwts_framework *fw)
>  		fwts_log_info_verbatim(fw, "  Address Size Offset:      0x%4.4" PRIx16, info->address_size_offset);
>  		fwts_log_nl(fw);
>  
> -		if (info->revision != 0) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"DBG2NonZeroRevision",
> -				"DBG2 Info Structure Revision is 0x%2.2" PRIx8
> -				" and was expecting 0x00",
> -				info->revision);
> -		}
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "DBG2", "Info Structure Revision", info->revision, 0, &passed);
> +
>  		if (port == NULL) {
>  			passed = false;
>  			fwts_failed(fw, LOG_LEVEL_HIGH,
> diff --git a/src/acpi/fpdt/fpdt.c b/src/acpi/fpdt/fpdt.c
> index 085477bf..88cd55e9 100644
> --- a/src/acpi/fpdt/fpdt.c
> +++ b/src/acpi/fpdt/fpdt.c
> @@ -71,14 +71,7 @@ static int fpdt_test1(fwts_framework *fw)
>  		goto done;
>  	}
>  
> -	if (header->revision != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"FPDTBadRevision",
> -			"FPDT revision is incorrect, expecting 0x01,"
> -			" instead got 0x%2.2" PRIx8,
> -			header->revision);
> -	}
> +	fwts_acpi_revision_check("FPDT", header->revision, 1, &passed);
>  
>  	ptr += sizeof(fwts_acpi_table_header);
>  	while (ptr < data + table->length) {
> @@ -120,14 +113,8 @@ static int fpdt_test1(fwts_framework *fw)
>  				fwts_log_info(fw, "Note: currently fwts does not check FBPT validity and the associated data");
>  			}
>  
> -			if (fbbpr->fpdt.revision != 1) {
> -				passed = false;
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					"FPDTInvalidRevision",
> -					"FPDT: Revision field is 0x%" PRIx8
> -					" and not the expected value of 0x01",
> -					fbbpr->fpdt.revision);
> -			}
> +			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "FPDT", "FBPT Revision", fbbpr->fpdt.revision, 1, &passed);
> +
>  			/* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
>  			break;
>  		case 0x0001:	/* S3 Performance Table Pointer Record */
> @@ -153,14 +140,8 @@ static int fpdt_test1(fwts_framework *fw)
>  				fwts_log_info(fw, "Note: currently fwts does not check S3PT validity and the associated data");
>  			}
>  
> -			if (s3ptpr->fpdt.revision != 1) {
> -				passed = false;
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -					"FPDTInvalidRevision",
> -					"FPDT: Revision field is 0x%" PRIx8
> -					" and not the expected value of 0x01",
> -					s3ptpr->fpdt.revision);
> -			}
> +			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "FPDT", "S3PT Revision", s3ptpr->fpdt.revision, 1, &passed);
> +
>  			/* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
>  			break;
>  		case 0x0002 ... 0x0fff:
> diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c
> index e3857c32..2f2d7838 100644
> --- a/src/acpi/iort/iort.c
> +++ b/src/acpi/iort/iort.c
> @@ -70,14 +70,8 @@ static void iort_node_check(
>  				node->revision);
>  		}
>  
> -	} else if (node->revision != 0) {
> -		*passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"IORTNodeRevisionNonZero",
> -			"IORT Node Revision field is 0x%2.2" PRIx8
> -			" and should be zero.",
> -			node->revision);
> -	}
> +	} else
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "IORT", "IORT Node Revision", node->revision, 0, passed);
>  
>  	fwts_acpi_reserved_zero_check(fw, "IORT", "Node Reserved", node->reserved, sizeof(node->reserved), passed);
>  
> diff --git a/src/acpi/msdm/msdm.c b/src/acpi/msdm/msdm.c
> index 9f9d0dc7..bffc8699 100644
> --- a/src/acpi/msdm/msdm.c
> +++ b/src/acpi/msdm/msdm.c
> @@ -71,14 +71,9 @@ static int msdm_test1(fwts_framework *fw)
>  	switch (msdm->data_type) {
>  	case 0x0001:
>  		/* Check license key size */
> -		if (msdm->data_length != 0x1d) {
> -			passed = false;
> -			fwts_failed(fw, LOG_LEVEL_HIGH,
> -				"MSDMDataLengthInvalid",
> -				"MSDM Data Length field should be 0x0000001d, got 0x8.8%" PRIx32
> -				" instead",
> -				msdm->data_length);
> -		} else {
> +		if (msdm->data_length != 0x1d)
> +			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "MSDM", "Data Length", msdm->data_length, 29, &passed);
> +		else {
>  			size_t i;
>  			bool invalid = false;
>  			/* Note, no checks to see if this is a valid key! */
> diff --git a/src/acpi/spcr/spcr.c b/src/acpi/spcr/spcr.c
> index bde4ec9c..c05da580 100644
> --- a/src/acpi/spcr/spcr.c
> +++ b/src/acpi/spcr/spcr.c
> @@ -202,22 +202,8 @@ static int spcr_test1(fwts_framework *fw)
>  			" is a reserved baud rate", spcr->baud_rate);
>  	}
>  
> -	if (spcr->parity != 0) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"SPCRReservedValueUsed",
> -			"SPCR Parity field must be zero, got 0x%2.2" PRIx8 " instead",
> -			spcr->parity);
> -	}
> -
> -	if (spcr->stop_bits != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"SPCRReservedValueUsed",
> -			"SPCR Stop field must be 1, got 0x%2.2" PRIx8 " instead",
> -			spcr->stop_bits);
> -	}
> -
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "SPCR", "Parity", spcr->parity, 0, &passed);
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "SPCR", "Stop", spcr->stop_bits, 1, &passed);
>  	fwts_acpi_reserved_bits_check(fw, "SPCR", "Flow control", spcr->flow_control, sizeof(spcr->flow_control), 3, 7, &passed);
>  
>  	reserved = false;
> diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
> index ed7886dd..edabb494 100644
> --- a/src/acpi/spmi/spmi.c
> +++ b/src/acpi/spmi/spmi.c
> @@ -96,14 +96,7 @@ static int spmi_test1(fwts_framework *fw)
>  			spmi->interface_type);
>  	}
>  
> -	if (spmi->reserved1 != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"SPMIReserved1Not1",
> -			"SPMI reserved field (offset 37) must be 0x01, got "
> -			"0x%2.2" PRIx8 " instead", spmi->reserved1);
> -	}
> -
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "SPMI", "Reserved1", spmi->reserved1, 1, &passed);
>  	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 */
> diff --git a/src/acpi/srat/srat.c b/src/acpi/srat/srat.c
> index 1dd19eae..56ee5e3d 100644
> --- a/src/acpi/srat/srat.c
> +++ b/src/acpi/srat/srat.c
> @@ -347,13 +347,7 @@ static int srat_test1(fwts_framework *fw)
>  	bool passed = true;
>  	ssize_t length = (ssize_t)srat->header.length;
>  
> -	if (srat->reserved1 != 1) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"SRATInterfaceReserved",
> -			"SRAT reserved field 1 should be 1, instead was "
> -			"0x%" PRIx32, srat->reserved1);
> -		passed = false;
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "SRAT", "Revision1", srat->reserved1, 1, &passed);
>  
>  	data += sizeof(fwts_acpi_table_srat);
>  	length -= sizeof(fwts_acpi_table_srat);
> diff --git a/src/acpi/tcpa/tcpa.c b/src/acpi/tcpa/tcpa.c
> index 5904858e..817f7273 100644
> --- a/src/acpi/tcpa/tcpa.c
> +++ b/src/acpi/tcpa/tcpa.c
> @@ -30,23 +30,8 @@ static int tcpa_client_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
>  {
>  	bool passed = true;
>  
> -	if (tcpa->header.length != 50) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadSize",
> -			"TCPA size is incorrect, expecting 0x32 bytes, "
> -			"instead got 0x%8.8" PRIx32 " bytes",
> -			tcpa->header.length);
> -	}
> -
> -	if (tcpa->header.revision != 2) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadRevision",
> -			"TCPA revision is incorrect, expecting 0x02, "
> -			"instead got 0x%2.2" PRIx8,
> -			tcpa->header.revision);
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "TCPA", "Length", tcpa->header.length, 50, &passed);
> +	fwts_acpi_revision_check("TCPA", tcpa->header.revision, 2, &passed);
>  
>  	fwts_log_info_verbatim(fw, "TCPA Table:");
>  	fwts_log_info_verbatim(fw, "  Platform Class:                  0x%4.4"   PRIx16, tcpa->platform_class);
> @@ -61,23 +46,8 @@ static int tcpa_server_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
>  	bool passed = true;
>  	uint32_t reserved2;
>  
> -	if (tcpa->header.length != 100) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadSize",
> -			"TCPA size is incorrect, expecting 0x64 bytes, "
> -			"instead got 0x%8.8" PRIx32 " bytes",
> -			tcpa->header.length);
> -	}
> -
> -	if (tcpa->header.revision != 2) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"TCPABadRevision",
> -			"TCPA revision is incorrect, expecting 0x02, "
> -			"instead got 0x%2.2" PRIx8,
> -			tcpa->header.revision);
> -	}
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "TCPA", "Length", tcpa->header.length, 100, &passed);
> +	fwts_acpi_revision_check("TCPA", tcpa->header.revision, 2, &passed);
>  
>  	reserved2 = tcpa->server.reserved2[0] + (tcpa->server.reserved2[1] << 4) + (tcpa->server.reserved2[2] << 8);
>  
> diff --git a/src/acpi/wpbt/wpbt.c b/src/acpi/wpbt/wpbt.c
> index 64eebf91..7082cff7 100644
> --- a/src/acpi/wpbt/wpbt.c
> +++ b/src/acpi/wpbt/wpbt.c
> @@ -44,22 +44,11 @@ static int wpbt_test1(fwts_framework *fw)
>  	fwts_log_info_verbatim(fw, "  Content Layout:           0x%2.2" PRIx8, wpbt->layout);
>  	fwts_log_info_verbatim(fw, "  Content Type:             0x%2.2" PRIx8, wpbt->type);
>  
> -	if (wpbt->layout != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"WPBTBadLayout",
> -			"WPBT supports Content Layout 1 only, got "
> -			"0x%2.2" PRIx8 " instead", wpbt->layout);
> -	}
> -
> -	if (wpbt->type != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"WPBTBadType",
> -			"WPBT supports Content Type 1 only, got "
> -			"0x%2.2" PRIx8 " instead", wpbt->type);
> +	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "WPBT", "Layout", wpbt->layout, 1, &passed);
>  
> -	} else {
> +	if (wpbt->type != 1)
> +		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "WPBT", "Type", wpbt->type, 1, &passed);
> +	else {
>  		fwts_acpi_table_wpbt_type1 *type = (fwts_acpi_table_wpbt_type1 *) (table->data + sizeof(fwts_acpi_table_wpbt));
>  		fwts_log_info_verbatim(fw, "  Arguments Length:         0x%4.4" PRIx16, type->arguments_length);
>  
> diff --git a/src/acpi/xenv/xenv.c b/src/acpi/xenv/xenv.c
> index 012faac6..4199e934 100644
> --- a/src/acpi/xenv/xenv.c
> +++ b/src/acpi/xenv/xenv.c
> @@ -43,14 +43,7 @@ static int xenv_test1(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	}
>  
> -	if (xenv->header.revision != 1) {
> -		passed = false;
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"XENVBadRevision",
> -			"XENV revision is incorrect, expecting 0x01, "
> -			"instead got 0x%2.2" PRIx8,
> -			xenv->header.revision);
> -	}
> +	fwts_acpi_revision_check("XENV", xenv->header.revision, 1, &passed);
>  
>  	fwts_log_info_verbatim(fw, "XENV Table:");
>  	fwts_log_info_verbatim(fw, "  GNT Start Address:               0x%16.16" PRIx64, xenv->gnt_start);
> 

Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox series

Patch

diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c
index e15044ce..0efee7dc 100644
--- a/src/acpi/bgrt/bgrt.c
+++ b/src/acpi/bgrt/bgrt.c
@@ -46,25 +46,9 @@  static int bgrt_test1(fwts_framework *fw)
 	fwts_log_info_verbatim(fw, "  Image Offset X:           0x%8.8" PRIx32, bgrt->image_offset_x);
 	fwts_log_info_verbatim(fw, "  Image Offset Y:           0x%8.8" PRIx32, bgrt->image_offset_y);
 
-	if (bgrt->version != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"BGRTInvalidVersion",
-			"BGRT: Version field is 0x%" PRIx8
-			" and not the expected value of 0x01",
-			bgrt->version);
-	}
-
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "BGRT", "Version", bgrt->version, 1, &passed);
 	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,
-			"BGRTInvalidImageType",
-			"BGRT: Image Type field is 0x%" PRIx8
-			", the only allowed type is 0x00",
-			bgrt->image_type);
-	}
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "BGRT", "Image Type", bgrt->image_type, 0, &passed);
 
 	if (passed)
 		fwts_passed(fw, "No issues found in BGRT table.");
diff --git a/src/acpi/cpep/cpep.c b/src/acpi/cpep/cpep.c
index e0b0772f..2e0f4eda 100644
--- a/src/acpi/cpep/cpep.c
+++ b/src/acpi/cpep/cpep.c
@@ -58,26 +58,16 @@  static int cpep_test1(fwts_framework *fw)
 
 	for (i = 0; i < n; i++) {
 		bool cpep_passed = true;
+		char label[40];
+
 		fwts_acpi_cpep_processor_info *info = &cpep->cpep_info[i];
 
-		if (info->type != 0) {
-			cpep_passed = false;
-			fwts_failed(fw, LOG_LEVEL_HIGH,
-				"CPEPInvalidProcStructureType",
-				"CPEP: Processor Structure %" PRIu32
-				" Type field is 0x%" PRIx8
-				" and was expected to be 0x00",
-				i, info->type);
-		}
-		if (info->length != 8) {
-			cpep_passed = false;
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"CPEPInvalidProcStructureType",
-				"CPEP: Processor Structure %" PRIu32
-				" Length field is 0x%" PRIx8
-				" and was expected to be 0x08",
-				i, info->length);
-		}
+		snprintf(label, 40, "Processor Structure %d Type", i);
+		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "CPEP", label, info->type, 0, &cpep_passed);
+
+		snprintf(label, 40, "Processor Structure %d Length", i);
+		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "CPEP", label, info->length, 8, &cpep_passed);
+
 		/* Should probably sanity check processor UID, EIDs at a later date */
 
 		/* Some feedback if polling interval is very short */
diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
index d0e72fe1..2b4eb73a 100644
--- a/src/acpi/dbg2/dbg2.c
+++ b/src/acpi/dbg2/dbg2.c
@@ -260,14 +260,8 @@  static int dbg2_test1(fwts_framework *fw)
 		fwts_log_info_verbatim(fw, "  Address Size Offset:      0x%4.4" PRIx16, info->address_size_offset);
 		fwts_log_nl(fw);
 
-		if (info->revision != 0) {
-			passed = false;
-			fwts_failed(fw, LOG_LEVEL_HIGH,
-				"DBG2NonZeroRevision",
-				"DBG2 Info Structure Revision is 0x%2.2" PRIx8
-				" and was expecting 0x00",
-				info->revision);
-		}
+		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "DBG2", "Info Structure Revision", info->revision, 0, &passed);
+
 		if (port == NULL) {
 			passed = false;
 			fwts_failed(fw, LOG_LEVEL_HIGH,
diff --git a/src/acpi/fpdt/fpdt.c b/src/acpi/fpdt/fpdt.c
index 085477bf..88cd55e9 100644
--- a/src/acpi/fpdt/fpdt.c
+++ b/src/acpi/fpdt/fpdt.c
@@ -71,14 +71,7 @@  static int fpdt_test1(fwts_framework *fw)
 		goto done;
 	}
 
-	if (header->revision != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"FPDTBadRevision",
-			"FPDT revision is incorrect, expecting 0x01,"
-			" instead got 0x%2.2" PRIx8,
-			header->revision);
-	}
+	fwts_acpi_revision_check("FPDT", header->revision, 1, &passed);
 
 	ptr += sizeof(fwts_acpi_table_header);
 	while (ptr < data + table->length) {
@@ -120,14 +113,8 @@  static int fpdt_test1(fwts_framework *fw)
 				fwts_log_info(fw, "Note: currently fwts does not check FBPT validity and the associated data");
 			}
 
-			if (fbbpr->fpdt.revision != 1) {
-				passed = false;
-				fwts_failed(fw, LOG_LEVEL_MEDIUM,
-					"FPDTInvalidRevision",
-					"FPDT: Revision field is 0x%" PRIx8
-					" and not the expected value of 0x01",
-					fbbpr->fpdt.revision);
-			}
+			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "FPDT", "FBPT Revision", fbbpr->fpdt.revision, 1, &passed);
+
 			/* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
 			break;
 		case 0x0001:	/* S3 Performance Table Pointer Record */
@@ -153,14 +140,8 @@  static int fpdt_test1(fwts_framework *fw)
 				fwts_log_info(fw, "Note: currently fwts does not check S3PT validity and the associated data");
 			}
 
-			if (s3ptpr->fpdt.revision != 1) {
-				passed = false;
-				fwts_failed(fw, LOG_LEVEL_MEDIUM,
-					"FPDTInvalidRevision",
-					"FPDT: Revision field is 0x%" PRIx8
-					" and not the expected value of 0x01",
-					s3ptpr->fpdt.revision);
-			}
+			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "FPDT", "S3PT Revision", s3ptpr->fpdt.revision, 1, &passed);
+
 			/* Spec doesn't mention Reserved field should be 0 or not, skip checking the reserved field */
 			break;
 		case 0x0002 ... 0x0fff:
diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c
index e3857c32..2f2d7838 100644
--- a/src/acpi/iort/iort.c
+++ b/src/acpi/iort/iort.c
@@ -70,14 +70,8 @@  static void iort_node_check(
 				node->revision);
 		}
 
-	} else if (node->revision != 0) {
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"IORTNodeRevisionNonZero",
-			"IORT Node Revision field is 0x%2.2" PRIx8
-			" and should be zero.",
-			node->revision);
-	}
+	} else
+		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "IORT", "IORT Node Revision", node->revision, 0, passed);
 
 	fwts_acpi_reserved_zero_check(fw, "IORT", "Node Reserved", node->reserved, sizeof(node->reserved), passed);
 
diff --git a/src/acpi/msdm/msdm.c b/src/acpi/msdm/msdm.c
index 9f9d0dc7..bffc8699 100644
--- a/src/acpi/msdm/msdm.c
+++ b/src/acpi/msdm/msdm.c
@@ -71,14 +71,9 @@  static int msdm_test1(fwts_framework *fw)
 	switch (msdm->data_type) {
 	case 0x0001:
 		/* Check license key size */
-		if (msdm->data_length != 0x1d) {
-			passed = false;
-			fwts_failed(fw, LOG_LEVEL_HIGH,
-				"MSDMDataLengthInvalid",
-				"MSDM Data Length field should be 0x0000001d, got 0x8.8%" PRIx32
-				" instead",
-				msdm->data_length);
-		} else {
+		if (msdm->data_length != 0x1d)
+			fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "MSDM", "Data Length", msdm->data_length, 29, &passed);
+		else {
 			size_t i;
 			bool invalid = false;
 			/* Note, no checks to see if this is a valid key! */
diff --git a/src/acpi/spcr/spcr.c b/src/acpi/spcr/spcr.c
index bde4ec9c..c05da580 100644
--- a/src/acpi/spcr/spcr.c
+++ b/src/acpi/spcr/spcr.c
@@ -202,22 +202,8 @@  static int spcr_test1(fwts_framework *fw)
 			" is a reserved baud rate", spcr->baud_rate);
 	}
 
-	if (spcr->parity != 0) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"SPCRReservedValueUsed",
-			"SPCR Parity field must be zero, got 0x%2.2" PRIx8 " instead",
-			spcr->parity);
-	}
-
-	if (spcr->stop_bits != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"SPCRReservedValueUsed",
-			"SPCR Stop field must be 1, got 0x%2.2" PRIx8 " instead",
-			spcr->stop_bits);
-	}
-
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "SPCR", "Parity", spcr->parity, 0, &passed);
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "SPCR", "Stop", spcr->stop_bits, 1, &passed);
 	fwts_acpi_reserved_bits_check(fw, "SPCR", "Flow control", spcr->flow_control, sizeof(spcr->flow_control), 3, 7, &passed);
 
 	reserved = false;
diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c
index ed7886dd..edabb494 100644
--- a/src/acpi/spmi/spmi.c
+++ b/src/acpi/spmi/spmi.c
@@ -96,14 +96,7 @@  static int spmi_test1(fwts_framework *fw)
 			spmi->interface_type);
 	}
 
-	if (spmi->reserved1 != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_LOW,
-			"SPMIReserved1Not1",
-			"SPMI reserved field (offset 37) must be 0x01, got "
-			"0x%2.2" PRIx8 " instead", spmi->reserved1);
-	}
-
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "SPMI", "Reserved1", spmi->reserved1, 1, &passed);
 	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 */
diff --git a/src/acpi/srat/srat.c b/src/acpi/srat/srat.c
index 1dd19eae..56ee5e3d 100644
--- a/src/acpi/srat/srat.c
+++ b/src/acpi/srat/srat.c
@@ -347,13 +347,7 @@  static int srat_test1(fwts_framework *fw)
 	bool passed = true;
 	ssize_t length = (ssize_t)srat->header.length;
 
-	if (srat->reserved1 != 1) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"SRATInterfaceReserved",
-			"SRAT reserved field 1 should be 1, instead was "
-			"0x%" PRIx32, srat->reserved1);
-		passed = false;
-	}
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_MEDIUM, "SRAT", "Revision1", srat->reserved1, 1, &passed);
 
 	data += sizeof(fwts_acpi_table_srat);
 	length -= sizeof(fwts_acpi_table_srat);
diff --git a/src/acpi/tcpa/tcpa.c b/src/acpi/tcpa/tcpa.c
index 5904858e..817f7273 100644
--- a/src/acpi/tcpa/tcpa.c
+++ b/src/acpi/tcpa/tcpa.c
@@ -30,23 +30,8 @@  static int tcpa_client_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
 {
 	bool passed = true;
 
-	if (tcpa->header.length != 50) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"TCPABadSize",
-			"TCPA size is incorrect, expecting 0x32 bytes, "
-			"instead got 0x%8.8" PRIx32 " bytes",
-			tcpa->header.length);
-	}
-
-	if (tcpa->header.revision != 2) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"TCPABadRevision",
-			"TCPA revision is incorrect, expecting 0x02, "
-			"instead got 0x%2.2" PRIx8,
-			tcpa->header.revision);
-	}
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "TCPA", "Length", tcpa->header.length, 50, &passed);
+	fwts_acpi_revision_check("TCPA", tcpa->header.revision, 2, &passed);
 
 	fwts_log_info_verbatim(fw, "TCPA Table:");
 	fwts_log_info_verbatim(fw, "  Platform Class:                  0x%4.4"   PRIx16, tcpa->platform_class);
@@ -61,23 +46,8 @@  static int tcpa_server_test(fwts_framework *fw, fwts_acpi_table_tcpa *tcpa)
 	bool passed = true;
 	uint32_t reserved2;
 
-	if (tcpa->header.length != 100) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"TCPABadSize",
-			"TCPA size is incorrect, expecting 0x64 bytes, "
-			"instead got 0x%8.8" PRIx32 " bytes",
-			tcpa->header.length);
-	}
-
-	if (tcpa->header.revision != 2) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"TCPABadRevision",
-			"TCPA revision is incorrect, expecting 0x02, "
-			"instead got 0x%2.2" PRIx8,
-			tcpa->header.revision);
-	}
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "TCPA", "Length", tcpa->header.length, 100, &passed);
+	fwts_acpi_revision_check("TCPA", tcpa->header.revision, 2, &passed);
 
 	reserved2 = tcpa->server.reserved2[0] + (tcpa->server.reserved2[1] << 4) + (tcpa->server.reserved2[2] << 8);
 
diff --git a/src/acpi/wpbt/wpbt.c b/src/acpi/wpbt/wpbt.c
index 64eebf91..7082cff7 100644
--- a/src/acpi/wpbt/wpbt.c
+++ b/src/acpi/wpbt/wpbt.c
@@ -44,22 +44,11 @@  static int wpbt_test1(fwts_framework *fw)
 	fwts_log_info_verbatim(fw, "  Content Layout:           0x%2.2" PRIx8, wpbt->layout);
 	fwts_log_info_verbatim(fw, "  Content Type:             0x%2.2" PRIx8, wpbt->type);
 
-	if (wpbt->layout != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"WPBTBadLayout",
-			"WPBT supports Content Layout 1 only, got "
-			"0x%2.2" PRIx8 " instead", wpbt->layout);
-	}
-
-	if (wpbt->type != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"WPBTBadType",
-			"WPBT supports Content Type 1 only, got "
-			"0x%2.2" PRIx8 " instead", wpbt->type);
+	fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "WPBT", "Layout", wpbt->layout, 1, &passed);
 
-	} else {
+	if (wpbt->type != 1)
+		fwts_acpi_fixed_value_check(fw, LOG_LEVEL_HIGH, "WPBT", "Type", wpbt->type, 1, &passed);
+	else {
 		fwts_acpi_table_wpbt_type1 *type = (fwts_acpi_table_wpbt_type1 *) (table->data + sizeof(fwts_acpi_table_wpbt));
 		fwts_log_info_verbatim(fw, "  Arguments Length:         0x%4.4" PRIx16, type->arguments_length);
 
diff --git a/src/acpi/xenv/xenv.c b/src/acpi/xenv/xenv.c
index 012faac6..4199e934 100644
--- a/src/acpi/xenv/xenv.c
+++ b/src/acpi/xenv/xenv.c
@@ -43,14 +43,7 @@  static int xenv_test1(fwts_framework *fw)
 		return FWTS_ERROR;
 	}
 
-	if (xenv->header.revision != 1) {
-		passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"XENVBadRevision",
-			"XENV revision is incorrect, expecting 0x01, "
-			"instead got 0x%2.2" PRIx8,
-			xenv->header.revision);
-	}
+	fwts_acpi_revision_check("XENV", xenv->header.revision, 1, &passed);
 
 	fwts_log_info_verbatim(fw, "XENV Table:");
 	fwts_log_info_verbatim(fw, "  GNT Start Address:               0x%16.16" PRIx64, xenv->gnt_start);