[PATCH] acpi: fadt: X_GPE0/1_BLK can be zero when not used

[Colin Ian King]

On 26/11/2020 03:01, Alex Hung wrote:
> ACPI spec states "if this register block is not supported, this field
> contains zero".
> 
> This patch also replaces spaces by a tab for fwts_acpi_gas struct.
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/fadt/fadt.c        | 33 +++++++++++++++++++++++----------
>  src/lib/include/fwts_acpi.h |  6 +++---
>  2 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index ab5a3b2b..3886b0ed 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -1519,6 +1519,17 @@ static void acpi_table_check_fadt_p_lvl3_lat(fwts_framework *fw, uint64_t pblk)
>  	return;
>  }
>  
> +static bool gas_nonzero(fwts_acpi_gas x_gpe0_blk) {
> +	char *byte = (char*) &x_gpe0_blk;
> +	uint8_t i;
> +
> +	for (i = 0; i < sizeof(fwts_acpi_gas); i++)
> +		if (*byte++)
> +			return true;
> +
> +	return false;
> +}
> +

It may be useful to write this helper function as a more generic "is it
zero" function and add it to the the fwts library, say in
src/lib/src/fwts_acpu.c

bool acpi_data_zero(void *data, const size_t len)
{
	uint8_t *ptr = (uint8_t *)data;
	size_t i;

	for (i = 0; i < len; i++)
		if (!*byte++)
			return false;

	return true;	
}

>  static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) {
>  
>  	if (fwts_acpi_is_reduced_hardware(fw))
> @@ -1527,21 +1538,23 @@ static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) {
>  	if (fadt->x_gpe0_blk.access_width == 1)
>  		fwts_passed(fw, "FADT X_GPE0_BLK has correct byte access width.");
>  	else {
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"X_GPE0_BLKBadAccessWidth",
> -			"FADT X_GPE0_BLK Access width 0x%2.2" PRIx8
> -			" but it should be 1 (byte access).",
> -			fadt->x_gpe0_blk.access_width);
> +		if (gas_nonzero(fadt->x_gpe0_blk))
> +			fwts_failed(fw, LOG_LEVEL_HIGH,
> +				"X_GPE0_BLKBadAccessWidth",
> +				"FADT X_GPE0_BLK Access width 0x%2.2" PRIx8
> +				" but it should be 1 (byte access).",
> +				fadt->x_gpe0_blk.access_width);
>  	}
>  
>  	if (fadt->x_gpe1_blk.access_width == 1)
>  		fwts_passed(fw, "FADT X_GPE1_BLK has correct byte access width.");
>  	else {
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"X_GPE1_BLKBadAccessWidth",
> -			"FADT X_GPE1_BLK Access width 0x%2.2" PRIx8
> -			" but it should be 1 (byte access).",
> -			fadt->x_gpe1_blk.access_width);
> +		if (gas_nonzero(fadt->x_gpe1_blk))
> +			fwts_failed(fw, LOG_LEVEL_HIGH,
> +				"X_GPE1_BLKBadAccessWidth",
> +				"FADT X_GPE1_BLK Access width 0x%2.2" PRIx8
> +				" but it should be 1 (byte access).",
> +				fadt->x_gpe1_blk.access_width);
>  	}
>  }
>  
> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
> index efa988f2..89fb128c 100644
> --- a/src/lib/include/fwts_acpi.h
> +++ b/src/lib/include/fwts_acpi.h
> @@ -117,9 +117,9 @@ extern const char *fwts_acpi_fadt_preferred_pm_profile[];
>  typedef struct {
>  	uint8_t 	address_space_id;
>  	uint8_t		register_bit_width;
> -        uint8_t 	register_bit_offset;
> -        uint8_t 	access_width;
> -        uint64_t 	address;
> +	uint8_t 	register_bit_offset;
> +	uint8_t 	access_width;
> +	uint64_t 	address;
>  } __attribute__ ((packed)) fwts_acpi_gas;
>  
>  /*
>