Patchwork [2/2] acpi: acpitables: better checking on FADT SCI_INT

login
register
mail settings
Submitter Colin King
Date March 28, 2014, 12:17 p.m.
Message ID <1396009052-3915-3-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/334682/
State Accepted
Headers show

Comments

Colin King - March 28, 2014, 12:17 p.m.
From: Colin Ian King <colin.king@canonical.com>

A SCI_INT failure should only be checked for if SMM is enabled,
that is, if SMI_CMD is defined.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/acpitables/acpitables.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
Alex Hung - April 7, 2014, 6:35 a.m.
On 03/28/2014 08:17 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> A SCI_INT failure should only be checked for if SMM is enabled,
> that is, if SMI_CMD is defined.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpi/acpitables/acpitables.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/acpi/acpitables/acpitables.c b/src/acpi/acpitables/acpitables.c
> index 9e562f6..439df2a 100644
> --- a/src/acpi/acpitables/acpitables.c
> +++ b/src/acpi/acpitables/acpitables.c
> @@ -121,15 +121,25 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
>  		}
>  	}
>  
> -	if (fadt->sci_int == 0)
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
> -	if (fadt->smi_cmd == 0) {
> +	/*
> +	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
> +	 *  specification states that if SMI_CMD is zero then it is
> +	 *  a system that does not support System Management Mode, so
> +	 *  in that case, don't check SCI_INT being valid.
> +	 */
> +	if (fadt->smi_cmd != 0) {
> +		if (fadt->sci_int == 0) {
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
> +		}
> +	} else {
>  		if ((fadt->acpi_enable == 0) &&
>  		    (fadt->acpi_disable == 0) &&
>  		    (fadt->s4bios_req == 0) &&
>  		    (fadt->pstate_cnt == 0) &&
> -		    (fadt->cst_cnt == 0))
> -			fwts_warning(fw, "FADT SMI_CMD is 0x00, system appears to not support System Management mode.");
> +		    (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.");
> +		}
>  		else {
>  			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSMICMDZero",
>  					"FADT SMI_CMD is 0x00, however, one or more of ACPI_ENABLE, ACPI_DISABLE, "
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - April 17, 2014, 3:50 a.m.
On 03/28/2014 08:17 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> A SCI_INT failure should only be checked for if SMM is enabled,
> that is, if SMI_CMD is defined.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/acpitables/acpitables.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/acpi/acpitables/acpitables.c b/src/acpi/acpitables/acpitables.c
> index 9e562f6..439df2a 100644
> --- a/src/acpi/acpitables/acpitables.c
> +++ b/src/acpi/acpitables/acpitables.c
> @@ -121,15 +121,25 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
>   		}
>   	}
>
> -	if (fadt->sci_int == 0)
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
> -	if (fadt->smi_cmd == 0) {
> +	/*
> +	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
> +	 *  specification states that if SMI_CMD is zero then it is
> +	 *  a system that does not support System Management Mode, so
> +	 *  in that case, don't check SCI_INT being valid.
> +	 */
> +	if (fadt->smi_cmd != 0) {
> +		if (fadt->sci_int == 0) {
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
> +		}
> +	} else {
>   		if ((fadt->acpi_enable == 0) &&
>   		    (fadt->acpi_disable == 0) &&
>   		    (fadt->s4bios_req == 0) &&
>   		    (fadt->pstate_cnt == 0) &&
> -		    (fadt->cst_cnt == 0))
> -			fwts_warning(fw, "FADT SMI_CMD is 0x00, system appears to not support System Management mode.");
> +		    (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.");
> +		}
>   		else {
>   			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSMICMDZero",
>   					"FADT SMI_CMD is 0x00, however, one or more of ACPI_ENABLE, ACPI_DISABLE, "
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/acpi/acpitables/acpitables.c b/src/acpi/acpitables/acpitables.c
index 9e562f6..439df2a 100644
--- a/src/acpi/acpitables/acpitables.c
+++ b/src/acpi/acpitables/acpitables.c
@@ -121,15 +121,25 @@  static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
 		}
 	}
 
-	if (fadt->sci_int == 0)
-		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
-	if (fadt->smi_cmd == 0) {
+	/*
+	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
+	 *  specification states that if SMI_CMD is zero then it is
+	 *  a system that does not support System Management Mode, so
+	 *  in that case, don't check SCI_INT being valid.
+	 */
+	if (fadt->smi_cmd != 0) {
+		if (fadt->sci_int == 0) {
+			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
+		}
+	} else {
 		if ((fadt->acpi_enable == 0) &&
 		    (fadt->acpi_disable == 0) &&
 		    (fadt->s4bios_req == 0) &&
 		    (fadt->pstate_cnt == 0) &&
-		    (fadt->cst_cnt == 0))
-			fwts_warning(fw, "FADT SMI_CMD is 0x00, system appears to not support System Management mode.");
+		    (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.");
+		}
 		else {
 			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSMICMDZero",
 					"FADT SMI_CMD is 0x00, however, one or more of ACPI_ENABLE, ACPI_DISABLE, "