[PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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

-- 
2.13.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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 <bp@suse.de>
---
 arch/x86/include/asm/mce.h       | 4 +++-
 arch/x86/kernel/cpu/mcheck/mce.c | 9 ++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 181264989db5..ba4c99ace544 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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,
 };
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 3b413065c613..dcf75f40e031 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -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);
 
-- 
2.13.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/cpu/mcheck/mce.c |  3 ++-
 include/trace/events/mce.h       | 11 +++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index dcf75f40e031..62bab5d5e96c 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -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);
 
diff --git a/include/trace/events/mce.h b/include/trace/events/mce.h
index 70f02149808c..82ff4014fab1 100644
--- a/include/trace/events/mce.h
+++ b/include/trace/events/mce.h
@@ -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 */
-- 
2.13.0

[PATCH 3/7] seq_buf: Add seq_buf_clear_buf()

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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 <bp@suse.de>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/seq_buf.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index fb7eb9ccb1cd..8705a482e76c 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -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)
 {
-- 
2.13.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Will be used in a module in a later patch.

Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/seq_buf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index cb18469e1f49..b5217cf1824d 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -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
 /**
-- 
2.13.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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 <bp@suse.de>
---
 drivers/edac/mce_amd.c | 215 +++++++++++++++++++++++++++++--------------------
 1 file changed, 127 insertions(+), 88 deletions(-)

diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index a11a671c7a38..b7c1f8f7e871 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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");
-- 
2.13.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

... 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 <bp@suse.de>
---
 drivers/edac/mce_amd.c | 106 +++++++++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 38 deletions(-)

diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index b7c1f8f7e871..41c09d5b81f0 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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)
-- 
2.13.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

mcelog cannot decode AMD MCEs.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 62bab5d5e96c..3ecc2b2db8f3 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -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 */
-- 
2.13.0

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

[Steven Rostedt]

On Fri, 25 Aug 2017 12:24:08 +0200
Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> Will be used in a module in a later patch.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  lib/seq_buf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/seq_buf.c b/lib/seq_buf.c
> index cb18469e1f49..b5217cf1824d 100644
> --- a/lib/seq_buf.c
> +++ b/lib/seq_buf.c
> @@ -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

>  
>  #ifdef CONFIG_BINARY_PRINTF
>  /**

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

[Steven Rostedt]

On Fri, 25 Aug 2017 12:24:09 +0200
Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> 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 <bp@suse.de>
> ---
>  drivers/edac/mce_amd.c | 215 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 127 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
> index a11a671c7a38..b7c1f8f7e871 100644
> --- a/drivers/edac/mce_amd.c
> +++ b/drivers/edac/mce_amd.c
> @@ -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");

Use seq_buf_puts()


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

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

>  		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");

seq_buf_puts()

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

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

>  
>  		switch (r4) {
>  		case R4_RD:
> -			pr_cont("TLB reload.\n");
> +			seq_buf_printf(&sb, "TLB reload.\n");

seq_buf_puts()

>  			break;
>  		case R4_DWR:
> -			pr_cont("store.\n");
> +			seq_buf_printf(&sb, "store.\n");

seq_buf_puts()

>  			break;
>  		case R4_DRD:
> -			pr_cont("load.\n");
> +			seq_buf_printf(&sb, "load.\n");

seq_buf_puts()

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

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

>  			break;
>  
>  		case 0x2:
>  		case 0x11:
> -			pr_cont("STQ access error.\n");
> +			seq_buf_printf(&sb, "STQ access error.\n");

seq_buf_puts()

>  			break;
>  
>  		case 0x3:
> -			pr_cont("SCB access error.\n");
> +			seq_buf_printf(&sb, "SCB access error.\n");

seq_buf_puts()

>  			break;
>  
>  		case 0x10:
> -			pr_cont("Tag error.\n");
> +			seq_buf_printf(&sb, "Tag error.\n");

seq_buf_puts()

>  			break;
>  
>  		case 0x12:
> -			pr_cont("LDQ access error.\n");
> +			seq_buf_printf(&sb, "LDQ access error.\n");

seq_buf_puts()

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

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

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

>  
>  	/* 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");

seq_buf_puts()

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

>  			break;
>  
>  		case R4_EVICT:
> -			pr_cont("Copyback Parity/Victim error.\n");
> +			seq_buf_printf(&sb, "Copyback Parity/Victim error.\n");

seq_buf_puts()

>  			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

>  			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");

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

[Steven Rostedt]

On Fri, 25 Aug 2017 12:24:10 +0200
Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> ... 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 <bp@suse.de>
> ---
>  drivers/edac/mce_amd.c | 106 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 68 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
> index b7c1f8f7e871..41c09d5b81f0 100644
> --- a/drivers/edac/mce_amd.c
> +++ b/drivers/edac/mce_amd.c
> @@ -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: ");

Use seq_buf_puts() in this patch too.

-- Steve

>  
>  	/* TLB error signatures are the same across families */
>  	if (TLB_ERROR(ec)) {

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Borislav Petkov]

On Fri, Aug 25, 2017 at 12:24:04PM +0200, Borislav Petkov wrote:
> 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.

---
diff --git a/ras-mce-handler.c b/ras-mce-handler.c
index 2e520d3663ac..ff6f4b373e56 100644
--- a/ras-mce-handler.c
+++ b/ras-mce-handler.c
@@ -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);
 
