ACPI, APEI, GHES: Remove strict check for memory error handling

PATCH Applied Chen, Gong 3 participants 14 messages Nov 25, 2013 – Dec 21, 2013

Subsystems (3)

x86 ACPI ACPI APEI

lore

Share & tools

lore

Chen, Gong

author

2013-11-25 07:32 UTC #0 lore raw

PATCH v2 1/2 ACPI, APEI, GHES: Remove strict check for memory error handling

Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error even fatal error if BIOS enable it. For this kind of situation, it should be logged, too.

v2 -> v1: make the event record more precisely

Signed-off-by Chen, Gong <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce-apei.c | 10 +++++++---
 drivers/acpi/apei/ghes.c              |  3 +--
 2 files changed, 8 insertions(+), 5 deletions(-)

arch/x86/kernel/cpu/mcheck/mce-apei.c +7 -3

index de8b60a..d137ab8 100644
@@ -33,6 +33,7 @@
 #include <linux/acpi.h>
 #include <linux/cper.h>
 #include <acpi/apei.h>
+#include <acpi/ghes.h>
 #include <asm/mce.h>
 
 #include "mce-internal.h"

@@ -41,14 +42,17 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;
 
-	/* Only corrected MC is reported */
-	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
 
 	mce_setup(&m);
 	m.bank = 1;
-	/* Fake a memory read corrected error with unknown channel */
+	/* Fake a memory read error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | 0x9f;
+	if (corrected >= GHES_SEV_RECOVERABLE)
+		m.status |= MCI_STATUS_UC;
+	if (corrected >= GHES_SEV_PANIC)
+		m.status |= MCI_STATUS_PCC;
 	m.addr = mem_err->physical_addr;
 	mce_log(&m);
 	mce_notify_irq();

drivers/acpi/apei/ghes.c +1 -2

index a30bc31..ce3683d 100644
@@ -453,8 +453,7 @@ static void ghes_do_proc(struct ghes *ghes,
 			ghes_edac_report_mem_error(ghes, sev, mem_err);
 
 #ifdef CONFIG_X86_MCE
-			apei_mce_report_mem_error(sev == GHES_SEV_CORRECTED,
-						  mem_err);
+			apei_mce_report_mem_error(sev, mem_err);
 #endif
 			ghes_handle_memory_failure(gdata, sev);
 		}

Signature · 1 line

1.8.4.3

Chen, Gong

author

2013-11-25 07:32 UTC #1 lore raw

PATCH v2 2/2 ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Cleanup the logic for function ghes_handle_memory_failure. Just make it simpler and cleaner.

v2 -> v1: fix a compile error & some minor changes.

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

drivers/acpi/apei/ghes.c +20 ~~-16~~

index ce3683d..46766ef 100644
@@ -413,27 +413,31 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
 {
 #ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE
 	unsigned long pfn;
+	int flags = -1;
 	int sec_sev = ghes_severity(gdata->error_severity);
 	struct cper_sec_mem_err *mem_err;
 	mem_err = (struct cper_sec_mem_err *)(gdata + 1);
 
-	if (sec_sev == GHES_SEV_CORRECTED &&
-	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
-	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		if (pfn_valid(pfn))
-			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
-		else if (printk_ratelimit())
-			pr_warn(FW_WARN GHES_PFX
-			"Invalid address in generic error data: %#llx\n",
-			mem_err->physical_addr);
-	}
-	if (sev == GHES_SEV_RECOVERABLE &&
-	    sec_sev == GHES_SEV_RECOVERABLE &&
-	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		memory_failure_queue(pfn, 0, 0);
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
+		return;
+
+	pfn = mem_err->physical_addr >> PAGE_SHIFT;
+	if (!pfn_valid(pfn)) {
+		pr_warn_ratelimited(FW_WARN GHES_PFX
+		"Invalid address in generic error data: %#llx\n",
+		mem_err->physical_addr);
+		return;
 	}
+
+	/* iff following two events can be handled properly by now */
+	if (sec_sev == GHES_SEV_CORRECTED &&
+	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED))
+		flags = MF_SOFT_OFFLINE;
+	if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
+		flags = 0;
+
+	if (flags != -1)
+		memory_failure_queue(pfn, 0, flags);
 #endif
 }

