> -----Original Message----- > From: [email protected] <[email protected]> > On Behalf Of Tony Luck > Sent: Monday, March 11, 2019 1:51 PM > To: Borislav Petkov <[email protected]> > Cc: Tony Luck <[email protected]>; [email protected]; > [email protected]; Ashok Raj <[email protected]> > Subject: [PATCH] x86, mce: Fix machine_check_poll() tests for which errors to > log > > There has been a lurking "TBD" in the machine check poll routine ever > since it was first split out from the machine check handler. The potential > issue is that the poll routine may have just begun a read from the STATUS > register in a machine check bank when the hardware logs an error in that > bank and signals a machine check. That race used to be pretty small back > when machine checks were broadcast, but the addition of local machine check > means that the poll code could continue running and clear the error from the > bank before the local machine check handler on another CPU gets around to > reading it. > > Fix the code to be sure to only process errors that need to be processed > in the poll code, leaving other logged errors alone for the machine check > handler to find and process. > > Fixes: b79109c3bbcf ("x86, mce: separate correct machine check poller and > fatal exception handler") > Fixes: ed7290d0ee8f ("x86, mce: implement new status bits") > Reported-by: Ashok Raj <[email protected]> > Signed-off-by: Tony Luck <[email protected]> > --- > arch/x86/kernel/cpu/mce/core.c | 42 ++++++++++++++++++++++++++++------ > 1 file changed, 35 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c > index 6ce290c506d9..806551b381ae 100644 > --- a/arch/x86/kernel/cpu/mce/core.c > +++ b/arch/x86/kernel/cpu/mce/core.c > @@ -712,19 +712,47 @@ bool machine_check_poll(enum mcp_flags flags, > mce_banks_t *b) > > barrier(); > m.status = mce_rdmsrl(msr_ops.status(i)); > + > + /* If this entry is not valid, ignore it */ > if (!(m.status & MCI_STATUS_VAL)) > continue; > > /* > - * Uncorrected or signalled events are handled by the exception > - * handler when it is enabled, so don't process those here. > - * > - * TBD do the same check for MCI_STATUS_EN here? > + * If we are logging everything (at CPU online) or this > + * is a corrected error, then we must log it. > */ > - if (!(flags & MCP_UC) && > - (m.status & (mca_cfg.ser ? MCI_STATUS_S : MCI_STATUS_UC))) > - continue; > + if ((flags & MCP_UC) || (m.status & MCI_STATUS_UC) == 0) > + goto log_it; > + > + /* > + * Older systems that do not support software error recovery > + * should skip over uncorrected errors, but log everything else > + */ > + if (!mca_cfg.ser) { > + if (m.status & MCI_STATUS_UC) > + continue; > + goto log_it; > + } > + > + /* Log "not enabled" (speculative) errors */ > + if (!(m.status & MCI_STATUS_EN)) > + goto log_it; > + > + /* > + * Log UCNA (SDM: 15.6.3 "UCR Error Classification") > + * UC == 1 && PCC == 0 && S == 0 > + */ > + if (!(m.status & MCI_STATUS_PCC) && !(m.status & MCI_STATUS_S)) > + goto log_it; > +
Can you please include a vendor check with this? MCi_STATUS[56] is not defined the same way on AMD systems. Thanks, Yazen
