Extended H/W error log driver

Commit aaf9d93 only catches condition check before print, but the similar check is needed during printing CPER error sections.

Signed-off-by Chen, Gong <[email protected]>

To satisfy the necessary of following patches and make related definition

more clear, update some definitions about CPER. No functional changes.

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/apei-internal.h | 12 ++++-----
 drivers/acpi/apei/cper.c          | 46 ++++++++++++++++-----------------
 drivers/acpi/apei/ghes.c          | 54 +++++++++++++++++++--------------------
 include/acpi/actbl1.h             | 14 +++++-----
 include/acpi/ghes.h               |  2 +-
 5 files changed, 64 insertions(+), 64 deletions(-)

drivers/acpi/apei/cper.c +2 -2

index f827f02..b2e4134 100644
@@ -5,7 +5,7 @@
  *	Author: Huang Ying <[email protected]>
  *
  * CPER is the format used to describe platform hardware error by
- * various APEI tables, such as ERST, BERT and HEST etc.
+ * various tables, such as ERST, BERT and HEST etc.
  *
  * For more information about CPER, please refer to Appendix N of UEFI
  * Specification version 2.3.

@@ -248,7 +248,7 @@ static const char *cper_pcie_port_type_strs[] = {
 };
 
 static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
-			    const struct acpi_hest_generic_data *gdata)
+			    const struct acpi_generic_data *gdata)
 {
 	if (pcie->validation_bits & CPER_PCIE_VALID_PORT_TYPE)
 		printk("%s""port_type: %d, %s\n", pfx, pcie->port_type,

drivers/acpi/apei/cper.c +5 -6

 	"resource not accessible",
 	"latent error",
 };
 
-static void apei_estatus_print_section(
-	const char *pfx, const struct acpi_hest_generic_data *gdata, int sec_no)
+static void cper_estatus_print_section(
+	const char *pfx, const struct acpi_generic_data *gdata, int sec_no)
 {
 	uuid_le *sec_type = (uuid_le *)gdata->section_type;
 	__u16 severity;

@@ -302,8 +302,8 @@ static void apei_estatus_print_section(
 	printk("%s""section: %d, severity: %d, %s\n", pfx, sec_no, severity,
 	       cper_severity_str(severity));
 	printk("%s""flags: 0x%02x\n", pfx, gdata->flags);
-	cper_print_bits(pfx, gdata->flags, apei_estatus_section_flag_strs,
-			ARRAY_SIZE(apei_estatus_section_flag_strs));
+	cper_print_bits(pfx, gdata->flags, cper_estatus_section_flag_strs,
+			ARRAY_SIZE(cper_estatus_section_flag_strs));
 	if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
 		printk("%s""fru_id: %pUl\n", pfx, (uuid_le *)gdata->fru_id);
 	if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)

@@ -339,34 +339,34 @@ err_section_too_small:
 	pr_err(FW_WARN "error section length is too small\n");
 }
 
-void apei_estatus_print(const char *pfx,
-			const struct acpi_hest_generic_status *estatus)
+void cper_estatus_print(const char *pfx,

drivers/acpi/apei/cper.c +3 -3

 	severity = estatus->error_severity;
 	printk("%s""severity: %d, %s\n", pfx, severity,
 	       cper_severity_str(severity));
 	data_len = estatus->data_length;
-	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
+	gdata = (struct acpi_generic_data *)(estatus + 1);
 	while (data_len >= sizeof(*gdata)) {
 		gedata_len = gdata->error_data_length;
-		apei_estatus_print_section(pfx, gdata, sec_no);
+		cper_estatus_print_section(pfx, gdata, sec_no);
 		data_len -= gedata_len + sizeof(*gdata);
 		gdata = (void *)(gdata + 1) + gedata_len;
 		sec_no++;
 	}
 }
-EXPORT_SYMBOL_GPL(apei_estatus_print);
+EXPORT_SYMBOL_GPL(cper_estatus_print);

include/acpi/actbl1.h +1 -1

index 0bd750e..3c62a0a 100644
@@ -596,7 +596,7 @@ struct acpi_hest_generic {
 
 /* Generic Error Status block */
 
-struct acpi_hest_generic_status {
+struct acpi_generic_status {
 	u32 block_status;
 	u32 raw_data_offset;
 	u32 raw_data_length;

@@ -606,15 +606,15 @@ struct acpi_hest_generic_status {
 
 /* Values for block_status flags above */

drivers/acpi/apei/cper.c err_section_too_small: · line 350

Quoted 2 lines

drivers/acpi/apei/cper.c +1 -1

-	printk("%s""APEI generic hardware error status\n", pfx);
+	printk("%s""Generic Hardware Error Status\n", pfx);

Btw, what's the story with printk not using KERN_x levels in this file? Why are we falling back to default printk levels for all printks here and shouldn't we rather prioritize them by urgency into, say, KERN_ERR, KERN_INFO, etc?

Quoted 7 lines · Borislav Petkov

drivers/acpi/apei/cper.c err_section_too_small: · line 350

Quoted 2 lines

drivers/acpi/apei/cper.c +1 -1

-	printk("%s""APEI generic hardware error status\n", pfx);
+	printk("%s""Generic Hardware Error Status\n", pfx);

Btw, what's the story with printk not using KERN_x levels in this file? Why are we falling back to default printk levels for all printks here and shouldn't we rather prioritize them by urgency into, say, KERN_ERR, KERN_INFO, etc?

Ignore that - checkpatch complained about it but I kinda missed that we're handing down the prefix.

Date: Tue, 15 Oct 2013 18:57:18 -0700
From: Joe Perches <[email protected]>
To: Borislav Petkov <[email protected]>
Cc: "Chen, Gong" <[email protected]>, [email protected],
[email protected], [email protected]
Subject: Re: [PATCH 2/8] ACPI, CPER: Update cper info
X-Mailer: Evolution 3.6.4-0ubuntu1

Quoted 9 lines · Borislav Petkov

Quoted 7 lines · Borislav Petkov

drivers/acpi/apei/cper.c err_section_too_small: · line 350

Quoted 2 lines

drivers/acpi/apei/cper.c +1 -1

-	printk("%s""APEI generic hardware error status\n", pfx);
+	printk("%s""Generic Hardware Error Status\n", pfx);

Btw, what's the story with printk not using KERN_x levels in this file? Why are we falling back to default printk levels for all printks here and shouldn't we rather prioritize them by urgency into, say, KERN_ERR, KERN_INFO, etc?

Quoted 2 lines

Ignore that - checkpatch complained about it but I kinda missed that we're handing down the prefix.

I think it'd be better to rename pfx to level as that's what printk.h calls them.

Quoted 18 lines · Joe Perches

Date: Tue, 15 Oct 2013 18:57:18 -0700
From: Joe Perches <[email protected]>
To: Borislav Petkov <[email protected]>
Cc: "Chen, Gong" <[email protected]>, [email protected],
[email protected], [email protected]
Subject: Re: [PATCH 2/8] ACPI, CPER: Update cper info
X-Mailer: Evolution 3.6.4-0ubuntu1

Quoted 9 lines · Borislav Petkov

Quoted 7 lines · Borislav Petkov

drivers/acpi/apei/cper.c err_section_too_small: · line 350

Quoted 2 lines

drivers/acpi/apei/cper.c +1 -1

-	printk("%s""APEI generic hardware error status\n", pfx);
+	printk("%s""Generic Hardware Error Status\n", pfx);

Btw, what's the story with printk not using KERN_x levels in this file? Why are we falling back to default printk levels for all printks here and shouldn't we rather prioritize them by urgency into, say, KERN_ERR, KERN_INFO, etc?

Quoted 6 lines

Quoted 2 lines

Ignore that - checkpatch complained about it but I kinda missed that we're handing down the prefix.

I think it'd be better to rename pfx to level as that's what printk.h calls them.

No. pfx includes log level and prefix string both.

To satisfy the necessary of following patches and make related definition more clear, update some definitions about CPER. No functional changes.

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/apei-internal.h | 12 ++++-----
 drivers/acpi/apei/cper.c          | 46 ++++++++++++++++-----------------
 drivers/acpi/apei/ghes.c          | 54 +++++++++++++++++++--------------------
 include/acpi/actbl1.h             | 14 +++++-----
 include/acpi/ghes.h               |  2 +-
 5 files changed, 64 insertions(+), 64 deletions(-)

drivers/acpi/apei/apei-internal.h +0 -0

index f220d64..21ba34a 100644

drivers/acpi/apei/apei-internal.h +0 -0

 {
 	if (estatus->raw_data_length)
 		return estatus->raw_data_offset + \

@@ -135,10 +135,10 @@ static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)
 		return sizeof(*estatus) + estatus->data_length;

Date: Tue, 15 Oct 2013 23:47:23 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 2/8] ACPI, CPER: Update cper info
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 17 lines · Chen Gong

To satisfy the necessary of following patches and make related definition more clear, update some definitions about CPER. No functional changes.

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/apei-internal.h | 12 ++++-----
 drivers/acpi/apei/cper.c          | 46 ++++++++++++++++-----------------
 drivers/acpi/apei/ghes.c          | 54 +++++++++++++++++++--------------------
 include/acpi/actbl1.h             | 14 +++++-----
 include/acpi/ghes.h               |  2 +-
 5 files changed, 64 insertions(+), 64 deletions(-)

drivers/acpi/apei/apei-internal.h +0 -0

index f220d64..21ba34a 100644

drivers/acpi/apei/apei-internal.h struct dentry; · line 125

Quoted 5 lines

drivers/acpi/apei/apei-internal.h +1 -1

@@ -122,11 +122,11 @@ struct dentry;
 struct dentry *apei_get_debugfs_dir(void);
 
 #define apei_estatus_for_each_section(estatus, section)			\
-	for (section = (struct acpi_hest_generic_data *)(estatus + 1);	\
+	for (section = (struct acpi_generic_data *)(estatus + 1);	\

This is a good one to rename, though I wonder if acpi_generic_error_data is more appropriate?

Quoted 3 lines

drivers/acpi/apei/apei-internal.h +0 -0

 	     (void *)section - (void *)estatus < estatus->data_length;	\
 	     section = (void *)(section+1) + section->error_data_length)
 

drivers/acpi/apei/apei-internal.h struct dentry; · line 129

Quoted 2 lines

drivers/acpi/apei/apei-internal.h +1 -1

-static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)
+static inline u32 cper_estatus_len(struct acpi_generic_status *estatus)

Not sure I understand the rationale for these changes - we are still dealing with ACPI/APEI generic error status/data structures. So, why the cper_ prefix?

Quoted 5 lines

drivers/acpi/apei/apei-internal.h +0 -0

 {
 	if (estatus->raw_data_length)
 		return estatus->raw_data_offset + \

@@ -135,10 +135,10 @@ static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)
 		return sizeof(*estatus) + estatus->data_length;

drivers/acpi/apei/apei-internal.h static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus) · line 141

Quoted 10 lines

drivers/acpi/apei/apei-internal.h +4 -4

 }
 
-void apei_estatus_print(const char *pfx,
-			const struct acpi_hest_generic_status *estatus);
-int apei_estatus_check_header(const struct acpi_hest_generic_status *estatus);
-int apei_estatus_check(const struct acpi_hest_generic_status *estatus);
+void cper_estatus_print(const char *pfx,
+			const struct acpi_generic_status *estatus);
+int cper_estatus_check_header(const struct acpi_generic_status *estatus);
+int cper_estatus_check(const struct acpi_generic_status *estatus);

Same here. All the above functions work on ACPI structures...

drivers/acpi/apei/apei-internal.h static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)

Quoted 12 lines

drivers/acpi/apei/apei-internal.h +5 -5

 /* Values for block_status flags above */
 
-#define ACPI_HEST_UNCORRECTABLE             (1)
-#define ACPI_HEST_CORRECTABLE               (1<<1)
-#define ACPI_HEST_MULTIPLE_UNCORRECTABLE    (1<<2)
-#define ACPI_HEST_MULTIPLE_CORRECTABLE      (1<<3)
-#define ACPI_HEST_ERROR_ENTRY_COUNT         (0xFF<<4)	/* 8 bits, error count */
+#define ACPI_GEN_ERR_UC			(1)
+#define ACPI_GEN_ERR_CE			(1<<1)
+#define ACPI_GEN_ERR_MULTI_UC		(1<<2)
+#define ACPI_GEN_ERR_MULTI_CE		(1<<3)
+#define ACPI_GEN_ERR_COUNT_SHIFT	(0xFF<<4) /* 8 bits, error count */

I'd prefer ACPI_GENERIC_ERR_ since ACPI_GEN_ERR sounds far too much like ACPI "Generated" :)

Thanks,
Naveen

Quoted 38 lines · Naveen N. Rao

Date: Tue, 15 Oct 2013 23:47:23 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 2/8] ACPI, CPER: Update cper info
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 17 lines · Chen Gong

