The mce handler for 'nfit' devices is called for memory errors on a Non-Volatile DIMM, and adds the error location to a 'badblocks' list. This list is used by the various NVDIMM drivers to avoid consuming known poison locations during IO.
The mce handler gets called for both corrected and uncorrectable errors. Until now, both kinds of errors have been added to the badblocks list. However, corrected memory errors indicate that the problem has already been fixed by hardware, and the resulting interrupt is merely a notification to Linux. As far as future accesses to that location are concerned, it is perfectly fine to use, and thus doesn't need to be included in the above badblocks list.
Add a check in the nfit mce handler to filter out corrected mce events, and only process uncorrectable errors.
Reported-by: Omar Avelar omar.avelar@intel.com Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Cc: stable@vger.kernel.org Cc: Dan Williams dan.j.williams@intel.com Cc: Tony Luck tony.luck@intel.com Cc: Borislav Petkov bp@alien8.de Signed-off-by: Vishal Verma vishal.l.verma@intel.com --- arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-)
v3: Unchanged from v2
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index 3a17107594c8..3111b3cee2ee 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -216,6 +216,7 @@ static inline int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *s
int mce_available(struct cpuinfo_x86 *c); bool mce_is_memory_error(struct mce *m); +bool mce_is_correctable(struct mce *m);
DECLARE_PER_CPU(unsigned, mce_exception_count); DECLARE_PER_CPU(unsigned, mce_poll_count); diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 953b3ce92dcc..27015948bc41 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -534,7 +534,7 @@ bool mce_is_memory_error(struct mce *m) } EXPORT_SYMBOL_GPL(mce_is_memory_error);
-static bool mce_is_correctable(struct mce *m) +bool mce_is_correctable(struct mce *m) { if (m->cpuvendor == X86_VENDOR_AMD && m->status & MCI_STATUS_DEFERRED) return false; @@ -544,6 +544,7 @@ static bool mce_is_correctable(struct mce *m)
return true; } +EXPORT_SYMBOL_GPL(mce_is_correctable);
static bool cec_add_mce(struct mce *m) { diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c index e9626bf6ca29..7a51707f87e9 100644 --- a/drivers/acpi/nfit/mce.c +++ b/drivers/acpi/nfit/mce.c @@ -25,8 +25,8 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val, struct acpi_nfit_desc *acpi_desc; struct nfit_spa *nfit_spa;
- /* We only care about memory errors */ - if (!mce_is_memory_error(mce)) + /* We only care about uncorrectable memory errors */ + if (!mce_is_memory_error(mce) || mce_is_correctable(mce)) return NOTIFY_DONE;
/*
The NFIT machine check handler uses the physical address from the 'mce' structure, and compares it against information in the ACPI NFIT table to determine whether that location lies on an NVDIMM. The mce->addr field however may not always be valid, and this is indicated by the MCI_STATUS_ADDRV bit in the status field.
Export mce_usable_address() which already performs validation for the address, and use it in the NFIT handler.
Reported-by: Robert Elliott elliott@hpe.com Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Cc: stable@vger.kernel.org Cc: Dan Williams dan.j.williams@intel.com Cc: Tony Luck tony.luck@intel.com Cc: Borislav Petkov bp@alien8.de Signed-off-by: Vishal Verma vishal.l.verma@intel.com --- arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-)
v3: Add this patch to address the address validation (Robert)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index 3111b3cee2ee..eb786f90f2d3 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -217,6 +217,7 @@ static inline int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *s int mce_available(struct cpuinfo_x86 *c); bool mce_is_memory_error(struct mce *m); bool mce_is_correctable(struct mce *m); +int mce_usable_address(struct mce *m);
DECLARE_PER_CPU(unsigned, mce_exception_count); DECLARE_PER_CPU(unsigned, mce_poll_count); diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 27015948bc41..cdbedeb3f3db 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -485,7 +485,7 @@ static void mce_report_event(struct pt_regs *regs) * be somewhat complicated (e.g. segment offset would require an instruction * parser). So only support physical addresses up to page granuality for now. */ -static int mce_usable_address(struct mce *m) +int mce_usable_address(struct mce *m) { if (!(m->status & MCI_STATUS_ADDRV)) return 0; @@ -505,6 +505,7 @@ static int mce_usable_address(struct mce *m)
return 1; } +EXPORT_SYMBOL_GPL(mce_usable_address);
bool mce_is_memory_error(struct mce *m) { diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c index 7a51707f87e9..d943ed50f0b4 100644 --- a/drivers/acpi/nfit/mce.c +++ b/drivers/acpi/nfit/mce.c @@ -29,6 +29,10 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val, if (!mce_is_memory_error(mce) || mce_is_correctable(mce)) return NOTIFY_DONE;
+ /* Verify the address reported in the mce is valid */ + if (!mce_usable_address(mce)) + return NOTIFY_DONE; + /* * mce->addr contains the physical addr accessed that caused the * machine check. We need to walk through the list of NFITs, and see
On Thu, Oct 25, 2018 at 06:37:29PM -0600, Vishal Verma wrote:
The NFIT machine check handler uses the physical address from the 'mce' structure, and compares it against information in the ACPI NFIT table to determine whether that location lies on an NVDIMM. The mce->addr field however may not always be valid, and this is indicated by the MCI_STATUS_ADDRV bit in the status field.
Export mce_usable_address() which already performs validation for the address, and use it in the NFIT handler.
Reported-by: Robert Elliott elliott@hpe.com Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Cc: stable@vger.kernel.org Cc: Dan Williams dan.j.williams@intel.com Cc: Tony Luck tony.luck@intel.com Cc: Borislav Petkov bp@alien8.de Signed-off-by: Vishal Verma vishal.l.verma@intel.com
arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-)
Is there any particular reason why is this a separate patch and not part of the first one?
Also, do s/mce/MCE/g.
Thx.
On Tue, Nov 6, 2018 at 6:51 AM Borislav Petkov bp@alien8.de wrote:
On Thu, Oct 25, 2018 at 06:37:29PM -0600, Vishal Verma wrote:
The NFIT machine check handler uses the physical address from the 'mce' structure, and compares it against information in the ACPI NFIT table to determine whether that location lies on an NVDIMM. The mce->addr field however may not always be valid, and this is indicated by the MCI_STATUS_ADDRV bit in the status field.
Export mce_usable_address() which already performs validation for the address, and use it in the NFIT handler.
Reported-by: Robert Elliott elliott@hpe.com Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Cc: stable@vger.kernel.org Cc: Dan Williams dan.j.williams@intel.com Cc: Tony Luck tony.luck@intel.com Cc: Borislav Petkov bp@alien8.de Signed-off-by: Vishal Verma vishal.l.verma@intel.com
arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-)
Is there any particular reason why is this a separate patch and not part of the first one?
I recommended the split so the fixes can be tracked and / or reverted independently if they cause problems.
On Tue, Nov 06, 2018 at 08:20:38AM -0800, Dan Williams wrote:
I recommended the split so the fixes can be tracked and / or reverted independently if they cause problems.
Do you have anything concrete in mind that might go wrong or just a general cautiousness?
Or do you think the hw might not spit what mce_usable_address() is checking for, for NVDIMM MCEs, so that you'd like to be able to revert that second patch?
Thx.
On Tue, Nov 6, 2018 at 9:53 AM Borislav Petkov bp@alien8.de wrote:
On Tue, Nov 06, 2018 at 08:20:38AM -0800, Dan Williams wrote:
I recommended the split so the fixes can be tracked and / or reverted independently if they cause problems.
Do you have anything concrete in mind that might go wrong or just a general cautiousness?
Just general cautiousness, I'm not opposed to squashing them.
Or do you think the hw might not spit what mce_usable_address() is checking for, for NVDIMM MCEs, so that you'd like to be able to revert that second patch?
mce_usable_address() should not have any sensitivity to NVDIMM vs DRAM MCEs.
On Tue, Nov 06, 2018 at 10:02:46AM -0800, Dan Williams wrote:
Just general cautiousness, I'm not opposed to squashing them.
Nah, I can keep them separate - I was just wondering why.
mce_usable_address() should not have any sensitivity to NVDIMM vs DRAM MCEs.
Ok.
Thx.
Commit-ID: e8a308e5f47e545e0d41d0686c00f5f5217c5f61 Gitweb: https://git.kernel.org/tip/e8a308e5f47e545e0d41d0686c00f5f5217c5f61 Author: Vishal Verma vishal.l.verma@intel.com AuthorDate: Thu, 25 Oct 2018 18:37:29 -0600 Committer: Borislav Petkov bp@suse.de CommitDate: Tue, 6 Nov 2018 19:13:26 +0100
acpi/nfit, x86/mce: Validate a MCE's address before using it
The NFIT machine check handler uses the physical address from the mce structure, and compares it against information in the ACPI NFIT table to determine whether that location lies on an NVDIMM. The mce->addr field however may not always be valid, and this is indicated by the MCI_STATUS_ADDRV bit in the status field.
Export mce_usable_address() which already performs validation for the address, and use it in the NFIT handler.
Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Reported-by: Robert Elliott elliott@hpe.com Signed-off-by: Vishal Verma vishal.l.verma@intel.com Signed-off-by: Borislav Petkov bp@suse.de CC: Arnd Bergmann arnd@arndb.de Cc: Dan Williams dan.j.williams@intel.com CC: Dave Jiang dave.jiang@intel.com CC: elliott@hpe.com CC: "H. Peter Anvin" hpa@zytor.com CC: Ingo Molnar mingo@redhat.com CC: Len Brown lenb@kernel.org CC: linux-acpi@vger.kernel.org CC: linux-edac linux-edac@vger.kernel.org CC: linux-nvdimm@lists.01.org CC: Qiuxu Zhuo qiuxu.zhuo@intel.com CC: "Rafael J. Wysocki" rjw@rjwysocki.net CC: Ross Zwisler zwisler@kernel.org CC: stable stable@vger.kernel.org CC: Thomas Gleixner tglx@linutronix.de CC: Tony Luck tony.luck@intel.com CC: x86-ml x86@kernel.org CC: Yazen Ghannam yazen.ghannam@amd.com Link: http://lkml.kernel.org/r/20181026003729.8420-2-vishal.l.verma@intel.com --- arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index dbd9fe2f6163..c1a812bd5a27 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -222,6 +222,7 @@ static inline void mce_hygon_feature_init(struct cpuinfo_x86 *c) { return mce_am int mce_available(struct cpuinfo_x86 *c); bool mce_is_memory_error(struct mce *m); bool mce_is_correctable(struct mce *m); +int mce_usable_address(struct mce *m);
DECLARE_PER_CPU(unsigned, mce_exception_count); DECLARE_PER_CPU(unsigned, mce_poll_count); diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 77527b8ea982..36d2696c9563 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -485,7 +485,7 @@ static void mce_report_event(struct pt_regs *regs) * be somewhat complicated (e.g. segment offset would require an instruction * parser). So only support physical addresses up to page granuality for now. */ -static int mce_usable_address(struct mce *m) +int mce_usable_address(struct mce *m) { if (!(m->status & MCI_STATUS_ADDRV)) return 0; @@ -505,6 +505,7 @@ static int mce_usable_address(struct mce *m)
return 1; } +EXPORT_SYMBOL_GPL(mce_usable_address);
bool mce_is_memory_error(struct mce *m) { diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c index 7a51707f87e9..d6c1b10f6c25 100644 --- a/drivers/acpi/nfit/mce.c +++ b/drivers/acpi/nfit/mce.c @@ -29,6 +29,10 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val, if (!mce_is_memory_error(mce) || mce_is_correctable(mce)) return NOTIFY_DONE;
+ /* Verify the address reported in the MCE is valid. */ + if (!mce_usable_address(mce)) + return NOTIFY_DONE; + /* * mce->addr contains the physical addr accessed that caused the * machine check. We need to walk through the list of NFITs, and see
Commit-ID: 5d96c9342c23ee1d084802dcf064caa67ecaa45b Gitweb: https://git.kernel.org/tip/5d96c9342c23ee1d084802dcf064caa67ecaa45b Author: Vishal Verma vishal.l.verma@intel.com AuthorDate: Thu, 25 Oct 2018 18:37:28 -0600 Committer: Borislav Petkov bp@suse.de CommitDate: Tue, 6 Nov 2018 19:13:10 +0100
acpi/nfit, x86/mce: Handle only uncorrectable machine checks
The MCE handler for nfit devices is called for memory errors on a Non-Volatile DIMM and adds the error location to a 'badblocks' list. This list is used by the various NVDIMM drivers to avoid consuming known poison locations during IO.
The MCE handler gets called for both corrected and uncorrectable errors. Until now, both kinds of errors have been added to the badblocks list. However, corrected memory errors indicate that the problem has already been fixed by hardware, and the resulting interrupt is merely a notification to Linux.
As far as future accesses to that location are concerned, it is perfectly fine to use, and thus doesn't need to be included in the above badblocks list.
Add a check in the nfit MCE handler to filter out corrected mce events, and only process uncorrectable errors.
Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Reported-by: Omar Avelar omar.avelar@intel.com Signed-off-by: Vishal Verma vishal.l.verma@intel.com Signed-off-by: Borislav Petkov bp@suse.de CC: Arnd Bergmann arnd@arndb.de CC: Dan Williams dan.j.williams@intel.com CC: Dave Jiang dave.jiang@intel.com CC: elliott@hpe.com CC: "H. Peter Anvin" hpa@zytor.com CC: Ingo Molnar mingo@redhat.com CC: Len Brown lenb@kernel.org CC: linux-acpi@vger.kernel.org CC: linux-edac linux-edac@vger.kernel.org CC: linux-nvdimm@lists.01.org CC: Qiuxu Zhuo qiuxu.zhuo@intel.com CC: "Rafael J. Wysocki" rjw@rjwysocki.net CC: Ross Zwisler zwisler@kernel.org CC: stable stable@vger.kernel.org CC: Thomas Gleixner tglx@linutronix.de CC: Tony Luck tony.luck@intel.com CC: x86-ml x86@kernel.org CC: Yazen Ghannam yazen.ghannam@amd.com Link: http://lkml.kernel.org/r/20181026003729.8420-1-vishal.l.verma@intel.com --- arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index 4da9b1c58d28..dbd9fe2f6163 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -221,6 +221,7 @@ static inline void mce_hygon_feature_init(struct cpuinfo_x86 *c) { return mce_am
int mce_available(struct cpuinfo_x86 *c); bool mce_is_memory_error(struct mce *m); +bool mce_is_correctable(struct mce *m);
DECLARE_PER_CPU(unsigned, mce_exception_count); DECLARE_PER_CPU(unsigned, mce_poll_count); diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 8c66d2fc8f81..77527b8ea982 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -534,7 +534,7 @@ bool mce_is_memory_error(struct mce *m) } EXPORT_SYMBOL_GPL(mce_is_memory_error);
-static bool mce_is_correctable(struct mce *m) +bool mce_is_correctable(struct mce *m) { if (m->cpuvendor == X86_VENDOR_AMD && m->status & MCI_STATUS_DEFERRED) return false; @@ -547,6 +547,7 @@ static bool mce_is_correctable(struct mce *m)
return true; } +EXPORT_SYMBOL_GPL(mce_is_correctable);
static bool cec_add_mce(struct mce *m) { diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c index e9626bf6ca29..7a51707f87e9 100644 --- a/drivers/acpi/nfit/mce.c +++ b/drivers/acpi/nfit/mce.c @@ -25,8 +25,8 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val, struct acpi_nfit_desc *acpi_desc; struct nfit_spa *nfit_spa;
- /* We only care about memory errors */ - if (!mce_is_memory_error(mce)) + /* We only care about uncorrectable memory errors */ + if (!mce_is_memory_error(mce) || mce_is_correctable(mce)) return NOTIFY_DONE;
/*
Hi, Vishal,
Vishal Verma vishal.l.verma@intel.com writes:
The mce handler for 'nfit' devices is called for memory errors on a Non-Volatile DIMM, and adds the error location to a 'badblocks' list. This list is used by the various NVDIMM drivers to avoid consuming known poison locations during IO.
The mce handler gets called for both corrected and uncorrectable errors.
Sorry for necroposting. I thought the point of the CEC was to make sure that the other registered decoders only ever saw uncorrected errors. How do we end up getting called with a correctable error?
Thanks, Jeff
Until now, both kinds of errors have been added to the badblocks list. However, corrected memory errors indicate that the problem has already been fixed by hardware, and the resulting interrupt is merely a notification to Linux. As far as future accesses to that location are concerned, it is perfectly fine to use, and thus doesn't need to be included in the above badblocks list.
Add a check in the nfit mce handler to filter out corrected mce events, and only process uncorrectable errors.
Reported-by: Omar Avelar omar.avelar@intel.com Fixes: 6839a6d96f4e ("nfit: do an ARS scrub on hitting a latent media error") Cc: stable@vger.kernel.org Cc: Dan Williams dan.j.williams@intel.com Cc: Tony Luck tony.luck@intel.com Cc: Borislav Petkov bp@alien8.de Signed-off-by: Vishal Verma vishal.l.verma@intel.com
arch/x86/include/asm/mce.h | 1 + arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- drivers/acpi/nfit/mce.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-)
v3: Unchanged from v2
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index 3a17107594c8..3111b3cee2ee 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -216,6 +216,7 @@ static inline int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *s int mce_available(struct cpuinfo_x86 *c); bool mce_is_memory_error(struct mce *m); +bool mce_is_correctable(struct mce *m); DECLARE_PER_CPU(unsigned, mce_exception_count); DECLARE_PER_CPU(unsigned, mce_poll_count); diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 953b3ce92dcc..27015948bc41 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -534,7 +534,7 @@ bool mce_is_memory_error(struct mce *m) } EXPORT_SYMBOL_GPL(mce_is_memory_error); -static bool mce_is_correctable(struct mce *m) +bool mce_is_correctable(struct mce *m) { if (m->cpuvendor == X86_VENDOR_AMD && m->status & MCI_STATUS_DEFERRED) return false; @@ -544,6 +544,7 @@ static bool mce_is_correctable(struct mce *m) return true; } +EXPORT_SYMBOL_GPL(mce_is_correctable); static bool cec_add_mce(struct mce *m) { diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c index e9626bf6ca29..7a51707f87e9 100644 --- a/drivers/acpi/nfit/mce.c +++ b/drivers/acpi/nfit/mce.c @@ -25,8 +25,8 @@ static int nfit_handle_mce(struct notifier_block *nb, unsigned long val, struct acpi_nfit_desc *acpi_desc; struct nfit_spa *nfit_spa;
- /* We only care about memory errors */
- if (!mce_is_memory_error(mce))
- /* We only care about uncorrectable memory errors */
- if (!mce_is_memory_error(mce) || mce_is_correctable(mce)) return NOTIFY_DONE;
/*
linux-stable-mirror@lists.linaro.org