ACK: [PATCH 12/21] FADT: restructure test sequence around reduced hardware mode

[Colin Ian King]

On 09/02/16 01:32, Al Stone wrote:
> Since a prior patch added a test of of the reduced hardware fields, we
> can now resequence the tests so that we only test the fields we must.
> 
> If we are not in reduced hardware mode, we test everything.  If we are,
> we ignore tests for many fields.  They are simply irrelevant, and if the
> fields are non-zero, that would have been caught in the prior patch.
> 
> There are also several fields that really cannot be reliably tested; we
> cannot gather enough info to know whether they are correct or not, or
> the field is allowed to be any value that it can hold.  Simply log the
> value for these fields, but only when in the proper mode.
> 
> Signed-off-by: Al Stone <al.stone at linaro.org>
> ---
>  src/acpi/fadt/fadt.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index fd430ec..430bfd4 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -848,10 +848,34 @@ static int fadt_test1(fwts_framework *fw)
>  	acpi_table_check_fadt_reserved(fw);
>  	acpi_table_check_fadt_pm_profile(fw);
>  	acpi_table_check_fadt_reduced_hardware(fw);
> -	acpi_table_check_fadt_smi(fw, fadt, &passed);
> -	acpi_table_check_fadt_pm_tmr(fw, fadt, &passed);
> -	acpi_table_check_fadt_gpe(fw, fadt, &passed);
> -	acpi_table_check_fadt_pm_addr(fw, fadt, &passed);
> +
> +	/*
> +	 * If a field can be tested, we call a function to do so.  If
> +	 * any value is reasonable and allowable, we simply log the value.
> +	 * For example, the SCI_INT is one byte and can be from 0..255, and
> +	 * 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)) {
> +		fwts_log_info(fw, "FADT SCI_INT is %" PRIu8, fadt->sci_int);
> +		acpi_table_check_fadt_smi(fw, fadt, &passed);
> +		acpi_table_check_fadt_pm_tmr(fw, fadt, &passed);
> +		acpi_table_check_fadt_gpe(fw, fadt, &passed);
> +		acpi_table_check_fadt_pm_addr(fw, fadt, &passed);
> +		fwts_log_info(fw, "FADT GPE1_BASE is %" PRIu8, fadt->gpe1_base);
> +
> +		fwts_log_info(fw, "FADT FLUSH_SIZE is %" PRIu16,
> +			      fadt->flush_size);
> +		fwts_log_info(fw, "FADT FLUSH_STRIDE is %" PRIu16,
> +			      fadt->flush_stride);
> +		fwts_log_info(fw, "FADT DUTY_OFFSET is %" PRIu8,
> +			      fadt->duty_offset);
> +		fwts_log_info(fw, "FADT DUTY_WIDTH is %" PRIu8,
> +			      fadt->duty_width);
> +		fwts_log_info(fw, "FADT DAY_ALRM is %" PRIu8, fadt->day_alrm);
> +		fwts_log_info(fw, "FADT MON_ALRM is %" PRIu8, fadt->mon_alrm);
> +		fwts_log_info(fw, "FADT CENTURY is %" PRIu8, fadt->century);
> +	}
>  
>  	/*
>  	 * Bug LP: #833644
> @@ -878,6 +902,14 @@ static int fadt_test1(fwts_framework *fw)
>  	}
>  	*/
>  
> +	/*
> +	 * Cannot really test the Hypervisor Vendor Identity since
> +	 * the value is provided by the hypervisor to the OS (as a
> +	 * sign that the ACPI tables have been fabricated), if it
> +	 * being used at all.  Or, it's set to zero.
> +	 */
> +	fwts_log_info(fw, "FADT Hypervisor Vendor Identity is %" PRIu64,
> +		      fadt->hypervisor_id);
>  	if (passed)
>  		fwts_passed(fw, "No issues found in FADT table.");
>  
> 
Seems very reasonable to me.

Acked-by: Colin Ian King <colin.king at canonucal.com>