diff mbox

[13/21] FADT: expand compliance tests for the SMI_CMD field

Message ID 1454981583-31872-14-git-send-email-al.stone@linaro.org
State Rejected
Headers show

Commit Message

Al Stone Feb. 9, 2016, 1:32 a.m. UTC
Minor tweaks to the tests of the SMI_CMD to make them only slightly
more extensive, but also to fit within the resequencing of tests for
reduced hardware mode..

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 src/acpi/fadt/fadt.c | 68 ++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

Comments

Colin Ian King Feb. 9, 2016, 12:21 p.m. UTC | #1
On 09/02/16 01:32, Al Stone wrote:
> Minor tweaks to the tests of the SMI_CMD to make them only slightly
> more extensive, but also to fit within the resequencing of tests for
> reduced hardware mode..
> 
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>  src/acpi/fadt/fadt.c | 68 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 430bfd4..3da297e 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -660,19 +660,8 @@ static void acpi_table_check_fadt_reduced_hardware(fwts_framework *fw)
>  			    "hardware mode.");
>  }
>  
> -static void acpi_table_check_fadt_smi(
> -	fwts_framework *fw,
> -	const fwts_acpi_table_fadt *fadt,
> -	bool *passed)
> +static void acpi_table_check_fadt_smi_cmd(fwts_framework *fw)
>  {
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> -		if (fadt->smi_cmd != 0) {
> -			fwts_warning(fw, "FADT SMI_CMD is not zero "
> -				"but should be in reduced hardware mode.");
> -		}
> -		return;
> -	}
> -
>  	/*
>  	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
>  	 *  specification states that if SMI_CMD is zero then it is
> @@ -680,38 +669,49 @@ static void acpi_table_check_fadt_smi(
>  	 *  in that case, don't check SCI_INT being valid.
>  	 */
>  	if (fadt->smi_cmd != 0) {
> -		if (fadt->sci_int == 0) {
> -			*passed = false;
> +		if (fadt->sci_int == 0)
>  			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"FADTSCIIRQZero",
> -				"FADT SCI Interrupt is 0x00, should be defined.");
> -		}
> +				    "FADTSCIIRQZero",
> +				    "FADT SCI Interrupt is 0x00, but must "
> +				    "be defined since SMI command indicates "
> +				    "System Management Mode is supported.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT SMI_CMD indicates System Management "
> +				    "Mode is supported, and the SCI Interrupt "
> +				    "is non-zero.");
>  	} else {
>  		if ((fadt->acpi_enable == 0) &&
>  		    (fadt->acpi_disable == 0) &&
>  		    (fadt->s4bios_req == 0) &&
>  		    (fadt->pstate_cnt == 0) &&
>  		    (fadt->cst_cnt == 0)) {
> -			/* Not an error, but intentional, but feedback this finding anyhow */
> -			fwts_log_info(fw, "The FADT SMI_CMD is zero, system "
> -				"does not support System Management Mode.");
> +			/*
> +			 * Not an error, but intentional, so feedback
> +			 * this finding.
> +			 */
> +			fwts_passed(fw, "The FADT SMI_CMD is zero, system "
> +				    "does not support System Management Mode.");
>  		}
>  		else {
> -			*passed = false;
>  			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"FADTSMICMDZero",
> -				"FADT SMI_CMD is 0x00, however, one or more of "
> -				"ACPI_ENABLE, ACPI_DISABLE, S4BIOS_REQ, PSTATE_CNT "
> -				"and CST_CNT are defined which means SMI_CMD should "
> -				"be defined otherwise SMI commands cannot be sent.");
> +				    "FADTSMICMDZero",
> +				    "FADT SMI_CMD is 0x00, however, one or "
> +				    "more of ACPI_ENABLE, ACPI_DISABLE, "
> +				    "S4BIOS_REQ, PSTATE_CNT and CST_CNT are "
> +				    "defined which means SMI_CMD should be "
> +				    "defined, otherwise SMI commands cannot "
> +				    "be sent.");
>  			fwts_advice(fw,
> -				"The configuration seems to suggest that SMI command "
> -				"should be defined to allow the kernel to trigger "
> -				"system management interrupts via the SMD_CMD port. "
> -				"The fact that SMD_CMD is zero which is invalid means "
> -				"that SMIs are not possible through the normal ACPI "
> -				"mechanisms. This means some firmware based machine "
> -				"specific functions will not work.");
> +				    "The configuration seems to suggest that "
> +				    "SMI command should be defined to allow "
> +				    "the kernel to trigger system management "
> +				    "interrupts via the SMD_CMD port.  The "
> +				    "fact that SMD_CMD is zero which is "
> +				    "invalid means that SMIs are not possible "
> +				    "through the normal ACPI mechanisms. This "
> +				    "means some firmware based machine "
> +				    "specific functions will not work.");
>  		}
>  	}
>  }
> @@ -858,7 +858,7 @@ static int fadt_test1(fwts_framework *fw)
>  	 */
>  	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_smi_cmd(fw);
>  		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);
> 
Acked-by: Colin Ian King <colin.king@canonucal.com>
Alex Hung Feb. 17, 2016, 6:11 a.m. UTC | #2
On 2016-02-09 09:32 AM, Al Stone wrote:
> Minor tweaks to the tests of the SMI_CMD to make them only slightly
> more extensive, but also to fit within the resequencing of tests for
> reduced hardware mode..
>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>   src/acpi/fadt/fadt.c | 68 ++++++++++++++++++++++++++--------------------------
>   1 file changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 430bfd4..3da297e 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -660,19 +660,8 @@ static void acpi_table_check_fadt_reduced_hardware(fwts_framework *fw)
>   			    "hardware mode.");
>   }
>
> -static void acpi_table_check_fadt_smi(
> -	fwts_framework *fw,
> -	const fwts_acpi_table_fadt *fadt,
> -	bool *passed)
> +static void acpi_table_check_fadt_smi_cmd(fwts_framework *fw)
>   {
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> -		if (fadt->smi_cmd != 0) {
> -			fwts_warning(fw, "FADT SMI_CMD is not zero "
> -				"but should be in reduced hardware mode.");
> -		}
> -		return;
> -	}
> -
>   	/*
>   	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
>   	 *  specification states that if SMI_CMD is zero then it is
> @@ -680,38 +669,49 @@ static void acpi_table_check_fadt_smi(
>   	 *  in that case, don't check SCI_INT being valid.
>   	 */
>   	if (fadt->smi_cmd != 0) {
> -		if (fadt->sci_int == 0) {
> -			*passed = false;
> +		if (fadt->sci_int == 0)
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"FADTSCIIRQZero",
> -				"FADT SCI Interrupt is 0x00, should be defined.");
> -		}
> +				    "FADTSCIIRQZero",
> +				    "FADT SCI Interrupt is 0x00, but must "
> +				    "be defined since SMI command indicates "
> +				    "System Management Mode is supported.");
> +		else
> +			fwts_passed(fw,
> +				    "FADT SMI_CMD indicates System Management "
> +				    "Mode is supported, and the SCI Interrupt "
> +				    "is non-zero.");
>   	} else {
>   		if ((fadt->acpi_enable == 0) &&
>   		    (fadt->acpi_disable == 0) &&
>   		    (fadt->s4bios_req == 0) &&
>   		    (fadt->pstate_cnt == 0) &&
>   		    (fadt->cst_cnt == 0)) {
> -			/* Not an error, but intentional, but feedback this finding anyhow */
> -			fwts_log_info(fw, "The FADT SMI_CMD is zero, system "
> -				"does not support System Management Mode.");
> +			/*
> +			 * Not an error, but intentional, so feedback
> +			 * this finding.
> +			 */
> +			fwts_passed(fw, "The FADT SMI_CMD is zero, system "
> +				    "does not support System Management Mode.");
>   		}
>   		else {
> -			*passed = false;
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -				"FADTSMICMDZero",
> -				"FADT SMI_CMD is 0x00, however, one or more of "
> -				"ACPI_ENABLE, ACPI_DISABLE, S4BIOS_REQ, PSTATE_CNT "
> -				"and CST_CNT are defined which means SMI_CMD should "
> -				"be defined otherwise SMI commands cannot be sent.");
> +				    "FADTSMICMDZero",
> +				    "FADT SMI_CMD is 0x00, however, one or "
> +				    "more of ACPI_ENABLE, ACPI_DISABLE, "
> +				    "S4BIOS_REQ, PSTATE_CNT and CST_CNT are "
> +				    "defined which means SMI_CMD should be "
> +				    "defined, otherwise SMI commands cannot "
> +				    "be sent.");
>   			fwts_advice(fw,
> -				"The configuration seems to suggest that SMI command "
> -				"should be defined to allow the kernel to trigger "
> -				"system management interrupts via the SMD_CMD port. "
> -				"The fact that SMD_CMD is zero which is invalid means "
> -				"that SMIs are not possible through the normal ACPI "
> -				"mechanisms. This means some firmware based machine "
> -				"specific functions will not work.");
> +				    "The configuration seems to suggest that "
> +				    "SMI command should be defined to allow "
> +				    "the kernel to trigger system management "
> +				    "interrupts via the SMD_CMD port.  The "
> +				    "fact that SMD_CMD is zero which is "
> +				    "invalid means that SMIs are not possible "
> +				    "through the normal ACPI mechanisms. This "
> +				    "means some firmware based machine "
> +				    "specific functions will not work.");
>   		}
>   	}
>   }
> @@ -858,7 +858,7 @@ static int fadt_test1(fwts_framework *fw)
>   	 */
>   	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_smi_cmd(fw);
>   		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);
>

Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox

Patch

diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
index 430bfd4..3da297e 100644
--- a/src/acpi/fadt/fadt.c
+++ b/src/acpi/fadt/fadt.c
@@ -660,19 +660,8 @@  static void acpi_table_check_fadt_reduced_hardware(fwts_framework *fw)
 			    "hardware mode.");
 }
 
-static void acpi_table_check_fadt_smi(
-	fwts_framework *fw,
-	const fwts_acpi_table_fadt *fadt,
-	bool *passed)
+static void acpi_table_check_fadt_smi_cmd(fwts_framework *fw)
 {
-	if (fwts_acpi_is_reduced_hardware(fadt)) {
-		if (fadt->smi_cmd != 0) {
-			fwts_warning(fw, "FADT SMI_CMD is not zero "
-				"but should be in reduced hardware mode.");
-		}
-		return;
-	}
-
 	/*
 	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
 	 *  specification states that if SMI_CMD is zero then it is
@@ -680,38 +669,49 @@  static void acpi_table_check_fadt_smi(
 	 *  in that case, don't check SCI_INT being valid.
 	 */
 	if (fadt->smi_cmd != 0) {
-		if (fadt->sci_int == 0) {
-			*passed = false;
+		if (fadt->sci_int == 0)
 			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"FADTSCIIRQZero",
-				"FADT SCI Interrupt is 0x00, should be defined.");
-		}
+				    "FADTSCIIRQZero",
+				    "FADT SCI Interrupt is 0x00, but must "
+				    "be defined since SMI command indicates "
+				    "System Management Mode is supported.");
+		else
+			fwts_passed(fw,
+				    "FADT SMI_CMD indicates System Management "
+				    "Mode is supported, and the SCI Interrupt "
+				    "is non-zero.");
 	} else {
 		if ((fadt->acpi_enable == 0) &&
 		    (fadt->acpi_disable == 0) &&
 		    (fadt->s4bios_req == 0) &&
 		    (fadt->pstate_cnt == 0) &&
 		    (fadt->cst_cnt == 0)) {
-			/* Not an error, but intentional, but feedback this finding anyhow */
-			fwts_log_info(fw, "The FADT SMI_CMD is zero, system "
-				"does not support System Management Mode.");
+			/*
+			 * Not an error, but intentional, so feedback
+			 * this finding.
+			 */
+			fwts_passed(fw, "The FADT SMI_CMD is zero, system "
+				    "does not support System Management Mode.");
 		}
 		else {
-			*passed = false;
 			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"FADTSMICMDZero",
-				"FADT SMI_CMD is 0x00, however, one or more of "
-				"ACPI_ENABLE, ACPI_DISABLE, S4BIOS_REQ, PSTATE_CNT "
-				"and CST_CNT are defined which means SMI_CMD should "
-				"be defined otherwise SMI commands cannot be sent.");
+				    "FADTSMICMDZero",
+				    "FADT SMI_CMD is 0x00, however, one or "
+				    "more of ACPI_ENABLE, ACPI_DISABLE, "
+				    "S4BIOS_REQ, PSTATE_CNT and CST_CNT are "
+				    "defined which means SMI_CMD should be "
+				    "defined, otherwise SMI commands cannot "
+				    "be sent.");
 			fwts_advice(fw,
-				"The configuration seems to suggest that SMI command "
-				"should be defined to allow the kernel to trigger "
-				"system management interrupts via the SMD_CMD port. "
-				"The fact that SMD_CMD is zero which is invalid means "
-				"that SMIs are not possible through the normal ACPI "
-				"mechanisms. This means some firmware based machine "
-				"specific functions will not work.");
+				    "The configuration seems to suggest that "
+				    "SMI command should be defined to allow "
+				    "the kernel to trigger system management "
+				    "interrupts via the SMD_CMD port.  The "
+				    "fact that SMD_CMD is zero which is "
+				    "invalid means that SMIs are not possible "
+				    "through the normal ACPI mechanisms. This "
+				    "means some firmware based machine "
+				    "specific functions will not work.");
 		}
 	}
 }
@@ -858,7 +858,7 @@  static int fadt_test1(fwts_framework *fw)
 	 */
 	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_smi_cmd(fw);
 		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);