Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1621250 > unrolled thread

RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers

Started by"Ghannam, Yazen" <Yazen.Ghannam@amd.com>
First post2017-04-11 15:00 +0200
Last post2017-04-11 16:00 +0200
Articles 8 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers "Ghannam, Yazen" <Yazen.Ghannam@amd.com> - 2017-04-11 15:00 +0200
    Re: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers Borislav Petkov <bp@alien8.de> - 2017-04-11 15:20 +0200
      RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers "Ghannam, Yazen" <Yazen.Ghannam@amd.com> - 2017-04-11 15:20 +0200
        Re: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers Borislav Petkov <bp@alien8.de> - 2017-04-11 15:30 +0200
          RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers "Ghannam, Yazen" <Yazen.Ghannam@amd.com> - 2017-04-11 15:40 +0200
            Re: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers Borislav Petkov <bp@alien8.de> - 2017-04-11 15:40 +0200
              Re: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers Borislav Petkov <bp@alien8.de> - 2017-04-11 16:00 +0200
              RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR}  registers "Ghannam, Yazen" <Yazen.Ghannam@amd.com> - 2017-04-11 16:00 +0200

#1621250 — RE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers

From"Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Date2017-04-11 15:00 +0200
SubjectRE: [PATCH v2 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers
Message-ID<tv3yO-77s-15@gated-at.bofh.it>
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Friday, April 07, 2017 5:35 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
> 
> On Fri, Apr 07, 2017 at 08:37:03PM +0000, Ghannam, Yazen wrote:
> > CEs will get picked up by polling or if we hit a threshold.
> 
> I think we should be pre-emptive here and simply log the error. No use
> waiting until something polling finally gets its hands on it - we're looking at the
> signature so we might just as well log it.
> 

Okay, will do.

> > Okay. But do we need a return value? I'm thinking we can go through
> > all banks and log any and all Deferred errors rather than just the
> > first one we find. I asked and this is the preferred method like how
> > we do in the #MC handler. The same applies to the thresholding interrupt
> handler.
> 
> ... and not only the deferred errors but the CEs too.
> 
> Which would make the whole code a *lot* simpler. You simply iterate over
> banks:
> 
> 	for_each_bank()
> 		log_error()
> 
> 		if (smca)
> 			log_error_from_de_regs()
> 
> Purely pseudocode of course.
> 
> And there's no need to go and look whether the error is a deferred error or
> whatnot. In the majority of the cases it will be because we're in the #DF
> handler and it better be raised for a #DF.
> 

If we do as above then we can possibly log the same deferred error twice. So
even if we log every error we should still check if the error is deferred to decide
whether or not to log what's in the DE* registers.

> But even if we see something else, we should simply log it. As long as it is a
> valid error signature there's nothing wrong with us logging it and clearing the
> regs. The earlier we do so, the lower the probability for setting the overflow
> bit.
> 

Okay.

Thanks,
Yazen

[toc] | [next] | [standalone]

#1621260

FromBorislav Petkov <bp@alien8.de>
Date2017-04-11 15:20 +0200
Message-ID<tv3Sa-7tH-15@gated-at.bofh.it>
In reply to#1621250
On Tue, Apr 11, 2017 at 12:53:56PM +0000, Ghannam, Yazen wrote:
> If we do as above then we can possibly log the same deferred error twice.

Why twice?

	for_each_bank()
		log_error()
		|-> clear MSRs after logging

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1621262

From"Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Date2017-04-11 15:20 +0200
Message-ID<tv3Sa-7tH-17@gated-at.bofh.it>
In reply to#1621260
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Tuesday, April 11, 2017 9:12 AM
> 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
> 
> On Tue, Apr 11, 2017 at 12:53:56PM +0000, Ghannam, Yazen wrote:
> > If we do as above then we can possibly log the same deferred error twice.
> 
> Why twice?
> 
> 	for_each_bank()
> 		log_error()
> 		|-> clear MSRs after logging
> 

So log_error() reads/clears MCA_STATUS, right? This won't affect MCA_DESTAT
on SMCA systems. So if we call log_error_smca() and unconditionally read
MCA_DESTAT, we will find the same deferred error that we logged in log_error().

Thanks,
Yazen

[toc] | [prev] | [next] | [standalone]

#1621263

FromBorislav Petkov <bp@alien8.de>
Date2017-04-11 15:30 +0200
Message-ID<tv41Q-7yR-7@gated-at.bofh.it>
In reply to#1621262
On Tue, Apr 11, 2017 at 01:18:50PM +0000, Ghannam, Yazen wrote:
> So log_error() reads/clears MCA_STATUS, right? This won't affect MCA_DESTAT
> on SMCA systems. So if we call log_error_smca() and unconditionally read
> MCA_DESTAT, we will find the same deferred error that we logged in log_error().

I'm reading this as, "we log the same deferred error in *both* the
original MCA MSRs and in the new DE* ones". Correct?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1621270

From"Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Date2017-04-11 15:40 +0200
Message-ID<tv4bw-7C6-21@gated-at.bofh.it>
In reply to#1621263
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Tuesday, April 11, 2017 9:25 AM
> 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
> 
> On Tue, Apr 11, 2017 at 01:18:50PM +0000, Ghannam, Yazen wrote:
> > So log_error() reads/clears MCA_STATUS, right? This won't affect
> > MCA_DESTAT on SMCA systems. So if we call log_error_smca() and
> > unconditionally read MCA_DESTAT, we will find the same deferred error
> that we logged in log_error().
> 
> I'm reading this as, "we log the same deferred error in *both* the original
> MCA MSRs and in the new DE* ones". Correct?
> 

Yes, exactly. Deferred errors are *always* logged in the DE* registers and
they are logged in the original MSRs based on the MCA_CONFIG bit.

The idea here is that if a deferred error is overwritten in MCA_STATUS we
will still have a copy logged in MCA_DESTAT.

Thanks,
Yazen

[toc] | [prev] | [next] | [standalone]

#1621272

FromBorislav Petkov <bp@alien8.de>
Date2017-04-11 15:40 +0200
Message-ID<tv4bw-7C6-19@gated-at.bofh.it>
In reply to#1621270
On Tue, Apr 11, 2017 at 01:32:03PM +0000, Ghannam, Yazen wrote:
> Yes, exactly. Deferred errors are *always* logged in the DE* registers and

Then the solution is simple:

        for_each_bank()
                if (log_error()) {
			clear_msrs();
			continue;
		}

                if (mca_cfg.smca) {
                        log_error_from_de_regs()
			clear_msrs();
		}
	}