Signature · 1 line

1.8.4.3

Chen, Gong

author

2013-11-26 07:12 UTC #2 lore raw

Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Quoted 60 lines · Gong

Date: Mon, 25 Nov 2013 02:15:01 -0500
From: "Chen, Gong" <[email protected]>
To: [email protected], [email protected], [email protected]
Cc: [email protected], "Chen, Gong" <[email protected]>
Subject: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
error handling
X-Mailer: git-send-email 1.8.4.3

Cleanup the logic for function ghes_handle_memory_failure. Just make it simpler and cleaner.

v2 -> v1: fix a compile error & some minor changes.

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

drivers/acpi/apei/ghes.c +13 ~~-16~~

index ce3683d..46766ef 100644
@@ -413,27 +413,31 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
 {
 #ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE
 	unsigned long pfn;
+	int flags = -1;
 	int sec_sev = ghes_severity(gdata->error_severity);
 	struct cper_sec_mem_err *mem_err;
 	mem_err = (struct cper_sec_mem_err *)(gdata + 1);
 
-	if (sec_sev == GHES_SEV_CORRECTED &&
-	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
-	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		if (pfn_valid(pfn))
-			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
-		else if (printk_ratelimit())
-			pr_warn(FW_WARN GHES_PFX
-			"Invalid address in generic error data: %#llx\n",
-			mem_err->physical_addr);
-	}
-	if (sev == GHES_SEV_RECOVERABLE &&
-	    sec_sev == GHES_SEV_RECOVERABLE &&
-	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		memory_failure_queue(pfn, 0, 0);
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
+		return;
+
+	pfn = mem_err->physical_addr >> PAGE_SHIFT;
+	if (!pfn_valid(pfn)) {
+		pr_warn_ratelimited(FW_WARN GHES_PFX
+		"Invalid address in generic error data: %#llx\n",
+		mem_err->physical_addr);
+		return;
 	}
+
+	/* iff following two events can be handled properly by now */
+	if (sec_sev == GHES_SEV_CORRECTED &&

drivers/acpi/apei/ghes.c static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev) · line 443

Quoted 10 lines

drivers/acpi/apei/ghes.c +7 -0

+	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED))
+		flags = MF_SOFT_OFFLINE;
+	if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
+		flags = 0;
+
+	if (flags != -1)
+		memory_failure_queue(pfn, 0, flags);
 #endif
 }

Hi, Boris

In this patch so-called cleanup includes an implied PFN check for UC error but missed in current codes.

Borislav Petkov

maintainer

2013-11-26 07:23 UTC #3 lore raw

Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Quoted 2 lines · Gong

In this patch so-called cleanup includes an implied PFN check for UC error but missed in current codes.

Right, I was about to look at it. You probably should add this to the commit message so that it is clear.

Thanks.

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Chen, Gong

author

2013-11-27 02:32 UTC #4 lore raw

Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Quoted 9 lines · Borislav Petkov

Date: Tue, 26 Nov 2013 08:23:35 +0100
From: Borislav Petkov <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
error handling
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 7 lines

Quoted 2 lines · Gong
In this patch so-called cleanup includes an implied PFN check for UC error but missed in current codes.
Right, I was about to look at it. You probably should add this to the commit message so that it is clear.

How about this:

Add proper PFN validity check for UC error and cleanup the code logic to make it simpler and cleaner.

If OK and reasonable for this patch, would you mind helping to update the introduction in the patch before merging it?

Chen, Gong

author

2013-12-14 14:00 UTC #5 lore raw

Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Quoted 27 lines · Gong