To satisfy the necessary of following patches and make related definition more clear, update some definitions about CPER. No functional changes.

Signed-off-by Chen, Gong <[email protected]>

---
  drivers/acpi/apei/apei-internal.h | 12 ++++-----
  drivers/acpi/apei/cper.c          | 46 ++++++++++++++++-----------------
  drivers/acpi/apei/ghes.c          | 54 +++++++++++++++++++--------------------
  include/acpi/actbl1.h             | 14 +++++-----
  include/acpi/ghes.h               |  2 +-
  5 files changed, 64 insertions(+), 64 deletions(-)

drivers/acpi/apei/apei-internal.h +0 -0

index f220d64..21ba34a 100644

drivers/acpi/apei/apei-internal.h struct dentry; · line 125

Quoted 5 lines

drivers/acpi/apei/apei-internal.h +1 -1

@@ -122,11 +122,11 @@ struct dentry;
  struct dentry *apei_get_debugfs_dir(void);
 
  #define apei_estatus_for_each_section(estatus, section)			\
-	for (section = (struct acpi_hest_generic_data *)(estatus + 1);	\
+	for (section = (struct acpi_generic_data *)(estatus + 1);	\

This is a good one to rename, though I wonder if acpi_generic_error_data is more appropriate?

Quoted 3 lines

drivers/acpi/apei/apei-internal.h +0 -0

  	     (void *)section - (void *)estatus < estatus->data_length;	\
  	     section = (void *)(section+1) + section->error_data_length)
 

Quoted 7 lines

drivers/acpi/apei/apei-internal.h struct dentry; · line 129

Quoted 2 lines

drivers/acpi/apei/apei-internal.h +1 -1

-static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)
+static inline u32 cper_estatus_len(struct acpi_generic_status *estatus)

Not sure I understand the rationale for these changes - we are still dealing with ACPI/APEI generic error status/data structures. So, why the cper_ prefix?

Because CPER is not APEI specific, beside APEI, some others like eMCA needs this.

Quoted 55 lines · Chen Gong

Quoted 44 lines · Naveen N. Rao

Date: Tue, 15 Oct 2013 23:47:23 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 2/8] ACPI, CPER: Update cper info
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 21 lines · Chen Gong

To satisfy the necessary of following patches and make related definition
more clear, update some definitions about CPER. No functional changes.

Signed-off-by Chen, Gong <[email protected]>

---
  drivers/acpi/apei/apei-internal.h | 12 ++++-----
  drivers/acpi/apei/cper.c          | 46
++++++++++++++++-----------------
  drivers/acpi/apei/ghes.c          | 54
+++++++++++++++++++--------------------
  include/acpi/actbl1.h             | 14 +++++-----
  include/acpi/ghes.h               |  2 +-
  5 files changed, 64 insertions(+), 64 deletions(-)

drivers/acpi/apei/apei-internal.h +0 -0

index f220d64..21ba34a 100644

drivers/acpi/apei/apei-internal.h struct dentry; · line 126

Quoted 6 lines

drivers/acpi/apei/apei-internal.h +1 -1

@@ -122,11 +122,11 @@ struct dentry;
  struct dentry *apei_get_debugfs_dir(void);
 
  #define apei_estatus_for_each_section(estatus, section)            \
