* [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors @ 2020-06-15 18:40 Tony Luck 2020-06-16 19:29 ` Borislav Petkov 0 siblings, 1 reply; 14+ messages in thread From: Tony Luck @ 2020-06-15 18:40 UTC (permalink / raw) To: Borislav Petkov; +Cc: Youquan Song, Tony Luck, x86, linux-kernel From: Youquan Song <youquan.song@intel.com> Skylake has a mode where the system administrator can use a BIOS setup option to request that the memory controller report uncorrected errors found by the patrol scrubber as corrected. This results in them being signalled using CMCI, which is less disruptive than a machine check. Add a quirk to detect that a "corrected" error is actually a downgraded uncorrected error with model specific checks for the "MSCOD" signature in MCi_STATUS and that the error was reported from a memory controller bank. Adjust the severity to MCE_AO_SEVERITY so that Linux will try to take the affected page offline. [Tony: Wordsmith commit comment] Signed-off-by: Youquan Song <youquan.song@intel.com> Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/mce/core.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index e9265e2f28c9..0dbd0a21a0bf 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -123,6 +123,8 @@ static struct irq_work mce_irq_work; static void (*quirk_no_way_out)(int bank, struct mce *m, struct pt_regs *regs); +static void no_adjust_mce_log(struct mce *m) {}; +static void (*adjust_mce_log)(struct mce *m) = no_adjust_mce_log; /* * CPU/chipset specific EDAC code can register a notifier call here to print * MCE errors in a human-readable form. @@ -772,6 +774,7 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b) if (mca_cfg.dont_log_ce && !mce_usable_address(&m)) goto clear_it; + adjust_mce_log(&m); mce_log(&m); clear_it: @@ -1640,6 +1643,30 @@ static void quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs) m->cs = regs->cs; } +/* + * Skylake family CPUs have a mode where the user can request that + * the memory controller report uncorrected errors found by the patrol + * scrubber as corrected (MCI_STATUS_UC == 0). This results in them being + * signalled using CMCI, which is less disruptive that a machine check. + * The following quirk detects such errors and adjusts the severity. + */ + +#define MSCOD_UCE_SCRUB (0x0010 << 16) /* UnCorrected Patrol Scrub Error */ +#define MSCOD_MASK GENMASK_ULL(31, 16) + +/* + * Check the error code to see if this is an uncorrected patrol + * scrub error from one of the memory controller banks. If so, + * then adjust the severity level to MCE_AO_SEVERITY + */ +static void quirk_skx_adjust_mce_log(struct mce *m) +{ + if (((m->status & MCACOD_SCRUBMSK) == MCACOD_SCRUB) && + ((m->status & MSCOD_MASK) == MSCOD_UCE_SCRUB) && + m->bank >= 13 && m->bank <= 18) + m->severity = MCE_AO_SEVERITY; +} + /* Add per CPU specific workarounds here */ static int __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c) { @@ -1714,6 +1741,9 @@ static int __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c) if (c->x86 == 6 && c->x86_model == 45) quirk_no_way_out = quirk_sandybridge_ifu; + + if (c->x86 == 6 && c->x86_model == INTEL_FAM6_SKYLAKE_X) + adjust_mce_log = quirk_skx_adjust_mce_log; } if (c->x86_vendor == X86_VENDOR_ZHAOXIN) { -- 2.21.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-06-15 18:40 [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck @ 2020-06-16 19:29 ` Borislav Petkov 2020-06-16 22:33 ` Luck, Tony 0 siblings, 1 reply; 14+ messages in thread From: Borislav Petkov @ 2020-06-16 19:29 UTC (permalink / raw) To: Tony Luck; +Cc: Youquan Song, x86, linux-kernel On Mon, Jun 15, 2020 at 11:40:56AM -0700, Tony Luck wrote: > From: Youquan Song <youquan.song@intel.com> > > Skylake has a mode where the system administrator can use a BIOS setup > option to request that the memory controller report uncorrected errors > found by the patrol scrubber as corrected. This results in them being > signalled using CMCI, which is less disruptive than a machine check. > > Add a quirk to detect that a "corrected" error is actually a downgraded > uncorrected error with model specific checks for the "MSCOD" signature in > MCi_STATUS and that the error was reported from a memory controller bank. > > Adjust the severity to MCE_AO_SEVERITY so that Linux will try to take > the affected page offline. > > [Tony: Wordsmith commit comment] > > Signed-off-by: Youquan Song <youquan.song@intel.com> > Signed-off-by: Tony Luck <tony.luck@intel.com> > --- > arch/x86/kernel/cpu/mce/core.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c > index e9265e2f28c9..0dbd0a21a0bf 100644 > --- a/arch/x86/kernel/cpu/mce/core.c > +++ b/arch/x86/kernel/cpu/mce/core.c > @@ -123,6 +123,8 @@ static struct irq_work mce_irq_work; > > static void (*quirk_no_way_out)(int bank, struct mce *m, struct pt_regs *regs); > > +static void no_adjust_mce_log(struct mce *m) {}; > +static void (*adjust_mce_log)(struct mce *m) = no_adjust_mce_log; > /* > * CPU/chipset specific EDAC code can register a notifier call here to print > * MCE errors in a human-readable form. > @@ -772,6 +774,7 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b) > if (mca_cfg.dont_log_ce && !mce_usable_address(&m)) > goto clear_it; > > + adjust_mce_log(&m); > mce_log(&m); Two things: can that error type be detected when #MC gets raised, i.e., in do_machine_check() as part of scanning all banks? If so, then the adjusting needs to happen inside mce_log(). Also, that assignment to the function pointer doesn't make much sense to me and I think you should do the vendor/family/model checking straight in a function adjust_mce_log() which gets called by whoever... Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-06-16 19:29 ` Borislav Petkov @ 2020-06-16 22:33 ` Luck, Tony 2020-06-17 7:41 ` Borislav Petkov 0 siblings, 1 reply; 14+ messages in thread From: Luck, Tony @ 2020-06-16 22:33 UTC (permalink / raw) To: Borislav Petkov; +Cc: Song, Youquan, x86, linux-kernel > Two things: can that error type be detected when #MC gets raised, i.e., in > do_machine_check() as part of scanning all banks? If the BIOS option is left in the default setting, uncorrectable errors found by the patrol scrubber are reported with a machine check. Those MSCOD and MCACOD signatures are the same ... but that's not important because MCi_STATUS.UC==1. So Linux doesn't need to jump through hoops to "upgrade" the severity. > If so, then the adjusting needs to happen inside mce_log(). So no, this adjust only needs to happen when polling the banks from CMCI or periodic timer. > Also, that assignment to the function pointer doesn't make much sense to > me and I think you should do the vendor/family/model checking straight > in a function adjust_mce_log() which gets called by whoever... The point was to avoid the runtime test for CPU model on every error. But this isn't a performance critical path, so we can refactor if you think that looks cleaner. There is some new set of validation tests running now to check the effectiveness of this BIOS + OS change. So it may be a while before updated version is posted. Thanks -Tony ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-06-16 22:33 ` Luck, Tony @ 2020-06-17 7:41 ` Borislav Petkov 2020-06-17 18:49 ` Luck, Tony 0 siblings, 1 reply; 14+ messages in thread From: Borislav Petkov @ 2020-06-17 7:41 UTC (permalink / raw) To: Luck, Tony; +Cc: Song, Youquan, x86, linux-kernel On Tue, Jun 16, 2020 at 10:33:08PM +0000, Luck, Tony wrote: > If the BIOS option is left in the default setting, uncorrectable errors found > by the patrol scrubber are reported with a machine check. Those MSCOD > and MCACOD signatures are the same ... but that's not important because > MCi_STATUS.UC==1. So Linux doesn't need to jump through hoops to > "upgrade" the severity. No, this is not what I meant: I meant when you have the setting enabled to downgrade those errors, can they be detected as part of another MCE being raised... > > If so, then the adjusting needs to happen inside mce_log(). > So no, this adjust only needs to happen when polling the banks from > CMCI or periodic timer. ... but since those downgraded errors raise CMCI then the answer to my question is no. > The point was to avoid the runtime test for CPU model on every error. But > this isn't a performance critical path, so we can refactor if you think that > looks cleaner. Yes please. > There is some new set of validation tests running now to check the effectiveness > of this BIOS + OS change. So it may be a while before updated version is > posted. Ok, thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-06-17 7:41 ` Borislav Petkov @ 2020-06-17 18:49 ` Luck, Tony 2020-08-28 20:21 ` [PATCH v2] " Luck, Tony 0 siblings, 1 reply; 14+ messages in thread From: Luck, Tony @ 2020-06-17 18:49 UTC (permalink / raw) To: Borislav Petkov; +Cc: Song, Youquan, x86, linux-kernel On Wed, Jun 17, 2020 at 09:41:58AM +0200, Borislav Petkov wrote: > On Tue, Jun 16, 2020 at 10:33:08PM +0000, Luck, Tony wrote: > > If the BIOS option is left in the default setting, uncorrectable errors found > > by the patrol scrubber are reported with a machine check. Those MSCOD > > and MCACOD signatures are the same ... but that's not important because > > MCi_STATUS.UC==1. So Linux doesn't need to jump through hoops to > > "upgrade" the severity. > > No, this is not what I meant: I meant when you have the setting enabled > to downgrade those errors, can they be detected as part of another MCE > being raised... Yes one of these downgraded error logs could be sitting in a bank whe a different machine check fires. But the severity calculation will ignore it as MCi_STATUS.EN==0 -Tony ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-06-17 18:49 ` Luck, Tony @ 2020-08-28 20:21 ` Luck, Tony 2020-09-25 19:19 ` Borislav Petkov 0 siblings, 1 reply; 14+ messages in thread From: Luck, Tony @ 2020-08-28 20:21 UTC (permalink / raw) To: Borislav Petkov; +Cc: Song, Youquan, x86, linux-kernel From: Youquan Song <youquan.song@intel.com> Skylake has a mode where the system administrator can use a BIOS setup option to request that the memory controller report uncorrected errors found by the patrol scrubber as corrected. This results in them being signalled using CMCI, which is less disruptive than a machine check. Add a quirk to detect that a "corrected" error is actually a downgraded uncorrected error with model specific checks for the "MSCOD" signature in MCi_STATUS and that the error was reported from a memory controller bank. Adjust the severity to MCE_AO_SEVERITY so that Linux will try to take the affected page offline. [Tony: Wordsmith commit comment] Signed-off-by: Youquan Song <youquan.song@intel.com> Signed-off-by: Tony Luck <tony.luck@intel.com> --- This updated patch slipped through some cracks. Sorry for the delay. V2: Boris: Don't optimize with pointer to quirk function. Just do the vendor/family/model check in the adjust_mce_log() function Tony: Add check for stepping >= 4 --- arch/x86/kernel/cpu/mce/core.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index e9265e2f28c9..2d5fe23adf29 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -673,6 +673,35 @@ static void mce_read_aux(struct mce *m, int i) } } +/* + * Skylake family CPUs have a mode where the user can request that + * the memory controller report uncorrected errors found by the patrol + * scrubber as corrected (MCI_STATUS_UC == 0). This results in them being + * signalled using CMCI, which is less disruptive that a machine check. + * The following quirk detects such errors and adjusts the severity. + */ + +#define MSCOD_UCE_SCRUB (0x0010 << 16) /* UnCorrected Patrol Scrub Error */ +#define MSCOD_MASK GENMASK_ULL(31, 16) + +static void adjust_mce_log(struct mce *m) +{ + struct cpuinfo_x86 *c = &boot_cpu_data; + + if (c->x86_vendor == X86_VENDOR_INTEL && c->x86 == 6 && + c->x86_model == INTEL_FAM6_SKYLAKE_X && c->x86_stepping >= 4) { + /* + * Check the error code to see if this is an uncorrected patrol + * scrub error from one of the memory controller banks. If so, + * then adjust the severity level to MCE_AO_SEVERITY + */ + if (((m->status & MCACOD_SCRUBMSK) == MCACOD_SCRUB) && + ((m->status & MSCOD_MASK) == MSCOD_UCE_SCRUB) && + m->bank >= 13 && m->bank <= 18) + m->severity = MCE_AO_SEVERITY; + } +} + DEFINE_PER_CPU(unsigned, mce_poll_count); /* @@ -772,6 +801,7 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b) if (mca_cfg.dont_log_ce && !mce_usable_address(&m)) goto clear_it; + adjust_mce_log(&m); mce_log(&m); clear_it: -- 2.21.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-08-28 20:21 ` [PATCH v2] " Luck, Tony @ 2020-09-25 19:19 ` Borislav Petkov 2020-09-25 23:06 ` Luck, Tony 0 siblings, 1 reply; 14+ messages in thread From: Borislav Petkov @ 2020-09-25 19:19 UTC (permalink / raw) To: Luck, Tony; +Cc: Song, Youquan, x86, linux-kernel On Fri, Aug 28, 2020 at 01:21:50PM -0700, Luck, Tony wrote: > +static void adjust_mce_log(struct mce *m) > +{ > + struct cpuinfo_x86 *c = &boot_cpu_data; > + > + if (c->x86_vendor == X86_VENDOR_INTEL && c->x86 == 6 && > + c->x86_model == INTEL_FAM6_SKYLAKE_X && c->x86_stepping >= 4) { > + /* > + * Check the error code to see if this is an uncorrected patrol > + * scrub error from one of the memory controller banks. If so, > + * then adjust the severity level to MCE_AO_SEVERITY > + */ > + if (((m->status & MCACOD_SCRUBMSK) == MCACOD_SCRUB) && > + ((m->status & MSCOD_MASK) == MSCOD_UCE_SCRUB) && > + m->bank >= 13 && m->bank <= 18) > + m->severity = MCE_AO_SEVERITY; > + } > +} > + > DEFINE_PER_CPU(unsigned, mce_poll_count); > > /* > @@ -772,6 +801,7 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b) > if (mca_cfg.dont_log_ce && !mce_usable_address(&m)) > goto clear_it; > > + adjust_mce_log(&m); > mce_log(&m); Coming back to this and looking at it, I can't say that I like it. We're sticking hooks to look at and massage the logged data everywhere on the MCE processing path and it is getting really unwieldy. And after staring at this a bit, it looks like all it wants to do is to adjust the severity. And we have a severity grading mechanism. So let's see how ugly it would become if we extended it to check that too. So how's that below instead? It builds here, I haven't even thought about testing it and I might've missed out on some aspects but tbh this looks much better to me. Because it is not bolted on the handling path but integral part of it. Thoughts? --- diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index e1da619add19..8c1a41aa5e40 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -9,9 +9,11 @@ #include <linux/seq_file.h> #include <linux/init.h> #include <linux/debugfs.h> -#include <asm/mce.h> #include <linux/uaccess.h> +#include <asm/mce.h> +#include <asm/intel-family.h> + #include "internal.h" /* @@ -40,9 +42,14 @@ static struct severity { unsigned char context; unsigned char excp; unsigned char covered; + unsigned char cpu_model; + unsigned char cpu_stepping; + unsigned char bank_lo, bank_hi; char *msg; } severities[] = { #define MCESEV(s, m, c...) { .sev = MCE_ ## s ## _SEVERITY, .msg = m, ## c } +#define BANK_RANGE(l, h) .bank_lo = l, .bank_hi = h +#define MODEL_STEPPING(m,s) .cpu_model = m, .cpu_stepping = s #define KERNEL .context = IN_KERNEL #define USER .context = IN_USER #define KERNEL_RECOV .context = IN_KERNEL_RECOV @@ -97,7 +104,10 @@ static struct severity { KEEP, "Corrected error", NOSER, BITCLR(MCI_STATUS_UC) ), - + MCESEV(AO, "UnCorrected Patrol Scrub Error", + NOSER, MASK(0xffffeff0, 0x001000c0), + MODEL_STEPPING(INTEL_FAM6_SKYLAKE_X, 4),BANK_RANGE(13,18) + ), /* * known AO MCACODs reported via MCE or CMC: * @@ -324,6 +334,12 @@ static int mce_severity_intel(struct mce *m, int tolerant, char **msg, bool is_e continue; if (s->excp && excp != s->excp) continue; + if (s->cpu_model && boot_cpu_data.x86_model != s->cpu_model) + continue; + if (s->cpu_stepping && boot_cpu_data.x86_stepping <= s->cpu_stepping) + continue; + if (s->bank_lo && (s->bank_lo <= m->bank && m->bank <= s->bank_hi)) + continue; if (msg) *msg = s->msg; s->covered = 1; -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-09-25 19:19 ` Borislav Petkov @ 2020-09-25 23:06 ` Luck, Tony 2020-09-27 22:19 ` Borislav Petkov 0 siblings, 1 reply; 14+ messages in thread From: Luck, Tony @ 2020-09-25 23:06 UTC (permalink / raw) To: Borislav Petkov; +Cc: Song, Youquan, x86, linux-kernel On Fri, Sep 25, 2020 at 09:19:12PM +0200, Borislav Petkov wrote: > And after staring at this a bit, it looks like all it wants to do is to > adjust the severity. And we have a severity grading mechanism. So let's > see how ugly it would become if we extended it to check that too. > > So how's that below instead? In some ways that's pretty neat. But it would still be ugly if we need to extend it further for other issues. Especially if they don't have such a simple rule to adjust the severity. > It builds here, I haven't even thought about testing it and I might've > missed out on some aspects but tbh this looks much better to me. Because > it is not bolted on the handling path but integral part of it. > > Thoughts? > > --- > diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c > index e1da619add19..8c1a41aa5e40 100644 > --- a/arch/x86/kernel/cpu/mce/severity.c > +++ b/arch/x86/kernel/cpu/mce/severity.c > @@ -9,9 +9,11 @@ > #include <linux/seq_file.h> > #include <linux/init.h> > #include <linux/debugfs.h> > -#include <asm/mce.h> > #include <linux/uaccess.h> > > +#include <asm/mce.h> > +#include <asm/intel-family.h> > + > #include "internal.h" > > /* > @@ -40,9 +42,14 @@ static struct severity { > unsigned char context; > unsigned char excp; > unsigned char covered; > + unsigned char cpu_model; > + unsigned char cpu_stepping; > + unsigned char bank_lo, bank_hi; This would be better as a bit mask. I don't think we need this same hack on the next generation of CPUs ... but if we did, the bank numbers that would be affected don't form a continuous sequence. > char *msg; > } severities[] = { > #define MCESEV(s, m, c...) { .sev = MCE_ ## s ## _SEVERITY, .msg = m, ## c } > +#define BANK_RANGE(l, h) .bank_lo = l, .bank_hi = h > +#define MODEL_STEPPING(m,s) .cpu_model = m, .cpu_stepping = s > #define KERNEL .context = IN_KERNEL > #define USER .context = IN_USER > #define KERNEL_RECOV .context = IN_KERNEL_RECOV > @@ -97,7 +104,10 @@ static struct severity { > KEEP, "Corrected error", > NOSER, BITCLR(MCI_STATUS_UC) > ), > - > + MCESEV(AO, "UnCorrected Patrol Scrub Error", > + NOSER, MASK(0xffffeff0, 0x001000c0), > + MODEL_STEPPING(INTEL_FAM6_SKYLAKE_X, 4),BANK_RANGE(13,18) > + ), I'd need to stare at the placement of this in the sequence of rules at some non-Friday-afternoon time. It might be right, but as we've grumbled together many times before that code is full of surprise side effects. > /* > * known AO MCACODs reported via MCE or CMC: > * > @@ -324,6 +334,12 @@ static int mce_severity_intel(struct mce *m, int tolerant, char **msg, bool is_e > continue; > if (s->excp && excp != s->excp) > continue; > + if (s->cpu_model && boot_cpu_data.x86_model != s->cpu_model) > + continue; > + if (s->cpu_stepping && boot_cpu_data.x86_stepping <= s->cpu_stepping) > + continue; > + if (s->bank_lo && (s->bank_lo <= m->bank && m->bank <= s->bank_hi)) > + continue; > if (msg) > *msg = s->msg; > s->covered = 1; -Tony ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-09-25 23:06 ` Luck, Tony @ 2020-09-27 22:19 ` Borislav Petkov 2020-09-30 2:13 ` [PATCH 0/2] mce severity quirk & cleanup Tony Luck 0 siblings, 1 reply; 14+ messages in thread From: Borislav Petkov @ 2020-09-27 22:19 UTC (permalink / raw) To: Luck, Tony; +Cc: Song, Youquan, x86, linux-kernel On Fri, Sep 25, 2020 at 04:06:20PM -0700, Luck, Tony wrote: > In some ways that's pretty neat. But it would still be ugly if we need > to extend it further for other issues. Especially if they don't have such > a simple rule to adjust the severity. I'm still secretly hoping that one fine day one of your minions will come with a conversion series around the corner; series, which gets rid of this unreadable mess and we get something *actually* readable like mce_severity_amd(). And on that day I'll even take one patch per severities[] member so that it is absolutely clear how the conversion has happened and review can be good and catch any lurking errors. One day... :-) Because after that day, we can do arbitrary rules to adjust the severity and there won't be any problems with extending it anymore because then it'll be nice and flexible C code only. > This would be better as a bit mask. I don't think we need this same > hack on the next generation of CPUs ... but if we did, the bank numbers > that would be affected don't form a continuous sequence. Ok, feel free to adjust it how you think it is better. > I'd need to stare at the placement of this in the sequence of rules at some > non-Friday-afternoon time. It might be right, but as we've grumbled together > many times before that code is full of surprise side effects. Nah, I just put it there so that I can see the macros in the same window and don't have to scroll - I'm purely relying on you here to place it in the right spot. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] mce severity quirk & cleanup 2020-09-27 22:19 ` Borislav Petkov @ 2020-09-30 2:13 ` Tony Luck 2020-09-30 2:13 ` [PATCH 1/2] x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck 2020-09-30 2:13 ` [PATCH 2/2] x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list Tony Luck 0 siblings, 2 replies; 14+ messages in thread From: Tony Luck @ 2020-09-30 2:13 UTC (permalink / raw) To: Borislav Petkov; +Cc: Tony Luck, Youquan Song, linux-kernel Part 0001 is a cleaned up version of a suggested patch from Boris. Changes: Moved the new rule after the "standard" action optional rules [The old location was OK, but this feels better] Added comment explaining what the new rule does Make formatting match existing style Add some spaces after commas the appease checkpatch Fix logic to match bank number range Change cpu_stepping to cpu_minstepping Change logic so rule applies to any stepping >= cpu_minstepping Dropped CamelCase from string (s/"UnCorrected/Uncorrected/) But those were all minor so I left Boris as Author. Which means the checkpatch hates this because it doesn't have a Signed-off-by: Boris. Part 0002 is just a cleanup I noticed while reading through the rules to decide where to put the new one. This AMD rule has been hiding in plain sight for years. Drop it. Borislav Petkov (1): x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck (1): x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list arch/x86/kernel/cpu/mce/severity.c | 32 ++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) -- 2.21.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-09-30 2:13 ` [PATCH 0/2] mce severity quirk & cleanup Tony Luck @ 2020-09-30 2:13 ` Tony Luck 2020-09-30 5:53 ` [tip: ras/core] " tip-bot2 for Borislav Petkov 2020-09-30 2:13 ` [PATCH 2/2] x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list Tony Luck 1 sibling, 1 reply; 14+ messages in thread From: Tony Luck @ 2020-09-30 2:13 UTC (permalink / raw) To: Borislav Petkov; +Cc: Youquan Song, Tony Luck, linux-kernel From: Borislav Petkov <bp@alien8.de> The patrol scrubber in Skylake and Cascade Lake systems can be configured to report uncorrected errors using a special signature in the machine check bank and to signal using CMCI instead of machine check. Update the severity calculation mechanism to allow specifying the model, minimum stepping and range of machine check bank numbers. Add a new rule to detect the special signature (on model 0x55, stepping >=4 in any of the memory controller banks). Suggested-by: Youquan Song <youquan.song@intel.com> Rewritten-by: Borislav Petkov <bp@alien8.de> Co-developed-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/mce/severity.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index e1da619add19..567ce09a0286 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -9,9 +9,11 @@ #include <linux/seq_file.h> #include <linux/init.h> #include <linux/debugfs.h> -#include <asm/mce.h> #include <linux/uaccess.h> +#include <asm/mce.h> +#include <asm/intel-family.h> + #include "internal.h" /* @@ -40,9 +42,14 @@ static struct severity { unsigned char context; unsigned char excp; unsigned char covered; + unsigned char cpu_model; + unsigned char cpu_minstepping; + unsigned char bank_lo, bank_hi; char *msg; } severities[] = { #define MCESEV(s, m, c...) { .sev = MCE_ ## s ## _SEVERITY, .msg = m, ## c } +#define BANK_RANGE(l, h) .bank_lo = l, .bank_hi = h +#define MODEL_STEPPING(m, s) .cpu_model = m, .cpu_minstepping = s #define KERNEL .context = IN_KERNEL #define USER .context = IN_USER #define KERNEL_RECOV .context = IN_KERNEL_RECOV @@ -97,7 +104,6 @@ static struct severity { KEEP, "Corrected error", NOSER, BITCLR(MCI_STATUS_UC) ), - /* * known AO MCACODs reported via MCE or CMC: * @@ -113,6 +119,18 @@ static struct severity { AO, "Action optional: last level cache writeback error", SER, MASK(MCI_UC_AR|MCACOD, MCI_STATUS_UC|MCACOD_L3WB) ), + /* + * Quirk for Skylake/Cascade Lake. Patrol scrubber may be configured + * to report uncorrected errors using CMCI with a special signature. + * UC=0, MSCOD=0x0010, MCACOD=binary(000X 0000 1100 XXXX) reported + * in one of the memory controller banks. + * Set severity to "AO" for same action as normal patrol scrub error. + */ + MCESEV( + AO, "Uncorrected Patrol Scrub Error", + SER, MASK(MCI_STATUS_UC|MCI_ADDR|0xffffeff0, MCI_ADDR|0x001000c0), + MODEL_STEPPING(INTEL_FAM6_SKYLAKE_X, 4), BANK_RANGE(13, 18) + ), /* ignore OVER for UCNA */ MCESEV( @@ -324,6 +342,12 @@ static int mce_severity_intel(struct mce *m, int tolerant, char **msg, bool is_e continue; if (s->excp && excp != s->excp) continue; + if (s->cpu_model && boot_cpu_data.x86_model != s->cpu_model) + continue; + if (s->cpu_minstepping && boot_cpu_data.x86_stepping < s->cpu_minstepping) + continue; + if (s->bank_lo && (m->bank < s->bank_lo || m->bank > s->bank_hi)) + continue; if (msg) *msg = s->msg; s->covered = 1; -- 2.21.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [tip: ras/core] x86/mce: Add Skylake quirk for patrol scrub reported errors 2020-09-30 2:13 ` [PATCH 1/2] x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck @ 2020-09-30 5:53 ` tip-bot2 for Borislav Petkov 0 siblings, 0 replies; 14+ messages in thread From: tip-bot2 for Borislav Petkov @ 2020-09-30 5:53 UTC (permalink / raw) To: linux-tip-commits; +Cc: Youquan Song, Borislav Petkov, Tony Luck, x86, LKML The following commit has been merged into the ras/core branch of tip: Commit-ID: fd258dc4442c5c1c069c6b5b42bfe7d10cddda95 Gitweb: https://git.kernel.org/tip/fd258dc4442c5c1c069c6b5b42bfe7d10cddda95 Author: Borislav Petkov <bp@suse.de> AuthorDate: Tue, 29 Sep 2020 19:13:12 -07:00 Committer: Borislav Petkov <bp@suse.de> CommitterDate: Wed, 30 Sep 2020 07:43:56 +02:00 x86/mce: Add Skylake quirk for patrol scrub reported errors The patrol scrubber in Skylake and Cascade Lake systems can be configured to report uncorrected errors using a special signature in the machine check bank and to signal using CMCI instead of machine check. Update the severity calculation mechanism to allow specifying the model, minimum stepping and range of machine check bank numbers. Add a new rule to detect the special signature (on model 0x55, stepping >=4 in any of the memory controller banks). [ bp: Rewrite it. aegl: Productize it. ] Suggested-by: Youquan Song <youquan.song@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Co-developed-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Link: https://lkml.kernel.org/r/20200930021313.31810-2-tony.luck@intel.com --- arch/x86/kernel/cpu/mce/severity.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index e1da619..567ce09 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -9,9 +9,11 @@ #include <linux/seq_file.h> #include <linux/init.h> #include <linux/debugfs.h> -#include <asm/mce.h> #include <linux/uaccess.h> +#include <asm/mce.h> +#include <asm/intel-family.h> + #include "internal.h" /* @@ -40,9 +42,14 @@ static struct severity { unsigned char context; unsigned char excp; unsigned char covered; + unsigned char cpu_model; + unsigned char cpu_minstepping; + unsigned char bank_lo, bank_hi; char *msg; } severities[] = { #define MCESEV(s, m, c...) { .sev = MCE_ ## s ## _SEVERITY, .msg = m, ## c } +#define BANK_RANGE(l, h) .bank_lo = l, .bank_hi = h +#define MODEL_STEPPING(m, s) .cpu_model = m, .cpu_minstepping = s #define KERNEL .context = IN_KERNEL #define USER .context = IN_USER #define KERNEL_RECOV .context = IN_KERNEL_RECOV @@ -97,7 +104,6 @@ static struct severity { KEEP, "Corrected error", NOSER, BITCLR(MCI_STATUS_UC) ), - /* * known AO MCACODs reported via MCE or CMC: * @@ -113,6 +119,18 @@ static struct severity { AO, "Action optional: last level cache writeback error", SER, MASK(MCI_UC_AR|MCACOD, MCI_STATUS_UC|MCACOD_L3WB) ), + /* + * Quirk for Skylake/Cascade Lake. Patrol scrubber may be configured + * to report uncorrected errors using CMCI with a special signature. + * UC=0, MSCOD=0x0010, MCACOD=binary(000X 0000 1100 XXXX) reported + * in one of the memory controller banks. + * Set severity to "AO" for same action as normal patrol scrub error. + */ + MCESEV( + AO, "Uncorrected Patrol Scrub Error", + SER, MASK(MCI_STATUS_UC|MCI_ADDR|0xffffeff0, MCI_ADDR|0x001000c0), + MODEL_STEPPING(INTEL_FAM6_SKYLAKE_X, 4), BANK_RANGE(13, 18) + ), /* ignore OVER for UCNA */ MCESEV( @@ -324,6 +342,12 @@ static int mce_severity_intel(struct mce *m, int tolerant, char **msg, bool is_e continue; if (s->excp && excp != s->excp) continue; + if (s->cpu_model && boot_cpu_data.x86_model != s->cpu_model) + continue; + if (s->cpu_minstepping && boot_cpu_data.x86_stepping < s->cpu_minstepping) + continue; + if (s->bank_lo && (m->bank < s->bank_lo || m->bank > s->bank_hi)) + continue; if (msg) *msg = s->msg; s->covered = 1; ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list 2020-09-30 2:13 ` [PATCH 0/2] mce severity quirk & cleanup Tony Luck 2020-09-30 2:13 ` [PATCH 1/2] x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck @ 2020-09-30 2:13 ` Tony Luck 2020-09-30 5:53 ` [tip: ras/core] x86/mce: Drop AMD-specific " tip-bot2 for Tony Luck 1 sibling, 1 reply; 14+ messages in thread From: Tony Luck @ 2020-09-30 2:13 UTC (permalink / raw) To: Borislav Petkov; +Cc: Tony Luck, Youquan Song, linux-kernel Way back in v3.19 Intel and AMD shared the same machine check severity grading code. So it made sense to add a case for AMD DEFERRED errors in commit e3480271f592 ("x86, mce, severity: Extend the the mce_severity mechanism to handle UCNA/DEFERRED error") But later in v4.2 AMD switched to a separate grading function in commit bf80bbd7dcf5 ("x86/mce: Add an AMD severities-grading function") Belatedly drop the DEFERRED case from the Intel rule list. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/mce/severity.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index 567ce09a0286..e0722461bb57 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -96,10 +96,6 @@ static struct severity { PANIC, "In kernel and no restart IP", EXCP, KERNEL_RECOV, MCGMASK(MCG_STATUS_RIPV, 0) ), - MCESEV( - DEFERRED, "Deferred error", - NOSER, MASK(MCI_STATUS_UC|MCI_STATUS_DEFERRED|MCI_STATUS_POISON, MCI_STATUS_DEFERRED) - ), MCESEV( KEEP, "Corrected error", NOSER, BITCLR(MCI_STATUS_UC) -- 2.21.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [tip: ras/core] x86/mce: Drop AMD-specific "DEFERRED" case from Intel severity rule list 2020-09-30 2:13 ` [PATCH 2/2] x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list Tony Luck @ 2020-09-30 5:53 ` tip-bot2 for Tony Luck 0 siblings, 0 replies; 14+ messages in thread From: tip-bot2 for Tony Luck @ 2020-09-30 5:53 UTC (permalink / raw) To: linux-tip-commits; +Cc: Tony Luck, Borislav Petkov, x86, LKML The following commit has been merged into the ras/core branch of tip: Commit-ID: ed9705e4ad1c19ae51ed0cb4c112f9eb6dfc69fc Gitweb: https://git.kernel.org/tip/ed9705e4ad1c19ae51ed0cb4c112f9eb6dfc69fc Author: Tony Luck <tony.luck@intel.com> AuthorDate: Tue, 29 Sep 2020 19:13:13 -07:00 Committer: Borislav Petkov <bp@suse.de> CommitterDate: Wed, 30 Sep 2020 07:49:58 +02:00 x86/mce: Drop AMD-specific "DEFERRED" case from Intel severity rule list Way back in v3.19 Intel and AMD shared the same machine check severity grading code. So it made sense to add a case for AMD DEFERRED errors in commit e3480271f592 ("x86, mce, severity: Extend the the mce_severity mechanism to handle UCNA/DEFERRED error") But later in v4.2 AMD switched to a separate grading function in commit bf80bbd7dcf5 ("x86/mce: Add an AMD severities-grading function") Belatedly drop the DEFERRED case from the Intel rule list. Signed-off-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Link: https://lkml.kernel.org/r/20200930021313.31810-3-tony.luck@intel.com --- arch/x86/kernel/cpu/mce/severity.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c index 567ce09..e072246 100644 --- a/arch/x86/kernel/cpu/mce/severity.c +++ b/arch/x86/kernel/cpu/mce/severity.c @@ -97,10 +97,6 @@ static struct severity { EXCP, KERNEL_RECOV, MCGMASK(MCG_STATUS_RIPV, 0) ), MCESEV( - DEFERRED, "Deferred error", - NOSER, MASK(MCI_STATUS_UC|MCI_STATUS_DEFERRED|MCI_STATUS_POISON, MCI_STATUS_DEFERRED) - ), - MCESEV( KEEP, "Corrected error", NOSER, BITCLR(MCI_STATUS_UC) ), ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-09-30 5:54 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-15 18:40 [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck 2020-06-16 19:29 ` Borislav Petkov 2020-06-16 22:33 ` Luck, Tony 2020-06-17 7:41 ` Borislav Petkov 2020-06-17 18:49 ` Luck, Tony 2020-08-28 20:21 ` [PATCH v2] " Luck, Tony 2020-09-25 19:19 ` Borislav Petkov 2020-09-25 23:06 ` Luck, Tony 2020-09-27 22:19 ` Borislav Petkov 2020-09-30 2:13 ` [PATCH 0/2] mce severity quirk & cleanup Tony Luck 2020-09-30 2:13 ` [PATCH 1/2] x86/mce: Add Skylake quirk for patrol scrub reported errors Tony Luck 2020-09-30 5:53 ` [tip: ras/core] " tip-bot2 for Borislav Petkov 2020-09-30 2:13 ` [PATCH 2/2] x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list Tony Luck 2020-09-30 5:53 ` [tip: ras/core] x86/mce: Drop AMD-specific " tip-bot2 for Tony Luck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.