[PATCH] acpi: method: check one element in _TSD only for acpi 6.2 and later

[Colin Ian King]

On 20/02/18 20:37, Alex Hung wrote:
> ACPI 6.1 and before allow more element in _TSD method
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/method/method.c           |  6 ++++--
>  src/lib/include/fwts_acpi.h        |  8 ++++++++
>  src/lib/include/fwts_acpi_tables.h |  2 ++
>  src/lib/src/fwts_acpi_tables.c     | 29 +++++++++++++++++++++++++++++
>  4 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 6839c3f..0df3526 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -2896,8 +2896,10 @@ static void method_test_TSD_return(
>  	if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
>  		return;
>  
> -	if (fwts_method_package_count_equal(fw, name, "_TSD", obj, 1) != FWTS_OK)
> -		return;
> +	if (fwts_get_acpi_version(fw) >= FWTS_ACPI_VERSION_62) {
> +		if (fwts_method_package_count_equal(fw, name, "_TSD", obj, 1) != FWTS_OK)
> +			return;
> +	}
>  
>  	/* Could be one or more packages */
>  	for (i = 0; i < obj->Package.Count; i++) {
> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
> index 3151b90..e22c4ab 100644
> --- a/src/lib/include/fwts_acpi.h
> +++ b/src/lib/include/fwts_acpi.h
> @@ -84,6 +84,14 @@
>  #define FWTS_GAS_ADDR_SPACE_ID_PCC		(0x0a)
>  #define FWTS_GAS_ADDR_SPACE_ID_FFH		(0x7f)
>  
> +#define FWTS_ACPI_VERSION_62 (0x620)
> +#define FWTS_ACPI_VERSION_61 (0x610)
> +#define FWTS_ACPI_VERSION_60 (0x600)
> +#define FWTS_ACPI_VERSION_51 (0x510)
> +#define FWTS_ACPI_VERSION_50 (0x500)
> +#define FWTS_ACPI_VERSION_40 (0x400)
> +#define FWTS_ACPI_VERSION_30 (0x300)
> +
>  #include "fwts_types.h"
>  #include "fwts_framework.h"
>  #include "fwts_log.h"
> diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
> index 11b870c..d307af8 100644
> --- a/src/lib/include/fwts_acpi_tables.h
> +++ b/src/lib/include/fwts_acpi_tables.h
> @@ -58,6 +58,8 @@ void fwts_acpi_reserved_bits_check(fwts_framework *fw, const char *table, const
>  void fwts_acpi_reserved_type_check(fwts_framework *fw, const char *table, uint8_t value, uint8_t min, uint8_t reserved, bool *passed);
>  bool fwts_acpi_subtable_length_check(fwts_framework *fw, const char *table, uint8_t subtable_type, uint32_t subtable_length, uint32_t size);
>  
> +uint32_t fwts_get_acpi_version(fwts_framework *fw);
> +
>  #endif
>  
>  #endif
> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
> index 2aa30f8..eb96700 100644
> --- a/src/lib/src/fwts_acpi_tables.c
> +++ b/src/lib/src/fwts_acpi_tables.c
> @@ -1532,4 +1532,33 @@ bool fwts_acpi_subtable_length_check(
>  	return true;
>  }
>  
> +static uint32_t acpi_version;
> +/*
> + *  fwts_get_acpi_version()
> + * 	get acpi version from facp
> + */
> +uint32_t fwts_get_acpi_version(fwts_framework *fw)
> +{
> +	fwts_acpi_table_fadt *fadt;
> +	fwts_acpi_table_info *table;
> +
> +	if (acpi_version != 0)
> +		return acpi_version;
> +
> +	if (fwts_acpi_find_table(fw, "FACP", 0, &table) != FWTS_OK) {
> +		fwts_log_error(fw, "Cannot read ACPI table FACP.");
> +		return acpi_version = 0;

assuming that acpi_version is zero at this point, the assignment looks
dubious to me.  Just return 0 will do.

> +	}
> +
> +	if (table == NULL) {
> +		fwts_log_error(fw, "ACPI table FACP does not exist!");
> +		return acpi_version = 0;

same here, return 0 will do.

> +	}
> +
> +	fadt = (fwts_acpi_table_fadt *)table->data;
> +	acpi_version = (fadt->header.revision << 8) + fadt->minor_version;

should this be sanity checked?; the table may be returning any old garbage.

> +
> +	return acpi_version;
> +}
> +
>  #endif
> 

Colin