-    for (section = (struct acpi_hest_generic_data *)(estatus +
 1);    \
+    for (section = (struct acpi_generic_data *)(estatus + 1);    \

This is a good one to rename, though I wonder if acpi_generic_error_data is more appropriate?

Quoted 4 lines

drivers/acpi/apei/apei-internal.h +0 -0

           (void *)section - (void *)estatus <
 estatus->data_length;    \
           section = (void *)(section+1) + section->error_data_length)
 

Quoted 9 lines

Quoted 4 lines

drivers/acpi/apei/apei-internal.h +1 -1

-static inline u32 apei_estatus_len(struct acpi_hest_generic_status
 *estatus)
+static inline u32 cper_estatus_len(struct acpi_generic_status

*estatus)

Not sure I understand the rationale for these changes - we are still dealing with ACPI/APEI generic error status/data structures. So, why the cper_ prefix?

Quoted 2 lines

Because CPER is not APEI specific, beside APEI, some others like eMCA needs this.

Right, but even the document you point to refers to these structures as what they are: ACPI Generic error status/data. Clearly, CPER is an incorrect prefix here since CPER/UEFI does *not* seem to have the same structure format.

Regards,
Naveen

This error log driver (a.k.a eMCA driver) is implemented based on
http://www.intel.com/content/www/us/en/architecture-and-technology/enhanced-mca-logging-xeon-paper.html.
After errors are captured, more valuable information can be got with this new enhanced error log driver.

Signed-off-by Chen, Gong <[email protected]>

---
 arch/x86/include/asm/mce.h       |   5 +
 arch/x86/kernel/cpu/mcheck/mce.c |  10 ++
 drivers/acpi/Kconfig             |   8 +
 drivers/acpi/Makefile            |   2 +
 drivers/acpi/acpi_extlog.c       | 339 +++++++++++++++++++++++++++++++++++++++
 drivers/acpi/bus.c               |   3 +-
 include/linux/acpi.h             |   1 +
 7 files changed, 367 insertions(+), 1 deletion(-)
 create mode 100644 drivers/acpi/acpi_extlog.c

arch/x86/include/asm/mce.h +0 -0

index cbe6b9e..f8c33e2 100644

arch/x86/include/asm/mce.h +2 -0

+/* Call the installed extended error log print function */
+extern int (*mce_ext_err_print)(const char *, int cpu, int bank);
 /* Call the installed machine check handler for this CPU setup. */
 extern void (*machine_check_vector)(struct pt_regs *, long error_code);
 void do_machine_check(struct pt_regs *, long);

arch/x86/kernel/cpu/mcheck/mce.c +0 -0

index b3218cd..6802637 100644

arch/x86/kernel/cpu/mcheck/mce.c +4 -0

+
+int (*mce_ext_err_print)(const char *, int, int) = NULL;
+EXPORT_SYMBOL_GPL(mce_ext_err_print);
+
 static DEFINE_MUTEX(mce_chrdev_read_mutex);
 
 #define rcu_dereference_check_mce(p) \

arch/x86/kernel/cpu/mcheck/mce.c +0 -0

 		mce_read_aux(&m, i);
 
 		if (!(flags & MCP_TIMESTAMP))

drivers/acpi/Kconfig +0 -0

index 22327e6..1465fa8 100644

drivers/acpi/Kconfig +1 -0

+
 endif	# ACPI

drivers/acpi/Makefile +2 -0

index cdaf68b..bce34af 100644
@@ -82,3 +82,5 @@ processor-$(CONFIG_CPU_FREQ)	+= processor_perflib.o
 obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
 
 obj-$(CONFIG_ACPI_APEI)		+= apei/
+
+obj-$(CONFIG_ACPI_EXTLOG)	+= acpi_extlog.o

drivers/acpi/acpi_extlog.c new file +8 -0

new file mode 100644
index 0000000..3e3e286
@@ -0,0 +1,339 @@
+/*
+ * Extended Error Log driver
+ *
+ * Copyright (C) 2013 Intel Corp.
+ * Author: Chen, Gong <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version

drivers/acpi/acpi_extlog.c new file +48 -0

+
+#define EXTLOG_DSM_REV		0x0
+#define	EXTLOG_FN_QUERY		0x0
+#define	EXTLOG_FN_ADDR		0x1
+
+#define FLAG_OS_OPTIN		BIT(0)
+#define EXTLOG_QUERY_L1_EXIST	BIT(1)
+#define ELOG_ENTRY_VALID	(1ULL<<63)
+#define ELOG_ENTRY_LEN		0x1000
+
+#define EMCA_BUG \
+	"Can not request iomem region <0x%016llx-0x%016llx> - eMCA disabled\n"
+
+struct extlog_l1_head {
+	u32 ver;	/* Header Version */
+	u32 hdr_len;	/* Header Length */
+	u64 total_len;	/* entire L1 Directory length including this header */
+	u64 elog_base;	/* MCA Error Log Directory base address */
+	u64 elog_len;	/* MCA Error Log Directory length */
+	u32 flags;	/* bit 0 - OS/VMM Opt-in */
+	u8  rev0[12];
+	u32 entries;	/* Valid L1 Directory entries per logical processor */
+	u8  rev1[12];
+};
+
+static u8 extlog_dsm_uuid[] = "663E35AF-CC10-41A4-88EA-5470AF055295";
+
+/* L1 table related physical address */
+static u64 elog_base;
+static size_t elog_size;
+static u64 l1_dirbase;
+static size_t l1_size;
+
+/* L1 table related virtual address */
+static void __iomem *extlog_l1_addr;
+static void __iomem *elog_addr;
+
+static void *elog_buf;
+
+static u64 *l1_entry_base;
+static u32 l1_percpu_entry;
+
+#define ELOG_IDX(cpu, bank) \
+	(cpu_physical_id(cpu) * l1_percpu_entry + (bank))
+
+#define ELOG_ENTRY_DATA(idx) \
+	(*(l1_entry_base + (idx)))
+

drivers/acpi/acpi_extlog.c new file +7 -0

+	idx = ELOG_IDX(cpu, bank);
+	data = ELOG_ENTRY_DATA(idx);
+	if ((data & ELOG_ENTRY_VALID) == 0)
+		return NULL;
+
+	data &= EXT_ELOG_ENTRY_MASK;
+	estatus = (struct acpi_generic_status *)ELOG_ENTRY_ADDR(data);

drivers/acpi/acpi_extlog.c new file +50 -0

+		return 0;
+	}
+
+	return 1;
+}
+
+static int extlog_print(const char *pfx, int cpu, int bank)
+{
+	struct acpi_generic_status *estatus;
+	int rc;
+
+	estatus = extlog_elog_entry_check(cpu, bank);
+	if (estatus == NULL)
+		return -EINVAL;
+
+	memcpy(elog_buf, (void *)estatus, ELOG_ENTRY_LEN);
+	/* clear record status to enable BIOS to update it again */
+	estatus->block_status = 0;
+
+	rc = print_extlog_rcd(pfx, (struct acpi_generic_status *)elog_buf, cpu);
+
+	return rc;
+}
+
+static int extlog_get_dsm(acpi_handle handle, int rev, int func, u64 *ret)
+{
+	struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
+	struct acpi_object_list input;
+	union acpi_object params[4], *obj;
+	u8 uuid[16];
+	int i;
+
+	acpi_str_to_uuid(extlog_dsm_uuid, uuid);
+	input.count = 4;
+	input.pointer = params;
+	params[0].type = ACPI_TYPE_BUFFER;
+	params[0].buffer.length = 16;
+	params[0].buffer.pointer = uuid;
+	params[1].type = ACPI_TYPE_INTEGER;
+	params[1].integer.value = rev;
+	params[2].type = ACPI_TYPE_INTEGER;
+	params[2].integer.value = func;
+	params[3].type = ACPI_TYPE_PACKAGE;
+	params[3].package.count = 0;
+	params[3].package.elements = NULL;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_DSM", &input, &buf)))
+		return -1;
+
+	*ret = 0;

drivers/acpi/acpi_extlog.c new file +12 -0

+
+	return 0;
+}
+
+static bool extlog_get_l1addr(void)
+{
+	acpi_handle handle;
+	u64 ret;
+
+	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
+		goto fail;
+

Date: Fri, 11 Oct 2013 17:24:52 +0200
From: Borislav Petkov <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 3/8] ACPI, x86: Extended error log driver for x86
platform
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 22 lines · Gong

This error log driver (a.k.a eMCA driver) is implemented based on
http://www.intel.com/content/www/us/en/architecture-and-technology/enhanced-mca-logging-xeon-paper.html.
After errors are captured, more valuable information can be got with this new enhanced error log driver.

Signed-off-by Chen, Gong <[email protected]>

---
 arch/x86/include/asm/mce.h       |   5 +
 arch/x86/kernel/cpu/mcheck/mce.c |  10 ++
 drivers/acpi/Kconfig             |   8 +
 drivers/acpi/Makefile            |   2 +
 drivers/acpi/acpi_extlog.c       | 339 +++++++++++++++++++++++++++++++++++++++
 drivers/acpi/bus.c               |   3 +-
 include/linux/acpi.h             |   1 +
 7 files changed, 367 insertions(+), 1 deletion(-)
 create mode 100644 drivers/acpi/acpi_extlog.c

arch/x86/include/asm/mce.h +0 -0

index cbe6b9e..f8c33e2 100644

arch/x86/include/asm/mce.h line 18

Quoted 7 lines

arch/x86/include/asm/mce.h +1 -0

