[PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors

[PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Tony Luck]

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

Re: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Borislav Petkov]

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

RE: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Luck, Tony]

> 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

Re: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Borislav Petkov]

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

Re: [PATCH] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Luck, Tony]

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

Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Luck, Tony]

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

Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Borislav Petkov]

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

Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Luck, Tony]

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

Re: [PATCH v2] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Borislav Petkov]

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

[PATCH 0/2] mce severity quirk & cleanup

[Tony Luck]

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

[PATCH 1/2] x86/mce: Add Skylake quirk for patrol scrub reported errors

[Tony Luck]

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

[PATCH 2/2] x86/mce: Drop AMD specific "DEFERRED" case from Intel severity rule list

[Tony Luck]

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

[tip: ras/core] x86/mce: Add Skylake quirk for patrol scrub reported errors

[tip-bot2 for Borislav Petkov]

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;

[tip: ras/core] x86/mce: Drop AMD-specific "DEFERRED" case from Intel severity rule list

[tip-bot2 for Tony Luck]

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)
 		),