Skip to contentSkip to conversation

x86/mce: retrieve poison range from hardware

PATCH Superseded Jane Chu 2 participants 3 messages Aug 2, 2022

Subsystems (4)

v6 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Jane Chu

author
#0 lore raw

PATCH v6 1/1 x86/mce: retrieve poison range from hardware

With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine poison granularity") that changed nfit_handle_mce() callback to report badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting 2 back-to-back poisons and the driver ends up logging 8 badblocks, because 0x1000 bytes is 8 512-byte.

Dan Williams noticed that apei_mce_report_mem_error() hardcode the LSB field to PAGE_SHIFT instead of consulting the input struct cper_sec_mem_err record. So change to rely on hardware whenever support is available.

---
 arch/x86/kernel/cpu/mce/apei.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/mce/apei.c +13 -1
index 717192915f28..e2439c7872ba 100644
@@ -29,15 +29,27 @@
 void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;
+	int lsb;
 
 	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
 
+	/*
+	 * Even if the ->validation_bits are set for address mask,
+	 * to be extra safe, check and reject an error radius '0',
+	 * and fallback to the default page size.
+	 */
+	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
+		lsb = find_first_bit((const unsigned long *)
+			&mem_err->physical_addr_mask, PAGE_SHIFT);
+	else
+		lsb = PAGE_SHIFT;
+
 	mce_setup(&m);
 	m.bank = -1;
 	/* Fake a memory read error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | MCI_STATUS_MISCV | 0x9f;
-	m.misc = (MCI_MISC_ADDR_PHYS << 6) | PAGE_SHIFT;
+	m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
 
 	if (severity >= GHES_SEV_RECOVERABLE)
 		m.status |= MCI_STATUS_UC;
Signature · 1 line
2.18.4
Reviewed-by on v6 1/1
#1 lore raw
Quoted 28 lines · Jane Chu

With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine poison granularity") that changed nfit_handle_mce() callback to report badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting 2 back-to-back poisons and the driver ends up logging 8 badblocks, because 0x1000 bytes is 8 512-byte.

Dan Williams noticed that apei_mce_report_mem_error() hardcode the LSB field to PAGE_SHIFT instead of consulting the input struct cper_sec_mem_err record. So change to rely on hardware whenever support is available.

---
 arch/x86/kernel/cpu/mce/apei.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/mce/apei.c +0 -0
index 717192915f28..e2439c7872ba 100644
@@ -29,15 +29,27 @@
 void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;

arch/x86/kernel/cpu/mce/apei.c line 41

Quoted 10 lines
arch/x86/kernel/cpu/mce/apei.c +6 -0
+	int lsb;
 
 	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
 
+	/*
+	 * Even if the ->validation_bits are set for address mask,
+	 * to be extra safe, check and reject an error radius '0',
+	 * and fallback to the default page size.
+	 */

speling nit:

s/fallback/fall back

arch/x86/kernel/cpu/mce/apei.c line 44

Quoted 3 lines
arch/x86/kernel/cpu/mce/apei.c +3 -0
+	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
+		lsb = find_first_bit((const unsigned long *)
+			&mem_err->physical_addr_mask, PAGE_SHIFT);

I think we can write this in a shorter form and in a single line:

		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);

(Ignore checkpatch if it wants to break the line.)

Untested.

Assuming my suggestion is correct and with those addressed:

Thanks,

Ingo

author
#2 lore raw
Quoted 54 lines · Ingo Molnar
Quoted 28 lines · Jane Chu

With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine poison granularity") that changed nfit_handle_mce() callback to report badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting 2 back-to-back poisons and the driver ends up logging 8 badblocks, because 0x1000 bytes is 8 512-byte.

Dan Williams noticed that apei_mce_report_mem_error() hardcode the LSB field to PAGE_SHIFT instead of consulting the input struct cper_sec_mem_err record. So change to rely on hardware whenever support is available.

---
  arch/x86/kernel/cpu/mce/apei.c | 14 +++++++++++++-
  1 file changed, 13 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/mce/apei.c +0 -0
index 717192915f28..e2439c7872ba 100644
@@ -29,15 +29,27 @@
  void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
  {
  	struct mce m;

arch/x86/kernel/cpu/mce/apei.c line 41

Quoted 10 lines
arch/x86/kernel/cpu/mce/apei.c +6 -0
+	int lsb;
 
  	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
  		return;
 
+	/*
+	 * Even if the ->validation_bits are set for address mask,
+	 * to be extra safe, check and reject an error radius '0',
+	 * and fallback to the default page size.
+	 */

speling nit:

s/fallback/fall back

arch/x86/kernel/cpu/mce/apei.c line 44

Quoted 3 lines
arch/x86/kernel/cpu/mce/apei.c +3 -0
+	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
+		lsb = find_first_bit((const unsigned long *)
+			&mem_err->physical_addr_mask, PAGE_SHIFT);

I think we can write this in a shorter form and in a single line:

		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);
Quoted 7 lines

(Ignore checkpatch if it wants to break the line.)

Untested.

Assuming my suggestion is correct and with those addressed:

Thanks! Just tested the change, v7 coming next.

-jane

Quoted 4 lines

Thanks,

Ingo