Skip to contentSkip to conversation

EDAC, mce_amd: Issue decoded MCE through the tracepoint

PATCH In review Borislav Petkov 3 participants 19 messages Aug 25, 2017 – Sep 4, 2017

Subsystems (6)
x86 Library code Tracing EDAC-core +2 more subsystems
lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

cover EDAC, mce_amd: Issue decoded MCE through the tracepoint

From: Borislav Petkov <[email protected]>

Hi all,

here's v2 incorporating all the feedback from last time. The main difference is that instead of adding yet another tracepoint, I extended mce_record with the decoded string. This way is much more natural and we should've done it like this since the get-go.

The TP record looks like this:

# tracer: nop
#
# _-----=> irqs-off
# / _----=> need-resched

#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
     kworker/1:1-91    [001] ....    97.021806: mce_record: CPU: 0, MCGc/s: 0/0, MC5: 9600410000000e0f, IPID: 0000000000000000, ADDR/MISC/SYND: 0000000097370d7b/0000000000000000/0000000000000000, RIP: 00:<0000000000000000>, TSC: 5c226747ec, PROCESSOR: 2:0, TIME: 0, SOCKET: 0, APIC: 0 MC5 Error: CPU Watchdog timer expire.

and userspace can pick apart the fields, as before.

Next step is adding that to rasdaemon.

Thanks.

Changelog:
==========

v1:

here's a first stab at adding a tracepoint which dumps the decoded MCE string to userspace. The main idea is to have the decoding functionality in the kernel and depending on whether you have userspace consumers listening or not, to dump the error to the tracepoint or to dmesg.

In either case, we do the decoding in the kernel and don't need special userspace. Furthermore, adding new CPU support will have to be done only in one place.

First 6 patches are cleanups which are good to have regardless, IMO.

Any constructive comments and suggestions are appreciated.

Thanks.

P.S., Thanks to Rostedt for the input!

Borislav Petkov (7):
x86/mce: Handle an in-kernel MCE decoder
x86/mce: Extend the MCE tracepoint with a decoded string
seq_buf: Add seq_buf_clear_buf()
seq_buf: Export seq_buf_printf() to modules
EDAC, mce_amd: Convert to seq_buf
EDAC, mce_amd: Issue the decoded info through the TP or printk() x86/mce: Issue the mcelog --ascii message on !AMD

 arch/x86/include/asm/mce.h       |   4 +-
 arch/x86/kernel/cpu/mcheck/mce.c |  14 +-
 drivers/edac/mce_amd.c           | 279 ++++++++++++++++++++++++---------------
 include/linux/seq_buf.h          |   7 +
 include/trace/events/mce.h       |  11 +-
 lib/seq_buf.c                    |   1 +
 6 files changed, 204 insertions(+), 112 deletions(-)
Signature · 1 line
2.13.0
author maintainer
#1 lore raw

PATCH 1/7 x86/mce: Handle an in-kernel MCE decoder

From: Borislav Petkov <[email protected]>

Change the MCE decoding flow to issue the trace_mce_record() tracepoint only when there's no decoder registered on the decoder chain. If there is, it will do some massaging on the MCE info before issuing it through the tracepoint.

Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/include/asm/mce.h       | 4 +++-
 arch/x86/kernel/cpu/mcheck/mce.c | 9 ++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)
arch/x86/include/asm/mce.h +3 -1
index 181264989db5..ba4c99ace544 100644
@@ -146,6 +146,7 @@ struct mca_config {
 	bool ser;
 	bool recovery;
 	bool bios_cmci_threshold;
+	bool has_decoder;
 	u8 banks;
 	s8 bootlog;
 	int tolerant;

@@ -195,7 +196,8 @@ enum mce_notifier_prios {
 	MCE_PRIO_SRAO		= INT_MAX - 1,
 	MCE_PRIO_EXTLOG		= INT_MAX - 2,
 	MCE_PRIO_NFIT		= INT_MAX - 3,
-	MCE_PRIO_EDAC		= INT_MAX - 4,
+	MCE_PRIO_DECODER	= INT_MAX - 4,
+	MCE_PRIO_EDAC		= INT_MAX - 5,
 	MCE_PRIO_MCELOG		= 1,
 	MCE_PRIO_LOWEST		= 0,
 };
arch/x86/kernel/cpu/mcheck/mce.c +8 -1
index 3b413065c613..dcf75f40e031 100644
@@ -163,6 +163,9 @@ void mce_register_decode_chain(struct notifier_block *nb)
 
 	atomic_inc(&num_notifiers);
 
+	if (nb->priority == MCE_PRIO_DECODER)
+		mca_cfg.has_decoder = true;
+
 	blocking_notifier_chain_register(&x86_mce_decoder_chain, nb);
 }
 EXPORT_SYMBOL_GPL(mce_register_decode_chain);

@@ -171,6 +174,9 @@ void mce_unregister_decode_chain(struct notifier_block *nb)
 {
 	atomic_dec(&num_notifiers);
 
+	if (nb->priority == MCE_PRIO_DECODER)
+		mca_cfg.has_decoder = false;
+
 	blocking_notifier_chain_unregister(&x86_mce_decoder_chain, nb);
 }
 EXPORT_SYMBOL_GPL(mce_unregister_decode_chain);