Date: Tue, 26 Nov 2013 21:15:42 -0500
From: "Chen, Gong" <[email protected]>
To: Borislav Petkov <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
error handling
User-Agent: Mutt/1.5.21 (2010-09-15)
Quoted 9 lines · Borislav Petkov
Date: Tue, 26 Nov 2013 08:23:35 +0100
From: Borislav Petkov <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory
error handling
User-Agent: Mutt/1.5.21 (2010-09-15)
Quoted 7 lines
Quoted 2 lines · Gong
In this patch so-called cleanup includes an implied PFN check for UC error but missed in current codes.
Right, I was about to look at it. You probably should add this to the commit message so that it is clear.

Quoted 7 lines

How about this:
Add proper PFN validity check for UC error and cleanup the code logic to make it simpler and cleaner.
If OK and reasonable for this patch, would you mind helping to update the introduction in the patch before merging it?

Hi, Boris

Will you pick up this patch in your RAS request pull?

Naveen N. Rao

Acked-by on v2 2/2

2013-11-26 09:06 UTC #6 lore raw

Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

In reply to Chen, Gong

Quoted 6 lines · Gong

Cleanup the logic for function ghes_handle_memory_failure. Just make it simpler and cleaner.
v2 -> v1: fix a compile error & some minor changes.
Signed-off-by Chen, Gong <[email protected]>

Acked-by Naveen N. Rao <[email protected]>

Quoted 56 lines

---
  drivers/acpi/apei/ghes.c | 36 ++++++++++++++++++++----------------
  1 file changed, 20 insertions(+), 16 deletions(-)

drivers/acpi/apei/ghes.c +20 ~~-16~~

index ce3683d..46766ef 100644
@@ -413,27 +413,31 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
  {
  #ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE
  	unsigned long pfn;
+	int flags = -1;
  	int sec_sev = ghes_severity(gdata->error_severity);
  	struct cper_sec_mem_err *mem_err;
  	mem_err = (struct cper_sec_mem_err *)(gdata + 1);
 
-	if (sec_sev == GHES_SEV_CORRECTED &&
-	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
-	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		if (pfn_valid(pfn))
-			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
-		else if (printk_ratelimit())
-			pr_warn(FW_WARN GHES_PFX
-			"Invalid address in generic error data: %#llx\n",
-			mem_err->physical_addr);
-	}
-	if (sev == GHES_SEV_RECOVERABLE &&
-	    sec_sev == GHES_SEV_RECOVERABLE &&
-	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
-		pfn = mem_err->physical_addr >> PAGE_SHIFT;
-		memory_failure_queue(pfn, 0, 0);
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
+		return;
+
+	pfn = mem_err->physical_addr >> PAGE_SHIFT;
+	if (!pfn_valid(pfn)) {
+		pr_warn_ratelimited(FW_WARN GHES_PFX
+		"Invalid address in generic error data: %#llx\n",
+		mem_err->physical_addr);
+		return;
  	}
+
+	/* iff following two events can be handled properly by now */
+	if (sec_sev == GHES_SEV_CORRECTED &&
+	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED))
+		flags = MF_SOFT_OFFLINE;
+	if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
+		flags = 0;
+
+	if (flags != -1)
+		memory_failure_queue(pfn, 0, flags);
  #endif
  }

Borislav Petkov

maintainer Applied

2013-12-21 12:41 UTC #7 lore raw

Re: [PATCH v2 2/2] ACPI, APEI, GHES: Cleanup ghes codes for memory error handling

Quoted 9 lines · Naveen N. Rao

Quoted 6 lines · Gong
Cleanup the logic for function ghes_handle_memory_failure. Just make it simpler and cleaner.
v2 -> v1: fix a compile error & some minor changes.
Signed-off-by Chen, Gong <[email protected]>
Acked-by Naveen N. Rao <[email protected]>

Applied, thanks.

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Borislav Petkov

maintainer Acked-by on v2 1/2

2013-11-25 17:13 UTC #8 lore raw

