ACK: [PATCH 1/2] lib: refactor fwts_acpi_is_reduced_hardware

Colin Ian King colin.king at canonical.com
Mon Nov 23 09:04:24 UTC 2020


On 30/09/2020 02:52, Alex Hung wrote:
> Removed the need to get fadt in advance so this function can be used
> more easily.
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/fadt/fadt.c               | 18 +++++++++---------
>  src/lib/include/fwts_acpi_tables.h |  2 +-
>  src/lib/src/fwts_acpi_tables.c     | 19 ++++++++++++++++---
>  src/sbbr/fadt/fadt.c               |  2 +-
>  4 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 9d35db03..ab5a3b2b 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -80,7 +80,7 @@ static int fadt_init(fwts_framework *fw)
>  	 * required (5.2.10) is we are not in reduced hardware
>  	 * mode
>  	 */
> -	if (!fwts_acpi_is_reduced_hardware(fadt)) {
> +	if (!fwts_acpi_is_reduced_hardware(fw)) {
>  		if (fwts_acpi_find_table(fw, "FACS", 0, &table) != FWTS_OK) {
>  			fwts_log_error(fw, "Cannot read ACPI table FACS.");
>  			return FWTS_ERROR;
> @@ -245,7 +245,7 @@ static void acpi_table_check_fadt_firmware_ctrl(fwts_framework *fw)
>  
>  	/* for more recent FADTs, things get more complicated */
>  	if (fadt->firmware_control == 0 && fadt->x_firmware_ctrl == 0) {
> -		if (fwts_acpi_is_reduced_hardware(fadt)) {
> +		if (fwts_acpi_is_reduced_hardware(fw)) {
>  			fwts_passed(fw,
>  				    "FADT 32 bit FIRMWARE_CONTROL and "
>  				    "64 bit X_FIRMWARE_CONTROL (FACS "
> @@ -443,7 +443,7 @@ static void acpi_table_check_fadt_reduced_hardware(fwts_framework *fw)
>  	static const fwts_acpi_gas null_gas;
>  	uint32_t flag_mask;
>  
> -	rhw = fwts_acpi_is_reduced_hardware(fadt);
> +	rhw = fwts_acpi_is_reduced_hardware(fw);
>  	fwts_log_info(fw, "FADT indicates ACPI %s in reduced hardware mode.",
>  		      rhw ? IS : IS_NOT);
>  
> @@ -1521,7 +1521,7 @@ static void acpi_table_check_fadt_p_lvl3_lat(fwts_framework *fw, uint64_t pblk)
>  
>  static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) {
>  
> -	if (fwts_acpi_is_reduced_hardware(fadt))
> +	if (fwts_acpi_is_reduced_hardware(fw))
>  		return;
>  
>  	if (fadt->x_gpe0_blk.access_width == 1)
> @@ -1547,7 +1547,7 @@ static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) {
>  
>  static void acpi_table_check_fadt_sleep_control_reg(fwts_framework *fw)
>  {
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +	if (fwts_acpi_is_reduced_hardware(fw)) {
>  		if (fadt->sleep_control_reg.address == 0)
>  			fwts_passed(fw, "FADT SLEEP_CONTROL_REG not in use.");
>  		else {
> @@ -1577,7 +1577,7 @@ static void acpi_table_check_fadt_sleep_control_reg(fwts_framework *fw)
>  
>  static void acpi_table_check_fadt_sleep_status_reg(fwts_framework *fw)
>  {
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +	if (fwts_acpi_is_reduced_hardware(fw)) {
>  		if (fadt->sleep_status_reg.address == 0)
>  			fwts_passed(fw, "FADT SLEEP_STATUS_REG not in use.");
>  		else {
> @@ -1620,7 +1620,7 @@ static int fadt_test1(fwts_framework *fw)
>  	 * there is no other info (as far as this author knows) that can be
>  	 * used to verify that the value is correct.
>  	 */
> -	if (!fwts_acpi_is_reduced_hardware(fadt)) {
> +	if (!fwts_acpi_is_reduced_hardware(fw)) {
>  		fwts_log_info(fw, "FADT SCI_INT is %" PRIu8, fadt->sci_int);
>  		acpi_table_check_fadt_smi_cmd(fw);
>  		acpi_table_check_fadt_acpi_enable(fw);
> @@ -1690,7 +1690,7 @@ static int fadt_test2(fwts_framework *fw)
>  	uint32_t port, width, val32;
>  	int ret = FWTS_OK;
>  
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +	if (fwts_acpi_is_reduced_hardware(fw)) {
>  		fwts_skipped(fw, "In reduced hardware mode, skipping test.");
>  		return FWTS_OK;
>  	}
> @@ -1770,7 +1770,7 @@ static int fadt_test2(fwts_framework *fw)
>  
>  static int fadt_test3(fwts_framework *fw)
>  {
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +	if (fwts_acpi_is_reduced_hardware(fw)) {
>  		fwts_skipped(fw, "In reduced hardware mode, skipping test.");
>  		return FWTS_OK;
>  	}
> diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
> index b557ae3a..7125ba2c 100644
> --- a/src/lib/include/fwts_acpi_tables.h
> +++ b/src/lib/include/fwts_acpi_tables.h
> @@ -51,7 +51,7 @@ int fwts_acpi_find_table_by_addr(fwts_framework *fw, const uint64_t addr, fwts_a
>  int fwts_acpi_get_table(fwts_framework *fw, const uint32_t index, fwts_acpi_table_info **info);
>  bool fwts_acpi_obj_find(fwts_framework *fw, const char *obj_name);
>  
> -fwts_bool fwts_acpi_is_reduced_hardware(const fwts_acpi_table_fadt *fadt);
> +fwts_bool fwts_acpi_is_reduced_hardware(fwts_framework *fw);
>  
>  void fwts_acpi_reserved_zero_check(fwts_framework *fw, const char *table, const char *field, uint64_t value, uint8_t size, bool *passed);
>  void fwts_acpi_reserved_zero_array_check(fwts_framework *fw, const char *table, const char *field, uint8_t* data, uint8_t length, bool *passed);
> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
> index 44dc411b..f0a809eb 100644
> --- a/src/lib/src/fwts_acpi_tables.c
> +++ b/src/lib/src/fwts_acpi_tables.c
> @@ -283,10 +283,23 @@ int fwts_acpi_free_tables(void)
>   *  fwts_acpi_is_reduced_hardware()
>   *	Check the ACPI tables for HW_REDUCED_ACPI bit in flag field.
>   */
> -fwts_bool fwts_acpi_is_reduced_hardware(const fwts_acpi_table_fadt *fadt)
> +fwts_bool fwts_acpi_is_reduced_hardware(fwts_framework *fw)
>  {
> +	fwts_acpi_table_info *table;
> +	const fwts_acpi_table_fadt *fadt;
> +
> +	if (fwts_acpi_find_table(fw, "FACP", 0, &table) != FWTS_OK) {
> +		fwts_log_error(fw, "Cannot read ACPI table FACP.");
> +		return FWTS_ERROR;
> +	}
> +	if (table == NULL) {
> +		fwts_log_error(fw, "ACPI table FACP does not exist!");
> +		return FWTS_ERROR;
> +	}
> +	fadt = (const fwts_acpi_table_fadt *) table->data;
> +
>  	if ((fadt->header.revision >= 5) &&
> -			(fadt->header.length >= 116)&&
> +			(fadt->header.length >= 116) &&
>  			(fadt->flags & FWTS_ACPI_FADT_FLAGS_HW_REDUCED_ACPI)) {
>  		return FWTS_TRUE;
>  	}
> @@ -402,7 +415,7 @@ PRAGMA_PACK_WARN_OFF
>  PRAGMA_POP
>  	if (result != FWTS_OK) {
>  		if ((result == FWTS_NULL_POINTER) &&
> -				fwts_acpi_is_reduced_hardware(fadt)) {
> +				fwts_acpi_is_reduced_hardware(fw)) {
>  			fwts_log_info(fw, "Ignore the missing FACS. "
>  					"It is optional in hardware-reduced mode");
>  		} else {
> diff --git a/src/sbbr/fadt/fadt.c b/src/sbbr/fadt/fadt.c
> index 2d81e9c5..c02c84e9 100644
> --- a/src/sbbr/fadt/fadt.c
> +++ b/src/sbbr/fadt/fadt.c
> @@ -92,7 +92,7 @@ static int fadt_sbbr_reduced_hw_test2(fwts_framework *fw)
>  	static const fwts_acpi_gas null_gas;
>  	uint32_t flag_mask;
>  
> -	rhw = fwts_acpi_is_reduced_hardware(fadt);
> +	rhw = fwts_acpi_is_reduced_hardware(fw);
>  	if (rhw == 0)
>  		fwts_failed(fw, LOG_LEVEL_CRITICAL, "fadt_reduced_hw:", "FADT indicates ACPI is not in reduced hardware mode.");
>  	else
> 
Acked-by: Colin Ian King <colin.king at canonical.com>



More information about the fwts-devel mailing list