[PATCH 0/2] x86, MCE: Catch the early worm

[PATCH 0/2] x86, MCE: Catch the early worm

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Currently, we don't decode MCEs which are being detected during MCA
initialization and before registration of the decoders on the decoding
notifier chain. These two patches add the functionality to drain the
mcelog buffer right after we've registered a decoder.

With it, early MCEs get caught and decoded into the dmesg:

...
[    0.068003] ... generic registers:      6
[    0.068003] ... value mask:             0000ffffffffffff
[    0.068003] ... max period:             00007fffffffffff
[    0.068003] ... fixed-purpose events:   0
[    0.068003] ... event mask:             000000000000003f
[    0.068003] MCE: In-kernel MCE decoding enabled.
[    0.068003] [Hardware Error]: CPU:0  MC0_STATUS[-|UE|MiscV|-|AddrV|-|-]: 0xac80000000000833
[    0.068003] [Hardware Error]:        MC0_ADDR: 0x00000000f8080000
[    0.068003] [Hardware Error]: Data Cache Error: System Read Data Error.
[    0.068003] [Hardware Error]: cache level: L3/GEN, mem/io: MEM, mem-tx: DRD, part-proc: SRC (no timeout)
[    0.068069] Booting Node   0, Processors  #1
[    0.068276] smpboot cpu 1: start_ip = 88000
[    0.160077]  #2
[    0.160206] smpboot cpu 2: start_ip = 88000
[    0.252073]  #3 Ok.
...

Comments and suggestions are welcome, as always.

Thanks.

[PATCH 1/2] x86, mce: Add wrappers for registering on the decode chain

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

No functionality change, this is done so that in a follow-on patch all
queued-up MCEs can be decoded after registering on the chain.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/include/asm/mce.h       |    3 ++-
 arch/x86/kernel/cpu/mcheck/mce.c |   25 ++++++++++++++++++-------
 drivers/edac/i7core_edac.c       |    4 ++--
 drivers/edac/mce_amd.c           |    4 ++--
 drivers/edac/sb_edac.c           |    6 ++----
 5 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 0e8ae57..b7c47a4 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -120,7 +120,8 @@ struct mce_log {
 
 #ifdef __KERNEL__
 
-extern struct atomic_notifier_head x86_mce_decoder_chain;
+extern void mce_register_decode_chain(struct notifier_block *nb);
+extern void mce_unregister_decode_chain(struct notifier_block *nb);
 
 #include <linux/percpu.h>
 #include <linux/init.h>
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 2af127d..c3c66ac 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -95,13 +95,6 @@ static DECLARE_WAIT_QUEUE_HEAD(mce_chrdev_wait);
 static DEFINE_PER_CPU(struct mce, mces_seen);
 static int			cpu_missing;
 
-/*
- * CPU/chipset specific EDAC code can register a notifier call here to print
- * MCE errors in a human-readable form.
- */
-ATOMIC_NOTIFIER_HEAD(x86_mce_decoder_chain);
-EXPORT_SYMBOL_GPL(x86_mce_decoder_chain);
-
 /* MCA banks polled by the period polling timer for corrected events */
 DEFINE_PER_CPU(mce_banks_t, mce_poll_banks) = {
 	[0 ... BITS_TO_LONGS(MAX_NR_BANKS)-1] = ~0UL
@@ -109,6 +102,12 @@ DEFINE_PER_CPU(mce_banks_t, mce_poll_banks) = {
 
 static DEFINE_PER_CPU(struct work_struct, mce_work);
 
+/*
+ * CPU/chipset specific EDAC code can register a notifier call here to print
+ * MCE errors in a human-readable form.
+ */
+ATOMIC_NOTIFIER_HEAD(x86_mce_decoder_chain);
+
 /* Do initial initialization of a struct mce */
 void mce_setup(struct mce *m)
 {
@@ -190,6 +189,18 @@ void mce_log(struct mce *mce)
 	set_bit(0, &mce_need_notify);
 }
 
+void mce_register_decode_chain(struct notifier_block *nb)
+{
+	atomic_notifier_chain_register(&x86_mce_decoder_chain, nb);
+}
+EXPORT_SYMBOL_GPL(mce_register_decode_chain);
+
+void mce_unregister_decode_chain(struct notifier_block *nb)
+{
+	atomic_notifier_chain_unregister(&x86_mce_decoder_chain, nb);
+}
+EXPORT_SYMBOL_GPL(mce_unregister_decode_chain);
+
 static void print_mce(struct mce *m)
 {
 	int ret = 0;
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 70ad892..8568d9b 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -2234,7 +2234,7 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
 	if (pvt->enable_scrub)
 		disable_sdram_scrub_setting(mci);
 
-	atomic_notifier_chain_unregister(&x86_mce_decoder_chain, &i7_mce_dec);
+	mce_unregister_decode_chain(&i7_mce_dec);
 
 	/* Disable EDAC polling */
 	i7core_pci_ctl_release(pvt);
@@ -2336,7 +2336,7 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
 	/* DCLK for scrub rate setting */
 	pvt->dclk_freq = get_dclk_freq();
 
-	atomic_notifier_chain_register(&x86_mce_decoder_chain, &i7_mce_dec);
+	mce_register_decode_chain(&i7_mce_dec);
 
 	return 0;
 
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index d0864d9..bd926ea 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -884,7 +884,7 @@ static int __init mce_amd_init(void)
 
 	pr_info("MCE: In-kernel MCE decoding enabled.\n");
 
-	atomic_notifier_chain_register(&x86_mce_decoder_chain, &amd_mce_dec_nb);
+	mce_register_decode_chain(&amd_mce_dec_nb);
 
 	return 0;
 }
@@ -893,7 +893,7 @@ early_initcall(mce_amd_init);
 #ifdef MODULE
 static void __exit mce_amd_exit(void)
 {
-	atomic_notifier_chain_unregister(&x86_mce_decoder_chain, &amd_mce_dec_nb);
+	mce_unregister_decode_chain(&amd_mce_dec_nb);
 	kfree(fam_ops);
 }
 
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 7a402bf..965bc0c 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1661,8 +1661,7 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
 	debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
 		__func__, mci, &sbridge_dev->pdev[0]->dev);
 
-	atomic_notifier_chain_unregister(&x86_mce_decoder_chain,
-					 &sbridge_mce_dec);
+	mce_unregister_decode_chain(&sbridge_mce_dec);
 
 	/* Remove MC sysfs nodes */
 	edac_mc_del_mc(mci->dev);
@@ -1731,8 +1730,7 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
 		goto fail0;
 	}
 
-	atomic_notifier_chain_register(&x86_mce_decoder_chain,
-				       &sbridge_mce_dec);
+	mce_register_decode_chain(&sbridge_mce_dec);
 	return 0;
 
 fail0:
-- 
1.7.8.rc0

[PATCH 2/2] x86, MCE: Drain mcelog buffer

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Add a function which drains whatever MCEs were logged in already during
boot and before the decoder chains were registered.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c |   39 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index c3c66ac..5be2464 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -189,9 +189,48 @@ void mce_log(struct mce *mce)
 	set_bit(0, &mce_need_notify);
 }
 
