[PATCH] acpi: method: only get pedantic about therm returns if values are hard coded
Keng-Yu Lin
kengyu at canonical.com
Tue Aug 7 03:11:42 UTC 2012
On Wed, Aug 1, 2012 at 5:25 PM, Colin King <colin.king at canonical.com> wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> The thermal evaluations are producing unnecessary noisy advice and warnings
> because the evals are from emulated memory or I/O space regions, which of
> course will return dummy values. Instead of doing this, make it more
> intelligent by keeping track of any memory space accesses and only checking
> the evaluated return value if we didn't access any memory or I/O space regions.
> This way we can catch any bogus hard-coded incorrect therm return values.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
> src/acpi/method/method.c | 54 +++++++++++++++++++++++++++++------------
> src/acpica/fwts_acpica.c | 14 +++++++++++
> src/lib/include/fwts_acpica.h | 2 ++
> 3 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 9e81f07..377b7bc 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -1222,22 +1222,43 @@ static void method_test_THERM_return(fwts_framework *fw, char *name, ACPI_BUFFER
> {
> if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) == FWTS_OK) {
> char *method = (char*)private;
> - if (obj->Integer.Value >= 2732)
> - fwts_passed(fw, "%s correctly returned sane looking value 0x%8.8x (%5.1f degrees K)",
> - method,
> - (uint32_t)obj->Integer.Value,
> - (float)((uint32_t)obj->Integer.Value) / 10.0);
> - else {
> - fwts_failed(fw, LOG_LEVEL_MEDIUM, "MethodBadTemp",
> - "%s returned a dubious value below 0 degrees C: 0x%8.8x (%5.1f degrees K)",
> - method,
> - (uint32_t)obj->Integer.Value,
> - (float)((uint32_t)obj->Integer.Value) / 10.0);
> - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> - fwts_advice(fw, "An incorrect value could be because the method requires interaction with "
> - "I/O ports or the embedded controller and this test frame work is spoofing "
> - "these operations. However, it is worth sanity checking these values in "
> +
> + if (fwts_acpi_region_handler_called_get()) {
> + /*
> + * We accessed some memory or I/O region during the evaluation
> + * which returns spoofed values, so we should not test the value
> + * being returned. In this case, just pass this as a valid
> + * return type.
> + */
> + fwts_passed(fw, "%s correctly returned sane looking return type.", name);
> + } else {
> + /*
> + * The evaluation probably was a hard-coded value, so sanity check it
> + */
> + if (obj->Integer.Value >= 2732)
> + fwts_passed(fw,
> + "%s correctly returned sane looking value "
> + "0x%8.8x (%5.1f degrees K)",
> + method,
> + (uint32_t)obj->Integer.Value,
> + (float)((uint32_t)obj->Integer.Value) / 10.0);
> + else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "MethodBadTemp",
> + "%s returned a dubious value below 0 degrees C: "
> + "0x%8.8x (%5.1f degrees K)",
> + method,
> + (uint32_t)obj->Integer.Value,
> + (float)((uint32_t)obj->Integer.Value) / 10.0);
> + fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> + fwts_advice(fw,
> + "The value returned was probably a hard-coded "
> + "thermal value which is out of range because "
> + "fwts did not detect any ACPI region handler "
> + "accesses of I/O or system memeory to evaluate "
> + "the thermal value. "
> + "It is worth sanity checking these values in "
> "/sys/class/thermal/thermal_zone*.");
> + }
> }
> }
> }
> @@ -1245,7 +1266,8 @@ static void method_test_THERM_return(fwts_framework *fw, char *name, ACPI_BUFFER
> #define method_test_THERM(name, type) \
> static int method_test ## name(fwts_framework *fw) \
> { \
> - return method_evaluate_method(fw, type, # name, NULL, 0, method_test_THERM_return, # name); \
> + fwts_acpi_region_handler_called_set(false); \
> + return method_evaluate_method(fw, type, # name, NULL, 0, method_test_THERM_return, # name); \
> }
>
> method_test_THERM(_CRT, METHOD_OPTIONAL)
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index 08b012d..4e28d60 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -74,6 +74,8 @@ static sem_hash sem_hash_table[MAX_SEMAPHORES];
>
> static ACPI_TABLE_DESC Tables[ACPI_MAX_INIT_TABLES];
>
> +static bool region_handler_called;
> +
> /*
> * Used to account for threads used by AcpiOsExecute
> */
> @@ -305,6 +307,16 @@ static ACPI_STATUS fwts_region_init(
> return AE_OK;
> }
>
> +void fwts_acpi_region_handler_called_set(bool val)
> +{
> + region_handler_called = val;
> +}
> +
> +bool fwts_acpi_region_handler_called_get(void)
> +{
> + return region_handler_called;
> +}
> +
> static ACPI_STATUS fwts_region_handler(
> UINT32 function,
> ACPI_PHYSICAL_ADDRESS address,
> @@ -323,6 +335,8 @@ static ACPI_STATUS fwts_region_handler(
> if (regionobject->Region.Type != ACPI_TYPE_REGION)
> return AE_OK;
>
> + fwts_acpi_region_handler_called_set(true);
> +
> context = ACPI_CAST_PTR (ACPI_CONNECTION_INFO, handlercontext);
> length = (ACPI_SIZE)regionobject->Region.Length;
>
> diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
> index e7008be..bfea77e 100644
> --- a/src/lib/include/fwts_acpica.h
> +++ b/src/lib/include/fwts_acpica.h
> @@ -32,5 +32,7 @@ fwts_list *fwts_acpica_get_object_names(int type);
> void fwts_acpica_sem_count_clear(void);
> void fwts_acpica_sem_count_get(int *acquired, int *released);
> void fwts_acpica_simulate_sem_timeout(int flag);
> +void fwts_acpi_region_handler_called_set(bool val);
> +bool fwts_acpi_region_handler_called_get(void);
>
> #endif
> --
> 1.7.10.4
>
Acked-by: Keng-Yu Lin <kengyu at canonical.com>
More information about the fwts-devel
mailing list