[PATCH] acpi: method: only get pedantic about therm returns if values are hard coded

[Colin King]

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