@@ -12,6 +12,7 @@
 #define MCG_CTL_P		(1ULL<<8)    /* MCG_CTL register available */
 #define MCG_EXT_P		(1ULL<<9)    /* Extended registers available */
 #define MCG_CMCI_P		(1ULL<<10)   /* CMCI supported */
+#define MCG_EXT_ERR_LOG		(1ULL<<26)   /* Extended error log supported */
 #define MCG_EXT_CNT_MASK	0xff0000     /* Number of Extended registers */
 #define MCG_EXT_CNT_SHIFT	16
 #define MCG_EXT_CNT(c)		(((c) & MCG_EXT_CNT_MASK) >> MCG_EXT_CNT_SHIFT)

Let's keep them numerically sorted by the bit numbers so put MCG_EXT_ERR_LOG after bit 24, MCG_SER_P.

Besides, this bit should be called MCG_ELOG_P as it is in the docs although your name is much more descriptive than what the hw guys came up with but I know they like to abbreviate to the lowest letter count possible :)

arch/x86/include/asm/mce.h extern void mce_disable_bank(int bank); · line 208

Quoted 5 lines

arch/x86/include/asm/mce.h +1 -0

@@ -204,6 +205,10 @@ extern void mce_disable_bank(int bank);
  * Exception handler
  */
 
+extern spinlock_t mce_elog_lock;

Uuh, I don't think we want to expose the naked spinlock. Rather, we'd need a couple of functions

mce_elog_lock
mce_elog_unlock

which get called by users.

Actually, it would be even better to keep all those details private to acpi_extlog.c and have machine_check_poll() call extlog_print() and in the cases where there's no extlog support, you have an empty extlog_print function.

arch/x86/include/asm/mce.h extern void mce_disable_bank(int bank); · line 209

Quoted 1 line

arch/x86/include/asm/mce.h +1 -0

+extern int (*mce_extlog_support)(void);

Unused leftover?

Quoted 10 lines

arch/x86/include/asm/mce.h +2 -0

+/* Call the installed extended error log print function */
+extern int (*mce_ext_err_print)(const char *, int cpu, int bank);
 /* Call the installed machine check handler for this CPU setup. */
 extern void (*machine_check_vector)(struct pt_regs *, long error_code);
 void do_machine_check(struct pt_regs *, long);

arch/x86/kernel/cpu/mcheck/mce.c +0 -0

index b3218cd..6802637 100644

arch/x86/kernel/cpu/mcheck/mce.c line 52

Quoted 5 lines

arch/x86/kernel/cpu/mcheck/mce.c +2 -0

@@ -48,6 +48,12 @@
 
 #include "mce-internal.h"
 
+DEFINE_SPINLOCK(mce_elog_lock);
+EXPORT_SYMBOL_GPL(mce_elog_lock);

Yeah, private to acpi_extlog and it can be simply 'elog_lock' there, without the "mce_" prefix.

Quoted 8 lines

arch/x86/kernel/cpu/mcheck/mce.c +4 -0

+
+int (*mce_ext_err_print)(const char *, int, int) = NULL;
+EXPORT_SYMBOL_GPL(mce_ext_err_print);
+
 static DEFINE_MUTEX(mce_chrdev_read_mutex);
 
 #define rcu_dereference_check_mce(p) \

arch/x86/kernel/cpu/mcheck/mce.c void machine_check_poll(enum mcp_flags flags, mce_banks_t *b) · line 636

Quoted 7 lines

arch/x86/kernel/cpu/mcheck/mce.c +4 -0

@@ -624,6 +630,10 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		    (m.status & (mca_cfg.ser ? MCI_STATUS_S : MCI_STATUS_UC)))
 			continue;
 
+		spin_lock(&mce_elog_lock);
+		if (mce_ext_err_print)
+			mce_ext_err_print(NULL, m.extcpu, i);
+		spin_unlock(&mce_elog_lock);

private to acpi_extlog.c

Quoted 8 lines

arch/x86/kernel/cpu/mcheck/mce.c +0 -0

 		mce_read_aux(&m, i);
 
 		if (!(flags & MCP_TIMESTAMP))

drivers/acpi/Kconfig +0 -0

index 22327e6..1465fa8 100644

drivers/acpi/Kconfig config ACPI_BGRT · line 377

Quoted 6 lines

drivers/acpi/Kconfig +3 -0

@@ -372,4 +372,12 @@ config ACPI_BGRT
 
 source "drivers/acpi/apei/Kconfig"
 
+config ACPI_EXTLOG
+	tristate "Extended Error Log support"
+	depends on X86 && X86_MCE

I guess we can depend only on X86_MCE

drivers/acpi/Kconfig config ACPI_BGRT · line 381

Quoted 4 lines

drivers/acpi/Kconfig +4 -0

+	default n
+	help
+	  This driver adds support for decoding extended errors from hardware.
+	  which allows the operating system to obtain data from trace.

Let's make this description a bit better:

"Enhanced MCA Logging allows firmware to provide additional error information to system software, synchronous with MCE or CMCI. This driver adds support for that functionality plus an additional special tracepoint which carries that information to userspace."

Quoted 26 lines

drivers/acpi/Kconfig +1 -0

+
 endif	# ACPI

drivers/acpi/Makefile +2 -0

index cdaf68b..bce34af 100644
@@ -82,3 +82,5 @@ processor-$(CONFIG_CPU_FREQ)	+= processor_perflib.o
 obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
 
 obj-$(CONFIG_ACPI_APEI)		+= apei/
+
+obj-$(CONFIG_ACPI_EXTLOG)	+= acpi_extlog.o

drivers/acpi/acpi_extlog.c new file +8 -0

new file mode 100644
index 0000000..3e3e286
@@ -0,0 +1,339 @@
+/*
+ * Extended Error Log driver
+ *
+ * Copyright (C) 2013 Intel Corp.
+ * Author: Chen, Gong <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version

drivers/acpi/acpi_extlog.c line 18

Quoted 10 lines

drivers/acpi/acpi_extlog.c new file +10 -0

+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Please no more of that boilerplate. Simply say that this file is licensed under GPLv2 and that's it.

drivers/acpi/acpi_extlog.c line 30

Quoted 12 lines

drivers/acpi/acpi_extlog.c new file +12 -0

+ */
+
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
+#include <linux/cper.h>
+#include <linux/ratelimit.h>
+#include <asm/mce.h>
+
+#include "apei/apei-internal.h"
+
+#define EXT_ELOG_ENTRY_MASK	0xfffffffffffff /* elog entry address mask */

Am I reading this correctly that these are bits [51:0]?

Btw, we have a GENMASK macro in drivers/edac/amd64_edac.h which can be used for generating contiguous bitmasks:

#define EXT_ELOG_ENTRY_MASK GENMASK(0, 51)

which makes it much more readable.

You could lift it into a more global header like, say, arch/x86/include/asm/edac.h maybe...

Quoted 52 lines

drivers/acpi/acpi_extlog.c new file +52 -0

+
+#define EXTLOG_DSM_REV		0x0
+#define	EXTLOG_FN_QUERY		0x0
+#define	EXTLOG_FN_ADDR		0x1
+
+#define FLAG_OS_OPTIN		BIT(0)
+#define EXTLOG_QUERY_L1_EXIST	BIT(1)
+#define ELOG_ENTRY_VALID	(1ULL<<63)
+#define ELOG_ENTRY_LEN		0x1000
+
+#define EMCA_BUG \
+	"Can not request iomem region <0x%016llx-0x%016llx> - eMCA disabled\n"
+
+struct extlog_l1_head {
+	u32 ver;	/* Header Version */
+	u32 hdr_len;	/* Header Length */
+	u64 total_len;	/* entire L1 Directory length including this header */
+	u64 elog_base;	/* MCA Error Log Directory base address */
+	u64 elog_len;	/* MCA Error Log Directory length */
+	u32 flags;	/* bit 0 - OS/VMM Opt-in */
+	u8  rev0[12];
+	u32 entries;	/* Valid L1 Directory entries per logical processor */
+	u8  rev1[12];
+};
+
+static u8 extlog_dsm_uuid[] = "663E35AF-CC10-41A4-88EA-5470AF055295";
+
+/* L1 table related physical address */
+static u64 elog_base;
+static size_t elog_size;
+static u64 l1_dirbase;
+static size_t l1_size;
+
+/* L1 table related virtual address */
+static void __iomem *extlog_l1_addr;
+static void __iomem *elog_addr;
+
+static void *elog_buf;
+
+static u64 *l1_entry_base;
+static u32 l1_percpu_entry;
+
+#define ELOG_IDX(cpu, bank) \
+	(cpu_physical_id(cpu) * l1_percpu_entry + (bank))
+
+#define ELOG_ENTRY_DATA(idx) \
+	(*(l1_entry_base + (idx)))
+
+#define ELOG_ENTRY_ADDR(phyaddr) \
+	(phyaddr - elog_base + (u8 *)elog_addr)
+
+static struct acpi_generic_status *extlog_elog_entry_check(int cpu, int bank)