In reply to Chen, Gong

Quoted 9 lines · Gong

Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error even fatal error if BIOS enable it. For this kind of situation, it should be logged, too.
v2 -> v1: make the event record more precisely
Signed-off-by Chen, Gong <[email protected]>

Looks ok to me.

Acked-by Borislav Petkov <[email protected]>

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Naveen N. Rao

Acked-by on v2 1/2

2013-11-26 09:04 UTC #9 lore raw

In reply to Chen, Gong

Quoted 34 lines · Gong

Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error even fatal error if BIOS enable it. For this kind of situation, it should be logged, too.
v2 -> v1: make the event record more precisely
Signed-off-by Chen, Gong <[email protected]>
---
  arch/x86/kernel/cpu/mcheck/mce-apei.c | 10 +++++++---
  drivers/acpi/apei/ghes.c              |  3 +--
  2 files changed, 8 insertions(+), 5 deletions(-)
arch/x86/kernel/cpu/mcheck/mce-apei.c +2 -2
index de8b60a..d137ab8 100644
@@ -33,6 +33,7 @@
  #include <linux/acpi.h>
  #include <linux/cper.h>
  #include <acpi/apei.h>
+#include <acpi/ghes.h>
  #include <asm/mce.h>
 
  #include "mce-internal.h"
@@ -41,14 +42,17 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
  {
  	struct mce m;
 
-	/* Only corrected MC is reported */
-	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
  		return;

arch/x86/kernel/cpu/mcheck/mce-apei.c void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err) · line 55

Quoted 10 lines

arch/x86/kernel/cpu/mcheck/mce-apei.c +5 -1

 
  	mce_setup(&m);
  	m.bank = 1;
-	/* Fake a memory read corrected error with unknown channel */
+	/* Fake a memory read error with unknown channel */
  	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | 0x9f;
+	if (corrected >= GHES_SEV_RECOVERABLE)
+		m.status |= MCI_STATUS_UC;
+	if (corrected >= GHES_SEV_PANIC)
+		m.status |= MCI_STATUS_PCC;

Hmm... so you only fill up the most basic information from the cper record. In the absence of 'S', 'AR' bits, I am not sure how useful this is - except for logging the error through /dev/mcelog for legacy users. If that is the intent, you have my

Acked-by Naveen N. Rao <[email protected]>

- Naveen

Quoted 18 lines

arch/x86/kernel/cpu/mcheck/mce-apei.c +0 -0

  	m.addr = mem_err->physical_addr;
  	mce_log(&m);
  	mce_notify_irq();

drivers/acpi/apei/ghes.c +1 -2

index a30bc31..ce3683d 100644
@@ -453,8 +453,7 @@ static void ghes_do_proc(struct ghes *ghes,
  			ghes_edac_report_mem_error(ghes, sev, mem_err);
 
  #ifdef CONFIG_X86_MCE
-			apei_mce_report_mem_error(sev == GHES_SEV_CORRECTED,
-						  mem_err);
+			apei_mce_report_mem_error(sev, mem_err);
  #endif
  			ghes_handle_memory_failure(gdata, sev);
  		}

Chen, Gong

author

2013-11-26 09:48 UTC #10 lore raw

Quoted 61 lines · Naveen N. Rao

Date: Tue, 26 Nov 2013 14:32:53 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>, [email protected],
[email protected]
CC: [email protected]
Subject: Re: [PATCH v2 1/2] ACPI, APEI, GHES: Remove strict check for
memory error handling
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101
Thunderbird/24.1.0
Quoted 34 lines · Gong
Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error even fatal error if BIOS enable it. For this kind of situation, it should be logged, too.
v2 -> v1: make the event record more precisely
Signed-off-by Chen, Gong <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce-apei.c | 10 +++++++---
 drivers/acpi/apei/ghes.c              |  3 +--
 2 files changed, 8 insertions(+), 5 deletions(-)
