Patchwork [RESEND] apci: checksum: RSDT and XSDT checksum failures should not be critical (LP: #1013168)

login
register
mail settings
Submitter Colin King
Date June 18, 2012, 8:18 a.m.
Message ID <1340007500-17169-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/165411/
State Accepted
Headers show

Comments

Colin King - June 18, 2012, 8:18 a.m.
From: Colin Ian King <colin.king@canonical.com>

It seems that the kernel is quite happy to handle RSDT and XSDT tables that
fail on their checksum checks, so lets not fail these as critical failures
anymore.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/checksum/checksum.c |   41 +++++++----------------------------------
 1 file changed, 7 insertions(+), 34 deletions(-)
Keng-Yu Lin - June 19, 2012, 2:30 a.m.
On Mon, Jun 18, 2012 at 4:18 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> It seems that the kernel is quite happy to handle RSDT and XSDT tables that
> fail on their checksum checks, so lets not fail these as critical failures
> anymore.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpi/checksum/checksum.c |   41 +++++++----------------------------------
>  1 file changed, 7 insertions(+), 34 deletions(-)
>
> diff --git a/src/acpi/checksum/checksum.c b/src/acpi/checksum/checksum.c
> index 264e7d0..9918a62 100644
> --- a/src/acpi/checksum/checksum.c
> +++ b/src/acpi/checksum/checksum.c
> @@ -89,16 +89,6 @@ static void checksum_rsdp(fwts_framework *fw, fwts_acpi_table_info *table)
>
>  }
>
> -/*
> - *  The following tables the kernel requires the checksum to be valid otherwise
> - *  it will not load them, so checksum failures here are considered critical errors.
> - */
> -static char *critical_checksum[] = {
> -       "RSDT",
> -       "XSDT",
> -       NULL
> -};
> -
>  static int checksum_scan_tables(fwts_framework *fw)
>  {
>        int i;
> @@ -131,35 +121,18 @@ static int checksum_scan_tables(fwts_framework *fw)
>                        fwts_passed(fw, "Table %s has correct checksum 0x%x.",
>                                table->name, hdr->checksum);
>                else {
> -                       int i;
> -                       int log_level = LOG_LEVEL_LOW;
> -
> -                       for (i = 0; critical_checksum[i]; i++) {
> -                               if (!strcmp(table->name, critical_checksum[i])) {
> -                                       log_level = LOG_LEVEL_CRITICAL;
> -                                       break;
> -                               }
> -                       }
> -
> -                       fwts_failed(fw, log_level, "ACPITableChecksum",
> +                       fwts_failed(fw, LOG_LEVEL_MEDIUM, "ACPITableChecksum",
>                                "Table %s has incorrect checksum, "
>                                "expected 0x%2.2x, got 0x%2.2x.",
>                                table->name, (uint8_t)(hdr->checksum-checksum),
>                                hdr->checksum);
>
> -                       /* Give some contextual explanation of the error */
> -                       if (log_level == LOG_LEVEL_CRITICAL)
> -                               fwts_advice(fw,
> -                                       "The kernel requires this table to have a "
> -                                       "valid checksum and will not load it. This "
> -                                       "will lead to ACPI not working correctly.");
> -                       else
> -                               fwts_advice(fw,
> -                                       "The kernel will warn that this table has "
> -                                       "an invalid checksum but will ignore the "
> -                                       "error and still load it. This is not a "
> -                                       "critical issue, but should be fixed if "
> -                                       "possible to avoid the warning messages.");
> +                       fwts_advice(fw,
> +                               "The kernel will warn that this table has "
> +                               "an invalid checksum but will ignore the "
> +                               "error and still load it. This is not a "
> +                               "critical issue, but should be fixed if "
> +                               "possible to avoid the warning messages.");
>
>                        fwts_tag_failed(fw, FWTS_TAG_ACPI_TABLE_CHECKSUM);
>                }
> --
> 1.7.10.4
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Keng-Yu Lin - June 19, 2012, 2:31 a.m.
Hi Colin,
  Will you also update the checksum testcase patch too?

  cheers,
-kengyu
Alex Hung - June 19, 2012, 5:24 a.m.
On 06/18/2012 04:18 PM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> It seems that the kernel is quite happy to handle RSDT and XSDT tables that
> fail on their checksum checks, so lets not fail these as critical failures
> anymore.
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   src/acpi/checksum/checksum.c |   41 +++++++----------------------------------
>   1 file changed, 7 insertions(+), 34 deletions(-)
>
> diff --git a/src/acpi/checksum/checksum.c b/src/acpi/checksum/checksum.c
> index 264e7d0..9918a62 100644
> --- a/src/acpi/checksum/checksum.c
> +++ b/src/acpi/checksum/checksum.c
> @@ -89,16 +89,6 @@ static void checksum_rsdp(fwts_framework *fw, fwts_acpi_table_info *table)
>
>   }
>
> -/*
> - *  The following tables the kernel requires the checksum to be valid otherwise
> - *  it will not load them, so checksum failures here are considered critical errors.
> - */
> -static char *critical_checksum[] = {
> -	"RSDT",
> -	"XSDT",
> -	NULL
> -};
> -
>   static int checksum_scan_tables(fwts_framework *fw)
>   {
>   	int i;
> @@ -131,35 +121,18 @@ static int checksum_scan_tables(fwts_framework *fw)
>   			fwts_passed(fw, "Table %s has correct checksum 0x%x.",
>   				table->name, hdr->checksum);
>   		else {
> -			int i;
> -			int log_level = LOG_LEVEL_LOW;
> -	
> -			for (i = 0; critical_checksum[i]; i++) {
> -				if (!strcmp(table->name, critical_checksum[i])) {
> -					log_level = LOG_LEVEL_CRITICAL;
> -					break;
> -				}
> -			}
> -
> -			fwts_failed(fw, log_level, "ACPITableChecksum",
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM, "ACPITableChecksum",
>   				"Table %s has incorrect checksum, "
>   				"expected 0x%2.2x, got 0x%2.2x.",
>   				table->name, (uint8_t)(hdr->checksum-checksum),
>   				hdr->checksum);
>
> -			/* Give some contextual explanation of the error */
> -			if (log_level == LOG_LEVEL_CRITICAL)
> -				fwts_advice(fw,
> -					"The kernel requires this table to have a "
> -					"valid checksum and will not load it. This "
> -					"will lead to ACPI not working correctly.");
> -			else
> -				fwts_advice(fw,
> -					"The kernel will warn that this table has "
> -					"an invalid checksum but will ignore the "
> -					"error and still load it. This is not a "
> -					"critical issue, but should be fixed if "
> -					"possible to avoid the warning messages.");
> +			fwts_advice(fw,
> +				"The kernel will warn that this table has "
> +				"an invalid checksum but will ignore the "
> +				"error and still load it. This is not a "
> +				"critical issue, but should be fixed if "
> +				"possible to avoid the warning messages.");
>
>   			fwts_tag_failed(fw, FWTS_TAG_ACPI_TABLE_CHECKSUM);
>   		}

Acked-by: Alex Hung <alex.hung@canonical.com>
Colin King - June 19, 2012, 5:33 a.m.
On 19/06/12 03:31, Keng-Yu Lin wrote:
> Hi Colin,
>    Will you also update the checksum testcase patch too?
>
>    cheers,
> -kengyu
>
Will do.

Colin

Patch

diff --git a/src/acpi/checksum/checksum.c b/src/acpi/checksum/checksum.c
index 264e7d0..9918a62 100644
--- a/src/acpi/checksum/checksum.c
+++ b/src/acpi/checksum/checksum.c
@@ -89,16 +89,6 @@  static void checksum_rsdp(fwts_framework *fw, fwts_acpi_table_info *table)
 
 }
 
-/*
- *  The following tables the kernel requires the checksum to be valid otherwise
- *  it will not load them, so checksum failures here are considered critical errors.
- */
-static char *critical_checksum[] = {
-	"RSDT",
-	"XSDT",
-	NULL
-};
-
 static int checksum_scan_tables(fwts_framework *fw)
 {
 	int i;
@@ -131,35 +121,18 @@  static int checksum_scan_tables(fwts_framework *fw)
 			fwts_passed(fw, "Table %s has correct checksum 0x%x.",
 				table->name, hdr->checksum);
 		else {
-			int i;
-			int log_level = LOG_LEVEL_LOW;
-	
-			for (i = 0; critical_checksum[i]; i++) {
-				if (!strcmp(table->name, critical_checksum[i])) {
-					log_level = LOG_LEVEL_CRITICAL;
-					break;
-				}
-			}
-
-			fwts_failed(fw, log_level, "ACPITableChecksum",
+			fwts_failed(fw, LOG_LEVEL_MEDIUM, "ACPITableChecksum",
 				"Table %s has incorrect checksum, "
 				"expected 0x%2.2x, got 0x%2.2x.",
 				table->name, (uint8_t)(hdr->checksum-checksum),
 				hdr->checksum);
 
-			/* Give some contextual explanation of the error */
-			if (log_level == LOG_LEVEL_CRITICAL)
-				fwts_advice(fw, 
-					"The kernel requires this table to have a "
-					"valid checksum and will not load it. This "
-					"will lead to ACPI not working correctly.");
-			else
-				fwts_advice(fw,
-					"The kernel will warn that this table has "
-					"an invalid checksum but will ignore the "
-					"error and still load it. This is not a "
-					"critical issue, but should be fixed if "
-					"possible to avoid the warning messages.");
+			fwts_advice(fw,
+				"The kernel will warn that this table has "
+				"an invalid checksum but will ignore the "
+				"error and still load it. This is not a "
+				"critical issue, but should be fixed if "
+				"possible to avoid the warning messages.");
 
 			fwts_tag_failed(fw, FWTS_TAG_ACPI_TABLE_CHECKSUM);
 		}