drivers/acpi/acpi_extlog.c line 88

Quoted 6 lines

drivers/acpi/acpi_extlog.c new file +6 -0

+{
+	int idx;
+	u64 data;
+	struct acpi_generic_status *estatus;
+
+	BUG_ON(cpu < 0);

What is this supposed to guard? And why such a heavy hammer?

Quoted 7 lines

drivers/acpi/acpi_extlog.c new file +7 -0

+	idx = ELOG_IDX(cpu, bank);
+	data = ELOG_ENTRY_DATA(idx);
+	if ((data & ELOG_ENTRY_VALID) == 0)
+		return NULL;
+
+	data &= EXT_ELOG_ENTRY_MASK;
+	estatus = (struct acpi_generic_status *)ELOG_ENTRY_ADDR(data);

drivers/acpi/acpi_extlog.c line 105

Quoted 10 lines

drivers/acpi/acpi_extlog.c new file +10 -0

+
+	/* if no valid data in elog entry, just return */
+	if (estatus->block_status == 0)
+		return NULL;
+
+	return estatus;
+}
+
+static void __print_extlog_rcd(const char *pfx,
+		struct acpi_generic_status *estatus, int cpu)

Please align args after the opening bracket.

drivers/acpi/acpi_extlog.c line 111

Quoted 6 lines

drivers/acpi/acpi_extlog.c new file +6 -0

+{
+	static atomic_t seqno;
+	unsigned int curr_seqno;
+	char pfx_seq[64];
+
+	if (pfx == NULL) {

	if (!pfx)

drivers/acpi/acpi_extlog.c

Quoted 13 lines

drivers/acpi/acpi_extlog.c new file +13 -0

+		if (estatus->error_severity <= CPER_SEV_CORRECTED)
+			pfx = KERN_INFO;
+		else
+			pfx = KERN_ERR;
+	}
+	curr_seqno = atomic_inc_return(&seqno);
+	snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}", pfx, curr_seqno);
+	printk("%s""Hardware error detected on CPU%d\n", pfx_seq, cpu);
+	cper_estatus_print(pfx_seq, estatus);
+}
+
+static int print_extlog_rcd(const char *pfx,
+		struct acpi_generic_status *estatus, int cpu)a

Ditto.

drivers/acpi/acpi_extlog.c line 132

Quoted 8 lines

drivers/acpi/acpi_extlog.c new file +8 -0

+{
+	/* Not more than 2 messages every 5 seconds */
+	static DEFINE_RATELIMIT_STATE(ratelimit_corrected, 5*HZ, 2);
+	static DEFINE_RATELIMIT_STATE(ratelimit_uncorrected, 5*HZ, 2);
+	struct ratelimit_state *ratelimit;
+
+	if (estatus->error_severity == CPER_SEV_CORRECTED ||
+			(estatus->error_severity == CPER_SEV_INFORMATIONAL))

alignment:

	if ( estatus->error_severity == CPER_SEV_CORRECTED ||
	    (estatus->error_severity == CPER_SEV_INFORMATIONAL))

drivers/acpi/acpi_extlog.c line 137

Quoted 5 lines

drivers/acpi/acpi_extlog.c new file +5 -0

+		ratelimit = &ratelimit_corrected;
+	else
+		ratelimit = &ratelimit_uncorrected;
+	if (__ratelimit(ratelimit)) {
+		__print_extlog_rcd(pfx, estatus, cpu);

Btw, __print_extlog_rcd is only called once, AFAICT. Why not fold it in here?

Quoted 56 lines

drivers/acpi/acpi_extlog.c new file +55 -0

+		return 0;
+	}
+
+	return 1;
+}
+
+static int extlog_print(const char *pfx, int cpu, int bank)
+{
+	struct acpi_generic_status *estatus;
+	int rc;
+
+	estatus = extlog_elog_entry_check(cpu, bank);
+	if (estatus == NULL)
+		return -EINVAL;
+
+	memcpy(elog_buf, (void *)estatus, ELOG_ENTRY_LEN);
+	/* clear record status to enable BIOS to update it again */
+	estatus->block_status = 0;
+
+	rc = print_extlog_rcd(pfx, (struct acpi_generic_status *)elog_buf, cpu);
+
+	return rc;
+}
+
+static int extlog_get_dsm(acpi_handle handle, int rev, int func, u64 *ret)
+{
+	struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
+	struct acpi_object_list input;
+	union acpi_object params[4], *obj;
+	u8 uuid[16];
+	int i;
+
+	acpi_str_to_uuid(extlog_dsm_uuid, uuid);
+	input.count = 4;
+	input.pointer = params;
+	params[0].type = ACPI_TYPE_BUFFER;
+	params[0].buffer.length = 16;
+	params[0].buffer.pointer = uuid;
+	params[1].type = ACPI_TYPE_INTEGER;
+	params[1].integer.value = rev;
+	params[2].type = ACPI_TYPE_INTEGER;
+	params[2].integer.value = func;
+	params[3].type = ACPI_TYPE_PACKAGE;
+	params[3].package.count = 0;
+	params[3].package.elements = NULL;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_DSM", &input, &buf)))
+		return -1;
+
+	*ret = 0;
+	obj = (union acpi_object *)buf.pointer;
+	if (obj->type == ACPI_TYPE_INTEGER)
+		*ret = obj->integer.value;
+	else if (obj->type == ACPI_TYPE_BUFFER) {
+		if (obj->buffer.length <= 8) {

drivers/acpi/acpi_extlog.c line 197

Quoted 5 lines

drivers/acpi/acpi_extlog.c new file +5 -0

+			for (i = 0; i < obj->buffer.length; i++)
+				*ret |= (obj->buffer.pointer[i] << (i * 8));
+		}
+	}
+	kfree(buf.pointer);

I'm guessing this is how acpi does allocation - ACPI_ALLOCATE_BUFFER and caller has to free it?

Quoted 12 lines

drivers/acpi/acpi_extlog.c new file +12 -0

+
+	return 0;
+}
+
+static bool extlog_get_l1addr(void)
+{
+	acpi_handle handle;
+	u64 ret;
+
+	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
+		goto fail;
+

drivers/acpi/acpi_extlog.c line 219

Quoted 10 lines

drivers/acpi/acpi_extlog.c new file +10 -0

+	if (extlog_get_dsm(handle, EXTLOG_DSM_REV, EXTLOG_FN_QUERY, &ret) ||
+			!(ret & EXTLOG_QUERY_L1_EXIST))
+		goto fail;
+
+	if (extlog_get_dsm(handle, EXTLOG_DSM_REV, EXTLOG_FN_ADDR, &ret))
+		goto fail;
+
+	l1_dirbase = ret;
+	/* Spec says L1 directory must be 4K aligned, bail out if it isn't */
+	if (l1_dirbase & ((1 << 12) - 1)) {

		       & (PAGE_SIZE - 1)

drivers/acpi/acpi_extlog.c line 227

Quoted 8 lines

drivers/acpi/acpi_extlog.c new file +8 -0

+		pr_warn(FW_BUG "L1 Directory is invalid at physical %llx\n",
+			l1_dirbase);
+		goto fail;
+	}
+
+	return true;
+fail:
+	return false;

You probably could drop the labels and simply do "return false" in the error cases as it is obvious.

Thanks.

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

But this driver can be loaded as a module. If this module is unloaded, extlog_print is gone. I can't keep such a pointer internally.

This macro is great and I'd loved to use it. But it looks like a litttle bit weird to let eMCA depends on a header file like edac.h. Meanwhile, I found in drivers/video/sis/init.c:3323 we have a very similar macro for this purpose. So how about writing a separate patch to clean it up first?

Because I think in theory "CPU < 0" is impossible. When it hits such situation, it should be a very serious H/W or firmware bug. At least, It think it should be a WARN_ON.

Date: Mon, 14 Oct 2013 12:26:35 +0200
From: Borislav Petkov <[email protected]>
To: Chen Gong <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 3/8] ACPI, x86: Extended error log driver for x86
platform
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 2 lines · Chen Gong

But this driver can be loaded as a module. If this module is unloaded, extlog_print is gone. I can't keep such a pointer internally.

Sure you can - you define a weak extlog_print() function in a compilation unit which is always builtin. Maybe mce.c or so.

Quoted 5 lines