-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Borislav Petkov]

Btw,

how about communicating stuff to the userspace daemon like this?

This'll simplify a lot of detection in userspace.

Thoughts?

---
diff --git a/drivers/ras/debugfs.c b/drivers/ras/debugfs.c
index 501603057dff..62d3da9d256f 100644
--- a/drivers/ras/debugfs.c
+++ b/drivers/ras/debugfs.c
@@ -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)

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[mark gross]

On Wed, Aug 30, 2017 at 01:48:43PM +0200, Borislav Petkov wrote:
> Btw,
> 
> how about communicating stuff to the userspace daemon like this?
> 
> This'll simplify a lot of detection in userspace.
> 
> Thoughts?
> 
> ---
> diff --git a/drivers/ras/debugfs.c b/drivers/ras/debugfs.c
> index 501603057dff..62d3da9d256f 100644
> --- a/drivers/ras/debugfs.c
> +++ b/drivers/ras/debugfs.c
> @@ -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. 


> +
>  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

> +	return 0;
>  }
>  
>  static int trace_open(struct inode *inode, struct file *file)
> 
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Borislav Petkov]

On Wed, Aug 30, 2017 at 02:47:19PM -0700, mark gross wrote:
> 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.

> 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!

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Steven Rostedt]

On Wed, 30 Aug 2017 14:47:19 -0700
mark gross <mgross@linux.intel.com> wrote:


> >  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?

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?

> 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

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Borislav Petkov]

On Thu, Aug 31, 2017 at 12:02:54AM +0200, Borislav Petkov wrote:
> 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):

---
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 67f951f0c840..b4855edac729 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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 */
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 3ecc2b2db8f3..6414fa70fd42 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -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);
diff --git a/drivers/ras/debugfs.c b/drivers/ras/debugfs.c
index 62d3da9d256f..b368e8ae4738 100644
--- a/drivers/ras/debugfs.c
+++ b/drivers/ras/debugfs.c
@@ -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;
 }
 
diff --git a/drivers/ras/debugfs.h b/drivers/ras/debugfs.h
index db72e4513191..9c75c4c5b84d 100644
--- a/drivers/ras/debugfs.h
+++ b/drivers/ras/debugfs.h
@@ -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__ */

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[mark gross]

On Wed, Aug 30, 2017 at 07:30:58PM -0400, Steven Rostedt wrote:
> On Wed, 30 Aug 2017 14:47:19 -0700
> mark gross <mgross@linux.intel.com> wrote:
> 
> 
> > >  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?
> 
> 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
> 
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Borislav Petkov]

On Sun, Sep 03, 2017 at 04:37:04PM -0700, mark gross wrote:
> > 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.

-- 
Regards/Gruss,
    Boris.

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