@@ -557,7 +563,8 @@ static int mce_first_notifier(struct notifier_block *nb, unsigned long val,
 		return NOTIFY_STOP;
 
 	/* Emit the trace record: */
-	trace_mce_record(m);
+	if (!mca_cfg.has_decoder)
+		trace_mce_record(m);
 
 	set_bit(0, &mce_need_notify);
author maintainer
#2 lore raw

PATCH 2/7 x86/mce: Extend the MCE tracepoint with a decoded string

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

... which will be filled in by an in-kernel decoder, if one present. Export it to modules too.

Signed-off-by Borislav Petkov <[email protected]>
Cc Steven Rostedt <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce.c |  3 ++-
 include/trace/events/mce.h       | 11 +++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -1
index dcf75f40e031..62bab5d5e96c 100644
@@ -59,6 +59,7 @@ static DEFINE_MUTEX(mce_log_mutex);
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/mce.h>
+EXPORT_TRACEPOINT_SYMBOL_GPL(mce_record);
 
 #define SPINUNIT		100	/* 100ns */
 

@@ -564,7 +565,7 @@ static int mce_first_notifier(struct notifier_block *nb, unsigned long val,
 
 	/* Emit the trace record: */
 	if (!mca_cfg.has_decoder)
-		trace_mce_record(m);
+		trace_mce_record(m, "");
 
 	set_bit(0, &mce_need_notify);
include/trace/events/mce.h +7 -4
index 70f02149808c..82ff4014fab1 100644
@@ -10,9 +10,9 @@
 
 TRACE_EVENT(mce_record,
 
-	TP_PROTO(struct mce *m),
+	TP_PROTO(struct mce *m, const char *param_str),
 
-	TP_ARGS(m),
+	TP_ARGS(m, param_str),
 
 	TP_STRUCT__entry(
 		__field(	u64,		mcgcap		)

@@ -32,6 +32,7 @@ TRACE_EVENT(mce_record,
 		__field(	u8,		cs		)
 		__field(	u8,		bank		)
 		__field(	u8,		cpuvendor	)
+		__string(	str,		param_str	)
 	),
 
 	TP_fast_assign(

@@ -52,9 +53,10 @@ TRACE_EVENT(mce_record,
 		__entry->cs		= m->cs;
 		__entry->bank		= m->bank;
 		__entry->cpuvendor	= m->cpuvendor;
+		__assign_str(str, param_str);
 	),
 
-	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, IPID: %016Lx, ADDR/MISC/SYND: %016Lx/%016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x",
+	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, IPID: %016Lx, ADDR/MISC/SYND: %016Lx/%016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x %s",
 		__entry->cpu,
 		__entry->mcgcap, __entry->mcgstatus,
 		__entry->bank, __entry->status,

@@ -65,7 +67,8 @@ TRACE_EVENT(mce_record,
 		__entry->cpuvendor, __entry->cpuid,
 		__entry->walltime,
 		__entry->socketid,
-		__entry->apicid)
+		__entry->apicid,
+		__get_str(str))
 );
 
 #endif /* _TRACE_MCE_H */
author maintainer
#3 lore raw

PATCH 3/7 seq_buf: Add seq_buf_clear_buf()

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

This is the version which clears the supplied buffer too. Useful when we're done with the buffer and want to clean it up and prepare it for reuse.

Signed-off-by Borislav Petkov <[email protected]>
Acked-by Steven Rostedt (VMware) <[email protected]>
---
 include/linux/seq_buf.h | 7 +++++++
 1 file changed, 7 insertions(+)
include/linux/seq_buf.h +7 -0
index fb7eb9ccb1cd..8705a482e76c 100644
@@ -28,6 +28,13 @@ static inline void seq_buf_clear(struct seq_buf *s)
 	s->readpos = 0;
 }
 
+/* Like seq_buf_clear() but zero out the buffer too. */
+static inline void seq_buf_clear_buf(struct seq_buf *s)
+{
+	seq_buf_clear(s);
+	memset(s->buffer, 0, s->size);
+}
+
 static inline void
 seq_buf_init(struct seq_buf *s, unsigned char *buf, unsigned int size)
 {
author maintainer
#4 lore raw

PATCH 4/7 seq_buf: Export seq_buf_printf() to modules

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Will be used in a module in a later patch.

Signed-off-by Borislav Petkov <[email protected]>
Acked-by Steven Rostedt (VMware) <[email protected]>
---
 lib/seq_buf.c | 1 +
 1 file changed, 1 insertion(+)
lib/seq_buf.c +1 -0
index cb18469e1f49..b5217cf1824d 100644
@@ -90,6 +90,7 @@ int seq_buf_printf(struct seq_buf *s, const char *fmt, ...)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(seq_buf_printf);
 
 #ifdef CONFIG_BINARY_PRINTF
 /**
maintainer
#5 lore raw

Re: [PATCH 4/7] seq_buf: Export seq_buf_printf() to modules

Quoted 15 lines · On Fri, 25 Aug 2017 12:24:08 +0200 Borislav Petkov <[email protected]> wrote:

From: Borislav Petkov <[email protected]>

Will be used in a module in a later patch.

Signed-off-by Borislav Petkov <[email protected]>
Acked-by Steven Rostedt (VMware) <[email protected]>
---
 lib/seq_buf.c | 1 +
 1 file changed, 1 insertion(+)
lib/seq_buf.c +0 -0
index cb18469e1f49..b5217cf1824d 100644

lib/seq_buf.c int seq_buf_printf(struct seq_buf *s, const char *fmt, ...) · line 93

Quoted 4 lines
lib/seq_buf.c +1 -0
@@ -90,6 +90,7 @@ int seq_buf_printf(struct seq_buf *s, const char *fmt, ...)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(seq_buf_printf);

May want to add seq_buf_putc() and seq_buf_puts() as well. Will explain in other patches.

-- Steve

lib/seq_buf.c int seq_buf_printf(struct seq_buf *s, const char *fmt, ...) · line 96

Quoted 3 lines
lib/seq_buf.c +0 -0
 #ifdef CONFIG_BINARY_PRINTF
 /**

---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#6 lore raw

PATCH 5/7 EDAC, mce_amd: Convert to seq_buf

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Convert the part which decodes the error description to the sequence buffer facility and thus save ourselves the many printk() invocations building the decoded string.

Also, use a genpool for the string buffers to handle concurrent invocations (and atomic context).

Signed-off-by Borislav Petkov <[email protected]>
---
 drivers/edac/mce_amd.c | 215 +++++++++++++++++++++++++++++--------------------
 1 file changed, 127 insertions(+), 88 deletions(-)
drivers/edac/mce_amd.c +127 -88
index a11a671c7a38..b7c1f8f7e871 100644
@@ -1,3 +1,5 @@
+#include <linux/genalloc.h>
+#include <linux/seq_buf.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 

@@ -34,6 +36,16 @@ void amd_unregister_ecc_decoder(void (*f)(int, struct mce *))
 }
 EXPORT_SYMBOL_GPL(amd_unregister_ecc_decoder);
 
+/* 128 because, well, nice and round - two cachelines. */
+#define ELEM_ORDER	7
+#define ELEM_SIZE	(1 << 7)
+#define DEC_POOL_SIZE	(2 * PAGE_SIZE)
+
+static char __err_buf[DEC_POOL_SIZE];
+static struct gen_pool *dec_pool;
+
+static struct seq_buf sb;
+
 /*
  * string representation for the different MCA reported error types, see F3x48
  * or MSR0000_0411.

@@ -315,9 +327,9 @@ static bool f12h_mc0_mce(u16 ec, u8 xec)
 		ret = true;
 
 		if (ll == LL_L2)
-			pr_cont("during L1 linefill from L2.\n");
+			seq_buf_printf(&sb, "during L1 linefill from L2.\n");
 		else if (ll == LL_L1)
-			pr_cont("Data/Tag %s error.\n", R4_MSG(ec));
+			seq_buf_printf(&sb, "Data/Tag %s error.\n", R4_MSG(ec));
 		else
 			ret = false;
 	}

@@ -327,7 +339,7 @@ static bool f12h_mc0_mce(u16 ec, u8 xec)
 static bool f10h_mc0_mce(u16 ec, u8 xec)
 {
 	if (R4(ec) == R4_GEN && LL(ec) == LL_L1) {
-		pr_cont("during data scrub.\n");
+		seq_buf_printf(&sb, "during data scrub.\n");
 		return true;
 	}
 	return f12h_mc0_mce(ec, xec);

@@ -336,7 +348,7 @@ static bool f10h_mc0_mce(u16 ec, u8 xec)
 static bool k8_mc0_mce(u16 ec, u8 xec)
 {
 	if (BUS_ERROR(ec)) {
-		pr_cont("during system linefill.\n");
+		seq_buf_printf(&sb, "during system linefill.\n");
 		return true;
 	}
 

@@ -356,14 +368,14 @@ static bool cat_mc0_mce(u16 ec, u8 xec)
 		switch (r4) {
 		case R4_DRD:
 		case R4_DWR:
-			pr_cont("Data/Tag parity error due to %s.\n",
-				(r4 == R4_DRD ? "load/hw prf" : "store"));
+			seq_buf_printf(&sb, "Data/Tag parity error due to %s.\n",
+					(r4 == R4_DRD ? "load/hw prf" : "store"));
 			break;
 		case R4_EVICT:
-			pr_cont("Copyback parity error on a tag miss.\n");
+			seq_buf_printf(&sb, "Copyback parity error on a tag miss.\n");
 			break;
 		case R4_SNOOP:
-			pr_cont("Tag parity error during snoop.\n");
+			seq_buf_printf(&sb, "Tag parity error during snoop.\n");
 			break;
 		default:
 			ret = false;

@@ -373,17 +385,17 @@ static bool cat_mc0_mce(u16 ec, u8 xec)
 		if ((II(ec) != II_MEM && II(ec) != II_IO) || LL(ec) != LL_LG)
 			return false;
 
-		pr_cont("System read data error on a ");
+		seq_buf_printf(&sb, "System read data error on a ");
 
 		switch (r4) {
 		case R4_RD:
-			pr_cont("TLB reload.\n");
+			seq_buf_printf(&sb, "TLB reload.\n");
 			break;
 		case R4_DWR:
-			pr_cont("store.\n");
+			seq_buf_printf(&sb, "store.\n");
 			break;
 		case R4_DRD:
-			pr_cont("load.\n");
+			seq_buf_printf(&sb, "load.\n");
 			break;
 		default:
 			ret = false;

@@ -403,28 +415,28 @@ static bool f15h_mc0_mce(u16 ec, u8 xec)
 
 		switch (xec) {
 		case 0x0:
-			pr_cont("Data Array access error.\n");
+			seq_buf_printf(&sb, "Data Array access error.\n");
 			break;
 
 		case 0x1:
-			pr_cont("UC error during a linefill from L2/NB.\n");
+			seq_buf_printf(&sb, "UC error during a linefill from L2/NB.\n");
 			break;
 
 		case 0x2:
 		case 0x11:
-			pr_cont("STQ access error.\n");
+			seq_buf_printf(&sb, "STQ access error.\n");
 			break;
 
 		case 0x3:
-			pr_cont("SCB access error.\n");
+			seq_buf_printf(&sb, "SCB access error.\n");
 			break;
 
 		case 0x10:
-			pr_cont("Tag error.\n");
+			seq_buf_printf(&sb, "Tag error.\n");
 			break;
 
 		case 0x12:
-			pr_cont("LDQ access error.\n");
+			seq_buf_printf(&sb, "LDQ access error.\n");
 			break;
 
 		default:

@@ -433,12 +445,12 @@ static bool f15h_mc0_mce(u16 ec, u8 xec)
 	} else if (BUS_ERROR(ec)) {
 
 		if (!xec)
-			pr_cont("System Read Data Error.\n");
+			seq_buf_printf(&sb, "System Read Data Error.\n");
 		else
-			pr_cont(" Internal error condition type %d.\n", xec);
+			seq_buf_printf(&sb, " Internal error condition type %d.\n", xec);
 	} else if (INT_ERROR(ec)) {
 		if (xec <= 0x1f)
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 		else
 			ret = false;
 

@@ -453,13 +465,13 @@ static void decode_mc0_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC0 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC0 Error: ");
 
 	/* TLB error signatures are the same across families */
 	if (TLB_ERROR(ec)) {
 		if (TT(ec) == TT_DATA) {
-			pr_cont("%s TLB %s.\n", LL_MSG(ec),
-				((xec == 2) ? "locked miss"
+			seq_buf_printf(&sb, "%s TLB %s.\n", LL_MSG(ec),
+					((xec == 2) ? "locked miss"
 					    : (xec ? "multimatch" : "parity")));
 			return;
 		}

@@ -478,19 +490,19 @@ static bool k8_mc1_mce(u16 ec, u8 xec)
 		return false;
 
 	if (ll == 0x2)
-		pr_cont("during a linefill from L2.\n");
+		seq_buf_printf(&sb, "during a linefill from L2.\n");
 	else if (ll == 0x1) {
 		switch (R4(ec)) {
 		case R4_IRD:
-			pr_cont("Parity error during data load.\n");
+			seq_buf_printf(&sb, "Parity error during data load.\n");
 			break;
 
 		case R4_EVICT:
-			pr_cont("Copyback Parity/Victim error.\n");
+			seq_buf_printf(&sb, "Copyback Parity/Victim error.\n");
 			break;
 
 		case R4_SNOOP:
-			pr_cont("Tag Snoop error.\n");
+			seq_buf_printf(&sb, "Tag Snoop error.\n");
 			break;
 
 		default:

@@ -515,13 +527,13 @@ static bool cat_mc1_mce(u16 ec, u8 xec)
 		return false;
 
 	if (r4 == R4_IRD)
-		pr_cont("Data/tag array parity error for a tag hit.\n");
+		seq_buf_printf(&sb, "Data/tag array parity error for a tag hit.\n");
 	else if (r4 == R4_SNOOP)
-		pr_cont("Tag error during snoop/victimization.\n");
+		seq_buf_printf(&sb, "Tag error during snoop/victimization.\n");
 	else if (xec == 0x0)
-		pr_cont("Tag parity error from victim castout.\n");
+		seq_buf_printf(&sb, "Tag parity error from victim castout.\n");
 	else if (xec == 0x2)
-		pr_cont("Microcode patch RAM parity error.\n");
+		seq_buf_printf(&sb, "Microcode patch RAM parity error.\n");
 	else
 		ret = false;
 

@@ -537,19 +549,19 @@ static bool f15h_mc1_mce(u16 ec, u8 xec)
 
 	switch (xec) {
 	case 0x0 ... 0xa:
-		pr_cont("%s.\n", f15h_mc1_mce_desc[xec]);
+		seq_buf_printf(&sb, "%s.\n", f15h_mc1_mce_desc[xec]);
 		break;
 
 	case 0xd:
-		pr_cont("%s.\n", f15h_mc1_mce_desc[xec-2]);
+		seq_buf_printf(&sb, "%s.\n", f15h_mc1_mce_desc[xec-2]);
 		break;
 
 	case 0x10:
-		pr_cont("%s.\n", f15h_mc1_mce_desc[xec-4]);
+		seq_buf_printf(&sb, "%s.\n", f15h_mc1_mce_desc[xec-4]);
 		break;
 
 	case 0x11 ... 0x15:
-		pr_cont("Decoder %s parity error.\n", f15h_mc1_mce_desc[xec-4]);
+		seq_buf_printf(&sb, "Decoder %s parity error.\n", f15h_mc1_mce_desc[xec-4]);
 		break;
 
 	default:

@@ -563,18 +575,18 @@ static void decode_mc1_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC1 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC1 Error: ");
 
 	if (TLB_ERROR(ec))
-		pr_cont("%s TLB %s.\n", LL_MSG(ec),
-			(xec ? "multimatch" : "parity error"));
+		seq_buf_printf(&sb, "%s TLB %s.\n", LL_MSG(ec),
+				(xec ? "multimatch" : "parity error"));
 	else if (BUS_ERROR(ec)) {
 		bool k8 = (boot_cpu_data.x86 == 0xf && (m->status & BIT_64(58)));
 
-		pr_cont("during %s.\n", (k8 ? "system linefill" : "NB data read"));
+		seq_buf_printf(&sb, "during %s.\n", (k8 ? "system linefill" : "NB data read"));
 	} else if (INT_ERROR(ec)) {
 		if (xec <= 0x3f)
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 		else
 			goto wrong_mc1_mce;
 	} else if (fam_ops->mc1_mce(ec, xec))

@@ -593,27 +605,27 @@ static bool k8_mc2_mce(u16 ec, u8 xec)
 	bool ret = true;
 
 	if (xec == 0x1)
-		pr_cont(" in the write data buffers.\n");
+		seq_buf_printf(&sb, " in the write data buffers.\n");
 	else if (xec == 0x3)
-		pr_cont(" in the victim data buffers.\n");
+		seq_buf_printf(&sb, " in the victim data buffers.\n");
 	else if (xec == 0x2 && MEM_ERROR(ec))
-		pr_cont(": %s error in the L2 cache tags.\n", R4_MSG(ec));
+		seq_buf_printf(&sb, ": %s error in the L2 cache tags.\n", R4_MSG(ec));
 	else if (xec == 0x0) {
 		if (TLB_ERROR(ec))
-			pr_cont("%s error in a Page Descriptor Cache or Guest TLB.\n",
-				TT_MSG(ec));
+			seq_buf_printf(&sb, "%s error in a Page Descriptor Cache or Guest TLB.\n",
+					TT_MSG(ec));
 		else if (BUS_ERROR(ec))
-			pr_cont(": %s/ECC error in data read from NB: %s.\n",
-				R4_MSG(ec), PP_MSG(ec));
+			seq_buf_printf(&sb, ": %s/ECC error in data read from NB: %s.\n",
+					R4_MSG(ec), PP_MSG(ec));
 		else if (MEM_ERROR(ec)) {
 			u8 r4 = R4(ec);
 
 			if (r4 >= 0x7)
-				pr_cont(": %s error during data copyback.\n",
-					R4_MSG(ec));
+				seq_buf_printf(&sb, ": %s error during data copyback.\n",
+						R4_MSG(ec));
 			else if (r4 <= 0x1)
-				pr_cont(": %s parity/ECC error during data "
-					"access from L2.\n", R4_MSG(ec));
+				seq_buf_printf(&sb,
+": %s parity/ECC error during data access from L2.\n", R4_MSG(ec));
 			else
 				ret = false;
 		} else

@@ -630,24 +642,24 @@ static bool f15h_mc2_mce(u16 ec, u8 xec)
 
 	if (TLB_ERROR(ec)) {
 		if (xec == 0x0)
-			pr_cont("Data parity TLB read error.\n");
+			seq_buf_printf(&sb, "Data parity TLB read error.\n");
 		else if (xec == 0x1)
-			pr_cont("Poison data provided for TLB fill.\n");
+			seq_buf_printf(&sb, "Poison data provided for TLB fill.\n");
 		else
 			ret = false;
 	} else if (BUS_ERROR(ec)) {
 		if (xec > 2)
 			ret = false;
 
-		pr_cont("Error during attempted NB data read.\n");
+		seq_buf_printf(&sb, "Error during attempted NB data read.\n");
 	} else if (MEM_ERROR(ec)) {
 		switch (xec) {
 		case 0x4 ... 0xc:
-			pr_cont("%s.\n", f15h_mc2_mce_desc[xec - 0x4]);
+			seq_buf_printf(&sb, "%s.\n", f15h_mc2_mce_desc[xec - 0x4]);
 			break;
 
 		case 0x10 ... 0x14:
-			pr_cont("%s.\n", f15h_mc2_mce_desc[xec - 0x7]);
+			seq_buf_printf(&sb, "%s.\n", f15h_mc2_mce_desc[xec - 0x7]);
 			break;
 
 		default:

@@ -655,7 +667,7 @@ static bool f15h_mc2_mce(u16 ec, u8 xec)
 		}
 	} else if (INT_ERROR(ec)) {
 		if (xec <= 0x3f)
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 		else
 			ret = false;
 	}

@@ -672,29 +684,29 @@ static bool f16h_mc2_mce(u16 ec, u8 xec)
 
 	switch (xec) {
 	case 0x04 ... 0x05:
-		pr_cont("%cBUFF parity error.\n", (r4 == R4_RD) ? 'I' : 'O');
+		seq_buf_printf(&sb, "%cBUFF parity error.\n", (r4 == R4_RD) ? 'I' : 'O');
 		break;
 
 	case 0x09 ... 0x0b:
 	case 0x0d ... 0x0f:
-		pr_cont("ECC error in L2 tag (%s).\n",
-			((r4 == R4_GEN)   ? "BankReq" :
-			((r4 == R4_SNOOP) ? "Prb"     : "Fill")));
+		seq_buf_printf(&sb, "ECC error in L2 tag (%s).\n",
+				((r4 == R4_GEN)   ? "BankReq" :
+				((r4 == R4_SNOOP) ? "Prb"     : "Fill")));
 		break;
 
 	case 0x10 ... 0x19:
 	case 0x1b:
-		pr_cont("ECC error in L2 data array (%s).\n",
-			(((r4 == R4_RD) && !(xec & 0x3)) ? "Hit"  :
-			((r4 == R4_GEN)   ? "Attr" :
-			((r4 == R4_EVICT) ? "Vict" : "Fill"))));
+		seq_buf_printf(&sb, "ECC error in L2 data array (%s).\n",
+				(((r4 == R4_RD) && !(xec & 0x3)) ? "Hit"  :
+				((r4 == R4_GEN)   ? "Attr" :
+				((r4 == R4_EVICT) ? "Vict" : "Fill"))));
 		break;
 
 	case 0x1c ... 0x1d:
 	case 0x1f:
-		pr_cont("Parity error in L2 attribute bits (%s).\n",
-			((r4 == R4_RD)  ? "Hit"  :
-			((r4 == R4_GEN) ? "Attr" : "Fill")));
+		seq_buf_printf(&sb, "Parity error in L2 attribute bits (%s).\n",
+				((r4 == R4_RD)  ? "Hit"  :
+				((r4 == R4_GEN) ? "Attr" : "Fill")));
 		break;
 
 	default:

@@ -709,10 +721,10 @@ static void decode_mc2_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC2 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC2 Error: ");
 
 	if (!fam_ops->mc2_mce(ec, xec))
-		pr_cont(HW_ERR "Corrupted MC2 MCE info?\n");
+		pr_emerg(HW_ERR "Corrupted MC2 MCE info?\n");
 }
 
 static void decode_mc3_mce(struct mce *m)

@@ -726,7 +738,7 @@ static void decode_mc3_mce(struct mce *m)
 		return;
 	}
 
-	pr_emerg(HW_ERR "MC3 Error");
+	seq_buf_printf(&sb, HW_ERR "MC3 Error");
 
 	if (xec == 0x0) {
 		u8 r4 = R4(ec);

@@ -734,7 +746,7 @@ static void decode_mc3_mce(struct mce *m)
 		if (!BUS_ERROR(ec) || (r4 != R4_DRD && r4 != R4_DWR))
 			goto wrong_mc3_mce;
 
-		pr_cont(" during %s.\n", R4_MSG(ec));
+		seq_buf_printf(&sb, " during %s.\n", R4_MSG(ec));
 	} else
 		goto wrong_mc3_mce;
 

@@ -752,7 +764,7 @@ static void decode_mc4_mce(struct mce *m)
 	u8 xec = XEC(m->status, 0x1f);
 	u8 offset = 0;
 
-	pr_emerg(HW_ERR "MC4 Error (node %d): ", node_id);
+	seq_buf_printf(&sb, HW_ERR "MC4 Error (node %d): ", node_id);
 
 	switch (xec) {
 	case 0x0 ... 0xe:

@@ -763,7 +775,7 @@ static void decode_mc4_mce(struct mce *m)
 			if (fam == 0x11)
 				goto wrong_mc4_mce;
 
-			pr_cont("%s.\n", mc4_mce_desc[xec]);
+			seq_buf_printf(&sb, "%s.\n", mc4_mce_desc[xec]);
 
 			if (decode_dram_ecc)
 				decode_dram_ecc(node_id, m);

@@ -773,16 +785,16 @@ static void decode_mc4_mce(struct mce *m)
 
 	case 0xf:
 		if (TLB_ERROR(ec))
-			pr_cont("GART Table Walk data error.\n");
+			seq_buf_printf(&sb, "GART Table Walk data error.\n");
 		else if (BUS_ERROR(ec))
-			pr_cont("DMA Exclusion Vector Table Walk error.\n");
+			seq_buf_printf(&sb, "DMA Exclusion Vector Table Walk error.\n");
 		else
 			goto wrong_mc4_mce;
 		return;
 
 	case 0x19:
 		if (fam == 0x15 || fam == 0x16)
-			pr_cont("Compute Unit Data Error.\n");
+			seq_buf_printf(&sb, "Compute Unit Data Error.\n");
 		else
 			goto wrong_mc4_mce;
 		return;

@@ -795,7 +807,7 @@ static void decode_mc4_mce(struct mce *m)
 		goto wrong_mc4_mce;
 	}
 
-	pr_cont("%s.\n", mc4_mce_desc[xec - offset]);
+	seq_buf_printf(&sb, "%s.\n", mc4_mce_desc[xec - offset]);
 	return;
 
  wrong_mc4_mce:

@@ -811,20 +823,20 @@ static void decode_mc5_mce(struct mce *m)
 	if (fam == 0xf || fam == 0x11)
 		goto wrong_mc5_mce;
 
-	pr_emerg(HW_ERR "MC5 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC5 Error: ");
 
 	if (INT_ERROR(ec)) {
 		if (xec <= 0x1f) {
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 			return;
 		} else
 			goto wrong_mc5_mce;
 	}
 
 	if (xec == 0x0 || xec == 0xc)
-		pr_cont("%s.\n", mc5_mce_desc[xec]);
+		seq_buf_printf(&sb, "%s.\n", mc5_mce_desc[xec]);
 	else if (xec <= 0xd)
-		pr_cont("%s parity error.\n", mc5_mce_desc[xec]);
+		seq_buf_printf(&sb, "%s parity error.\n", mc5_mce_desc[xec]);
 	else
 		goto wrong_mc5_mce;
 

@@ -838,12 +850,12 @@ static void decode_mc6_mce(struct mce *m)
 {
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC6 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC6 Error: ");
 
 	if (xec > 0x5)
 		goto wrong_mc6_mce;
 
-	pr_cont("%s parity error.\n", mc6_mce_desc[xec]);
+	seq_buf_printf(&sb, "%s parity error.\n", mc6_mce_desc[xec]);
 	return;
 
  wrong_mc6_mce:

@@ -871,13 +883,13 @@ static void decode_smca_error(struct mce *m)
 	bank_type = hwid->bank_type;
 	ip_name = smca_get_long_name(bank_type);
 
-	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
+	seq_buf_printf(&sb, HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
 
 	/* Only print the decode of valid error codes */
 	if (xec < smca_mce_descs[bank_type].num_descs &&
 			(hwid->xec_bitmap & BIT_ULL(xec))) {
-		pr_emerg(HW_ERR "%s Error: ", ip_name);
-		pr_cont("%s.\n", smca_mce_descs[bank_type].descs[xec]);
+		seq_buf_printf(&sb, HW_ERR "%s Error: ", ip_name);
+		seq_buf_printf(&sb, "%s.\n", smca_mce_descs[bank_type].descs[xec]);
 	}
 
 	if (bank_type == SMCA_UMC && xec == 0 && decode_dram_ecc)

@@ -944,10 +956,21 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 	struct mce *m = (struct mce *)data;
 	unsigned int fam = x86_family(m->cpuid);
 	int ecc;
+	char *dec_buf;
 
 	if (amd_filter_mce(m))
 		return NOTIFY_STOP;
 
+	dec_buf = (void *)gen_pool_alloc(dec_pool, ELEM_SIZE);
+	if (!dec_buf) {
+		pr_warn("Decode buffer full!\n");
+		return NOTIFY_STOP;
+	}
+
+	/* \0 terminated */
+	seq_buf_init(&sb, dec_buf, ELEM_SIZE);
+	seq_buf_clear_buf(&sb);
+
 	pr_emerg(HW_ERR "%s\n", decode_error_status(m));
 
 	pr_emerg(HW_ERR "CPU:%d (%x:%x:%x) MC%d_STATUS[%s|%s|%s|%s|%s",

@@ -1044,6 +1067,10 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
  err_code:
 	amd_decode_err_code(m->status & 0xffff);
 
+	pr_emerg("%.*s\n", (int)sb.len, sb.buffer);
+
+	gen_pool_free(dec_pool, (unsigned long)dec_buf, ELEM_SIZE);
+
 	return NOTIFY_STOP;
 }
 

@@ -1055,6 +1082,7 @@ static struct notifier_block amd_mce_dec_nb = {
 static int __init mce_amd_init(void)
 {
 	struct cpuinfo_x86 *c = &boot_cpu_data;
+	int ret;
 
 	if (c->x86_vendor != X86_VENDOR_AMD)
 		return -ENODEV;

@@ -1122,6 +1150,16 @@ static int __init mce_amd_init(void)
 		goto err_out;
 	}
 
+	dec_pool = gen_pool_create(ELEM_ORDER, -1);
+	if (!dec_pool)
+		goto err_out;
+
+	ret = gen_pool_add(dec_pool, (unsigned long)__err_buf, DEC_POOL_SIZE, -1);
+	if (ret) {
+		gen_pool_destroy(dec_pool);
+		goto err_out;
+	}
+
 	pr_info("MCE: In-kernel MCE decoding enabled.\n");
 
 	mce_register_decode_chain(&amd_mce_dec_nb);

@@ -1140,6 +1178,7 @@ static void __exit mce_amd_exit(void)
 {
 	mce_unregister_decode_chain(&amd_mce_dec_nb);
 	kfree(fam_ops);
+	gen_pool_destroy(dec_pool);
 }
 
 MODULE_DESCRIPTION("AMD MCE decoder");
maintainer
#7 lore raw

Re: [PATCH 5/7] EDAC, mce_amd: Convert to seq_buf

Quoted 42 lines · On Fri, 25 Aug 2017 12:24:09 +0200 Borislav Petkov <[email protected]> wrote:

From: Borislav Petkov <[email protected]>

Convert the part which decodes the error description to the sequence buffer facility and thus save ourselves the many printk() invocations building the decoded string.

Also, use a genpool for the string buffers to handle concurrent invocations (and atomic context).

Signed-off-by Borislav Petkov <[email protected]>
---
 drivers/edac/mce_amd.c | 215 +++++++++++++++++++++++++++++--------------------
 1 file changed, 127 insertions(+), 88 deletions(-)
drivers/edac/mce_amd.c +12 -0
index a11a671c7a38..b7c1f8f7e871 100644
@@ -1,3 +1,5 @@
+#include <linux/genalloc.h>
+#include <linux/seq_buf.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 

@@ -34,6 +36,16 @@ void amd_unregister_ecc_decoder(void (*f)(int, struct mce *))
 }
 EXPORT_SYMBOL_GPL(amd_unregister_ecc_decoder);
 
+/* 128 because, well, nice and round - two cachelines. */
+#define ELEM_ORDER	7
+#define ELEM_SIZE	(1 << 7)
+#define DEC_POOL_SIZE	(2 * PAGE_SIZE)
+
+static char __err_buf[DEC_POOL_SIZE];
+static struct gen_pool *dec_pool;
+
+static struct seq_buf sb;
+
 /*
  * string representation for the different MCA reported error types, see F3x48
  * or MSR0000_0411.

drivers/edac/mce_amd.c static bool f12h_mc0_mce(u16 ec, u8 xec) · line 330

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
@@ -315,9 +327,9 @@ static bool f12h_mc0_mce(u16 ec, u8 xec)
 		ret = true;
 
 		if (ll == LL_L2)
-			pr_cont("during L1 linefill from L2.\n");
+			seq_buf_printf(&sb, "during L1 linefill from L2.\n");

Use seq_buf_puts()

drivers/edac/mce_amd.c static bool f12h_mc0_mce(u16 ec, u8 xec) · line 342

Quoted 12 lines
drivers/edac/mce_amd.c +2 -2
 		else if (ll == LL_L1)
-			pr_cont("Data/Tag %s error.\n", R4_MSG(ec));
+			seq_buf_printf(&sb, "Data/Tag %s error.\n", R4_MSG(ec));
 		else
 			ret = false;
 	}

@@ -327,7 +339,7 @@ static bool f12h_mc0_mce(u16 ec, u8 xec)
 static bool f10h_mc0_mce(u16 ec, u8 xec)
 {
 	if (R4(ec) == R4_GEN && LL(ec) == LL_L1) {
-		pr_cont("during data scrub.\n");
+		seq_buf_printf(&sb, "during data scrub.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f10h_mc0_mce(u16 ec, u8 xec) · line 351

Quoted 9 lines
drivers/edac/mce_amd.c +1 -1
 		return true;
 	}
 	return f12h_mc0_mce(ec, xec);

@@ -336,7 +348,7 @@ static bool f10h_mc0_mce(u16 ec, u8 xec)
 static bool k8_mc0_mce(u16 ec, u8 xec)
 {
 	if (BUS_ERROR(ec)) {
-		pr_cont("during system linefill.\n");
+		seq_buf_printf(&sb, "during system linefill.\n");

seq_buf_puts()

Quoted 5 lines
drivers/edac/mce_amd.c +0 -0
 		return true;
 	}
 

@@ -356,14 +368,14 @@ static bool cat_mc0_mce(u16 ec, u8 xec)
 		switch (r4) {

drivers/edac/mce_amd.c static bool cat_mc0_mce(u16 ec, u8 xec) · line 375

Quoted 10 lines
drivers/edac/mce_amd.c +3 -3
 		case R4_DRD:
 		case R4_DWR:
-			pr_cont("Data/Tag parity error due to %s.\n",
-				(r4 == R4_DRD ? "load/hw prf" : "store"));
+			seq_buf_printf(&sb, "Data/Tag parity error due to %s.\n",
+					(r4 == R4_DRD ? "load/hw prf" : "store"));
 			break;
 		case R4_EVICT:
-			pr_cont("Copyback parity error on a tag miss.\n");
+			seq_buf_printf(&sb, "Copyback parity error on a tag miss.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool cat_mc0_mce(u16 ec, u8 xec) · line 378

Quoted 4 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 		case R4_SNOOP:
-			pr_cont("Tag parity error during snoop.\n");
+			seq_buf_printf(&sb, "Tag parity error during snoop.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool cat_mc0_mce(u16 ec, u8 xec) · line 388

Quoted 9 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 		default:
 			ret = false;

@@ -373,17 +385,17 @@ static bool cat_mc0_mce(u16 ec, u8 xec)
 		if ((II(ec) != II_MEM && II(ec) != II_IO) || LL(ec) != LL_LG)
 			return false;
 
-		pr_cont("System read data error on a ");
+		seq_buf_printf(&sb, "System read data error on a ");

seq_buf_puts()

drivers/edac/mce_amd.c static bool cat_mc0_mce(u16 ec, u8 xec) · line 392

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 		switch (r4) {
 		case R4_RD:
-			pr_cont("TLB reload.\n");
+			seq_buf_printf(&sb, "TLB reload.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool cat_mc0_mce(u16 ec, u8 xec) · line 395

Quoted 4 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 		case R4_DWR:
-			pr_cont("store.\n");
+			seq_buf_printf(&sb, "store.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool cat_mc0_mce(u16 ec, u8 xec) · line 398

Quoted 4 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 		case R4_DRD:
-			pr_cont("load.\n");
+			seq_buf_printf(&sb, "load.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 418

Quoted 9 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 		default:
 			ret = false;

@@ -403,28 +415,28 @@ static bool f15h_mc0_mce(u16 ec, u8 xec)
 
 		switch (xec) {
 		case 0x0:
-			pr_cont("Data Array access error.\n");
+			seq_buf_printf(&sb, "Data Array access error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 422

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case 0x1:
-			pr_cont("UC error during a linefill from L2/NB.\n");
+			seq_buf_printf(&sb, "UC error during a linefill from L2/NB.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 427

Quoted 6 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case 0x2:
 		case 0x11:
-			pr_cont("STQ access error.\n");
+			seq_buf_printf(&sb, "STQ access error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 431

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case 0x3:
-			pr_cont("SCB access error.\n");
+			seq_buf_printf(&sb, "SCB access error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 435

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case 0x10:
-			pr_cont("Tag error.\n");
+			seq_buf_printf(&sb, "Tag error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 439

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case 0x12:
-			pr_cont("LDQ access error.\n");
+			seq_buf_printf(&sb, "LDQ access error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 448

Quoted 9 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		default:

@@ -433,12 +445,12 @@ static bool f15h_mc0_mce(u16 ec, u8 xec)
 	} else if (BUS_ERROR(ec)) {
 
 		if (!xec)
-			pr_cont("System Read Data Error.\n");
+			seq_buf_printf(&sb, "System Read Data Error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool f15h_mc0_mce(u16 ec, u8 xec) · line 453

Quoted 7 lines
drivers/edac/mce_amd.c +2 -2
 		else
-			pr_cont(" Internal error condition type %d.\n", xec);
+			seq_buf_printf(&sb, " Internal error condition type %d.\n", xec);
 	} else if (INT_ERROR(ec)) {
 		if (xec <= 0x1f)
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static void decode_mc0_mce(struct mce *m) · line 468

Quoted 9 lines
drivers/edac/mce_amd.c +1 -1
 		else
 			ret = false;
 

@@ -453,13 +465,13 @@ static void decode_mc0_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC0 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC0 Error: ");

seq_buf_puts()

Quoted 12 lines
drivers/edac/mce_amd.c +2 -2
 	/* TLB error signatures are the same across families */
 	if (TLB_ERROR(ec)) {
 		if (TT(ec) == TT_DATA) {
-			pr_cont("%s TLB %s.\n", LL_MSG(ec),
-				((xec == 2) ? "locked miss"
+			seq_buf_printf(&sb, "%s TLB %s.\n", LL_MSG(ec),
+					((xec == 2) ? "locked miss"
 					    : (xec ? "multimatch" : "parity")));
 			return;
 		}

drivers/edac/mce_amd.c static bool k8_mc1_mce(u16 ec, u8 xec) · line 493

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
@@ -478,19 +490,19 @@ static bool k8_mc1_mce(u16 ec, u8 xec)
 		return false;
 
 	if (ll == 0x2)
-		pr_cont("during a linefill from L2.\n");
+		seq_buf_printf(&sb, "during a linefill from L2.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool k8_mc1_mce(u16 ec, u8 xec) · line 497

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 	else if (ll == 0x1) {
 		switch (R4(ec)) {
 		case R4_IRD:
-			pr_cont("Parity error during data load.\n");
+			seq_buf_printf(&sb, "Parity error during data load.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool k8_mc1_mce(u16 ec, u8 xec) · line 501

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case R4_EVICT:
-			pr_cont("Copyback Parity/Victim error.\n");
+			seq_buf_printf(&sb, "Copyback Parity/Victim error.\n");

seq_buf_puts()

drivers/edac/mce_amd.c static bool k8_mc1_mce(u16 ec, u8 xec) · line 505

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
 			break;
 
 		case R4_SNOOP:
-			pr_cont("Tag Snoop error.\n");
+			seq_buf_printf(&sb, "Tag Snoop error.\n");

seq_buf_puts()

OK, I'm getting tired of adding these ;-) You get the idea.

-- Steve

Quoted 382 lines
drivers/edac/mce_amd.c +86 -60
 			break;
 
 		default:

@@ -515,13 +527,13 @@ static bool cat_mc1_mce(u16 ec, u8 xec)
 		return false;
 
 	if (r4 == R4_IRD)
-		pr_cont("Data/tag array parity error for a tag hit.\n");
+		seq_buf_printf(&sb, "Data/tag array parity error for a tag hit.\n");
 	else if (r4 == R4_SNOOP)
-		pr_cont("Tag error during snoop/victimization.\n");
+		seq_buf_printf(&sb, "Tag error during snoop/victimization.\n");
 	else if (xec == 0x0)
-		pr_cont("Tag parity error from victim castout.\n");
+		seq_buf_printf(&sb, "Tag parity error from victim castout.\n");
 	else if (xec == 0x2)
-		pr_cont("Microcode patch RAM parity error.\n");
+		seq_buf_printf(&sb, "Microcode patch RAM parity error.\n");
 	else
 		ret = false;
 

@@ -537,19 +549,19 @@ static bool f15h_mc1_mce(u16 ec, u8 xec)
 
 	switch (xec) {
 	case 0x0 ... 0xa:
-		pr_cont("%s.\n", f15h_mc1_mce_desc[xec]);
+		seq_buf_printf(&sb, "%s.\n", f15h_mc1_mce_desc[xec]);
 		break;
 
 	case 0xd:
-		pr_cont("%s.\n", f15h_mc1_mce_desc[xec-2]);
+		seq_buf_printf(&sb, "%s.\n", f15h_mc1_mce_desc[xec-2]);
 		break;
 
 	case 0x10:
-		pr_cont("%s.\n", f15h_mc1_mce_desc[xec-4]);
+		seq_buf_printf(&sb, "%s.\n", f15h_mc1_mce_desc[xec-4]);
 		break;
 
 	case 0x11 ... 0x15:
-		pr_cont("Decoder %s parity error.\n", f15h_mc1_mce_desc[xec-4]);
+		seq_buf_printf(&sb, "Decoder %s parity error.\n", f15h_mc1_mce_desc[xec-4]);
 		break;
 
 	default:

@@ -563,18 +575,18 @@ static void decode_mc1_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC1 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC1 Error: ");
 
 	if (TLB_ERROR(ec))
-		pr_cont("%s TLB %s.\n", LL_MSG(ec),
-			(xec ? "multimatch" : "parity error"));
+		seq_buf_printf(&sb, "%s TLB %s.\n", LL_MSG(ec),
+				(xec ? "multimatch" : "parity error"));
 	else if (BUS_ERROR(ec)) {
 		bool k8 = (boot_cpu_data.x86 == 0xf && (m->status & BIT_64(58)));
 
-		pr_cont("during %s.\n", (k8 ? "system linefill" : "NB data read"));
+		seq_buf_printf(&sb, "during %s.\n", (k8 ? "system linefill" : "NB data read"));
 	} else if (INT_ERROR(ec)) {
 		if (xec <= 0x3f)
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 		else
 			goto wrong_mc1_mce;
 	} else if (fam_ops->mc1_mce(ec, xec))

@@ -593,27 +605,27 @@ static bool k8_mc2_mce(u16 ec, u8 xec)
 	bool ret = true;
 
 	if (xec == 0x1)
-		pr_cont(" in the write data buffers.\n");
+		seq_buf_printf(&sb, " in the write data buffers.\n");
 	else if (xec == 0x3)
-		pr_cont(" in the victim data buffers.\n");
+		seq_buf_printf(&sb, " in the victim data buffers.\n");
 	else if (xec == 0x2 && MEM_ERROR(ec))
-		pr_cont(": %s error in the L2 cache tags.\n", R4_MSG(ec));
+		seq_buf_printf(&sb, ": %s error in the L2 cache tags.\n", R4_MSG(ec));
 	else if (xec == 0x0) {
 		if (TLB_ERROR(ec))
-			pr_cont("%s error in a Page Descriptor Cache or Guest TLB.\n",
-				TT_MSG(ec));
+			seq_buf_printf(&sb, "%s error in a Page Descriptor Cache or Guest TLB.\n",
+					TT_MSG(ec));
 		else if (BUS_ERROR(ec))
-			pr_cont(": %s/ECC error in data read from NB: %s.\n",
-				R4_MSG(ec), PP_MSG(ec));
+			seq_buf_printf(&sb, ": %s/ECC error in data read from NB: %s.\n",
+					R4_MSG(ec), PP_MSG(ec));
 		else if (MEM_ERROR(ec)) {
 			u8 r4 = R4(ec);
 
 			if (r4 >= 0x7)
-				pr_cont(": %s error during data copyback.\n",
-					R4_MSG(ec));
+				seq_buf_printf(&sb, ": %s error during data copyback.\n",
+						R4_MSG(ec));
 			else if (r4 <= 0x1)
-				pr_cont(": %s parity/ECC error during data "
-					"access from L2.\n", R4_MSG(ec));
+				seq_buf_printf(&sb,
+": %s parity/ECC error during data access from L2.\n", R4_MSG(ec));
 			else
 				ret = false;
 		} else

@@ -630,24 +642,24 @@ static bool f15h_mc2_mce(u16 ec, u8 xec)
 
 	if (TLB_ERROR(ec)) {
 		if (xec == 0x0)
-			pr_cont("Data parity TLB read error.\n");
+			seq_buf_printf(&sb, "Data parity TLB read error.\n");
 		else if (xec == 0x1)
-			pr_cont("Poison data provided for TLB fill.\n");
+			seq_buf_printf(&sb, "Poison data provided for TLB fill.\n");
 		else
 			ret = false;
 	} else if (BUS_ERROR(ec)) {
 		if (xec > 2)
 			ret = false;
 
-		pr_cont("Error during attempted NB data read.\n");
+		seq_buf_printf(&sb, "Error during attempted NB data read.\n");
 	} else if (MEM_ERROR(ec)) {
 		switch (xec) {
 		case 0x4 ... 0xc:
-			pr_cont("%s.\n", f15h_mc2_mce_desc[xec - 0x4]);
+			seq_buf_printf(&sb, "%s.\n", f15h_mc2_mce_desc[xec - 0x4]);
 			break;
 
 		case 0x10 ... 0x14:
-			pr_cont("%s.\n", f15h_mc2_mce_desc[xec - 0x7]);
+			seq_buf_printf(&sb, "%s.\n", f15h_mc2_mce_desc[xec - 0x7]);
 			break;
 
 		default:

@@ -655,7 +667,7 @@ static bool f15h_mc2_mce(u16 ec, u8 xec)
 		}
 	} else if (INT_ERROR(ec)) {
 		if (xec <= 0x3f)
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 		else
 			ret = false;
 	}

@@ -672,29 +684,29 @@ static bool f16h_mc2_mce(u16 ec, u8 xec)
 
 	switch (xec) {
 	case 0x04 ... 0x05:
-		pr_cont("%cBUFF parity error.\n", (r4 == R4_RD) ? 'I' : 'O');
+		seq_buf_printf(&sb, "%cBUFF parity error.\n", (r4 == R4_RD) ? 'I' : 'O');
 		break;
 
 	case 0x09 ... 0x0b:
 	case 0x0d ... 0x0f:
-		pr_cont("ECC error in L2 tag (%s).\n",
-			((r4 == R4_GEN)   ? "BankReq" :
-			((r4 == R4_SNOOP) ? "Prb"     : "Fill")));
+		seq_buf_printf(&sb, "ECC error in L2 tag (%s).\n",
+				((r4 == R4_GEN)   ? "BankReq" :
+				((r4 == R4_SNOOP) ? "Prb"     : "Fill")));
 		break;
 
 	case 0x10 ... 0x19:
 	case 0x1b:
-		pr_cont("ECC error in L2 data array (%s).\n",
-			(((r4 == R4_RD) && !(xec & 0x3)) ? "Hit"  :
-			((r4 == R4_GEN)   ? "Attr" :
-			((r4 == R4_EVICT) ? "Vict" : "Fill"))));
+		seq_buf_printf(&sb, "ECC error in L2 data array (%s).\n",
+				(((r4 == R4_RD) && !(xec & 0x3)) ? "Hit"  :
+				((r4 == R4_GEN)   ? "Attr" :
+				((r4 == R4_EVICT) ? "Vict" : "Fill"))));
 		break;
 
 	case 0x1c ... 0x1d:
 	case 0x1f:
-		pr_cont("Parity error in L2 attribute bits (%s).\n",
-			((r4 == R4_RD)  ? "Hit"  :
-			((r4 == R4_GEN) ? "Attr" : "Fill")));
+		seq_buf_printf(&sb, "Parity error in L2 attribute bits (%s).\n",
+				((r4 == R4_RD)  ? "Hit"  :
+				((r4 == R4_GEN) ? "Attr" : "Fill")));
 		break;
 
 	default:

@@ -709,10 +721,10 @@ static void decode_mc2_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC2 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC2 Error: ");
 
 	if (!fam_ops->mc2_mce(ec, xec))
-		pr_cont(HW_ERR "Corrupted MC2 MCE info?\n");
+		pr_emerg(HW_ERR "Corrupted MC2 MCE info?\n");
 }
 
 static void decode_mc3_mce(struct mce *m)

@@ -726,7 +738,7 @@ static void decode_mc3_mce(struct mce *m)
 		return;
 	}
 
-	pr_emerg(HW_ERR "MC3 Error");
+	seq_buf_printf(&sb, HW_ERR "MC3 Error");
 
 	if (xec == 0x0) {
 		u8 r4 = R4(ec);

@@ -734,7 +746,7 @@ static void decode_mc3_mce(struct mce *m)
 		if (!BUS_ERROR(ec) || (r4 != R4_DRD && r4 != R4_DWR))
 			goto wrong_mc3_mce;
 
-		pr_cont(" during %s.\n", R4_MSG(ec));
+		seq_buf_printf(&sb, " during %s.\n", R4_MSG(ec));
 	} else
 		goto wrong_mc3_mce;
 

@@ -752,7 +764,7 @@ static void decode_mc4_mce(struct mce *m)
 	u8 xec = XEC(m->status, 0x1f);
 	u8 offset = 0;
 
-	pr_emerg(HW_ERR "MC4 Error (node %d): ", node_id);
+	seq_buf_printf(&sb, HW_ERR "MC4 Error (node %d): ", node_id);
 
 	switch (xec) {
 	case 0x0 ... 0xe:

@@ -763,7 +775,7 @@ static void decode_mc4_mce(struct mce *m)
 			if (fam == 0x11)
 				goto wrong_mc4_mce;
 
-			pr_cont("%s.\n", mc4_mce_desc[xec]);
+			seq_buf_printf(&sb, "%s.\n", mc4_mce_desc[xec]);
 
 			if (decode_dram_ecc)
 				decode_dram_ecc(node_id, m);

@@ -773,16 +785,16 @@ static void decode_mc4_mce(struct mce *m)
 
 	case 0xf:
 		if (TLB_ERROR(ec))
-			pr_cont("GART Table Walk data error.\n");
+			seq_buf_printf(&sb, "GART Table Walk data error.\n");
 		else if (BUS_ERROR(ec))
-			pr_cont("DMA Exclusion Vector Table Walk error.\n");
+			seq_buf_printf(&sb, "DMA Exclusion Vector Table Walk error.\n");
 		else
 			goto wrong_mc4_mce;
 		return;
 
 	case 0x19:
 		if (fam == 0x15 || fam == 0x16)
-			pr_cont("Compute Unit Data Error.\n");
+			seq_buf_printf(&sb, "Compute Unit Data Error.\n");
 		else
 			goto wrong_mc4_mce;
 		return;

@@ -795,7 +807,7 @@ static void decode_mc4_mce(struct mce *m)
 		goto wrong_mc4_mce;
 	}
 
-	pr_cont("%s.\n", mc4_mce_desc[xec - offset]);
+	seq_buf_printf(&sb, "%s.\n", mc4_mce_desc[xec - offset]);
 	return;
 
  wrong_mc4_mce:

@@ -811,20 +823,20 @@ static void decode_mc5_mce(struct mce *m)
 	if (fam == 0xf || fam == 0x11)
 		goto wrong_mc5_mce;
 
-	pr_emerg(HW_ERR "MC5 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC5 Error: ");
 
 	if (INT_ERROR(ec)) {
 		if (xec <= 0x1f) {
-			pr_cont("Hardware Assert.\n");
+			seq_buf_printf(&sb, "Hardware Assert.\n");
 			return;
 		} else
 			goto wrong_mc5_mce;
 	}
 
 	if (xec == 0x0 || xec == 0xc)
-		pr_cont("%s.\n", mc5_mce_desc[xec]);
+		seq_buf_printf(&sb, "%s.\n", mc5_mce_desc[xec]);
 	else if (xec <= 0xd)
-		pr_cont("%s parity error.\n", mc5_mce_desc[xec]);
+		seq_buf_printf(&sb, "%s parity error.\n", mc5_mce_desc[xec]);
 	else
 		goto wrong_mc5_mce;
 

@@ -838,12 +850,12 @@ static void decode_mc6_mce(struct mce *m)
 {
 	u8 xec = XEC(m->status, xec_mask);
 
-	pr_emerg(HW_ERR "MC6 Error: ");
+	seq_buf_printf(&sb, HW_ERR "MC6 Error: ");
 
 	if (xec > 0x5)
 		goto wrong_mc6_mce;
 
-	pr_cont("%s parity error.\n", mc6_mce_desc[xec]);
+	seq_buf_printf(&sb, "%s parity error.\n", mc6_mce_desc[xec]);
 	return;
 
  wrong_mc6_mce:

@@ -871,13 +883,13 @@ static void decode_smca_error(struct mce *m)
 	bank_type = hwid->bank_type;
 	ip_name = smca_get_long_name(bank_type);
 
-	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
+	seq_buf_printf(&sb, HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
 
 	/* Only print the decode of valid error codes */
 	if (xec < smca_mce_descs[bank_type].num_descs &&
 			(hwid->xec_bitmap & BIT_ULL(xec))) {
-		pr_emerg(HW_ERR "%s Error: ", ip_name);
-		pr_cont("%s.\n", smca_mce_descs[bank_type].descs[xec]);
+		seq_buf_printf(&sb, HW_ERR "%s Error: ", ip_name);
+		seq_buf_printf(&sb, "%s.\n", smca_mce_descs[bank_type].descs[xec]);
 	}
 
 	if (bank_type == SMCA_UMC && xec == 0 && decode_dram_ecc)

@@ -944,10 +956,21 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 	struct mce *m = (struct mce *)data;
 	unsigned int fam = x86_family(m->cpuid);
 	int ecc;
+	char *dec_buf;
 
 	if (amd_filter_mce(m))
 		return NOTIFY_STOP;
 
+	dec_buf = (void *)gen_pool_alloc(dec_pool, ELEM_SIZE);
+	if (!dec_buf) {
+		pr_warn("Decode buffer full!\n");
+		return NOTIFY_STOP;
+	}
+
+	/* \0 terminated */
+	seq_buf_init(&sb, dec_buf, ELEM_SIZE);
+	seq_buf_clear_buf(&sb);
+
 	pr_emerg(HW_ERR "%s\n", decode_error_status(m));
 
 	pr_emerg(HW_ERR "CPU:%d (%x:%x:%x) MC%d_STATUS[%s|%s|%s|%s|%s",

@@ -1044,6 +1067,10 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
  err_code:
 	amd_decode_err_code(m->status & 0xffff);
 
+	pr_emerg("%.*s\n", (int)sb.len, sb.buffer);
+
+	gen_pool_free(dec_pool, (unsigned long)dec_buf, ELEM_SIZE);
+
 	return NOTIFY_STOP;
 }
 

@@ -1055,6 +1082,7 @@ static struct notifier_block amd_mce_dec_nb = {
 static int __init mce_amd_init(void)
 {
 	struct cpuinfo_x86 *c = &boot_cpu_data;
+	int ret;
 
 	if (c->x86_vendor != X86_VENDOR_AMD)
 		return -ENODEV;

@@ -1122,6 +1150,16 @@ static int __init mce_amd_init(void)
 		goto err_out;
 	}
 
+	dec_pool = gen_pool_create(ELEM_ORDER, -1);
+	if (!dec_pool)
+		goto err_out;
+
+	ret = gen_pool_add(dec_pool, (unsigned long)__err_buf, DEC_POOL_SIZE, -1);
+	if (ret) {
+		gen_pool_destroy(dec_pool);
+		goto err_out;
+	}
+
 	pr_info("MCE: In-kernel MCE decoding enabled.\n");
 
 	mce_register_decode_chain(&amd_mce_dec_nb);

drivers/edac/mce_amd.c static void __exit mce_amd_exit(void) · line 1184

Quoted 7 lines
drivers/edac/mce_amd.c +1 -0
@@ -1140,6 +1178,7 @@ static void __exit mce_amd_exit(void)
 {
 	mce_unregister_decode_chain(&amd_mce_dec_nb);
 	kfree(fam_ops);
+	gen_pool_destroy(dec_pool);
 }
 
 MODULE_DESCRIPTION("AMD MCE decoder");

---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#8 lore raw

PATCH 6/7 EDAC, mce_amd: Issue the decoded info through the TP or printk()

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

... depending on whether we have userspace consumers. Handle the HW_ERR prefix accordingly - for printk() I need to do some strsep() monkey business.

Signed-off-by Borislav Petkov <[email protected]>
---
 drivers/edac/mce_amd.c | 106 +++++++++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 38 deletions(-)
drivers/edac/mce_amd.c +68 -38
index b7c1f8f7e871..41c09d5b81f0 100644
@@ -2,9 +2,12 @@
 #include <linux/seq_buf.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/ras.h>
 
 #include <asm/cpu.h>
 
+#include <trace/events/mce.h>
+
 #include "mce_amd.h"
 
 static struct amd_decoder_ops *fam_ops;

@@ -465,7 +468,7 @@ static void decode_mc0_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	seq_buf_printf(&sb, HW_ERR "MC0 Error: ");
+	seq_buf_printf(&sb, "MC0 Error: ");
 
 	/* TLB error signatures are the same across families */
 	if (TLB_ERROR(ec)) {

@@ -575,7 +578,7 @@ static void decode_mc1_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	seq_buf_printf(&sb, HW_ERR "MC1 Error: ");
+	seq_buf_printf(&sb, "MC1 Error: ");
 
 	if (TLB_ERROR(ec))
 		seq_buf_printf(&sb, "%s TLB %s.\n", LL_MSG(ec),

@@ -721,7 +724,7 @@ static void decode_mc2_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	seq_buf_printf(&sb, HW_ERR "MC2 Error: ");
+	seq_buf_printf(&sb, "MC2 Error: ");
 
 	if (!fam_ops->mc2_mce(ec, xec))
 		pr_emerg(HW_ERR "Corrupted MC2 MCE info?\n");

@@ -738,7 +741,7 @@ static void decode_mc3_mce(struct mce *m)
 		return;
 	}
 
-	seq_buf_printf(&sb, HW_ERR "MC3 Error");
+	seq_buf_printf(&sb, "MC3 Error");
 
 	if (xec == 0x0) {
 		u8 r4 = R4(ec);

@@ -764,7 +767,7 @@ static void decode_mc4_mce(struct mce *m)
 	u8 xec = XEC(m->status, 0x1f);
 	u8 offset = 0;
 
-	seq_buf_printf(&sb, HW_ERR "MC4 Error (node %d): ", node_id);
+	seq_buf_printf(&sb, "MC4 Error (node %d): ", node_id);
 
 	switch (xec) {
 	case 0x0 ... 0xe:

@@ -823,7 +826,7 @@ static void decode_mc5_mce(struct mce *m)
 	if (fam == 0xf || fam == 0x11)
 		goto wrong_mc5_mce;
 
-	seq_buf_printf(&sb, HW_ERR "MC5 Error: ");
+	seq_buf_printf(&sb, "MC5 Error: ");
 
 	if (INT_ERROR(ec)) {
 		if (xec <= 0x1f) {

@@ -850,7 +853,7 @@ static void decode_mc6_mce(struct mce *m)
 {
 	u8 xec = XEC(m->status, xec_mask);
 
-	seq_buf_printf(&sb, HW_ERR "MC6 Error: ");
+	seq_buf_printf(&sb, "MC6 Error: ");
 
 	if (xec > 0x5)
 		goto wrong_mc6_mce;

@@ -883,12 +886,12 @@ static void decode_smca_error(struct mce *m)
 	bank_type = hwid->bank_type;
 	ip_name = smca_get_long_name(bank_type);
 
-	seq_buf_printf(&sb, HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
+	seq_buf_printf(&sb, "%s Extended Error Code: %d\n", ip_name, xec);
 
 	/* Only print the decode of valid error codes */
 	if (xec < smca_mce_descs[bank_type].num_descs &&
 			(hwid->xec_bitmap & BIT_ULL(xec))) {
-		seq_buf_printf(&sb, HW_ERR "%s Error: ", ip_name);
+		seq_buf_printf(&sb, "%s Error: ", ip_name);
 		seq_buf_printf(&sb, "%s.\n", smca_mce_descs[bank_type].descs[xec]);
 	}
 

@@ -950,26 +953,10 @@ static const char *decode_error_status(struct mce *m)
 	return "Corrected error, no action required.";
 }
 
-static int
-amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
+static void __decode_mce(struct mce *m)
 {
-	struct mce *m = (struct mce *)data;
 	unsigned int fam = x86_family(m->cpuid);
 	int ecc;
-	char *dec_buf;
-
-	if (amd_filter_mce(m))
-		return NOTIFY_STOP;
-
-	dec_buf = (void *)gen_pool_alloc(dec_pool, ELEM_SIZE);
-	if (!dec_buf) {
-		pr_warn("Decode buffer full!\n");
-		return NOTIFY_STOP;
-	}
-
-	/* \0 terminated */
-	seq_buf_init(&sb, dec_buf, ELEM_SIZE);
-	seq_buf_clear_buf(&sb);
 
 	pr_emerg(HW_ERR "%s\n", decode_error_status(m));
 

@@ -1011,7 +998,12 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 	pr_cont("]: 0x%016llx\n", m->status);
 
 	if (m->status & MCI_STATUS_ADDRV)
-		pr_emerg(HW_ERR "Error Addr: 0x%016llx\n", m->addr);
+		pr_emerg(HW_ERR "Error Addr: 0x%016llx ", m->addr);
+
+	if (m->tsc)
+		pr_cont("TSC: %llu", m->tsc);
+
+	pr_cont("\n");
 
 	if (boot_cpu_has(X86_FEATURE_SMCA)) {
 		pr_emerg(HW_ERR "IPID: 0x%016llx", m->ipid);

@@ -1020,16 +1012,13 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 			pr_cont(", Syndrome: 0x%016llx", m->synd);
 
 		pr_cont("\n");
-
-		decode_smca_error(m);
-		goto err_code;
 	}
+}
 
-	if (m->tsc)
-		pr_emerg(HW_ERR "TSC: %llu\n", m->tsc);
-
+static void decode_legacy_error(struct mce *m)
+{
 	if (!fam_ops)
-		goto err_code;
+		return;
 
 	switch (m->bank) {
 	case 0:

@@ -1063,11 +1052,52 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 	default:
 		break;
 	}
+}
 
- err_code:
-	amd_decode_err_code(m->status & 0xffff);
+static int
+amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
+{
+	struct mce *m = (struct mce *)data;
+	char *dec_buf;
+
+	if (amd_filter_mce(m))
+		return NOTIFY_STOP;
 
-	pr_emerg("%.*s\n", (int)sb.len, sb.buffer);
+	dec_buf = (void *)gen_pool_alloc(dec_pool, ELEM_SIZE);
+	if (!dec_buf) {
+		pr_warn("Decode buffer full!\n");
+		return NOTIFY_STOP;
+	}
+
+	/* \0 terminated */
+	seq_buf_init(&sb, dec_buf, ELEM_SIZE);
+	seq_buf_clear_buf(&sb);
+
+	if (!ras_userspace_consumers())
+		__decode_mce(m);
+
+	if (boot_cpu_has(X86_FEATURE_SMCA))
+		decode_smca_error(m);
+	else
+		decode_legacy_error(m);
+
+	if (ras_userspace_consumers()) {
+                trace_mce_record(m, sb.buffer);
+	} else {
+		char *l;
+
+		while ((l = strsep(&sb.buffer, "\n"))) {
+			if (!strnlen(l, ELEM_SIZE))
+				break;
+
+			pr_emerg(HW_ERR "%s\n", l);
+		}
+
+		/* Restore original address because strsep() mangles it. */
+		sb.buffer = __err_buf;
+
+		amd_decode_err_code(m->status & 0xffff);
+	}
 
 	gen_pool_free(dec_pool, (unsigned long)dec_buf, ELEM_SIZE);
 

@@ -1076,7 +1106,7 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 
 static struct notifier_block amd_mce_dec_nb = {
 	.notifier_call	= amd_decode_mce,
-	.priority	= MCE_PRIO_EDAC,
+	.priority	= MCE_PRIO_DECODER,
 };
 
 static int __init mce_amd_init(void)
maintainer
#9 lore raw

Re: [PATCH 6/7] EDAC, mce_amd: Issue the decoded info through the TP or printk()

Quoted 29 lines · On Fri, 25 Aug 2017 12:24:10 +0200 Borislav Petkov <[email protected]> wrote:

From: Borislav Petkov <[email protected]>

... depending on whether we have userspace consumers. Handle the HW_ERR prefix accordingly - for printk() I need to do some strsep() monkey business.

Signed-off-by Borislav Petkov <[email protected]>
---
 drivers/edac/mce_amd.c | 106 +++++++++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 38 deletions(-)
drivers/edac/mce_amd.c +3 -0
index b7c1f8f7e871..41c09d5b81f0 100644
@@ -2,9 +2,12 @@
 #include <linux/seq_buf.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/ras.h>
 
 #include <asm/cpu.h>
 
+#include <trace/events/mce.h>
+
 #include "mce_amd.h"
 
 static struct amd_decoder_ops *fam_ops;

drivers/edac/mce_amd.c static void decode_mc0_mce(struct mce *m) · line 471

Quoted 5 lines
drivers/edac/mce_amd.c +1 -1
@@ -465,7 +468,7 @@ static void decode_mc0_mce(struct mce *m)
 	u16 ec = EC(m->status);
 	u8 xec = XEC(m->status, xec_mask);
 
-	seq_buf_printf(&sb, HW_ERR "MC0 Error: ");
+	seq_buf_printf(&sb, "MC0 Error: ");

Use seq_buf_puts() in this patch too.

-- Steve

drivers/edac/mce_amd.c static void decode_mc0_mce(struct mce *m) · line 474

Quoted 3 lines
drivers/edac/mce_amd.c +0 -0
 	/* TLB error signatures are the same across families */
 	if (TLB_ERROR(ec)) {

---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#10 lore raw

PATCH 7/7 x86/mce: Issue the mcelog --ascii message on !AMD

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

mcelog cannot decode AMD MCEs.

Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/mcheck/mce.c +3 -1
index 62bab5d5e96c..3ecc2b2db8f3 100644
@@ -272,7 +272,9 @@ static void __print_mce(struct mce *m)
 static void print_mce(struct mce *m)
 {
 	__print_mce(m);
-	pr_emerg_ratelimited(HW_ERR "Run the above through 'mcelog --ascii'\n");
+
+	if (m->cpuvendor != X86_VENDOR_AMD)
+		pr_emerg_ratelimited(HW_ERR "Run the above through 'mcelog --ascii'\n");
 }
 
 #define PANIC_TIMEOUT 5 /* 5 seconds */
author maintainer
#11 lore raw
In reply to Borislav Petkov
Quoted 1 line · Borislav Petkov

Next step is adding that to rasdaemon.

Ok, below is the dirty version of the changes that need to go into rasdaemon, I'll clean that up later. With it, I get:

# ./rasdaemon -f
overriding event (541) ras:mc_event with new print handler
rasdaemon: ras:mc_event event enabled
rasdaemon: Enabled event ras:mc_event
overriding event (58) mce:mce_record with new print handler
rasdaemon: mce:mce_record event enabled
rasdaemon: Enabled event mce:mce_record
overriding event (542) ras:extlog_mem_event with new print handler
rasdaemon: ras:extlog_mem_event event enabled
rasdaemon: Enabled event ras:extlog_mem_event
rasdaemon: Listening to events for cpus 0 to 7
cpu 07:           <...>-104   [1433913776]     0.000006: mce_record:           2017-08-28 17:41:01 +0200 bank=4, status= 9c7d410092080813, MC4 Error (node 2): DRAM ECC error detected on the NB.
, cpu_type= generic CPU, cpu= 2, socketid= 0, misc= 0, addr= 6d3d483b, , apicid= 0

and looking at it now, I don't need that "MC%d Error..:" thing either.

All queued for the next version.

ras-mce-handler.c +11 -1
index 2e520d3663ac..ff6f4b373e56 100644
@@ -23,6 +23,7 @@
 #include <unistd.h>
 #include <stdint.h>
 #include "libtrace/kbuffer.h"
+#include "libtrace/event-utils.h"
 #include "ras-mce-handler.h"
 #include "ras-record.h"
 #include "ras-logger.h"

@@ -185,6 +186,10 @@ static int detect_cpu(struct ras_events *ras)
 	ret = 0;
 
 	if (!strcmp(mce->vendor, "AuthenticAMD")) {
+
+		ret = 0;
+		goto ret;
+
 		if (mce->family == 15)
 			mce->cputype = CPU_K8;
 		if (mce->family > 15) {

@@ -357,8 +362,9 @@ int ras_mce_event_handler(struct trace_seq *s,
 	unsigned long long val;
 	struct ras_events *ras = context;
 	struct mce_priv *mce = ras->mce_priv;
+	const char *decoded_mce;
 	struct mce_event e;
-	int rc = 0;
+	int rc = 0, len;
 
 	memset(&e, 0, sizeof(e));
 

@@ -422,6 +428,10 @@ int ras_mce_event_handler(struct trace_seq *s,
 	if (rc)
 		return rc;
 
+	decoded_mce = pevent_get_field_raw(s, event, "decoded_str", record, &len, 1);
+	if (decoded_mce)
+		strncpy(e.error_msg, decoded_mce, min(len, 4096));
+
 	if (!*e.error_msg && *e.mcastatus_msg)
 		mce_snprintf(e.error_msg, "%s", e.mcastatus_msg);
author maintainer
#12 lore raw

Btw,

how about communicating stuff to the userspace daemon like this?

This'll simplify a lot of detection in userspace.

Thoughts?

drivers/ras/debugfs.c +5 -1
index 501603057dff..62d3da9d256f 100644
@@ -1,5 +1,7 @@
 #include <linux/debugfs.h>
 
+#include "../../arch/x86/kernel/cpu/mcheck/mce-internal.h"
+
 struct dentry *ras_debugfs_dir;
 
 static atomic_t trace_count = ATOMIC_INIT(0);

@@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
 
 static int trace_show(struct seq_file *m, void *v)
 {
-	return atomic_read(&trace_count);
+	seq_printf(m, "readers:%d\n", atomic_read(&trace_count));
+	seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);
+	return 0;
 }
 
 static int trace_open(struct inode *inode, struct file *file)
#13 lore raw
Quoted 14 lines · Borislav Petkov

Btw,

how about communicating stuff to the userspace daemon like this?

This'll simplify a lot of detection in userspace.

Thoughts?

---

drivers/ras/debugfs.c +0 -0
index 501603057dff..62d3da9d256f 100644

drivers/ras/debugfs.c line 3

Quoted 3 lines
drivers/ras/debugfs.c +1 -0
@@ -1,5 +1,7 @@
 #include <linux/debugfs.h>
 
+#include "../../arch/x86/kernel/cpu/mcheck/mce-internal.h"

FWIW I this looks fishy in arch independent code.
I assume this include is for the definition of the mca_cfg global used in the printf below.

drivers/ras/debugfs.c EXPORT_SYMBOL_GPL(ras_userspace_consumers); · line 18

Quoted 11 lines
drivers/ras/debugfs.c +3 -1
+
 struct dentry *ras_debugfs_dir;
 
 static atomic_t trace_count = ATOMIC_INIT(0);

@@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
 
 static int trace_show(struct seq_file *m, void *v)
 {
-	return atomic_read(&trace_count);
+	seq_printf(m, "readers:%d\n", atomic_read(&trace_count));
+	seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);

do you want to worry about this debugfs interfaces as ABI? debugfs changes have bitten me on one specific OS in irritating ways.

I'm not sure what the word is for debugfs interfaces as ABI these days so this feedback may be not so useful.

--mark

Quoted 14 lines
drivers/ras/debugfs.c +1 -0
+	return 0;
 }
 
 static int trace_open(struct inode *inode, struct file *file)
Signature · 9 lines
-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#14 lore raw
Quoted 3 lines · mark gross

FWIW I this looks fishy in arch independent code. I assume this include is for the definition of the mca_cfg global used in the printf below.

Yeah, it is a bit ad-hoc. I guess I can add an arch_has_decoder() wrapper which each arch can define. That would be cleaner.

Quoted 5 lines

do you want to worry about this debugfs interfaces as ABI? debugfs changes have bitten me on one specific OS in irritating ways.

I'm not sure what the word is for debugfs interfaces as ABI these days so this feedback may be not so useful.

Yeah, the moment something starts using it, it is ABI. The good thing about it is, though, that it can get stuff appended to it, just like tracepoints get fields added. And tools should be able to handle parsing appended lines fine. Removing lines OTOH is always problematic. But we'll make that append-only.

Thx!

author maintainer
#15 lore raw
Quoted 2 lines · Borislav Petkov

Yeah, it is a bit ad-hoc. I guess I can add an arch_has_decoder() wrapper which each arch can define. That would be cleaner.

Something like this, I guess (ontop):

arch/x86/include/asm/mce.h +2 -0
index 67f951f0c840..b4855edac729 100644
@@ -388,4 +388,6 @@ static inline int mce_threshold_remove_device(unsigned int cpu) { return 0; };
 
 #endif
 
+#define ras_arch_has_decoder ras_arch_has_decoder
+bool ras_arch_has_decoder(void);
 #endif /* _ASM_X86_MCE_H */
arch/x86/kernel/cpu/mcheck/mce.c +6 -0
index 3ecc2b2db8f3..6414fa70fd42 100644
@@ -182,6 +182,12 @@ void mce_unregister_decode_chain(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(mce_unregister_decode_chain);
 
+bool ras_arch_has_decoder(void)
+{
+	return mca_cfg.has_decoder;
+}
+EXPORT_SYMBOL_GPL(ras_arch_has_decoder);
+
 static inline u32 ctl_reg(int bank)
 {
 	return MSR_IA32_MCx_CTL(bank);
drivers/ras/debugfs.c +2 -2
index 62d3da9d256f..b368e8ae4738 100644
@@ -1,6 +1,6 @@
 #include <linux/debugfs.h>
 
-#include "../../arch/x86/kernel/cpu/mcheck/mce-internal.h"
+#include <asm/mce.h>
 
 struct dentry *ras_debugfs_dir;
 

@@ -15,7 +15,7 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
 static int trace_show(struct seq_file *m, void *v)
 {
 	seq_printf(m, "readers:%d\n", atomic_read(&trace_count));
-	seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);
+	seq_printf(m, "has decoder:%d\n", ras_arch_has_decoder());
 	return 0;
 }
drivers/ras/debugfs.h +5 -0
index db72e4513191..9c75c4c5b84d 100644
@@ -5,4 +5,9 @@
 
 extern struct dentry *ras_debugfs_dir;
 
+#ifndef ras_arch_has_decoder
+#define ras_arch_has_decoder ras_arch_has_decoder
+static inline bool ras_arch_has_decoder(void)	{ return false; }
+#endif
+
 #endif /* __RAS_DEBUGFS_H__ */
maintainer
#16 lore raw
In reply to mark gross
Quoted 5 lines · On Wed, 30 Aug 2017 14:47:19 -0700 mark gross <[email protected]> wrote:
Quoted 5 lines

struct dentry *ras_debugfs_dir;

static atomic_t trace_count = ATOMIC_INIT(0);

(quoted diff) +0 -0
@@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
Quoted 7 lines

drivers/ras/debugfs.c EXPORT_SYMBOL_GPL(ras_userspace_consumers); · line 18

Quoted 5 lines
drivers/ras/debugfs.c +2 -1
 static int trace_show(struct seq_file *m, void *v)
 {
-	return atomic_read(&trace_count);
+	seq_printf(m, "readers:%d\n", atomic_read(&trace_count));
+	seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);

do you want to worry about this debugfs interfaces as ABI?

It's the old, if a tree falls in the forest issue. If you break the ABI but nobody is around to notice, did it really break?

Quoted 4 lines

debugfs changes have bitten me on one specific OS in irritating ways.

I'm not sure what the word is for debugfs interfaces as ABI these days so this feedback may be not so useful.

I discussed this with Boris before he sent it out. The current code is actually broken. The trace_show() looks to be just a stub for a hack to have userspace tell the kernel it's reading something (the trace_count). The trace_show() function here returns the trace_count, which is just wrong. The seq_file show function is suppose to return less than zero on error, zero on success, or greater than zero if the show is suppose to skip the current field but not error out. This trace_show() function doesn't actually show anything. If you cat the file, it will be blank. Returning zero or greater than zero has the same effect. Which is nothing.

-- Steve
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#17 lore raw
Quoted 17 lines · Steven Rostedt
Quoted 5 lines · On Wed, 30 Aug 2017 14:47:19 -0700 mark gross <[email protected]> wrote:
Quoted 5 lines

struct dentry *ras_debugfs_dir;

static atomic_t trace_count = ATOMIC_INIT(0);

(quoted diff) +0 -0
@@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
Quoted 7 lines

drivers/ras/debugfs.c EXPORT_SYMBOL_GPL(ras_userspace_consumers); · line 18

Quoted 5 lines
drivers/ras/debugfs.c +2 -1
 static int trace_show(struct seq_file *m, void *v)
 {
-	return atomic_read(&trace_count);
+	seq_printf(m, "readers:%d\n", atomic_read(&trace_count));
+	seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);

do you want to worry about this debugfs interfaces as ABI?

Quoted 2 lines

It's the old, if a tree falls in the forest issue. If you break the ABI but nobody is around to notice, did it really break?

perhaps, but what I was trying to point out was: "multi line debugFS printf's like this are very easy to change to append other information and you might want to worry about the ABI implications that could happen in the future."

--mark

Quoted 17 lines
Quoted 5 lines

debugfs changes have bitten me on one specific OS in irritating ways.

I'm not sure what the word is for debugfs interfaces as ABI these days so this feedback may be not so useful.

I discussed this with Boris before he sent it out. The current code is actually broken. The trace_show() looks to be just a stub for a hack to have userspace tell the kernel it's reading something (the trace_count). The trace_show() function here returns the trace_count, which is just wrong. The seq_file show function is suppose to return less than zero on error, zero on success, or greater than zero if the show is suppose to skip the current field but not error out. This trace_show() function doesn't actually show anything. If you cat the file, it will be blank. Returning zero or greater than zero has the same effect. Which is nothing.

Quoted 5 lines

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#18 lore raw
Quoted 6 lines · mark gross
Quoted 2 lines

It's the old, if a tree falls in the forest issue. If you break the ABI but nobody is around to notice, did it really break?

perhaps, but what I was trying to point out was: "multi line debugFS printf's like this are very easy to change to append other information and you might want to worry about the ABI implications that could happen in the future."

Right, as I said to Mark earlier:

"Yeah, the moment something starts using it, it is ABI. The good thing about it is, though, that it can get stuff appended to it, just like tracepoints get fields added. And tools should be able to handle parsing appended lines fine. Removing lines OTOH is always problematic. But we'll make that append-only."

It is basically the same principle as with tracepoints where we can add members easily.