[PATCH V2] acpi: fadt: Fix FACS checking

Jeffrey Hugo jhugo at codeaurora.org
Wed Jan 31 19:43:28 UTC 2018


In case of HW reduced platforms, FIRMWARE_CTRL and X_FIRMWARE_CTRL are
allowed to be 0 in the FADT as a FACS may not be provided.  The FACS
validation which checks these fields does register a success for HW reduced
platforms when the fields are both 0, or registers a failure for non-HW
reduced platforms as FACS is required in that case.  However, the FACS
validation continues with other checks, which can override the previous
success with a failure, causing an invalid test result and confusing output
messages.

We can do better by reorganizing the checks to follow in logical succession,
only applying pass/fail results appropiate to the specific scenario (both
fields zero, both fields non-zero, one field zero with the other non-zero).

Signed-off-by: Jeffrey Hugo <jhugo at codeaurora.org>
---
 src/acpi/fadt/fadt.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
index 9863566..93a5862 100644
--- a/src/acpi/fadt/fadt.c
+++ b/src/acpi/fadt/fadt.c
@@ -271,28 +271,7 @@ static void acpi_table_check_fadt_firmware_ctrl(fwts_framework *fw)
 				    "reduced mode is not set. ");
 		}
 
-	}
-
-	if ((fadt->firmware_control != 0 && fadt->x_firmware_ctrl == 0) ||
-	    (fadt->firmware_control == 0 && fadt->x_firmware_ctrl != 0)) {
-		fwts_passed(fw,
-			    "Only one of FIRMWARE_CTRL and X_FIRMWARE_CTRL "
-			    "is non-zero.");
-	} else {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			    "FADTFACSBothSet",
-			    "Both 32-bit FIRMWARE_CTRL and 64-bit "
-			    "X_FIRMWARE_CTRL pointers to the FACS are "
-			    "set but only one is allowed.");
-		fwts_advice(fw,
-			    "Having both FACS pointers set is ambiguous; "
-			    "there is no way to determine which one is "
-			    "the correct address.  The kernel workaround "
-			    "is to always use the 64-bit X_FIRMWARE_CTRL.");
-	}
-
-
-	if (fadt->firmware_control != 0 && fadt->x_firmware_ctrl != 0) {
+	} else 	if (fadt->firmware_control != 0 && fadt->x_firmware_ctrl != 0) {
 		if ((uint64_t)fadt->firmware_control == fadt->x_firmware_ctrl) {
 			fwts_passed(fw,
 				    "Both FIRMWARE_CTRL and X_FIRMWARE_CTRL "
@@ -326,6 +305,10 @@ static void acpi_table_check_fadt_firmware_ctrl(fwts_framework *fw)
 				    "64-bit X_FIRMWARE_CTRL pointer to the "
 				    "FACS. ");
 		}
+	} else { /* only one of firmware_control/x_firmware_ctrl is 0 */
+		fwts_passed(fw,
+			    "Only one of FIRMWARE_CTRL and X_FIRMWARE_CTRL "
+			    "is non-zero.");
 	}
 }
 
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.




More information about the fwts-devel mailing list