arch/x86/kernel/cpu/mcheck/mce-apei.c +2 -2
index de8b60a..d137ab8 100644
@@ -33,6 +33,7 @@
 #include <linux/acpi.h>
 #include <linux/cper.h>
 #include <acpi/apei.h>
+#include <acpi/ghes.h>
 #include <asm/mce.h>
 
 #include "mce-internal.h"
@@ -41,14 +42,17 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;
 
-	/* Only corrected MC is reported */
-	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
arch/x86/kernel/cpu/mcheck/mce-apei.c void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err) · line 55
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce-apei.c +5 -1
 
 	mce_setup(&m);
 	m.bank = 1;
-	/* Fake a memory read corrected error with unknown channel */
+	/* Fake a memory read error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | 0x9f;
+	if (corrected >= GHES_SEV_RECOVERABLE)
+		m.status |= MCI_STATUS_UC;
+	if (corrected >= GHES_SEV_PANIC)
+		m.status |= MCI_STATUS_PCC;
Hmm... so you only fill up the most basic information from the cper record. In the absence of 'S', 'AR' bits, I am not sure how useful this is - except for logging the error through /dev/mcelog for legacy users. If that is the intent, you have my

Quoted 5 lines

Acked-by Naveen N. Rao <[email protected]>
- Naveen

Thanks for your ACK. We want to record more information but you know UEFI/CPER is not related to MCE in essentially. So we can't figure out all necessary information to construct MCE record. IOW, we can just apply the most valuable information like physical address and fake other fields. From this point of view, this kind of H/W error event report method is still not perfect.

Chen, Gong

author

2013-12-14 14:01 UTC #11 lore raw

Quoted 76 lines · Gong

Date: Tue, 26 Nov 2013 04:31:36 -0500
From: "Chen, Gong" <[email protected]>
To: "Naveen N. Rao" <[email protected]>
Cc: [email protected], [email protected], [email protected]
Subject: Re: [PATCH v2 1/2] ACPI, APEI, GHES: Remove strict check for
memory error handling
User-Agent: Mutt/1.5.21 (2010-09-15)
Quoted 61 lines · Naveen N. Rao
Date: Tue, 26 Nov 2013 14:32:53 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>, [email protected],
[email protected]
CC: [email protected]
Subject: Re: [PATCH v2 1/2] ACPI, APEI, GHES: Remove strict check for
memory error handling
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101
Thunderbird/24.1.0
Quoted 34 lines · Gong
Usually SCI is employed to handle corrected error, especially for memory corrected error but in fact SCI still can be used to handle any error like memory uncorrected error even fatal error if BIOS enable it. For this kind of situation, it should be logged, too.
v2 -> v1: make the event record more precisely
Signed-off-by Chen, Gong <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce-apei.c | 10 +++++++---
 drivers/acpi/apei/ghes.c              |  3 +--
 2 files changed, 8 insertions(+), 5 deletions(-)
arch/x86/kernel/cpu/mcheck/mce-apei.c +2 -2
index de8b60a..d137ab8 100644
@@ -33,6 +33,7 @@
 #include <linux/acpi.h>
 #include <linux/cper.h>
 #include <acpi/apei.h>
+#include <acpi/ghes.h>
 #include <asm/mce.h>
 
 #include "mce-internal.h"
@@ -41,14 +42,17 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;
 
-	/* Only corrected MC is reported */
-	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
arch/x86/kernel/cpu/mcheck/mce-apei.c void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err) · line 55
Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce-apei.c +5 -1
 
 	mce_setup(&m);
 	m.bank = 1;
