From patchwork Wed Aug 1 09:25:20 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 174416 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id BA5F72C009B for ; Wed, 1 Aug 2012 19:25:26 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1SwVBD-00033K-V1 for incoming@patchwork.ozlabs.org; Wed, 01 Aug 2012 09:25:24 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1SwVBB-00033D-8V for fwts-devel@lists.ubuntu.com; Wed, 01 Aug 2012 09:25:21 +0000 Received: from cpc37-craw6-2-0-cust191.16-3.cable.virginmedia.com ([92.239.39.192] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1SwVBB-0008Gs-5E for fwts-devel@lists.ubuntu.com; Wed, 01 Aug 2012 09:25:21 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH] acpi: method: only get pedantic about therm returns if values are hard coded Date: Wed, 1 Aug 2012 10:25:20 +0100 Message-Id: <1343813120-13282-1-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 1.7.10.4 X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: fwts-devel-bounces@lists.ubuntu.com Errors-To: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King 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 Acked-by: Keng-Yu Lin Acked-by: Ivan Hu --- 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