+static void drain_mcelog_buffer(void)
+{
+	unsigned int next, i, prev = 0;
+
+	next = rcu_dereference_check_mce(mcelog.next);
+
+	do {
+		struct mce *m;
+
+		/* drain what was logged during boot */
+		for (i = prev; i < next; i++) {
+			unsigned long start = jiffies;
+			unsigned retries = 1;
+
+			m = &mcelog.entry[i];
+
+			while (!m->finished) {
+				if (time_after_eq(jiffies, start + 2*retries))
+					retries++;
+
+				cpu_relax();
+
+				if (!m->finished && retries >= 4) {
+					pr_err("MCE: skipping error being logged currently!\n");
+					break;
+				}
+			}
+			smp_rmb();
+			atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, m);
+		}
+
+		memset(mcelog.entry + prev, 0, (next - prev) * sizeof(*m));
+		prev = next;
+		next = cmpxchg(&mcelog.next, prev, 0);
+	} while (next != prev);
+}
+
+
 void mce_register_decode_chain(struct notifier_block *nb)
 {
 	atomic_notifier_chain_register(&x86_mce_decoder_chain, nb);
+	drain_mcelog_buffer();
 }
 EXPORT_SYMBOL_GPL(mce_register_decode_chain);
 
-- 
1.7.8.rc0

Re: [PATCH 0/2] x86, MCE: Catch the early worm

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> Currently, we don't decode MCEs which are being detected during MCA
> initialization and before registration of the decoders on the decoding
> notifier chain. These two patches add the functionality to drain the
> mcelog buffer right after we've registered a decoder.
> 
> With it, early MCEs get caught and decoded into the dmesg:
> 
> ...
> [    0.068003] ... generic registers:      6
> [    0.068003] ... value mask:             0000ffffffffffff
> [    0.068003] ... max period:             00007fffffffffff
> [    0.068003] ... fixed-purpose events:   0
> [    0.068003] ... event mask:             000000000000003f
> [    0.068003] MCE: In-kernel MCE decoding enabled.
> [    0.068003] [Hardware Error]: CPU:0  MC0_STATUS[-|UE|MiscV|-|AddrV|-|-]: 0xac80000000000833
> [    0.068003] [Hardware Error]:        MC0_ADDR: 0x00000000f8080000
> [    0.068003] [Hardware Error]: Data Cache Error: System Read Data Error.
> [    0.068003] [Hardware Error]: cache level: L3/GEN, mem/io: MEM, mem-tx: DRD, part-proc: SRC (no timeout)
> [    0.068069] Booting Node   0, Processors  #1
> [    0.068276] smpboot cpu 1: start_ip = 88000
> [    0.160077]  #2
> [    0.160206] smpboot cpu 2: start_ip = 88000
> [    0.252073]  #3 Ok.
> ...
> 
> Comments and suggestions are welcome, as always.