This macro is great and I'd loved to use it. But it looks like a litttle bit weird to let eMCA depends on a header file like edac.h. Meanwhile, I found in drivers/video/sis/init.c:3323 we have a very similar macro for this purpose. So how about writing a separate patch to clean it up first?

Actually, you're right. Those macros are much more generic and could be exposed to the general public by putting them, say into include/include/bitops.h, for example?

Btw, the sis one generates unsigneds (4 byte on x86) while the edac one 8 byte ULLs. So you could call them

GENMASK
and
GENMASK_ULL

How does that sound?

Quoted 4 lines

Because I think in theory "CPU < 0" is impossible. When it hits such situation, it should be a very serious H/W or firmware bug. At least, It think it should be a WARN_ON.

Yes, I think a WARN_ON is much better than the heavy hammer. We can always turn it into a FW_BUG later if it really starts to trigger anywhere...

Thanks.

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

This kind of mask is often unsigned. So how about following mode:

GENMASKL / GENMASKQ
or
GENMASK_L / GENMASK_Q

Quoted 2 lines · Chen Gong

But this driver can be loaded as a module. If this module is unloaded, extlog_print is gone. I can't keep such a pointer internally.

Sure you can - you define a weak extlog_print() function in a compilation unit which is always builtin. Maybe mce.c or so.

Now load the module with the not-weak definition of the function - the module loader would have to go do a relocation fix-up in the base kernel to point to this new function. At module unload it would have to undo that.

Ok, then. How about a reg/dereg functionality, something like what I did in drivers/edac/mce_amd.c, near the top? We're basically handing down a proper function pointer to call and at module unload time we clear it.

exporting a naked function pointer.

This patch adds a new interface to decode memory device (type 17) to help error reporting on DIMMs.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 arch/ia64/kernel/setup.c    |  1 +
 arch/x86/kernel/setup.c     |  1 +
 drivers/firmware/dmi_scan.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dmi.h         |  5 ++++
 4 files changed, 67 insertions(+)

drivers/firmware/dmi_scan.c +14 -0

index fa0affb..ca3619d 100644
@@ -25,6 +25,13 @@ static int dmi_initialized;
 /* DMI system identification string used during boot */
 static char dmi_ids_string[128] __initdata;
 
+static struct dmi_memdev_info {
+	const char *device;
+	const char *bank;
+	u16 handle;
+} *dmi_memdev;
+static int dmi_memdev_nr;
+
 static const char * __init dmi_string_nosave(const struct dmi_header *dm, u8 s)
 {
 	const u8 *bp = ((u8 *) dm) + dm->length;

@@ -322,6 +329,42 @@ static void __init dmi_save_extended_devices(const struct dmi_header *dm)
 	dmi_save_one_device(*d & 0x7f, dmi_string_nosave(dm, *(d - 1)));
 }
 
+static void __init count_mem_devices(const struct dmi_header *dm, void *v)
+{
+	if (dm->type != DMI_ENTRY_MEM_DEVICE)
+		return;
+	dmi_memdev_nr++;
+}
+

Date: Fri, 11 Oct 2013 17:40:48 +0200
From: Borislav Petkov <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 4/8] DMI: Parse memory device (type 17) in SMBIOS
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 5 lines · Gong

This patch adds a new interface to decode memory device (type 17) to help error reporting on DIMMs.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

Reviewed-by Borislav Petkov <[email protected]>

Just a question below:

Quoted 7 lines

---
 arch/ia64/kernel/setup.c    |  1 +
 arch/x86/kernel/setup.c     |  1 +
 drivers/firmware/dmi_scan.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dmi.h         |  5 ++++
 4 files changed, 67 insertions(+)

[ ... ]

Quoted 34 lines

drivers/firmware/dmi_scan.c +19 -0

index fa0affb..ca3619d 100644
@@ -25,6 +25,13 @@ static int dmi_initialized;
 /* DMI system identification string used during boot */
 static char dmi_ids_string[128] __initdata;
 
+static struct dmi_memdev_info {
+	const char *device;
+	const char *bank;
+	u16 handle;
+} *dmi_memdev;
+static int dmi_memdev_nr;
+
 static const char * __init dmi_string_nosave(const struct dmi_header *dm, u8 s)
 {
 	const u8 *bp = ((u8 *) dm) + dm->length;

@@ -322,6 +329,42 @@ static void __init dmi_save_extended_devices(const struct dmi_header *dm)
 	dmi_save_one_device(*d & 0x7f, dmi_string_nosave(dm, *(d - 1)));
 }
 
+static void __init count_mem_devices(const struct dmi_header *dm, void *v)
+{
+	if (dm->type != DMI_ENTRY_MEM_DEVICE)
+		return;
+	dmi_memdev_nr++;
+}
+
+static void __init save_mem_devices(const struct dmi_header *dm, void *v)
+{
+	const char *d = (const char *)dm;
+	static int nr;
+

drivers/firmware/dmi_scan.c static void __init dmi_save_extended_devices(const struct dmi_header *dm) · line 348

Quoted 5 lines

drivers/firmware/dmi_scan.c +5 -0

+	if (dm->type != DMI_ENTRY_MEM_DEVICE)
+		return;
+	if (nr >= dmi_memdev_nr) {
+		pr_warn_once("Too many DIMM entries in SMBIOS table\n");
+		return;

Is this and count_mem_devices() some sort of precaution against insane DMI tables?

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Yes, but we highly expect BIOS manufactors to make is valid and complete.

This patch adds a new interface to decode memory device (type 17) to help error reporting on DIMMs.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 arch/ia64/kernel/setup.c    |  1 +
 arch/x86/kernel/setup.c     |  1 +
 drivers/firmware/dmi_scan.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dmi.h         |  5 ++++
 4 files changed, 67 insertions(+)

In latest UEFI spec(by now it is 2.4) memory error definition for CPER (UEFI 2.4 Appendix N Common Platform Error Record) adds some new fields. These fields help people to locate memory error on actual DIMM location.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

In latest UEFI spec(by now it is 2.4) memory error definition for CPER (UEFI 2.4 Appendix N Common Platform Error Record) adds some new fields. These fields help people to locate memory error on actual DIMM location.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/cper.c | 3 ++-
 include/linux/cper.h     | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

drivers/acpi/apei/cper.c +1 -1

index b2e4134..680230c 100644
@@ -8,7 +8,7 @@
  * various tables, such as ERST, BERT and HEST etc.
  *
  * For more information about CPER, please refer to Appendix N of UEFI
- * Specification version 2.3.
+ * Specification version 2.4.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License version

drivers/acpi/apei/cper.c +0 -0

 };
 
 static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)

include/linux/cper.h +3 -0

index c230494..bd01c9a 100644
@@ -232,6 +232,9 @@ enum {
 #define CPER_MEM_VALID_RESPONDER_ID		0x1000
 #define CPER_MEM_VALID_TARGET_ID		0x2000
 #define CPER_MEM_VALID_ERROR_TYPE		0x4000
+#define CPER_MEM_VALID_RANK_NUMBER		0x8000
+#define CPER_MEM_VALID_CARD_HANDLE		0x10000
+#define CPER_MEM_VALID_MODULE_HANDLE		0x20000
 
 #define CPER_PCIE_VALID_PORT_TYPE		0x0001
 #define CPER_PCIE_VALID_VERSION			0x0002

include/linux/cper.h +0 -0

 };
 
 struct cper_sec_pcie {

Signature · 1 line

1.8.4.rc3

Date: Tue, 15 Oct 2013 22:56:25 +0530
From: "Naveen N. Rao" <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 5/8] ACPI, APEI, CPER: Add UEFI 2.4 support for memory
error
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 25 lines · Chen Gong

In latest UEFI spec(by now it is 2.4) memory error definition for CPER (UEFI 2.4 Appendix N Common Platform Error Record) adds some new fields. These fields help people to locate memory error on actual DIMM location.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/cper.c | 3 ++-
 include/linux/cper.h     | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

drivers/acpi/apei/cper.c +1 -1

index b2e4134..680230c 100644
@@ -8,7 +8,7 @@
  * various tables, such as ERST, BERT and HEST etc.
  *
  * For more information about CPER, please refer to Appendix N of UEFI
- * Specification version 2.3.
+ * Specification version 2.4.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License version

drivers/acpi/apei/cper.c static const char *cper_mem_err_type_strs[] = { · line 194

Quoted 5 lines

drivers/acpi/apei/cper.c +1 -0

@@ -191,6 +191,7 @@ static const char *cper_mem_err_type_strs[] = {
 	"memory sparing",
 	"scrub corrected error",
 	"scrub uncorrected error",
+	"Physical Memory Map-out event",

All small letters to match the rest of the items:
"physical memory map-out event"

Quoted 15 lines

drivers/acpi/apei/cper.c +0 -0

 };
 
 static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)

