x86/mce: retrieve poison range from hardware
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
@@ -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
Quoted 28 lines · Jane Chu
With Commit
7917f9cdb503("acpi/nfit: rely onmce->miscto determine poison granularity") that changednfit_handle_mce()callback to report badrange according to 1ULL <<MCI_MISC_ADDR_LSB(mce->misc), it's been discovered that themce->miscLSB field is0x1000bytes, hence injecting 2 back-to-back poisons and the driver ends up logging 8 badblocks, because0x1000bytes is 8 512-byte.Dan Williams noticed that
apei_mce_report_mem_error()hardcode the LSB field toPAGE_SHIFTinstead of consulting the input structcper_sec_mem_errrecord. So change to rely on hardware whenever support is available.Reviewed-by Dan Williams <[email protected]>Signed-off-by Jane Chu <[email protected]>--- arch/x86/kernel/cpu/mce/apei.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)arch/x86/kernel/cpu/mce/apei.c +0
-0@@ -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
Quoted 54 lines · Ingo Molnar
Quoted 28 lines · Jane Chu
With Commit
7917f9cdb503("acpi/nfit: rely onmce->miscto determine poison granularity") that changednfit_handle_mce()callback to report badrange according to 1ULL <<MCI_MISC_ADDR_LSB(mce->misc), it's been discovered that themce->miscLSB field is0x1000bytes, hence injecting 2 back-to-back poisons and the driver ends up logging 8 badblocks, because0x1000bytes is 8 512-byte.Dan Williams noticed that
apei_mce_report_mem_error()hardcode the LSB field toPAGE_SHIFTinstead of consulting the input structcper_sec_mem_errrecord. So change to rely on hardware whenever support is available.Reviewed-by Dan Williams <[email protected]>Signed-off-by Jane Chu <[email protected]>--- arch/x86/kernel/cpu/mce/apei.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)arch/x86/kernel/cpu/mce/apei.c +0
-0@@ -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:
Reviewed-by Ingo Molnar <[email protected]>
Thanks! Just tested the change, v7 coming next.
-jane
Quoted 4 lines
Thanks,
Ingo