Looks good to me.

Thanks,

	Ingo

RE: [PATCH 2/2] x86, MCE: Drain mcelog buffer

[Luck, Tony]

void mce_register_decode_chain(struct notifier_block *nb)
 {
 	atomic_notifier_chain_register(&x86_mce_decoder_chain, nb);
+	drain_mcelog_buffer();
 }

Won't this dump all the pended stuff to the *first* guy to
register - but subsequent interested parties will still
miss out?

If so, then we might care who gets here first.

-Tony

Re: [PATCH 2/2] x86, MCE: Drain mcelog buffer

[Borislav Petkov]

On Fri, Dec 09, 2011 at 10:19:02AM -0800, Luck, Tony wrote:
> void mce_register_decode_chain(struct notifier_block *nb)
>  {
>  	atomic_notifier_chain_register(&x86_mce_decoder_chain, nb);
> +	drain_mcelog_buffer();
>  }
> 
> Won't this dump all the pended stuff to the *first* guy to
> register - but subsequent interested parties will still
> miss out?

Yeah,

this is currently assuming that we have one consumer per system. At
least this is the case on AMD: MCE decoder registers and decodes the
errors. It will pass a subset of the MCEs (DRAM ECCs) to amd64_edac for
additional massaging.

Do you see a usecase for multiple consumers?

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

RE: [PATCH 2/2] x86, MCE: Drain mcelog buffer

[Luck, Tony]

> Do you see a usecase for multiple consumers?

Only a theoretical one ... we might have a cpu model specific decoder
and an OEM supplied platform decoder - each as an independent module.

-Tony

Re: [PATCH 2/2] x86, MCE: Drain mcelog buffer

[Borislav Petkov]

On Fri, Dec 09, 2011 at 10:40:10AM -0800, Luck, Tony wrote:
> > Do you see a usecase for multiple consumers?
> 
> Only a theoretical one ... we might have a cpu model specific decoder
> and an OEM supplied platform decoder - each as an independent module.

Well, off the top of my head, we could probably _not_ delete the already
logged MCEs (bool keep arg, or similar) and when the last one registers,
it passes keep=false and cleans them up. And since we probably know
which one is the last - order is enforced by the initcalls order -
we're done.

However, is the platform module using MCA at all or will it probably use
PCIe AER instead?

Also, at the time the platform module inits, we've already regged the
CPU decoders so we can go ahead and safely delete the logged MCEs.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

RE: [PATCH 2/2] x86, MCE: Drain mcelog buffer

[Luck, Tony]

> Well, off the top of my head, we could probably _not_ delete the already
> logged MCEs (bool keep arg, or similar) and when the last one registers,
> it passes keep=false and cleans them up. And since we probably know
> which one is the last - order is enforced by the initcalls order -
> we're done.

Knowing when the last one has come is hard if there are modules as well
as built-in registrants.  We could save the whole list, and deliver a
copy of the history of the world so far to each as they register (never
deleting anything). But that sounds odd - why should a module loaded
6 months after system boot expect to see everything.

> However, is the platform module using MCA at all or will it probably use
> PCIe AER instead?
Don't know - its hypothetical - but it is possible that MCA might be used
for not-cpu stuff.

> Also, at the time the platform module inits, we've already regged the
> CPU decoders so we can go ahead and safely delete the logged MCEs.

I think if we ensure that the cpu decoder gets first chance to register,
we should be OK with giving it all the pended stuff. We can defer doing
something more complicated until the point that someone has a real use
case.

-Tony

Re: [PATCH 2/2] x86, MCE: Drain mcelog buffer

[Borislav Petkov]

On Fri, Dec 09, 2011 at 02:08:59PM -0800, Luck, Tony wrote:
> > Well, off the top of my head, we could probably _not_ delete the already
> > logged MCEs (bool keep arg, or similar) and when the last one registers,
> > it passes keep=false and cleans them up. And since we probably know
> > which one is the last - order is enforced by the initcalls order -
> > we're done.
> 
> Knowing when the last one has come is hard if there are modules as well
> as built-in registrants.  We could save the whole list, and deliver a
> copy of the history of the world so far to each as they register (never
> deleting anything). But that sounds odd - why should a module loaded
> 6 months after system boot expect to see everything.

Well, currently the buffer overflows at the 32th entry so something that
registers 6 months after system boot will probably see unrelated errors
from 6 months ago depending on the error rate.

> I think if we ensure that the cpu decoder gets first chance to register,
> we should be OK with giving it all the pended stuff. We can defer doing
> something more complicated until the point that someone has a real use
> case.

Agreed, and this is currently the case on AMD and Intel, AFAICT, where
the i7 or sb edac module registers. We can always change it later if
required.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551