Messages in this thread | | | From | "Ghannam, Yazen" <> | Subject | RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers | Date | Wed, 5 Apr 2017 17:06:19 +0000 |
| |
> -----Original Message----- > From: Borislav Petkov [mailto:bp@alien8.de] > Sent: Wednesday, April 05, 2017 12:45 PM > To: Ghannam, Yazen <Yazen.Ghannam@amd.com> > Cc: linux-edac@vger.kernel.org; Tony Luck <tony.luck@intel.com>; > x86@kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA > MCA_DE{STAT,ADDR} registers > > > > I'd rather we keep as many checks as possible out of __log_error(). > > What checks? >
Checking if we have a valid deferred error. Since we call __log_error() on thresholding interrupts too we would need to tell it which handler is calling it to do the correct check. This is what we currently do.
> > Your suggestion gave me an idea. Let's drop __log_error_deferred() and > > just select the correct registers in the deferred error interrupt handler. > > > > /* > > * APIC interrupt handler for deferred errors > > * > > * We have three scenarios for checking for Deferred errors. > > * 1) Non-SMCA systems check MCA_STATUS and log error if found. > > * 2) SMCA systems check MCA_STATUS. If error is found then log it and also > > * clear MCA_DESTAT. > > * 3) SMCA systems check MCA_DESTAT, if error was not found in > MCA_STATUS, and > > * log it. > > */ > > static void amd_deferred_error_interrupt(void) > > { > > unsigned int bank; > > u64 status; > > > > for (bank = 0; bank < mca_cfg.banks; ++bank) { > > rdmsrl(msr_ops.status(bank), status); > > > > if (is_deferred_error(status)) { > > __log_error(bank, msr_ops.status(bank), > > msr_ops.addr(bank), 0); > > So we're an SMCA box and we land here on a deferred error, we don't have > anything in the standard MSRs... >
What do you mean " we don't have anything"? We check if we have a valid deferred error in is_deferred_error(). Otherwise, we don't log anything.
> > /* Clear MCA_DESTAT even if we used MCA_STATUS. */ > > if (mce_flags.smca) > > > > wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(bank), 0); > > ... and here we clear the info which we wanted to log before we log it! >
No we don't. If we don't have a valid deferred error in MCA_STATUS then we don't get here.
> > > > } else if (mce_flags.smca) { > > rdmsrl(MSR_AMD64_SMCA_MCx_DESTAT(bank), > > status); > > > > if (is_deferred_error(status)) > > __log_error(bank, > > MSR_AMD64_SMCA_MCx_DESTAT(bank), > MSR_AMD64_SMCA_MCx_DEADDR(bank), 0); > > So we execute __log_error() twice on an SMCA box for a deferred error. >
No we don't. This is an if/else-if statement.
Thanks, Yazen
| |