include/linux/cper.h +3 -0

index c230494..bd01c9a 100644
@@ -232,6 +232,9 @@ enum {
 #define CPER_MEM_VALID_RESPONDER_ID		0x1000
 #define CPER_MEM_VALID_TARGET_ID		0x2000
 #define CPER_MEM_VALID_ERROR_TYPE		0x4000
+#define CPER_MEM_VALID_RANK_NUMBER		0x8000
+#define CPER_MEM_VALID_CARD_HANDLE		0x10000
+#define CPER_MEM_VALID_MODULE_HANDLE		0x20000
 

include/linux/cper.h struct cper_sec_mem_err { · line 356

Quoted 10 lines

include/linux/cper.h +4 -0

 #define CPER_PCIE_VALID_PORT_TYPE		0x0001
 #define CPER_PCIE_VALID_VERSION			0x0002

@@ -347,6 +350,10 @@ struct cper_sec_mem_err {
 	__u64	responder_id;
 	__u64	target_id;
 	__u8	error_type;
+	__u8	reserved;
+	__u16	rank;
+	__u16	mem_array_handle;
+	__u16	mem_dev_handle;

Nit: could you name those fields similar to what the spec has:
card_handle and module_handle, with perhaps a comment to indicate relationship to SMBIOS type 16/17 tables?

Regards,
Naveen

Quoted 6 lines

include/linux/cper.h +0 -0

 };
 
 struct cper_sec_pcie {

Signature · 1 line

1.8.4.rc3

After H/W error happens under FFM enabled mode, lots of information are shown but some important parts like DIMM location missed. This patch is used to show these extra fileds.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/cper.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/acpi/apei/cper.c +3 -0

index 680230c..2a4389f 100644
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/time.h>
 #include <linux/cper.h>
+#include <linux/dmi.h>
 #include <linux/acpi.h>
 #include <linux/pci.h>
 #include <linux/aer.h>

@@ -210,6 +211,8 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 		printk("%s""card: %d\n", pfx, mem->card);
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
 		printk("%s""module: %d\n", pfx, mem->module);
+	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
+		printk("%s""rank: %d\n", pfx, mem->rank);
 	if (mem->validation_bits & CPER_MEM_VALID_BANK)
 		printk("%s""bank: %d\n", pfx, mem->bank);
 	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)

@@ -232,6 +235,15 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 		       etype < ARRAY_SIZE(cper_mem_err_type_strs) ?

After H/W error happens under FFM enabled mode, lots of information are shown but some important parts like DIMM location missed. This patch is used to show these extra fileds.

Original-author: Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/cper.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/acpi/apei/cper.c +12 -0

index 680230c..2a4389f 100644
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/time.h>
 #include <linux/cper.h>
+#include <linux/dmi.h>
 #include <linux/acpi.h>
 #include <linux/pci.h>
 #include <linux/aer.h>

@@ -210,6 +211,8 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 		printk("%s""card: %d\n", pfx, mem->card);
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
 		printk("%s""module: %d\n", pfx, mem->module);
+	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
+		printk("%s""rank: %d\n", pfx, mem->rank);
 	if (mem->validation_bits & CPER_MEM_VALID_BANK)
 		printk("%s""bank: %d\n", pfx, mem->bank);
 	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)

@@ -232,6 +235,15 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 		       etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
 		       cper_mem_err_type_strs[etype] : "unknown");
 	}
+	if (mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE) {
+		const char *bank = NULL, *device = NULL;
+		dmi_memdev_name(mem->mem_dev_handle, &bank, &device);
+		if (bank != NULL && device != NULL)
+			printk("%s""DIMM location: %s %s", pfx, bank, device);
+		else
+			printk("%s""DIMM DMI handle: 0x%.4x", pfx,
+					mem->mem_dev_handle);
+	}
 }
 
 static const char *cper_pcie_port_type_strs[] = {

Signature · 1 line

1.8.4.rc3

Keep up only the most important fields for memory error reporting. The detail information will be moved to perf/trace interface.

Suggested-by Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/cper.c | 42 ++++++++++++++----------------------------
 1 file changed, 14 insertions(+), 28 deletions(-)

drivers/acpi/apei/cper.c +9 -10

index 2a4389f..567410e 100644
@@ -206,29 +206,29 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 		printk("%s""physical_address_mask: 0x%016llx\n",
 		       pfx, mem->physical_addr_mask);
 	if (mem->validation_bits & CPER_MEM_VALID_NODE)
-		printk("%s""node: %d\n", pfx, mem->node);
+		pr_debug("node: %d\n", mem->node);
 	if (mem->validation_bits & CPER_MEM_VALID_CARD)
-		printk("%s""card: %d\n", pfx, mem->card);
+		pr_debug("card: %d\n", mem->card);
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
-		printk("%s""module: %d\n", pfx, mem->module);
+		pr_debug("module: %d\n", mem->module);
 	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
-		printk("%s""rank: %d\n", pfx, mem->rank);
+		pr_debug("rank: %d\n", mem->rank);
 	if (mem->validation_bits & CPER_MEM_VALID_BANK)
-		printk("%s""bank: %d\n", pfx, mem->bank);
+		pr_debug("bank: %d\n", mem->bank);
 	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)
-		printk("%s""device: %d\n", pfx, mem->device);
+		pr_debug("device: %d\n", mem->device);
 	if (mem->validation_bits & CPER_MEM_VALID_ROW)
-		printk("%s""row: %d\n", pfx, mem->row);
+		pr_debug("row: %d\n", mem->row);
 	if (mem->validation_bits & CPER_MEM_VALID_COLUMN)
-		printk("%s""column: %d\n", pfx, mem->column);
+		pr_debug("column: %d\n", mem->column);
 	if (mem->validation_bits & CPER_MEM_VALID_BIT_POSITION)
-		printk("%s""bit_position: %d\n", pfx, mem->bit_pos);
+		pr_debug("bit_position: %d\n", mem->bit_pos);
 	if (mem->validation_bits & CPER_MEM_VALID_REQUESTOR_ID)
-		printk("%s""requestor_id: 0x%016llx\n", pfx, mem->requestor_id);

Date: Fri, 11 Oct 2013 18:02:08 +0200
From: Borislav Petkov <[email protected]>
To: "Chen, Gong" <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Subject: Re: [PATCH 7/8] ACPI, APEI, CPER: Cleanup CPER memory error output
format
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 46 lines · Gong

Keep up only the most important fields for memory error reporting. The detail information will be moved to perf/trace interface.

Suggested-by Tony Luck <[email protected]>

Signed-off-by Chen, Gong <[email protected]>

---
 drivers/acpi/apei/cper.c | 42 ++++++++++++++----------------------------
 1 file changed, 14 insertions(+), 28 deletions(-)

drivers/acpi/apei/cper.c +9 -10

index 2a4389f..567410e 100644
@@ -206,29 +206,29 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 		printk("%s""physical_address_mask: 0x%016llx\n",
 		       pfx, mem->physical_addr_mask);
 	if (mem->validation_bits & CPER_MEM_VALID_NODE)
-		printk("%s""node: %d\n", pfx, mem->node);
+		pr_debug("node: %d\n", mem->node);
 	if (mem->validation_bits & CPER_MEM_VALID_CARD)
-		printk("%s""card: %d\n", pfx, mem->card);
+		pr_debug("card: %d\n", mem->card);
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
-		printk("%s""module: %d\n", pfx, mem->module);
+		pr_debug("module: %d\n", mem->module);
 	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
-		printk("%s""rank: %d\n", pfx, mem->rank);
+		pr_debug("rank: %d\n", mem->rank);
 	if (mem->validation_bits & CPER_MEM_VALID_BANK)
-		printk("%s""bank: %d\n", pfx, mem->bank);
+		pr_debug("bank: %d\n", mem->bank);
 	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)
-		printk("%s""device: %d\n", pfx, mem->device);
+		pr_debug("device: %d\n", mem->device);
 	if (mem->validation_bits & CPER_MEM_VALID_ROW)
-		printk("%s""row: %d\n", pfx, mem->row);
+		pr_debug("row: %d\n", mem->row);
 	if (mem->validation_bits & CPER_MEM_VALID_COLUMN)
-		printk("%s""column: %d\n", pfx, mem->column);
+		pr_debug("column: %d\n", mem->column);
 	if (mem->validation_bits & CPER_MEM_VALID_BIT_POSITION)