-	/* Fake a memory read corrected error with unknown channel */
+	/* Fake a memory read error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | 0x9f;
+	if (corrected >= GHES_SEV_RECOVERABLE)
+		m.status |= MCI_STATUS_UC;
+	if (corrected >= GHES_SEV_PANIC)
+		m.status |= MCI_STATUS_PCC;
Hmm... so you only fill up the most basic information from the cper record. In the absence of 'S', 'AR' bits, I am not sure how useful this is - except for logging the error through /dev/mcelog for legacy users. If that is the intent, you have my
Quoted 5 lines
Acked-by Naveen N. Rao <[email protected]>
- Naveen

Quoted 6 lines

Thanks for your ACK. We want to record more information but you know UEFI/CPER is not related to MCE in essentially. So we can't figure out all necessary information to construct MCE record. IOW, we can just apply the most valuable information like physical address and fake other fields. From this point of view, this kind of H/W error event report method is still not perfect.

Hi, Boris

Will you pick up this patch in your RAS request pull?

Borislav Petkov

maintainer Acked-by Naveen N. Rao Applied

2013-12-16 14:51 UTC #12 lore raw

Quoted 1 line · Gong

Will you pick up this patch in your RAS request pull?

Applied, with commit message massaging and s/corrected/severity/ automatic variable change, version below:

--
From: "Chen, Gong" <[email protected]>
Subject: [PATCH] ACPI, APEI, GHES: Do not report only correctable errors with SCI

Currently SCI is employed to handle corrected errors, and memory corrected errors, more specifically but in fact SCI still can be used to handle any errors, e.g. uncorrected or even fatal ones if enabled by the BIOS. Enable logging for those kinds of errors too.

Signed-off-by Chen, Gong <[email protected]>

Acked-by Naveen N. Rao <[email protected]>

Cc Tony Luck <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ Boris: massage commit message, rename automatic variable. ]

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce-apei.c | 14 ++++++++++----
 drivers/acpi/apei/ghes.c              |  3 +--
 2 files changed, 11 insertions(+), 6 deletions(-)

arch/x86/kernel/cpu/mcheck/mce-apei.c +10 -4

index de8b60a53f69..a1aef9533154 100644
@@ -33,22 +33,28 @@
 #include <linux/acpi.h>
 #include <linux/cper.h>
 #include <acpi/apei.h>
+#include <acpi/ghes.h>
 #include <asm/mce.h>
 
 #include "mce-internal.h"
 
-void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
+void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;
 
-	/* Only corrected MC is reported */
-	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
+	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
 
 	mce_setup(&m);
 	m.bank = 1;
-	/* Fake a memory read corrected error with unknown channel */
+	/* Fake a memory read error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | 0x9f;
+
+	if (severity >= GHES_SEV_RECOVERABLE)
+		m.status |= MCI_STATUS_UC;
+	if (severity >= GHES_SEV_PANIC)
+		m.status |= MCI_STATUS_PCC;
+
 	m.addr = mem_err->physical_addr;
 	mce_log(&m);
 	mce_notify_irq();

drivers/acpi/apei/ghes.c +1 -2

index a30bc313787b..ce3683d93a13 100644
@@ -453,8 +453,7 @@ static void ghes_do_proc(struct ghes *ghes,
 			ghes_edac_report_mem_error(ghes, sev, mem_err);
 
 #ifdef CONFIG_X86_MCE
-			apei_mce_report_mem_error(sev == GHES_SEV_CORRECTED,
-						  mem_err);
+			apei_mce_report_mem_error(sev, mem_err);
 #endif
 			ghes_handle_memory_failure(gdata, sev);
 		}

Signature · 8 lines

1.8.4

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Chen, Gong

author

2013-12-16 15:00 UTC #13 lore raw

Quoted 9 lines · Borislav Petkov

Date: Mon, 16 Dec 2013 15:51:29 +0100
From: Borislav Petkov <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: "Naveen N. Rao" <[email protected]>, [email protected],
[email protected]
Subject: Re: [PATCH v2 1/2] ACPI, APEI, GHES: Remove strict check for
memory error handling
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 6 lines

Quoted 1 line · Gong
Will you pick up this patch in your RAS request pull?
Applied, with commit message massaging and s/corrected/severity/ automatic variable change, version below:

Thanks very much for your effort.