linux-kernel - RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Apr 2017 17:06:19 +0000
From:   "Ghannam, Yazen" <Yazen.Ghannam@....com>
To:     Borislav Petkov <bp@...en8.de>
CC:     "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        Tony Luck <tony.luck@...el.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}
 registers

> -----Original Message-----
> From: Borislav Petkov [mailto:bp@...en8.de]
> Sent: Wednesday, April 05, 2017 12:45 PM
> To: Ghannam, Yazen <Yazen.Ghannam@....com>
> Cc: linux-edac@...r.kernel.org; Tony Luck <tony.luck@...el.com>;
> x86@...nel.org; linux-kernel@...r.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