-		printk("%s""bit_position: %d\n", pfx, mem->bit_pos);
+		pr_debug("bit_position: %d\n", mem->bit_pos);
 	if (mem->validation_bits & CPER_MEM_VALID_REQUESTOR_ID)
-		printk("%s""requestor_id: 0x%016llx\n", pfx, mem->requestor_id);

drivers/acpi/apei/cper.c static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem) · line 234

Quoted 10 lines

drivers/acpi/apei/cper.c +3 -2

+		pr_debug("requestor_id: 0x%016llx\n", mem->requestor_id);
 	if (mem->validation_bits & CPER_MEM_VALID_RESPONDER_ID)
-		printk("%s""responder_id: 0x%016llx\n", pfx, mem->responder_id);
+		pr_debug("responder_id: 0x%016llx\n", mem->responder_id);
 	if (mem->validation_bits & CPER_MEM_VALID_TARGET_ID)
-		printk("%s""target_id: 0x%016llx\n", pfx, mem->target_id);
+		pr_debug("target_id: 0x%016llx\n", mem->target_id);
 	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE) {
 		u8 etype = mem->error_type;
 		printk("%s""error_type: %d, %s\n", pfx, etype,

Hmm, so this cper_print_mem is called for CPER_SEC_PLATFORM_MEM section type.

With the change above, the other caller __ghes_print_estatus() won't see the error messages if they're debug. Do we want that?

Signature · 5 lines

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Because most of data in CPER are empty or unimportant.

Quoted 1 line · Chen Gong

Because most of data in CPER are empty or unimportant.

It is not about whether it is important or not - the question is whether changing existing functionality which someone might rely upon is a problem here? Someone might be expecting exactly those messages to appear in dmesg.

Btw, what is the real reason for this patch, to save yourself the output in dmesg? If so, you can disable this output when eMCA module has been loaded but leave it intact otherwise.

This is an excellent idea - if the full data is being logged via TRACE, then we can drop to virtually nothing on the console (just have something similar to the "Machine check events logged" message so that people will get a tip that they might want to go dig into other logs). Maybe something like: %d corrected memory errors\n", count [rate limited]

But we'd have to make sure that the existing user(s) of this code also have a TRACE path.

It is basically the same idea as with the ras daemon - if we have a userspace consumer of ras trace events, we disable dmesg output. So the decision will be left to the userspace tool to disable dmesg output as a last step of its initialization.

Do you have a suggested mechanism for this disabling of dmesg?

Date: Mon, 14 Oct 2013 23:50:47 +0200
From: Borislav Petkov <[email protected]>
To: Tony Luck <[email protected]>
Cc: Chen Gong <[email protected]>, Linux Kernel Mailing List
<[email protected]>, linux-acpi <[email protected]>, Lance Ortiz <[email protected]>, "Naveen N. Rao" <[email protected]>
Subject: Re: [PATCH 7/8] ACPI, APEI, CPER: Cleanup CPER memory error output
format
User-Agent: Mutt/1.5.21 (2010-09-15)

Quoted 1 line · Tony Luck

Do you have a suggested mechanism for this disabling of dmesg?

Hmm, how about a 64-bit flag variable (we can use the remaining bits for other stuff later) called x86_ras_flags which is private to arch/x86/ras/core.c (a new file)...

[ btw, I'm thinking of something similar to efi's x86_efi_facility which we nicely query with test_bit and set with set_bit and clear_bit, etc, etc ]

Also, look at arch/x86/platform/efi/efi.c::efi_enabled() how it hides the actual variable and we can do something similar so that eMCA and other users like cper.c can do

apei_estatus_print_section:

	if (!ras_tracepoint_enabled())
		cper_print_mem(...)

We set the bit in x86_ras_flags from, say, debugfs, i.e., a userspace tool sets it and from that moment on all RAS output is rerouted to the trace event. I.e., the output for which there is a trace event...

How does that look like?

Looks fine to me. But a little bit out of current patch series. How about adding such interfaces after this patch series is merged?

And from your words, it looks like you prefer to reserve all current fields to avoid breaking user space things. IOW, drop patch [7/8]

and use another patch with above idea to get the same purpose. This is what you want, Boris?

Quoted 7 lines · Borislav Petkov

Quoted 1 line · Chen Gong

Because most of data in CPER are empty or unimportant.

It is not about whether it is important or not - the question is whether changing existing functionality which someone might rely upon is a problem here? Someone might be expecting exactly those messages to appear in dmesg.

Pulling in a couple more people who have been touching error reporting code in the last year or so (Hi Lance, Naveen ... feel free to drag more people to look at this thread).

I prodded Chen Gong in to make this change because our console messages are way to verbose (and scary) for simple corrected errors. There are 18 fields in the memory error section (as of UEFI 2.4 ... more are likely to be added because there are issues that some of the 16-bit wide fields are too small to handle increased internal values in modern DIMMs). Whether you print that one item per line, or a few very long lines - it is way more information
than the average user will ever want or need to see.

Quoted 7 lines · Borislav Petkov

Quoted 1 line · Chen Gong

Because most of data in CPER are empty or unimportant.

It is not about whether it is important or not - the question is whether changing existing functionality which someone might rely upon is a problem here? Someone might be expecting exactly those messages to appear in dmesg.

If so, how will disabling dmesg output and switching to a trace event help? The user-space program will still break unless they add support for that trace event, right?

I'm not sure if we actually provide guarantees w.r.t kernel log messages. The issue in this specific scenario is the sheer verbosity of the log messages.

Once we exposed it to userspace, we cannot simply assume that nothing is using it.

No guarantees are made about dmesg output.

Quoted 1 line · Joe Perches

No guarantees are made about dmesg output.

I'm with Joe here. Current users that parse dmesg have grumbled a bit that format changes - but they know that's the way life is. Perhaps they'll be too busy cheering about the TRACE formats that we are going to give them that they won't whine too much about this.

drivers/acpi/extlog_trace.h new file +1 -0

new file mode 100644
index 0000000..21f0887
@@ -0,0 +1,77 @@
+#if !defined(_TRACE_EXTLOG_H) || defined(TRACE_HEADER_MULTI_READ)

drivers/acpi/extlog_trace.h new file +66 -0

+ *
+ * These events are generated when hardware detects a corrected or
+ * uncorrected event.
+ *
+ */
+
+/* memory trace event */
+
+#define LOC_LEN		512
+#define MSG_LEN		((LOC_LEN) * 2)
+
+TRACE_EVENT(extlog_mem_event,
+	TP_PROTO(u32 etype,
+		char *dimm_loc,
+		const uuid_le *fru_id,
+		char *fru_text,
+		u64 error_count,
+		u32 severity,
+		u64 phy_addr,
+		char *mem_loc),
+
+	TP_ARGS(etype, dimm_loc, fru_id, fru_text, error_count, severity,
+		phy_addr, mem_loc),
+
+	TP_STRUCT__entry(
+		__field(u32, etype)
+		__dynamic_array(char, dimm_info, LOC_LEN)
+		__field(u64, error_count)
+		__field(u32, severity)
+		__dynamic_array(char, msg, MSG_LEN)
+	),
+
+	TP_fast_assign(
+		__entry->error_count = error_count;
+		__entry->severity = severity;
+		__entry->etype = etype;
+		if (dimm_loc[0] != '\0')
+			snprintf(__get_dynamic_array(dimm_info), LOC_LEN - 1,
+				"on %s", dimm_loc);
+		else
+			__assign_str(dimm_info, "");
+		if (phy_addr != 0)
+			snprintf(__get_dynamic_array(msg), MSG_LEN - 1,
+				"(FRU: %pUl %.20s physical addr: 0x%016llx%s)",
+				fru_id, fru_text, phy_addr, mem_loc);
+		else
+			__assign_str(msg, "");
+	),
+
+	TP_printk("%llu %s error%s:%s %s%s",
+			__entry->error_count,
+			cper_severity_str(__entry->severity),
+			__entry->error_count > 1 ? "s" : "",
+			cper_mem_err_type_str(__entry->etype),
+			__get_str(dimm_info),
+			__get_str(msg))
+);
+
+#endif /* _TRACE_EXTLOG_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE extlog_trace
+#include <trace/define_trace.h>

include/linux/cper.h +2 -0

index bd01c9a..c00eb55 100644
@@ -395,6 +395,8 @@ struct cper_sec_pcie {
 #pragma pack()
 
 u64 cper_next_record_id(void);
+const char *cper_severity_str(unsigned int);
+const char *cper_mem_err_type_str(unsigned int);
 void cper_print_bits(const char *prefix, unsigned int bits,
 		     const char *strs[], unsigned int strs_size);
 

Signature · 1 line

1.8.4.rc3