and clear_msrs() clears them all.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1621298

FromBorislav Petkov <bp@alien8.de>
Date2017-04-11 16:00 +0200
Message-ID<tv4uR-7Jp-11@gated-at.bofh.it>
In reply to#1621272
On Tue, Apr 11, 2017 at 01:50:02PM +0000, Ghannam, Yazen wrote:
> We need to make sure log_error() logged the deferred error before clearing
> MCA_DESTAT.

I can think of a couple of ways how to do that...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1621303

From"Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Date2017-04-11 16:00 +0200
Message-ID<tv4uR-7Jp-13@gated-at.bofh.it>
In reply to#1621272
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Tuesday, April 11, 2017 9:36 AM
> 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
> 
> On Tue, Apr 11, 2017 at 01:32:03PM +0000, Ghannam, Yazen wrote:
> > Yes, exactly. Deferred errors are *always* logged in the DE* registers and
> 
> Then the solution is simple:
> 
>         for_each_bank()
>                 if (log_error()) {
> 			clear_msrs();

We need to make sure log_error() logged the deferred error before clearing
MCA_DESTAT. We shouldn't assume that it did because we're in the #DF handler.
There's still a possibility that it was overwritten even if very rare.

> 			continue;
> 		}
> 
>                 if (mca_cfg.smca) {
>                         log_error_from_de_regs()
> 			clear_msrs();
> 		}
> 	}
> 
> and clear_msrs() clears them all.
> 

Thanks,
Yazen

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web