[PATCH v2 0/9] Extended H/W error log driver

[PATCH v2 0/9] Extended H/W error log driver

[Chen, Gong]

[PATCH v2 1/9] ACPI, APEI, CPER: Fix status check during error printing
[PATCH v2 2/9] ACPI, CPER: Update cper info
[PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro
[PATCH v2 4/9] ACPI, x86: Extended error log driver for x86 platform
[PATCH v2 5/9] DMI: Parse memory device (type 17) in SMBIOS
[PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error
[PATCH v2 7/9] ACPI, APEI, CPER: Enhance memory reporting capability
[PATCH v2 8/9] ACPI, APEI, CPER: Cleanup CPER memory error output format
[PATCH v2 9/9] ACPI / trace: Add trace interface for eMCA driver

This patch series adds an enhanced MCA event logging driver provided by Intel.
Please refer to this link: htpp://www.intel.com/content/www/us/en/architecture-and-technology/enhanced-mca-logging-xeon-paper.html

Certain usages such as Predictive Failure Analysis (PFA) require more
information about the error than what can be described in processor
machine check banks. Most server processors log additional information
about the error in processor uncore registers. Since the addresses
and layout of these registers vary widely from one processor to another,
system software cannot readily make use of them. To complicate matters
further, some of the additionalerror information cannot be constructed
without detailed knowledge about platform topology. This enhanced MCA
logging driver allows firmware to provide additional error information
to MCE/CMCI handler and thus addresses this gap.

After applying this patch series, when a memory corrected error happens,
we can get following information:

dmesg output:

[  949.545817] {1}Hardware error detected on CPU15
[  949.549786] {1}event severity: corrected
[  949.549786] {1} Error 0, type: corrected
[  949.549786] {1}  section_type: memory error
[  949.549786] {1}  physical_address: 0x0000001057eb0000
[  949.549786] {1}  DIMM location: Memriser3 CHANNEL A DIMM 0
[  949.549786] {1}Above error has been corrected by h/w and require no further action
[  949.549786] mce: [Hardware Error]: Machine check events logged
[ 1010.902124] {2}Hardware error detected on CPU15
[ 1010.906064] {2}event severity: corrected
[ 1010.906064] {2} Error 0, type: corrected
[ 1010.906064] {2}  section_type: memory error
[ 1010.906064] {2}  physical_address: 0x0000001057eb0000
[ 1010.906064] {2}  DIMM location: Memriser3 CHANNEL A DIMM 0
[ 1010.906064] {2}Above error has been corrected by h/w and require no further action
[ 1010.906064] mce: [Hardware Error]: Machine check events logged




trace output:

# tracer: nop
#
# entries-in-buffer/entries-written: 2/2   #P:120
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
          <idle>-0     [015] d.h.   951.584641: extlog_mem_event: 1 corrected error: unknown on Memriser3 CHANNEL A DIMM 0 (FRU: 00000000-0000-0000-0000-000000000000  physical addr: 0x0000001057eb0000 node: 1 card: 0 module: 0 rank: 0 bank: 0 row: 28917 column: 1400)
          <idle>-0     [015] d.h.  1013.008596: extlog_mem_event: 2 corrected errors: unknown on Memriser3 CHANNEL A DIMM 0 (FRU: 00000000-0000-0000-0000-000000000000  physical addr: 0x0000001057eb0000 node: 1 card: 0 module: 0 rank: 0 bank: 0 row: 28917 column: 1400)


dmesg output format has been updated based on the suggestion from Boris.
For trace output format we still need further discussion. In the last
patch(support trace interface) I have to reserve previous Kconfig format
because I find once I put trace_event interface in the module, it will
not work. I will paste another trace patch(it only works when acpi_extlog is
builtin) for your answer.

[PATCH v2 1/9] ACPI, APEI, CPER: Fix status check during error printing

[Chen, Gong]

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

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
 drivers/acpi/apei/cper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index 33dc6a0..f827f02 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -353,7 +353,7 @@ void apei_estatus_print(const char *pfx,
 	       cper_severity_str(severity));
 	data_len = estatus->data_length;
 	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
-	while (data_len > sizeof(*gdata)) {
+	while (data_len >= sizeof(*gdata)) {
 		gedata_len = gdata->error_data_length;
 		apei_estatus_print_section(pfx, gdata, sec_no);
 		data_len -= gedata_len + sizeof(*gdata);
-- 
1.8.4.rc3

Re: [PATCH v2 1/9] ACPI, APEI, CPER: Fix status check during error printing

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:55:58 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> Commit aaf9d93 only catches condition check before print,
> but the similar check is needed during printing CPER error
> sections.
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Reviewed-by: Borislav Petkov <bp@suse.de>

Reviewed-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> ---
>  drivers/acpi/apei/cper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index 33dc6a0..f827f02 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -353,7 +353,7 @@ void apei_estatus_print(const char *pfx,
>  	       cper_severity_str(severity));
>  	data_len = estatus->data_length;
>  	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
> -	while (data_len > sizeof(*gdata)) {
> +	while (data_len >= sizeof(*gdata)) {
>  		gedata_len = gdata->error_data_length;
>  		apei_estatus_print_section(pfx, gdata, sec_no);
>  		data_len -= gedata_len + sizeof(*gdata);


-- 

Cheers,
Mauro

[PATCH v2 2/9] ACPI, CPER: Update cper info

[Chen, Gong]

To prepare for the following patches and make related
definition more clear, update some definitions about CPER.

v2 -> v1: Update some more definitions suggested by Boris

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/acpi/apei/apei-internal.h | 12 ++++----
 drivers/acpi/apei/cper.c          | 58 +++++++++++++++++++--------------------
 drivers/acpi/apei/ghes.c          | 54 ++++++++++++++++++------------------
 include/acpi/actbl1.h             | 14 +++++-----
 include/acpi/ghes.h               |  2 +-
 include/linux/cper.h              |  2 +-
 6 files changed, 71 insertions(+), 71 deletions(-)

diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index f220d64..21ba34a 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -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);	\
 	     (void *)section - (void *)estatus < estatus->data_length;	\
 	     section = (void *)(section+1) + section->error_data_length)
 
-static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)
+static inline u32 cper_estatus_len(struct acpi_generic_status *estatus)
 {
 	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;
 }
 
-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);
 
 int apei_osc_setup(void);
 #endif
diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index f827f02..eb5f6d6 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -5,7 +5,7 @@
  *	Author: Huang Ying <ying.huang@intel.com>
  *
  * 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.
@@ -73,7 +73,7 @@ static const char *cper_severity_str(unsigned int severity)
  * printed, with @pfx is printed at the beginning of each line.
  */
 void cper_print_bits(const char *pfx, unsigned int bits,
-		     const char *strs[], unsigned int strs_size)
+		     const char * const strs[], unsigned int strs_size)
 {
 	int i, len = 0;
 	const char *str;
@@ -98,32 +98,32 @@ void cper_print_bits(const char *pfx, unsigned int bits,
 		printk("%s\n", buf);
 }
 
-static const char *cper_proc_type_strs[] = {
+static const char * const cper_proc_type_strs[] = {
 	"IA32/X64",
 	"IA64",
 };
 
-static const char *cper_proc_isa_strs[] = {
+static const char * const cper_proc_isa_strs[] = {
 	"IA32",
 	"IA64",
 	"X64",
 };
 
-static const char *cper_proc_error_type_strs[] = {
+static const char * const cper_proc_error_type_strs[] = {
 	"cache error",
 	"TLB error",
 	"bus error",
 	"micro-architectural error",
 };
 
-static const char *cper_proc_op_strs[] = {
+static const char * const cper_proc_op_strs[] = {
 	"unknown or generic",
 	"data read",
 	"data write",
 	"instruction execution",
 };
 
-static const char *cper_proc_flag_strs[] = {
+static const char * const cper_proc_flag_strs[] = {
 	"restartable",
 	"precise IP",
 	"overflow",
@@ -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,
@@ -283,17 +283,17 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
 }
 
-static const char *apei_estatus_section_flag_strs[] = {
+static const char * const cper_estatus_section_flag_strs[] = {
 	"primary",
 	"containment warning",
 	"reset",
-	"threshold exceeded",
+	"error threshold exceeded",
 	"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,
+			const struct acpi_generic_status *estatus)
 {
-	struct acpi_hest_generic_data *gdata;
+	struct acpi_generic_data *gdata;
 	unsigned int data_len, gedata_len;
 	int sec_no = 0;
 	__u16 severity;
 
-	printk("%s""APEI generic hardware error status\n", pfx);
+	printk("%s""Generic Hardware Error Status\n", pfx);
 	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);
 
-int apei_estatus_check_header(const struct acpi_hest_generic_status *estatus)
+int cper_estatus_check_header(const struct acpi_generic_status *estatus)
 {
 	if (estatus->data_length &&
-	    estatus->data_length < sizeof(struct acpi_hest_generic_data))
+	    estatus->data_length < sizeof(struct acpi_generic_data))
 		return -EINVAL;
 	if (estatus->raw_data_length &&
 	    estatus->raw_data_offset < sizeof(*estatus) + estatus->data_length)
@@ -374,19 +374,19 @@ int apei_estatus_check_header(const struct acpi_hest_generic_status *estatus)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(apei_estatus_check_header);
+EXPORT_SYMBOL_GPL(cper_estatus_check_header);
 
-int apei_estatus_check(const struct acpi_hest_generic_status *estatus)
+int cper_estatus_check(const struct acpi_generic_status *estatus)
 {
-	struct acpi_hest_generic_data *gdata;
+	struct acpi_generic_data *gdata;
 	unsigned int data_len, gedata_len;
 	int rc;
 
-	rc = apei_estatus_check_header(estatus);
+	rc = cper_estatus_check_header(estatus);
 	if (rc)
 		return rc;
 	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;
 		if (gedata_len > data_len - sizeof(*gdata))
@@ -399,4 +399,4 @@ int apei_estatus_check(const struct acpi_hest_generic_status *estatus)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(apei_estatus_check);
+EXPORT_SYMBOL_GPL(cper_estatus_check);
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 8ec37bb..0db6e4f 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -75,13 +75,13 @@
 #define GHES_ESTATUS_CACHE_LEN(estatus_len)			\
 	(sizeof(struct ghes_estatus_cache) + (estatus_len))
 #define GHES_ESTATUS_FROM_CACHE(estatus_cache)			\
-	((struct acpi_hest_generic_status *)			\
+	((struct acpi_generic_status *)				\
 	 ((struct ghes_estatus_cache *)(estatus_cache) + 1))
 
 #define GHES_ESTATUS_NODE_LEN(estatus_len)			\
 	(sizeof(struct ghes_estatus_node) + (estatus_len))
-#define GHES_ESTATUS_FROM_NODE(estatus_node)				\
-	((struct acpi_hest_generic_status *)				\
+#define GHES_ESTATUS_FROM_NODE(estatus_node)			\
+	((struct acpi_generic_status *)				\
 	 ((struct ghes_estatus_node *)(estatus_node) + 1))
 
 bool ghes_disable;
@@ -378,17 +378,17 @@ static int ghes_read_estatus(struct ghes *ghes, int silent)
 	ghes->flags |= GHES_TO_CLEAR;
 
 	rc = -EIO;
-	len = apei_estatus_len(ghes->estatus);
+	len = cper_estatus_len(ghes->estatus);
 	if (len < sizeof(*ghes->estatus))
 		goto err_read_block;
 	if (len > ghes->generic->error_block_length)
 		goto err_read_block;
-	if (apei_estatus_check_header(ghes->estatus))
+	if (cper_estatus_check_header(ghes->estatus))
 		goto err_read_block;
 	ghes_copy_tofrom_phys(ghes->estatus + 1,
 			      buf_paddr + sizeof(*ghes->estatus),
 			      len - sizeof(*ghes->estatus), 1);
-	if (apei_estatus_check(ghes->estatus))
+	if (cper_estatus_check(ghes->estatus))
 		goto err_read_block;
 	rc = 0;
 
@@ -409,7 +409,7 @@ static void ghes_clear_estatus(struct ghes *ghes)
 	ghes->flags &= ~GHES_TO_CLEAR;
 }
 
-static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int sev)
+static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
 {
 #ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE
 	unsigned long pfn;
@@ -438,10 +438,10 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
 }
 
 static void ghes_do_proc(struct ghes *ghes,
-			 const struct acpi_hest_generic_status *estatus)
+			 const struct acpi_generic_status *estatus)
 {
 	int sev, sec_sev;
-	struct acpi_hest_generic_data *gdata;
+	struct acpi_generic_data *gdata;
 
 	sev = ghes_severity(estatus->error_severity);
 	apei_estatus_for_each_section(estatus, gdata) {
@@ -496,7 +496,7 @@ static void ghes_do_proc(struct ghes *ghes,
 
 static void __ghes_print_estatus(const char *pfx,
 				 const struct acpi_hest_generic *generic,
-				 const struct acpi_hest_generic_status *estatus)
+				 const struct acpi_generic_status *estatus)
 {
 	static atomic_t seqno;
 	unsigned int curr_seqno;
@@ -513,12 +513,12 @@ static void __ghes_print_estatus(const char *pfx,
 	snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}" HW_ERR, pfx, curr_seqno);
 	printk("%s""Hardware error from APEI Generic Hardware Error Source: %d\n",
 	       pfx_seq, generic->header.source_id);
-	apei_estatus_print(pfx_seq, estatus);
+	cper_estatus_print(pfx_seq, estatus);
 }
 
 static int ghes_print_estatus(const char *pfx,
 			      const struct acpi_hest_generic *generic,
-			      const struct acpi_hest_generic_status *estatus)
+			      const struct acpi_generic_status *estatus)
 {
 	/* Not more than 2 messages every 5 seconds */
 	static DEFINE_RATELIMIT_STATE(ratelimit_corrected, 5*HZ, 2);
@@ -540,15 +540,15 @@ static int ghes_print_estatus(const char *pfx,
  * GHES error status reporting throttle, to report more kinds of
  * errors, instead of just most frequently occurred errors.
  */
-static int ghes_estatus_cached(struct acpi_hest_generic_status *estatus)
+static int ghes_estatus_cached(struct acpi_generic_status *estatus)
 {
 	u32 len;
 	int i, cached = 0;
 	unsigned long long now;
 	struct ghes_estatus_cache *cache;
-	struct acpi_hest_generic_status *cache_estatus;
+	struct acpi_generic_status *cache_estatus;
 
-	len = apei_estatus_len(estatus);
+	len = cper_estatus_len(estatus);
 	rcu_read_lock();
 	for (i = 0; i < GHES_ESTATUS_CACHES_SIZE; i++) {
 		cache = rcu_dereference(ghes_estatus_caches[i]);
@@ -571,19 +571,19 @@ static int ghes_estatus_cached(struct acpi_hest_generic_status *estatus)
 
 static struct ghes_estatus_cache *ghes_estatus_cache_alloc(
 	struct acpi_hest_generic *generic,
-	struct acpi_hest_generic_status *estatus)
+	struct acpi_generic_status *estatus)
 {
 	int alloced;
 	u32 len, cache_len;
 	struct ghes_estatus_cache *cache;
-	struct acpi_hest_generic_status *cache_estatus;
+	struct acpi_generic_status *cache_estatus;
 
 	alloced = atomic_add_return(1, &ghes_estatus_cache_alloced);
 	if (alloced > GHES_ESTATUS_CACHE_ALLOCED_MAX) {
 		atomic_dec(&ghes_estatus_cache_alloced);
 		return NULL;
 	}
-	len = apei_estatus_len(estatus);
+	len = cper_estatus_len(estatus);
 	cache_len = GHES_ESTATUS_CACHE_LEN(len);
 	cache = (void *)gen_pool_alloc(ghes_estatus_pool, cache_len);
 	if (!cache) {
@@ -603,7 +603,7 @@ static void ghes_estatus_cache_free(struct ghes_estatus_cache *cache)
 {
 	u32 len;
 
-	len = apei_estatus_len(GHES_ESTATUS_FROM_CACHE(cache));
+	len = cper_estatus_len(GHES_ESTATUS_FROM_CACHE(cache));
 	len = GHES_ESTATUS_CACHE_LEN(len);
 	gen_pool_free(ghes_estatus_pool, (unsigned long)cache, len);
 	atomic_dec(&ghes_estatus_cache_alloced);
@@ -619,7 +619,7 @@ static void ghes_estatus_cache_rcu_free(struct rcu_head *head)
 
 static void ghes_estatus_cache_add(
 	struct acpi_hest_generic *generic,
-	struct acpi_hest_generic_status *estatus)
+	struct acpi_generic_status *estatus)
 {
 	int i, slot = -1, count;
 	unsigned long long now, duration, period, max_period = 0;
@@ -751,7 +751,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
 	struct llist_node *llnode, *next;
 	struct ghes_estatus_node *estatus_node;
 	struct acpi_hest_generic *generic;
-	struct acpi_hest_generic_status *estatus;
+	struct acpi_generic_status *estatus;
 	u32 len, node_len;
 
 	llnode = llist_del_all(&ghes_estatus_llist);
@@ -765,7 +765,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
 		estatus_node = llist_entry(llnode, struct ghes_estatus_node,
 					   llnode);
 		estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
-		len = apei_estatus_len(estatus);
+		len = cper_estatus_len(estatus);
 		node_len = GHES_ESTATUS_NODE_LEN(len);
 		ghes_do_proc(estatus_node->ghes, estatus);
 		if (!ghes_estatus_cached(estatus)) {
@@ -784,7 +784,7 @@ static void ghes_print_queued_estatus(void)
 	struct llist_node *llnode;
 	struct ghes_estatus_node *estatus_node;
 	struct acpi_hest_generic *generic;
-	struct acpi_hest_generic_status *estatus;
+	struct acpi_generic_status *estatus;
 	u32 len, node_len;
 
 	llnode = llist_del_all(&ghes_estatus_llist);
@@ -797,7 +797,7 @@ static void ghes_print_queued_estatus(void)
 		estatus_node = llist_entry(llnode, struct ghes_estatus_node,
 					   llnode);
 		estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
-		len = apei_estatus_len(estatus);
+		len = cper_estatus_len(estatus);
 		node_len = GHES_ESTATUS_NODE_LEN(len);
 		generic = estatus_node->generic;
 		ghes_print_estatus(NULL, generic, estatus);
@@ -843,7 +843,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 #ifdef CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG
 		u32 len, node_len;
 		struct ghes_estatus_node *estatus_node;
-		struct acpi_hest_generic_status *estatus;
+		struct acpi_generic_status *estatus;
 #endif
 		if (!(ghes->flags & GHES_TO_CLEAR))
 			continue;
@@ -851,7 +851,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 		if (ghes_estatus_cached(ghes->estatus))
 			goto next;
 		/* Save estatus for further processing in IRQ context */
-		len = apei_estatus_len(ghes->estatus);
+		len = cper_estatus_len(ghes->estatus);
 		node_len = GHES_ESTATUS_NODE_LEN(len);
 		estatus_node = (void *)gen_pool_alloc(ghes_estatus_pool,
 						      node_len);
@@ -923,7 +923,7 @@ static int ghes_probe(struct platform_device *ghes_dev)
 
 	rc = -EIO;
 	if (generic->error_block_length <
-	    sizeof(struct acpi_hest_generic_status)) {
+	    sizeof(struct acpi_generic_status)) {
 		pr_warning(FW_BUG GHES_PFX "Invalid error block length: %u for generic hardware error source: %d\n",
 			   generic->error_block_length,
 			   generic->header.source_id);
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 0bd750e..556c83ee 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -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 */
 
-#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			BIT(0)
+#define ACPI_GEN_ERR_CE			BIT(1)
+#define ACPI_GEN_ERR_MULTI_UC		BIT(2)
+#define ACPI_GEN_ERR_MULTI_CE		BIT(3)
+#define ACPI_GEN_ERR_COUNT_SHIFT	(0xFF<<4) /* 8 bits, error count */
 
 /* Generic Error Data entry */
 
-struct acpi_hest_generic_data {
+struct acpi_generic_data {
 	u8 section_type[16];
 	u32 error_severity;
 	u16 revision;
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 720446c..dfd60d0 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -14,7 +14,7 @@
 
 struct ghes {
 	struct acpi_hest_generic *generic;
-	struct acpi_hest_generic_status *estatus;
+	struct acpi_generic_status *estatus;
 	u64 buffer_paddr;
 	unsigned long flags;
 	union {
diff --git a/include/linux/cper.h b/include/linux/cper.h
index c230494..09ebe21 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -389,6 +389,6 @@ struct cper_sec_pcie {
 
 u64 cper_next_record_id(void);
 void cper_print_bits(const char *prefix, unsigned int bits,
-		     const char *strs[], unsigned int strs_size);
+		     const char * const strs[], unsigned int strs_size);
 
 #endif
-- 
1.8.4.rc3

Re: [PATCH v2 2/9] ACPI, CPER: Update cper info

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:55:59AM -0400, Chen, Gong wrote:
> To prepare for the following patches and make related
> definition more clear, update some definitions about CPER.
> 
> v2 -> v1: Update some more definitions suggested by Boris
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 2/9] ACPI, CPER: Update cper info

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:55:59 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> To prepare for the following patches and make related
> definition more clear, update some definitions about CPER.
> 
> v2 -> v1: Update some more definitions suggested by Boris
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>

Reviewed-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> ---
>  drivers/acpi/apei/apei-internal.h | 12 ++++----
>  drivers/acpi/apei/cper.c          | 58 +++++++++++++++++++--------------------
>  drivers/acpi/apei/ghes.c          | 54 ++++++++++++++++++------------------
>  include/acpi/actbl1.h             | 14 +++++-----
>  include/acpi/ghes.h               |  2 +-
>  include/linux/cper.h              |  2 +-
>  6 files changed, 71 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
> index f220d64..21ba34a 100644
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -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);	\
>  	     (void *)section - (void *)estatus < estatus->data_length;	\
>  	     section = (void *)(section+1) + section->error_data_length)
>  
> -static inline u32 apei_estatus_len(struct acpi_hest_generic_status *estatus)
> +static inline u32 cper_estatus_len(struct acpi_generic_status *estatus)
>  {
>  	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;
>  }
>  
> -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);
>  
>  int apei_osc_setup(void);
>  #endif
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index f827f02..eb5f6d6 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -5,7 +5,7 @@
>   *	Author: Huang Ying <ying.huang@intel.com>
>   *
>   * 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.
> @@ -73,7 +73,7 @@ static const char *cper_severity_str(unsigned int severity)
>   * printed, with @pfx is printed at the beginning of each line.
>   */
>  void cper_print_bits(const char *pfx, unsigned int bits,
> -		     const char *strs[], unsigned int strs_size)
> +		     const char * const strs[], unsigned int strs_size)
>  {
>  	int i, len = 0;
>  	const char *str;
> @@ -98,32 +98,32 @@ void cper_print_bits(const char *pfx, unsigned int bits,
>  		printk("%s\n", buf);
>  }
>  
> -static const char *cper_proc_type_strs[] = {
> +static const char * const cper_proc_type_strs[] = {
>  	"IA32/X64",
>  	"IA64",
>  };
>  
> -static const char *cper_proc_isa_strs[] = {
> +static const char * const cper_proc_isa_strs[] = {
>  	"IA32",
>  	"IA64",
>  	"X64",
>  };
>  
> -static const char *cper_proc_error_type_strs[] = {
> +static const char * const cper_proc_error_type_strs[] = {
>  	"cache error",
>  	"TLB error",
>  	"bus error",
>  	"micro-architectural error",
>  };
>  
> -static const char *cper_proc_op_strs[] = {
> +static const char * const cper_proc_op_strs[] = {
>  	"unknown or generic",
>  	"data read",
>  	"data write",
>  	"instruction execution",
>  };
>  
> -static const char *cper_proc_flag_strs[] = {
> +static const char * const cper_proc_flag_strs[] = {
>  	"restartable",
>  	"precise IP",
>  	"overflow",
> @@ -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,
> @@ -283,17 +283,17 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
>  	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
>  }
>  
> -static const char *apei_estatus_section_flag_strs[] = {
> +static const char * const cper_estatus_section_flag_strs[] = {
>  	"primary",
>  	"containment warning",
>  	"reset",
> -	"threshold exceeded",
> +	"error threshold exceeded",
>  	"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,
> +			const struct acpi_generic_status *estatus)
>  {
> -	struct acpi_hest_generic_data *gdata;
> +	struct acpi_generic_data *gdata;
>  	unsigned int data_len, gedata_len;
>  	int sec_no = 0;
>  	__u16 severity;
>  
> -	printk("%s""APEI generic hardware error status\n", pfx);
> +	printk("%s""Generic Hardware Error Status\n", pfx);
>  	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);
>  
> -int apei_estatus_check_header(const struct acpi_hest_generic_status *estatus)
> +int cper_estatus_check_header(const struct acpi_generic_status *estatus)
>  {
>  	if (estatus->data_length &&
> -	    estatus->data_length < sizeof(struct acpi_hest_generic_data))
> +	    estatus->data_length < sizeof(struct acpi_generic_data))
>  		return -EINVAL;
>  	if (estatus->raw_data_length &&
>  	    estatus->raw_data_offset < sizeof(*estatus) + estatus->data_length)
> @@ -374,19 +374,19 @@ int apei_estatus_check_header(const struct acpi_hest_generic_status *estatus)
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(apei_estatus_check_header);
> +EXPORT_SYMBOL_GPL(cper_estatus_check_header);
>  
> -int apei_estatus_check(const struct acpi_hest_generic_status *estatus)
> +int cper_estatus_check(const struct acpi_generic_status *estatus)
>  {
> -	struct acpi_hest_generic_data *gdata;
> +	struct acpi_generic_data *gdata;
>  	unsigned int data_len, gedata_len;
>  	int rc;
>  
> -	rc = apei_estatus_check_header(estatus);
> +	rc = cper_estatus_check_header(estatus);
>  	if (rc)
>  		return rc;
>  	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;
>  		if (gedata_len > data_len - sizeof(*gdata))
> @@ -399,4 +399,4 @@ int apei_estatus_check(const struct acpi_hest_generic_status *estatus)
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(apei_estatus_check);
> +EXPORT_SYMBOL_GPL(cper_estatus_check);
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 8ec37bb..0db6e4f 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -75,13 +75,13 @@
>  #define GHES_ESTATUS_CACHE_LEN(estatus_len)			\
>  	(sizeof(struct ghes_estatus_cache) + (estatus_len))
>  #define GHES_ESTATUS_FROM_CACHE(estatus_cache)			\
> -	((struct acpi_hest_generic_status *)			\
> +	((struct acpi_generic_status *)				\
>  	 ((struct ghes_estatus_cache *)(estatus_cache) + 1))
>  
>  #define GHES_ESTATUS_NODE_LEN(estatus_len)			\
>  	(sizeof(struct ghes_estatus_node) + (estatus_len))
> -#define GHES_ESTATUS_FROM_NODE(estatus_node)				\
> -	((struct acpi_hest_generic_status *)				\
> +#define GHES_ESTATUS_FROM_NODE(estatus_node)			\
> +	((struct acpi_generic_status *)				\
>  	 ((struct ghes_estatus_node *)(estatus_node) + 1))
>  
>  bool ghes_disable;
> @@ -378,17 +378,17 @@ static int ghes_read_estatus(struct ghes *ghes, int silent)
>  	ghes->flags |= GHES_TO_CLEAR;
>  
>  	rc = -EIO;
> -	len = apei_estatus_len(ghes->estatus);
> +	len = cper_estatus_len(ghes->estatus);
>  	if (len < sizeof(*ghes->estatus))
>  		goto err_read_block;
>  	if (len > ghes->generic->error_block_length)
>  		goto err_read_block;
> -	if (apei_estatus_check_header(ghes->estatus))
> +	if (cper_estatus_check_header(ghes->estatus))
>  		goto err_read_block;
>  	ghes_copy_tofrom_phys(ghes->estatus + 1,
>  			      buf_paddr + sizeof(*ghes->estatus),
>  			      len - sizeof(*ghes->estatus), 1);
> -	if (apei_estatus_check(ghes->estatus))
> +	if (cper_estatus_check(ghes->estatus))
>  		goto err_read_block;
>  	rc = 0;
>  
> @@ -409,7 +409,7 @@ static void ghes_clear_estatus(struct ghes *ghes)
>  	ghes->flags &= ~GHES_TO_CLEAR;
>  }
>  
> -static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int sev)
> +static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
>  {
>  #ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE
>  	unsigned long pfn;
> @@ -438,10 +438,10 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
>  }
>  
>  static void ghes_do_proc(struct ghes *ghes,
> -			 const struct acpi_hest_generic_status *estatus)
> +			 const struct acpi_generic_status *estatus)
>  {
>  	int sev, sec_sev;
> -	struct acpi_hest_generic_data *gdata;
> +	struct acpi_generic_data *gdata;
>  
>  	sev = ghes_severity(estatus->error_severity);
>  	apei_estatus_for_each_section(estatus, gdata) {
> @@ -496,7 +496,7 @@ static void ghes_do_proc(struct ghes *ghes,
>  
>  static void __ghes_print_estatus(const char *pfx,
>  				 const struct acpi_hest_generic *generic,
> -				 const struct acpi_hest_generic_status *estatus)
> +				 const struct acpi_generic_status *estatus)
>  {
>  	static atomic_t seqno;
>  	unsigned int curr_seqno;
> @@ -513,12 +513,12 @@ static void __ghes_print_estatus(const char *pfx,
>  	snprintf(pfx_seq, sizeof(pfx_seq), "%s{%u}" HW_ERR, pfx, curr_seqno);
>  	printk("%s""Hardware error from APEI Generic Hardware Error Source: %d\n",
>  	       pfx_seq, generic->header.source_id);
> -	apei_estatus_print(pfx_seq, estatus);
> +	cper_estatus_print(pfx_seq, estatus);
>  }
>  
>  static int ghes_print_estatus(const char *pfx,
>  			      const struct acpi_hest_generic *generic,
> -			      const struct acpi_hest_generic_status *estatus)
> +			      const struct acpi_generic_status *estatus)
>  {
>  	/* Not more than 2 messages every 5 seconds */
>  	static DEFINE_RATELIMIT_STATE(ratelimit_corrected, 5*HZ, 2);
> @@ -540,15 +540,15 @@ static int ghes_print_estatus(const char *pfx,
>   * GHES error status reporting throttle, to report more kinds of
>   * errors, instead of just most frequently occurred errors.
>   */
> -static int ghes_estatus_cached(struct acpi_hest_generic_status *estatus)
> +static int ghes_estatus_cached(struct acpi_generic_status *estatus)
>  {
>  	u32 len;
>  	int i, cached = 0;
>  	unsigned long long now;
>  	struct ghes_estatus_cache *cache;
> -	struct acpi_hest_generic_status *cache_estatus;
> +	struct acpi_generic_status *cache_estatus;
>  
> -	len = apei_estatus_len(estatus);
> +	len = cper_estatus_len(estatus);
>  	rcu_read_lock();
>  	for (i = 0; i < GHES_ESTATUS_CACHES_SIZE; i++) {
>  		cache = rcu_dereference(ghes_estatus_caches[i]);
> @@ -571,19 +571,19 @@ static int ghes_estatus_cached(struct acpi_hest_generic_status *estatus)
>  
>  static struct ghes_estatus_cache *ghes_estatus_cache_alloc(
>  	struct acpi_hest_generic *generic,
> -	struct acpi_hest_generic_status *estatus)
> +	struct acpi_generic_status *estatus)
>  {
>  	int alloced;
>  	u32 len, cache_len;
>  	struct ghes_estatus_cache *cache;
> -	struct acpi_hest_generic_status *cache_estatus;
> +	struct acpi_generic_status *cache_estatus;
>  
>  	alloced = atomic_add_return(1, &ghes_estatus_cache_alloced);
>  	if (alloced > GHES_ESTATUS_CACHE_ALLOCED_MAX) {
>  		atomic_dec(&ghes_estatus_cache_alloced);
>  		return NULL;
>  	}
> -	len = apei_estatus_len(estatus);
> +	len = cper_estatus_len(estatus);
>  	cache_len = GHES_ESTATUS_CACHE_LEN(len);
>  	cache = (void *)gen_pool_alloc(ghes_estatus_pool, cache_len);
>  	if (!cache) {
> @@ -603,7 +603,7 @@ static void ghes_estatus_cache_free(struct ghes_estatus_cache *cache)
>  {
>  	u32 len;
>  
> -	len = apei_estatus_len(GHES_ESTATUS_FROM_CACHE(cache));
> +	len = cper_estatus_len(GHES_ESTATUS_FROM_CACHE(cache));
>  	len = GHES_ESTATUS_CACHE_LEN(len);
>  	gen_pool_free(ghes_estatus_pool, (unsigned long)cache, len);
>  	atomic_dec(&ghes_estatus_cache_alloced);
> @@ -619,7 +619,7 @@ static void ghes_estatus_cache_rcu_free(struct rcu_head *head)
>  
>  static void ghes_estatus_cache_add(
>  	struct acpi_hest_generic *generic,
> -	struct acpi_hest_generic_status *estatus)
> +	struct acpi_generic_status *estatus)
>  {
>  	int i, slot = -1, count;
>  	unsigned long long now, duration, period, max_period = 0;
> @@ -751,7 +751,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
>  	struct llist_node *llnode, *next;
>  	struct ghes_estatus_node *estatus_node;
>  	struct acpi_hest_generic *generic;
> -	struct acpi_hest_generic_status *estatus;
> +	struct acpi_generic_status *estatus;
>  	u32 len, node_len;
>  
>  	llnode = llist_del_all(&ghes_estatus_llist);
> @@ -765,7 +765,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
>  		estatus_node = llist_entry(llnode, struct ghes_estatus_node,
>  					   llnode);
>  		estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
> -		len = apei_estatus_len(estatus);
> +		len = cper_estatus_len(estatus);
>  		node_len = GHES_ESTATUS_NODE_LEN(len);
>  		ghes_do_proc(estatus_node->ghes, estatus);
>  		if (!ghes_estatus_cached(estatus)) {
> @@ -784,7 +784,7 @@ static void ghes_print_queued_estatus(void)
>  	struct llist_node *llnode;
>  	struct ghes_estatus_node *estatus_node;
>  	struct acpi_hest_generic *generic;
> -	struct acpi_hest_generic_status *estatus;
> +	struct acpi_generic_status *estatus;
>  	u32 len, node_len;
>  
>  	llnode = llist_del_all(&ghes_estatus_llist);
> @@ -797,7 +797,7 @@ static void ghes_print_queued_estatus(void)
>  		estatus_node = llist_entry(llnode, struct ghes_estatus_node,
>  					   llnode);
>  		estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
> -		len = apei_estatus_len(estatus);
> +		len = cper_estatus_len(estatus);
>  		node_len = GHES_ESTATUS_NODE_LEN(len);
>  		generic = estatus_node->generic;
>  		ghes_print_estatus(NULL, generic, estatus);
> @@ -843,7 +843,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
>  #ifdef CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG
>  		u32 len, node_len;
>  		struct ghes_estatus_node *estatus_node;
> -		struct acpi_hest_generic_status *estatus;
> +		struct acpi_generic_status *estatus;
>  #endif
>  		if (!(ghes->flags & GHES_TO_CLEAR))
>  			continue;
> @@ -851,7 +851,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
>  		if (ghes_estatus_cached(ghes->estatus))
>  			goto next;
>  		/* Save estatus for further processing in IRQ context */
> -		len = apei_estatus_len(ghes->estatus);
> +		len = cper_estatus_len(ghes->estatus);
>  		node_len = GHES_ESTATUS_NODE_LEN(len);
>  		estatus_node = (void *)gen_pool_alloc(ghes_estatus_pool,
>  						      node_len);
> @@ -923,7 +923,7 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  
>  	rc = -EIO;
>  	if (generic->error_block_length <
> -	    sizeof(struct acpi_hest_generic_status)) {
> +	    sizeof(struct acpi_generic_status)) {
>  		pr_warning(FW_BUG GHES_PFX "Invalid error block length: %u for generic hardware error source: %d\n",
>  			   generic->error_block_length,
>  			   generic->header.source_id);
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 0bd750e..556c83ee 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -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 */
>  
> -#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			BIT(0)
> +#define ACPI_GEN_ERR_CE			BIT(1)
> +#define ACPI_GEN_ERR_MULTI_UC		BIT(2)
> +#define ACPI_GEN_ERR_MULTI_CE		BIT(3)
> +#define ACPI_GEN_ERR_COUNT_SHIFT	(0xFF<<4) /* 8 bits, error count */
>  
>  /* Generic Error Data entry */
>  
> -struct acpi_hest_generic_data {
> +struct acpi_generic_data {
>  	u8 section_type[16];
>  	u32 error_severity;
>  	u16 revision;
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 720446c..dfd60d0 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -14,7 +14,7 @@
>  
>  struct ghes {
>  	struct acpi_hest_generic *generic;
> -	struct acpi_hest_generic_status *estatus;
> +	struct acpi_generic_status *estatus;
>  	u64 buffer_paddr;
>  	unsigned long flags;
>  	union {
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index c230494..09ebe21 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -389,6 +389,6 @@ struct cper_sec_pcie {
>  
>  u64 cper_next_record_id(void);
>  void cper_print_bits(const char *prefix, unsigned int bits,
> -		     const char *strs[], unsigned int strs_size);
> +		     const char * const strs[], unsigned int strs_size);
>  
>  #endif


-- 

Cheers,
Mauro

[PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Chen, Gong]

GENMASK is used to create a contiguous bitmask([hi:lo]). It is
implemented twice in current kernel. One is in EDAC driver, the other
is in SiS/XGI FB driver. Move it to a more generic place for other
usage.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Winischhofer <thomas@winischhofer.net>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/edac/amd64_edac.c | 46 ++++++++++++++++++++++++----------------------
 drivers/edac/amd64_edac.h |  8 --------
 drivers/video/sis/init.c  |  5 ++---
 include/linux/bitops.h    |  8 ++++++++
 4 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 3c9e4e9..b53d0de 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -339,8 +339,8 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
 	if (pvt->fam == 0xf && pvt->ext_model < K8_REV_F) {
 		csbase		= pvt->csels[dct].csbases[csrow];
 		csmask		= pvt->csels[dct].csmasks[csrow];
-		base_bits	= GENMASK(21, 31) | GENMASK(9, 15);
-		mask_bits	= GENMASK(21, 29) | GENMASK(9, 15);
+		base_bits	= GENMASK_ULL(31, 21) | GENMASK_ULL(15, 9);
+		mask_bits	= GENMASK_ULL(29, 21) | GENMASK_ULL(15, 9);
 		addr_shift	= 4;
 
 	/*
@@ -352,16 +352,16 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
 		csbase          = pvt->csels[dct].csbases[csrow];
 		csmask          = pvt->csels[dct].csmasks[csrow >> 1];
 
-		*base  = (csbase & GENMASK(5,  15)) << 6;
-		*base |= (csbase & GENMASK(19, 30)) << 8;
+		*base  = (csbase & GENMASK_ULL(15,  5)) << 6;
+		*base |= (csbase & GENMASK_ULL(30, 19)) << 8;
 
 		*mask = ~0ULL;
 		/* poke holes for the csmask */
-		*mask &= ~((GENMASK(5, 15)  << 6) |
-			   (GENMASK(19, 30) << 8));
+		*mask &= ~((GENMASK_ULL(15, 5)  << 6) |
+			   (GENMASK_ULL(30, 19) << 8));
 
-		*mask |= (csmask & GENMASK(5, 15))  << 6;
-		*mask |= (csmask & GENMASK(19, 30)) << 8;
+		*mask |= (csmask & GENMASK_ULL(15, 5))  << 6;
+		*mask |= (csmask & GENMASK_ULL(30, 19)) << 8;
 
 		return;
 	} else {
@@ -370,9 +370,11 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
 		addr_shift	= 8;
 
 		if (pvt->fam == 0x15)
-			base_bits = mask_bits = GENMASK(19,30) | GENMASK(5,13);
+			base_bits = mask_bits =
+				GENMASK_ULL(30,19) | GENMASK_ULL(13,5);
 		else
-			base_bits = mask_bits = GENMASK(19,28) | GENMASK(5,13);
+			base_bits = mask_bits =
+				GENMASK_ULL(28,19) | GENMASK_ULL(13,5);
 	}
 
 	*base  = (csbase & base_bits) << addr_shift;
@@ -561,7 +563,7 @@ static u64 sys_addr_to_dram_addr(struct mem_ctl_info *mci, u64 sys_addr)
 	 * section 3.4.2 of AMD publication 24592: AMD x86-64 Architecture
 	 * Programmer's Manual Volume 1 Application Programming.
 	 */
-	dram_addr = (sys_addr & GENMASK(0, 39)) - dram_base;
+	dram_addr = (sys_addr & GENMASK_ULL(39, 0)) - dram_base;
 
 	edac_dbg(2, "using DRAM Base register to translate SysAddr 0x%lx to DramAddr 0x%lx\n",
 		 (unsigned long)sys_addr, (unsigned long)dram_addr);
@@ -597,7 +599,7 @@ static u64 dram_addr_to_input_addr(struct mem_ctl_info *mci, u64 dram_addr)
 	 * concerning translating a DramAddr to an InputAddr.
 	 */
 	intlv_shift = num_node_interleave_bits(dram_intlv_en(pvt, 0));
-	input_addr = ((dram_addr >> intlv_shift) & GENMASK(12, 35)) +
+	input_addr = ((dram_addr >> intlv_shift) & GENMASK_ULL(35, 12)) +
 		      (dram_addr & 0xfff);
 
 	edac_dbg(2, "  Intlv Shift=%d DramAddr=0x%lx maps to InputAddr=0x%lx\n",
@@ -849,7 +851,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
 		end_bit   = 39;
 	}
 
-	addr = m->addr & GENMASK(start_bit, end_bit);
+	addr = m->addr & GENMASK_ULL(end_bit, start_bit);
 
 	/*
 	 * Erratum 637 workaround
@@ -861,7 +863,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
 		u16 mce_nid;
 		u8 intlv_en;
 
-		if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
+		if ((addr & GENMASK_ULL(47, 24)) >> 24 != 0x00fdf7)
 			return addr;
 
 		mce_nid	= amd_get_nb_id(m->extcpu);
@@ -871,7 +873,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
 		intlv_en = tmp >> 21 & 0x7;
 
 		/* add [47:27] + 3 trailing bits */
-		cc6_base  = (tmp & GENMASK(0, 20)) << 3;
+		cc6_base  = (tmp & GENMASK_ULL(20, 0)) << 3;
 
 		/* reverse and add DramIntlvEn */
 		cc6_base |= intlv_en ^ 0x7;
@@ -880,18 +882,18 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
 		cc6_base <<= 24;
 
 		if (!intlv_en)
-			return cc6_base | (addr & GENMASK(0, 23));
+			return cc6_base | (addr & GENMASK_ULL(23, 0));
 
 		amd64_read_pci_cfg(pvt->F1, DRAM_LOCAL_NODE_BASE, &tmp);
 
 							/* faster log2 */
-		tmp_addr  = (addr & GENMASK(12, 23)) << __fls(intlv_en + 1);
+		tmp_addr  = (addr & GENMASK_ULL(23, 12)) << __fls(intlv_en + 1);
 
 		/* OR DramIntlvSel into bits [14:12] */
-		tmp_addr |= (tmp & GENMASK(21, 23)) >> 9;
+		tmp_addr |= (tmp & GENMASK_ULL(23, 21)) >> 9;
 
 		/* add remaining [11:0] bits from original MC4_ADDR */
-		tmp_addr |= addr & GENMASK(0, 11);
+		tmp_addr |= addr & GENMASK_ULL(11, 0);
 
 		return cc6_base | tmp_addr;
 	}
@@ -952,12 +954,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
 
 	amd64_read_pci_cfg(f1, DRAM_LOCAL_NODE_LIM, &llim);
 
-	pvt->ranges[range].lim.lo &= GENMASK(0, 15);
+	pvt->ranges[range].lim.lo &= GENMASK_ULL(15, 0);
 
 				    /* {[39:27],111b} */
 	pvt->ranges[range].lim.lo |= ((llim & 0x1fff) << 3 | 0x7) << 16;
 
-	pvt->ranges[range].lim.hi &= GENMASK(0, 7);
+	pvt->ranges[range].lim.hi &= GENMASK_ULL(7, 0);
 
 				    /* [47:40] */
 	pvt->ranges[range].lim.hi |= llim >> 13;
@@ -1330,7 +1332,7 @@ static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, u8 range,
 			chan_off = dram_base;
 	}
 
-	return (sys_addr & GENMASK(6,47)) - (chan_off & GENMASK(23,47));
+	return (sys_addr & GENMASK_ULL(47,6)) - (chan_off & GENMASK_ULL(47,23));
 }
 
 /*
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index d2443cf..6dc1fcc 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -160,14 +160,6 @@
 #define OFF false
 
 /*
- * Create a contiguous bitmask starting at bit position @lo and ending at
- * position @hi. For example
- *
- * GENMASK(21, 39) gives us the 64bit vector 0x000000ffffe00000.
- */
-#define GENMASK(lo, hi)			(((1ULL << ((hi) - (lo) + 1)) - 1) << (lo))
-
-/*
  * PCI-defined configuration space registers
  */
 #define PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 0x141b
diff --git a/drivers/video/sis/init.c b/drivers/video/sis/init.c
index f082ae5..4f26bc2 100644
--- a/drivers/video/sis/init.c
+++ b/drivers/video/sis/init.c
@@ -3320,9 +3320,8 @@ SiSSetMode(struct SiS_Private *SiS_Pr, unsigned short ModeNo)
 }
 
 #ifndef GETBITSTR
-#define BITMASK(h,l)    	(((unsigned)(1U << ((h)-(l)+1))-1)<<(l))
-#define GENMASK(mask)   	BITMASK(1?mask,0?mask)
-#define GETBITS(var,mask)   	(((var) & GENMASK(mask)) >> (0?mask))
+#define GENBITSMASK(mask)   	GENMASK(1?mask,0?mask)
+#define GETBITS(var,mask)   	(((var) & GENBITSMASK(mask)) >> (0?mask))
 #define GETBITSTR(val,from,to)  ((GETBITS(val,from)) << (0?to))
 #endif
 
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a3b6b82..bd0c459 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -10,6 +10,14 @@
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #endif
 
+/*
+ * Create a contiguous bitmask starting at bit position @l and ending at
+ * position @h. For example
+ * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
+ */
+#define GENMASK(h, l)		(((U32_C(1) << ((h) - (l) + 1)) - 1) << (l))
+#define GENMASK_ULL(h, l)	(((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
+
 extern unsigned int __sw_hweight8(unsigned int w);
 extern unsigned int __sw_hweight16(unsigned int w);
 extern unsigned int __sw_hweight32(unsigned int w);
-- 
1.8.4.rc3

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:56:00AM -0400, Chen, Gong wrote:
> GENMASK is used to create a contiguous bitmask([hi:lo]). It is
> implemented twice in current kernel. One is in EDAC driver, the other
> is in SiS/XGI FB driver. Move it to a more generic place for other
> usage.
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Thomas Winischhofer <thomas@winischhofer.net>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:56:00 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> GENMASK is used to create a contiguous bitmask([hi:lo]). It is
> implemented twice in current kernel. One is in EDAC driver, the other
> is in SiS/XGI FB driver. Move it to a more generic place for other
> usage.
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Thomas Winischhofer <thomas@winischhofer.net>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

Btw, there's another incarnation of it at sb_edac.c (GET_BITFIELD).
It could make sense to also replace it by the newly bitops.h macro.

Regards,
Mauro


> ---
>  drivers/edac/amd64_edac.c | 46 ++++++++++++++++++++++++----------------------
>  drivers/edac/amd64_edac.h |  8 --------
>  drivers/video/sis/init.c  |  5 ++---
>  include/linux/bitops.h    |  8 ++++++++
>  4 files changed, 34 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 3c9e4e9..b53d0de 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -339,8 +339,8 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
>  	if (pvt->fam == 0xf && pvt->ext_model < K8_REV_F) {
>  		csbase		= pvt->csels[dct].csbases[csrow];
>  		csmask		= pvt->csels[dct].csmasks[csrow];
> -		base_bits	= GENMASK(21, 31) | GENMASK(9, 15);
> -		mask_bits	= GENMASK(21, 29) | GENMASK(9, 15);
> +		base_bits	= GENMASK_ULL(31, 21) | GENMASK_ULL(15, 9);
> +		mask_bits	= GENMASK_ULL(29, 21) | GENMASK_ULL(15, 9);
>  		addr_shift	= 4;
>  
>  	/*
> @@ -352,16 +352,16 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
>  		csbase          = pvt->csels[dct].csbases[csrow];
>  		csmask          = pvt->csels[dct].csmasks[csrow >> 1];
>  
> -		*base  = (csbase & GENMASK(5,  15)) << 6;
> -		*base |= (csbase & GENMASK(19, 30)) << 8;
> +		*base  = (csbase & GENMASK_ULL(15,  5)) << 6;
> +		*base |= (csbase & GENMASK_ULL(30, 19)) << 8;
>  
>  		*mask = ~0ULL;
>  		/* poke holes for the csmask */
> -		*mask &= ~((GENMASK(5, 15)  << 6) |
> -			   (GENMASK(19, 30) << 8));
> +		*mask &= ~((GENMASK_ULL(15, 5)  << 6) |
> +			   (GENMASK_ULL(30, 19) << 8));
>  
> -		*mask |= (csmask & GENMASK(5, 15))  << 6;
> -		*mask |= (csmask & GENMASK(19, 30)) << 8;
> +		*mask |= (csmask & GENMASK_ULL(15, 5))  << 6;
> +		*mask |= (csmask & GENMASK_ULL(30, 19)) << 8;
>  
>  		return;
>  	} else {
> @@ -370,9 +370,11 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
>  		addr_shift	= 8;
>  
>  		if (pvt->fam == 0x15)
> -			base_bits = mask_bits = GENMASK(19,30) | GENMASK(5,13);
> +			base_bits = mask_bits =
> +				GENMASK_ULL(30,19) | GENMASK_ULL(13,5);
>  		else
> -			base_bits = mask_bits = GENMASK(19,28) | GENMASK(5,13);
> +			base_bits = mask_bits =
> +				GENMASK_ULL(28,19) | GENMASK_ULL(13,5);
>  	}
>  
>  	*base  = (csbase & base_bits) << addr_shift;
> @@ -561,7 +563,7 @@ static u64 sys_addr_to_dram_addr(struct mem_ctl_info *mci, u64 sys_addr)
>  	 * section 3.4.2 of AMD publication 24592: AMD x86-64 Architecture
>  	 * Programmer's Manual Volume 1 Application Programming.
>  	 */
> -	dram_addr = (sys_addr & GENMASK(0, 39)) - dram_base;
> +	dram_addr = (sys_addr & GENMASK_ULL(39, 0)) - dram_base;
>  
>  	edac_dbg(2, "using DRAM Base register to translate SysAddr 0x%lx to DramAddr 0x%lx\n",
>  		 (unsigned long)sys_addr, (unsigned long)dram_addr);
> @@ -597,7 +599,7 @@ static u64 dram_addr_to_input_addr(struct mem_ctl_info *mci, u64 dram_addr)
>  	 * concerning translating a DramAddr to an InputAddr.
>  	 */
>  	intlv_shift = num_node_interleave_bits(dram_intlv_en(pvt, 0));
> -	input_addr = ((dram_addr >> intlv_shift) & GENMASK(12, 35)) +
> +	input_addr = ((dram_addr >> intlv_shift) & GENMASK_ULL(35, 12)) +
>  		      (dram_addr & 0xfff);
>  
>  	edac_dbg(2, "  Intlv Shift=%d DramAddr=0x%lx maps to InputAddr=0x%lx\n",
> @@ -849,7 +851,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
>  		end_bit   = 39;
>  	}
>  
> -	addr = m->addr & GENMASK(start_bit, end_bit);
> +	addr = m->addr & GENMASK_ULL(end_bit, start_bit);
>  
>  	/*
>  	 * Erratum 637 workaround
> @@ -861,7 +863,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
>  		u16 mce_nid;
>  		u8 intlv_en;
>  
> -		if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7)
> +		if ((addr & GENMASK_ULL(47, 24)) >> 24 != 0x00fdf7)
>  			return addr;
>  
>  		mce_nid	= amd_get_nb_id(m->extcpu);
> @@ -871,7 +873,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
>  		intlv_en = tmp >> 21 & 0x7;
>  
>  		/* add [47:27] + 3 trailing bits */
> -		cc6_base  = (tmp & GENMASK(0, 20)) << 3;
> +		cc6_base  = (tmp & GENMASK_ULL(20, 0)) << 3;
>  
>  		/* reverse and add DramIntlvEn */
>  		cc6_base |= intlv_en ^ 0x7;
> @@ -880,18 +882,18 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
>  		cc6_base <<= 24;
>  
>  		if (!intlv_en)
> -			return cc6_base | (addr & GENMASK(0, 23));
> +			return cc6_base | (addr & GENMASK_ULL(23, 0));
>  
>  		amd64_read_pci_cfg(pvt->F1, DRAM_LOCAL_NODE_BASE, &tmp);
>  
>  							/* faster log2 */
> -		tmp_addr  = (addr & GENMASK(12, 23)) << __fls(intlv_en + 1);
> +		tmp_addr  = (addr & GENMASK_ULL(23, 12)) << __fls(intlv_en + 1);
>  
>  		/* OR DramIntlvSel into bits [14:12] */
> -		tmp_addr |= (tmp & GENMASK(21, 23)) >> 9;
> +		tmp_addr |= (tmp & GENMASK_ULL(23, 21)) >> 9;
>  
>  		/* add remaining [11:0] bits from original MC4_ADDR */
> -		tmp_addr |= addr & GENMASK(0, 11);
> +		tmp_addr |= addr & GENMASK_ULL(11, 0);
>  
>  		return cc6_base | tmp_addr;
>  	}
> @@ -952,12 +954,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
>  
>  	amd64_read_pci_cfg(f1, DRAM_LOCAL_NODE_LIM, &llim);
>  
> -	pvt->ranges[range].lim.lo &= GENMASK(0, 15);
> +	pvt->ranges[range].lim.lo &= GENMASK_ULL(15, 0);
>  
>  				    /* {[39:27],111b} */
>  	pvt->ranges[range].lim.lo |= ((llim & 0x1fff) << 3 | 0x7) << 16;
>  
> -	pvt->ranges[range].lim.hi &= GENMASK(0, 7);
> +	pvt->ranges[range].lim.hi &= GENMASK_ULL(7, 0);
>  
>  				    /* [47:40] */
>  	pvt->ranges[range].lim.hi |= llim >> 13;
> @@ -1330,7 +1332,7 @@ static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, u8 range,
>  			chan_off = dram_base;
>  	}
>  
> -	return (sys_addr & GENMASK(6,47)) - (chan_off & GENMASK(23,47));
> +	return (sys_addr & GENMASK_ULL(47,6)) - (chan_off & GENMASK_ULL(47,23));
>  }
>  
>  /*
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index d2443cf..6dc1fcc 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -160,14 +160,6 @@
>  #define OFF false
>  
>  /*
> - * Create a contiguous bitmask starting at bit position @lo and ending at
> - * position @hi. For example
> - *
> - * GENMASK(21, 39) gives us the 64bit vector 0x000000ffffe00000.
> - */
> -#define GENMASK(lo, hi)			(((1ULL << ((hi) - (lo) + 1)) - 1) << (lo))
> -
> -/*
>   * PCI-defined configuration space registers
>   */
>  #define PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 0x141b
> diff --git a/drivers/video/sis/init.c b/drivers/video/sis/init.c
> index f082ae5..4f26bc2 100644
> --- a/drivers/video/sis/init.c
> +++ b/drivers/video/sis/init.c
> @@ -3320,9 +3320,8 @@ SiSSetMode(struct SiS_Private *SiS_Pr, unsigned short ModeNo)
>  }
>  
>  #ifndef GETBITSTR
> -#define BITMASK(h,l)    	(((unsigned)(1U << ((h)-(l)+1))-1)<<(l))
> -#define GENMASK(mask)   	BITMASK(1?mask,0?mask)
> -#define GETBITS(var,mask)   	(((var) & GENMASK(mask)) >> (0?mask))
> +#define GENBITSMASK(mask)   	GENMASK(1?mask,0?mask)
> +#define GETBITS(var,mask)   	(((var) & GENBITSMASK(mask)) >> (0?mask))
>  #define GETBITSTR(val,from,to)  ((GETBITS(val,from)) << (0?to))
>  #endif
>  
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index a3b6b82..bd0c459 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -10,6 +10,14 @@
>  #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
>  #endif
>  
> +/*
> + * Create a contiguous bitmask starting at bit position @l and ending at
> + * position @h. For example
> + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
> + */
> +#define GENMASK(h, l)		(((U32_C(1) << ((h) - (l) + 1)) - 1) << (l))
> +#define GENMASK_ULL(h, l)	(((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
> +
>  extern unsigned int __sw_hweight8(unsigned int w);
>  extern unsigned int __sw_hweight16(unsigned int w);
>  extern unsigned int __sw_hweight32(unsigned int w);


-- 

Cheers,
Mauro

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Chen Gong]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On Wed, Oct 16, 2013 at 02:02:21PM -0300, Mauro Carvalho Chehab wrote:
> Date: Wed, 16 Oct 2013 14:02:21 -0300
> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, bp@alien8.de, joe@perches.com,
>  naveen.n.rao@linux.vnet.ibm.com, arozansk@redhat.com,
>  linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, Thomas
>  Winischhofer <thomas@winischhofer.net>, Jean-Christophe Plagniol-Villard
>  <plagnioj@jcrosoft.com>, Tomi Valkeinen <tomi.valkeinen@ti.com>
> Subject: Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro
> X-Mailer: Claws Mail 3.9.2 (GTK+ 2.24.19; x86_64-redhat-linux-gnu)
> 
> Em Wed, 16 Oct 2013 10:56:00 -0400
> "Chen, Gong" <gong.chen@linux.intel.com> escreveu:
> 
> > GENMASK is used to create a contiguous bitmask([hi:lo]). It is
> > implemented twice in current kernel. One is in EDAC driver, the other
> > is in SiS/XGI FB driver. Move it to a more generic place for other
> > usage.
> > 
> > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Thomas Winischhofer <thomas@winischhofer.net>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> 
> Btw, there's another incarnation of it at sb_edac.c (GET_BITFIELD).
> It could make sense to also replace it by the newly bitops.h macro.
> 
> Regards,
> Mauro

Aha, I will update it in the next version.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Joe Perches]

On Wed, 2013-10-16 at 10:56 -0400, Chen, Gong wrote:
> GENMASK is used to create a contiguous bitmask([hi:lo]). It is
> implemented twice in current kernel. One is in EDAC driver, the other
> is in SiS/XGI FB driver. Move it to a more generic place for other
> usage.
[]
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
[]
> @@ -10,6 +10,14 @@
>  #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
>  #endif
>  
> +/*
> + * Create a contiguous bitmask starting at bit position @l and ending at
> + * position @h. For example
> + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.

ull

> + */
> +#define GENMASK(h, l)		(((U32_C(1) << ((h) - (l) + 1)) - 1) << (l))
> +#define GENMASK_ULL(h, l)	(((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))


Maybe add a

BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \
	     (h) < (l))

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Chen Gong]

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On Wed, Oct 16, 2013 at 07:59:09PM -0700, Joe Perches wrote:
> Date: Wed, 16 Oct 2013 19:59:09 -0700
> From: Joe Perches <joe@perches.com>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, bp@alien8.de, naveen.n.rao@linux.vnet.ibm.com,
>  m.chehab@samsung.com, arozansk@redhat.com, linux-acpi@vger.kernel.org,
>  linux-kernel@vger.kernel.org, Thomas Winischhofer
>  <thomas@winischhofer.net>, Jean-Christophe Plagniol-Villard
>  <plagnioj@jcrosoft.com>, Tomi Valkeinen <tomi.valkeinen@ti.com>
> Subject: Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro
> X-Mailer: Evolution 3.6.4-0ubuntu1 
> 
> On Wed, 2013-10-16 at 10:56 -0400, Chen, Gong wrote:
> > GENMASK is used to create a contiguous bitmask([hi:lo]). It is
> > implemented twice in current kernel. One is in EDAC driver, the other
> > is in SiS/XGI FB driver. Move it to a more generic place for other
> > usage.
> []
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> []
> > @@ -10,6 +10,14 @@
> >  #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
> >  #endif
> >  
> > +/*
> > + * Create a contiguous bitmask starting at bit position @l and ending at
> > + * position @h. For example
> > + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
> 
> ull
> 
> > + */
> > +#define GENMASK(h, l)		(((U32_C(1) << ((h) - (l) + 1)) - 1) << (l))
> > +#define GENMASK_ULL(h, l)	(((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
> 
> 
> Maybe add a
> 
> BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \
> 	     (h) < (l))
> 
No, if so, users can't use variables for this macro.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Joe Perches]

On Thu, 2013-10-17 at 02:30 -0400, Chen Gong wrote:
> On Wed, Oct 16, 2013 at 07:59:09PM -0700, Joe Perches wrote:
[]
> > Maybe add a
> > 
> > BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \
> > 	     (h) < (l))
> > 
> No, if so, users can't use variables for this macro.

__builtin_constant_p checks for constants

Built-in Function: int __builtin_constant_p (exp)
        You can use the built-in function __builtin_constant_p to
        determine if a value is known to be constant at compile-time and
        hence that GCC can perform constant-folding on expressions
        involving that value. The argument of the function is the value
        to test. The function returns the integer 1 if the argument is
        known to be a compile-time constant and 0 if it is not known to
        be a compile-time constant. A return of 0 does not indicate that
        the value is not a constant, but merely that GCC cannot prove it
        is a constant with the specified value of the -O option. 
        

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Chen Gong]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

On Wed, Oct 16, 2013 at 11:58:56PM -0700, Joe Perches wrote:
> Date: Wed, 16 Oct 2013 23:58:56 -0700
> From: Joe Perches <joe@perches.com>
> To: Chen Gong <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, bp@alien8.de, naveen.n.rao@linux.vnet.ibm.com,
>  m.chehab@samsung.com, arozansk@redhat.com, linux-acpi@vger.kernel.org,
>  linux-kernel@vger.kernel.org, Thomas Winischhofer
>  <thomas@winischhofer.net>, Jean-Christophe Plagniol-Villard
>  <plagnioj@jcrosoft.com>, Tomi Valkeinen <tomi.valkeinen@ti.com>
> Subject: Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro
> X-Mailer: Evolution 3.6.4-0ubuntu1 
> 
> On Thu, 2013-10-17 at 02:30 -0400, Chen Gong wrote:
> > On Wed, Oct 16, 2013 at 07:59:09PM -0700, Joe Perches wrote:
> []
> > > Maybe add a
> > > 
> > > BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \
> > > 	     (h) < (l))
> > > 
> > No, if so, users can't use variables for this macro.
> 
> __builtin_constant_p checks for constants
> 
> Built-in Function: int __builtin_constant_p (exp)
>         You can use the built-in function __builtin_constant_p to
>         determine if a value is known to be constant at compile-time and
>         hence that GCC can perform constant-folding on expressions
>         involving that value. The argument of the function is the value
>         to test. The function returns the integer 1 if the argument is
>         known to be a compile-time constant and 0 if it is not known to
>         be a compile-time constant. A return of 0 does not indicate that
>         the value is not a constant, but merely that GCC cannot prove it
>         is a constant with the specified value of the -O option. 
>         

Yes, even we have following codes __builtin_constant_p still can return 1,
so long as the value of variable can be identified.

int len = sizeof(int);
if (__builtin_constant_p(len)) {
        do_1;
} else {
        do_0;
}

but the point is we can use GENMASK like GENMASK(end_bit, start_bit) but
we don't know the value of end_bit/start_bit at compile-time.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Joe Perches]

On Thu, 2013-10-17 at 03:38 -0400, Chen Gong wrote:
> the point is we can use GENMASK like GENMASK(end_bit, start_bit) but
> we don't know the value of end_bit/start_bit at compile-time.

True.

The BUILD_BUG_ON idea is just to avoid people using
	GENMASK(1, 2)
instead of
	GENMASK(2, 1)

#define GENMASK(h, l)						\
({								\
	BUILD_BUG_ON(__builtin_constant_p(l) &&			\
		     __builtin_constant_p(h) &&			\
		     (l) > (h));				\
	(((U32_C(1) << ((h) - (l) + 1)) - 1) << (l));		\
})

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Borislav Petkov]

On Thu, Oct 17, 2013 at 01:32:30AM -0700, Joe Perches wrote:
> On Thu, 2013-10-17 at 03:38 -0400, Chen Gong wrote:
> > the point is we can use GENMASK like GENMASK(end_bit, start_bit) but
> > we don't know the value of end_bit/start_bit at compile-time.
> 
> True.
> 
> The BUILD_BUG_ON idea is just to avoid people using
> 	GENMASK(1, 2)

They'll notice the 0 pretty quickly when they test their code.

Let's add those checks only when it is really necessary and people have
actually made that mistake repeatedly.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Joe Perches]

On Thu, 2013-10-17 at 10:40 +0200, Borislav Petkov wrote:
> On Thu, Oct 17, 2013 at 01:32:30AM -0700, Joe Perches wrote:
> > On Thu, 2013-10-17 at 03:38 -0400, Chen Gong wrote:
> > > the point is we can use GENMASK like GENMASK(end_bit, start_bit) but
> > > we don't know the value of end_bit/start_bit at compile-time.
> > 
> > True.
> > 
> > The BUILD_BUG_ON idea is just to avoid people using
> > 	GENMASK(1, 2)
> 
> They'll notice the 0 pretty quickly when they test their code.
> 
> Let's add those checks only when it is really necessary and people have
> actually made that mistake repeatedly.

It's cost free to add the BUILD_BUG_ON
and perhaps you underestimate the runtime
bug checking effort,

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Tony Luck]

On Thu, Oct 17, 2013 at 1:55 AM, Joe Perches <joe@perches.com> wrote:
> It's cost free to add the BUILD_BUG_ON
> and perhaps you underestimate the runtime
> bug checking effort,

This looks OK to me.  Gong: This doesn't stop people from using variables
as arguments ... they just won't get a check for (h) < (l).

-Tony

Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro

[Joe Perches]

On Thu, 2013-10-17 at 09:10 -0700, Tony Luck wrote:
> On Thu, Oct 17, 2013 at 1:55 AM, Joe Perches <joe@perches.com> wrote:
> > It's cost free to add the BUILD_BUG_ON
> > and perhaps you underestimate the runtime
> > bug checking effort,
> 
> This looks OK to me.  Gong: This doesn't stop people from using variables
> as arguments ... they just won't get a check for (h) < (l).

Another possibility is to swap high and low if necessary and
maybe warn when either is negative or too large for the bit width.

#define GENMASK(h, l)							\
({									\
	size_t high = h;						\
	size_t low = l;							\
	BUILD_BUG_ON(__builtin_constant_p(l) &&				\
		     __builtin_constant_p(h) &&				\
		     (l) > (h));					\
	BUILD_BUG_ON(__builtin_constant_p(l) &&				\
		     __builtin_constant_p(h) &&				\
		     ((l) >= BITS_PER_LONG ||				\
		      (h) >= BITS_PER_LONG));				\
	WARN_ONCE((!__builtin_constant_p(l) &&				\
		   ((l) < 0 || (l) > BITS_PER_LONG)) ||			\
		  (!__builtin_constant_p(h) &&				\
		   ((h) < 0 || (h) > BITS_PER_LONG)) ||			\
		  (l) > (h),						\
		  "GENMASK: invalid mask values: l: %u, h: %d\n",	\
		  (l), (h));						\
	if (low > high)							\
		swap(low, high);					\
	(((U32_C(1) << (high - low + 1)) - 1) << low);			\
})

And maybe this should be renamed something like
#define BIT_MASK_RANGE(h, l)
or
#define BIT_MASK_SHIFTED(h, l)

[PATCH v2 4/9] ACPI, x86: Extended error log driver for x86 platform

[Chen, 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.htm l.
After errors are captured, more valuable information can be
got with this new enhanced error log driver.

v2 -> v1: eliminate spin_lock & minor fixes suggested by Boris

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 arch/x86/include/asm/mce.h       |   5 +
 arch/x86/kernel/cpu/mcheck/mce.c |  20 +++
 drivers/acpi/Kconfig             |   9 ++
 drivers/acpi/Makefile            |   2 +
 drivers/acpi/acpi_extlog.c       | 319 +++++++++++++++++++++++++++++++++++++++
 drivers/acpi/bus.c               |   3 +-
 include/linux/acpi.h             |   1 +
 7 files changed, 358 insertions(+), 1 deletion(-)
 create mode 100644 drivers/acpi/acpi_extlog.c

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index cbe6b9e..072b2f8 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -16,6 +16,7 @@
 #define MCG_EXT_CNT_SHIFT	16
 #define MCG_EXT_CNT(c)		(((c) & MCG_EXT_CNT_MASK) >> MCG_EXT_CNT_SHIFT)
 #define MCG_SER_P		(1ULL<<24)   /* MCA recovery/new status bits */
+#define MCG_ELOG_P		(1ULL<<26)   /* Extended error log supported */
 
 /* MCG_STATUS register defines */
 #define MCG_STATUS_RIPV  (1ULL<<0)   /* restart ip valid */
@@ -186,6 +187,10 @@ enum mcp_flags {
 	MCP_UC = (1 << 1),		/* log uncorrected errors */
 	MCP_DONTLOG = (1 << 2),		/* only clear, don't log */
 };
+
+void register_elog_handler(int (*f)(const char *, int, int));
+void unregister_elog_handler(int (*f)(const char *, int, int));
+
 void machine_check_poll(enum mcp_flags flags, mce_banks_t *b);
 
 int mce_notify_irq(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index b3218cd..c11a53f 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -48,6 +48,8 @@
 
 #include "mce-internal.h"
 
+static int (*mce_ext_err_print)(const char *, int, int) = NULL;
+
 static DEFINE_MUTEX(mce_chrdev_read_mutex);
 
 #define rcu_dereference_check_mce(p) \
@@ -576,6 +578,21 @@ static void mce_read_aux(struct mce *m, int i)
 
 DEFINE_PER_CPU(unsigned, mce_poll_count);
 
+void register_elog_handler(int (*f)(const char *, int, int))
+{
+	mce_ext_err_print = f;
+}
+EXPORT_SYMBOL_GPL(register_elog_handler);
+
+void unregister_elog_handler(int (*f)(const char *, int, int))
+{
+	if (f) {
+		WARN_ON(mce_ext_err_print != f);
+		mce_ext_err_print = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(unregister_elog_handler);
+
 /*
  * Poll for corrected events or events that happened before reset.
  * Those are just logged through /dev/mcelog.
@@ -624,6 +641,9 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		    (m.status & (mca_cfg.ser ? MCI_STATUS_S : MCI_STATUS_UC)))
 			continue;
 
+		if (mce_ext_err_print)
+			mce_ext_err_print(NULL, m.extcpu, i);
+
 		mce_read_aux(&m, i);
 
 		if (!(flags & MCP_TIMESTAMP))
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 22327e6..819c06b 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -372,4 +372,13 @@ config ACPI_BGRT
 
 source "drivers/acpi/apei/Kconfig"
 
+config ACPI_EXTLOG
+	tristate "Extended Error Log support"
+	depends on X86_MCE
+	default n
+	help
+	  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.
+
 endif	# ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index cdaf68b..bce34af 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -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
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
new file mode 100644
index 0000000..d55b072
--- /dev/null
+++ b/drivers/acpi/acpi_extlog.c
@@ -0,0 +1,319 @@
+/*
+ * Extended Error Log driver
+ *
+ * Copyright (C) 2013 Intel Corp.
+ * Author: Chen, Gong <gong.chen@intel.com>
+ *
+ * This file is licensed under GPLv2.
+ */
+
+#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	GENMASK_ULL(52, 0) /* elog entry address mask */
+
+#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)
+{
+	int idx;
+	u64 data;
+	struct acpi_generic_status *estatus;
+
+	WARN_ON(cpu < 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);
+
+	/* 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)
+{
+	static atomic_t seqno;
+	unsigned int curr_seqno;
+	char pfx_seq[64];
+
+	if (!pfx) {
+		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)
+{
+	/* 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))
+		ratelimit = &ratelimit_corrected;
+	else
+		ratelimit = &ratelimit_uncorrected;
+	if (__ratelimit(ratelimit)) {
+		__print_extlog_rcd(pfx, estatus, cpu);
+		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) {
+			for (i = 0; i < obj->buffer.length; i++)
+				*ret |= (obj->buffer.pointer[i] << (i * 8));
+		}
+	}
+	kfree(buf.pointer);
+
+	return 0;
+}
+
+static bool extlog_get_l1addr(void)
+{
+	acpi_handle handle;
+	u64 ret;
+
+	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
+		return false;
+
+	if (extlog_get_dsm(handle, EXTLOG_DSM_REV, EXTLOG_FN_QUERY, &ret) ||
+			!(ret & EXTLOG_QUERY_L1_EXIST))
+		return false;
+
+	if (extlog_get_dsm(handle, EXTLOG_DSM_REV, EXTLOG_FN_ADDR, &ret))
+		return false;
+
+	l1_dirbase = ret;
+	/* Spec says L1 directory must be 4K aligned, bail out if it isn't */
+	if (l1_dirbase & ((1 << 12) - 1)) {
+		pr_warn(FW_BUG "L1 Directory is invalid at physical %llx\n",
+			l1_dirbase);
+		return false;
+	}
+
+	return true;
+}
+
+static int __init extlog_init(void)
+{
+	struct extlog_l1_head *l1_head;
+	void __iomem *extlog_l1_hdr;
+	size_t l1_hdr_size;
+	struct resource *r;
+	u64 cap;
+	int rc;
+
+	rc = -ENODEV;
+
+	rdmsrl(MSR_IA32_MCG_CAP, cap);
+	if (!(cap & MCG_ELOG_P))
+		return rc;
+
+	if (!extlog_get_l1addr())
+		return rc;
+
+	rc = -EINVAL;
+	/* get L1 header to fetch necessary information */
+	l1_hdr_size = sizeof(struct extlog_l1_head);
+	r = request_mem_region(l1_dirbase, l1_hdr_size, "L1 DIR HDR");
+	if (!r) {
+		pr_warn(FW_BUG EMCA_BUG,
+			(unsigned long long)l1_dirbase,
+			(unsigned long long)l1_dirbase + l1_hdr_size);
+		goto err;
+	}
+
+	extlog_l1_hdr = acpi_os_map_memory(l1_dirbase, l1_hdr_size);
+	l1_head = (struct extlog_l1_head *)extlog_l1_hdr;
+	l1_size = l1_head->total_len;
+	l1_percpu_entry = l1_head->entries;
+	elog_base = l1_head->elog_base;
+	elog_size = l1_head->elog_len;
+	acpi_os_unmap_memory(extlog_l1_hdr, l1_hdr_size);
+	release_mem_region(l1_dirbase, l1_hdr_size);
+
+	/* remap L1 header again based on completed information */
+	r = request_mem_region(l1_dirbase, l1_size, "L1 Table");
+	if (!r) {
+		pr_warn(FW_BUG EMCA_BUG,
+			(unsigned long long)l1_dirbase,
+			(unsigned long long)l1_dirbase + l1_size);
+		goto err;
+	}
+	extlog_l1_addr = acpi_os_map_memory(l1_dirbase, l1_size);
+	l1_entry_base = (u64 *)((u8 *)extlog_l1_addr + l1_hdr_size);
+
+	/* remap elog table */
+	r = request_mem_region(elog_base, elog_size, "Elog Table");
+	if (!r) {
+		pr_warn(FW_BUG EMCA_BUG,
+			(unsigned long long)elog_base,
+			(unsigned long long)elog_base + elog_size);
+		goto err_release_l1_dir;
+	}
+	elog_addr = acpi_os_map_memory(elog_base, elog_size);
+
+	rc = -ENOMEM;
+	/* allocate buffer to save elog record */
+	elog_buf = kmalloc(ELOG_ENTRY_LEN, GFP_KERNEL);
+	if (elog_buf == NULL)
+		goto err_release_elog;
+
+	register_elog_handler(extlog_print);
+	/* enable OS to be involved to take over management from BIOS */
+	((struct extlog_l1_head *)extlog_l1_addr)->flags |= FLAG_OS_OPTIN;
+
+	return 0;
+
+err_release_elog:
+	if (elog_addr)
+		acpi_os_unmap_memory(elog_addr, elog_size);
+	release_mem_region(elog_base, elog_size);
+err_release_l1_dir:
+	if (extlog_l1_addr)
+		acpi_os_unmap_memory(extlog_l1_addr, l1_size);
+	release_mem_region(l1_dirbase, l1_size);
+err:
+	pr_warn(FW_BUG "Extended error log disabled because of problems parsing f/w tables\n");
+	return rc;
+}
+
+static void __exit extlog_exit(void)
+{
+	unregister_elog_handler(extlog_print);
+	((struct extlog_l1_head *)extlog_l1_addr)->flags &= ~FLAG_OS_OPTIN;
+	if (extlog_l1_addr)
+		acpi_os_unmap_memory(extlog_l1_addr, l1_size);
+	if (elog_addr)
+		acpi_os_unmap_memory(elog_addr, elog_size);
+	release_mem_region(elog_base, elog_size);
+	release_mem_region(l1_dirbase, l1_size);
+	kfree(elog_buf);
+}
+
+module_init(extlog_init);
+module_exit(extlog_exit);
+
+MODULE_AUTHOR("Chen, Gong <gong.chen@intel.com>");
+MODULE_DESCRIPTION("Extended Error Log Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index b587ec8..e1bd9a1 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -174,7 +174,7 @@ static void acpi_print_osc_error(acpi_handle handle,
 	printk("\n");
 }
 
-static acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
+acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
 {
 	int i;
 	static int opc_map_to_uuid[16] = {6, 4, 2, 0, 11, 9, 16, 14, 19, 21,
@@ -195,6 +195,7 @@ static acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
 	}
 	return AE_OK;
 }
+EXPORT_SYMBOL_GPL(acpi_str_to_uuid);
 
 acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
 {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a5db4ae..c30bac8 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -311,6 +311,7 @@ struct acpi_osc_context {
 #define OSC_INVALID_REVISION_ERROR	8
 #define OSC_CAPABILITIES_MASK_ERROR	16
 
+acpi_status acpi_str_to_uuid(char *str, u8 *uuid);
 acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
 
 /* platform-wide _OSC bits */
-- 
1.8.4.rc3

Re: [PATCH v2 4/9] ACPI, x86: Extended error log driver for x86 platform

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:56:01AM -0400, Chen, Gong wrote:
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index b3218cd..c11a53f 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -48,6 +48,8 @@
>  
>  #include "mce-internal.h"
>  
> +static int (*mce_ext_err_print)(const char *, int, int) = NULL;

Applying: ACPI, x86: Extended error log driver for x86 platform
ERROR: do not initialise statics to 0 or NULL
#32: FILE: arch/x86/kernel/cpu/mcheck/mce.c:51:
+static int (*mce_ext_err_print)(const char *, int, int) = NULL;

> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 22327e6..819c06b 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -372,4 +372,13 @@ config ACPI_BGRT
>  
>  source "drivers/acpi/apei/Kconfig"
>  
> +config ACPI_EXTLOG
> +	tristate "Extended Error Log support"
> +	depends on X86_MCE
> +	default n
> +	help
> +	  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.

Yep, you can use the description from your 0/9 mail here.

> +
>  endif	# ACPI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index cdaf68b..bce34af 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -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
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> new file mode 100644
> index 0000000..d55b072
> --- /dev/null
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -0,0 +1,319 @@
> +/*
> + * Extended Error Log driver
> + *
> + * Copyright (C) 2013 Intel Corp.
> + * Author: Chen, Gong <gong.chen@intel.com>
> + *
> + * This file is licensed under GPLv2.
> + */
> +
> +#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	GENMASK_ULL(52, 0) /* elog entry address mask */

So was this supposed to be 0xfffffffffffff, right?

Because that's bits 0 up to and including 51. In which case, it should
be GENMASK_ULL(51,0).

The rest are checkpatch minor issues below.

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

CHECK: braces {} should be used on all arms of this statement
#280: FILE: drivers/acpi/acpi_extlog.c:178:
+       if (obj->type == ACPI_TYPE_INTEGER)
[...]
+       else if (obj->type == ACPI_TYPE_BUFFER) {
[...]

> +		if (obj->buffer.length <= 8) {
> +			for (i = 0; i < obj->buffer.length; i++)
> +				*ret |= (obj->buffer.pointer[i] << (i * 8));
> +		}
> +	}
> +	kfree(buf.pointer);
> +
> +	return 0;
> +}
> +
> +static bool extlog_get_l1addr(void)
> +{
> +	acpi_handle handle;
> +	u64 ret;
> +
> +	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> +		return false;
> +
> +	if (extlog_get_dsm(handle, EXTLOG_DSM_REV, EXTLOG_FN_QUERY, &ret) ||
> +			!(ret & EXTLOG_QUERY_L1_EXIST))

CHECK: Alignment should match open parenthesis
#302: FILE: drivers/acpi/acpi_extlog.c:200:
+       if (extlog_get_dsm(handle, EXTLOG_DSM_REV, EXTLOG_FN_QUERY, &ret) ||
+                       !(ret & EXTLOG_QUERY_L1_EXIST))

Thanks.

-- 
Regards/Gruss,
    Boris.

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

[PATCH v2 5/9] DMI: Parse memory device (type 17) in SMBIOS

[Chen, Gong]

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

Original-author: Tony Luck <tony.luck@intel.com>
Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 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(+)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 4fc2e95..d86669b 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -1063,6 +1063,7 @@ check_bugs (void)
 static int __init run_dmi_scan(void)
 {
 	dmi_scan_machine();
+	dmi_memdev_walk();
 	dmi_set_dump_stack_arch_desc();
 	return 0;
 }
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..918d489 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -993,6 +993,7 @@ void __init setup_arch(char **cmdline_p)
 		efi_init();
 
 	dmi_scan_machine();
+	dmi_memdev_walk();
 	dmi_set_dump_stack_arch_desc();
 
 	/*
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index fa0affb..59579a7 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -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;
+
+	if (dm->type != DMI_ENTRY_MEM_DEVICE)
+		return;
+	if (nr >= dmi_memdev_nr) {
+		pr_warn(FW_BUG "Too many DIMM entries in SMBIOS table\n");
+		return;
+	}
+	dmi_memdev[nr].handle = dm->handle;
+	dmi_memdev[nr].device = dmi_string(dm, d[0x10]);
+	dmi_memdev[nr].bank = dmi_string(dm, d[0x11]);
+	nr++;
+}
+
+void __init dmi_memdev_walk(void)
+{
+	if (!dmi_available)
+		return;
+
+	if (dmi_walk_early(count_mem_devices) == 0 && dmi_memdev_nr) {
+		dmi_memdev = dmi_alloc(sizeof(*dmi_memdev) * dmi_memdev_nr);
+		if (dmi_memdev)
+			dmi_walk_early(save_mem_devices);
+	}
+}
+
 /*
  *	Process a DMI table entry. Right now all we care about are the BIOS
  *	and machine entries. For 2.5 we should pull the smbus controller info
@@ -815,3 +858,20 @@ bool dmi_match(enum dmi_field f, const char *str)
 	return !strcmp(info, str);
 }
 EXPORT_SYMBOL_GPL(dmi_match);
+
+void dmi_memdev_name(u16 handle, const char **bank, const char **device)
+{
+	int n;
+
+	if (dmi_memdev == NULL)
+		return;
+
+	for (n = 0; n < dmi_memdev_nr; n++) {
+		if (handle == dmi_memdev[n].handle) {
+			*bank = dmi_memdev[n].bank;
+			*device = dmi_memdev[n].device;
+			break;
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(dmi_memdev_name);
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index b6eb7a0..f820f0a 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -99,6 +99,7 @@ extern const char * dmi_get_system_info(int field);
 extern const struct dmi_device * dmi_find_device(int type, const char *name,
 	const struct dmi_device *from);
 extern void dmi_scan_machine(void);
+extern void dmi_memdev_walk(void);
 extern void dmi_set_dump_stack_arch_desc(void);
 extern bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp);
 extern int dmi_name_in_vendors(const char *str);
@@ -107,6 +108,7 @@ extern int dmi_available;
 extern int dmi_walk(void (*decode)(const struct dmi_header *, void *),
 	void *private_data);
 extern bool dmi_match(enum dmi_field f, const char *str);
+extern void dmi_memdev_name(u16 handle, const char **bank, const char **device);
 
 #else
 
@@ -115,6 +117,7 @@ static inline const char * dmi_get_system_info(int field) { return NULL; }
 static inline const struct dmi_device * dmi_find_device(int type, const char *name,
 	const struct dmi_device *from) { return NULL; }
 static inline void dmi_scan_machine(void) { return; }
+static inline void dmi_memdev_walk(void) { }
 static inline void dmi_set_dump_stack_arch_desc(void) { }
 static inline bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp)
 {
@@ -133,6 +136,8 @@ static inline int dmi_walk(void (*decode)(const struct dmi_header *, void *),
 	void *private_data) { return -1; }
 static inline bool dmi_match(enum dmi_field f, const char *str)
 	{ return false; }
+static inline void dmi_memdev_name(u16 handle, const char **bank,
+		const char **device) { }
 static inline const struct dmi_system_id *
 	dmi_first_match(const struct dmi_system_id *list) { return NULL; }
 
-- 
1.8.4.rc3

Re: [PATCH v2 5/9] DMI: Parse memory device (type 17) in SMBIOS

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:56:02AM -0400, Chen, Gong wrote:
> This patch adds a new interface to decode memory device (type 17)
> to help error reporting on DIMMs.
> 
> Original-author: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 5/9] DMI: Parse memory device (type 17) in SMBIOS

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:56:02 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> This patch adds a new interface to decode memory device (type 17)
> to help error reporting on DIMMs.
> 
> Original-author: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Reviewed-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> ---
>  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(+)
> 
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 4fc2e95..d86669b 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -1063,6 +1063,7 @@ check_bugs (void)
>  static int __init run_dmi_scan(void)
>  {
>  	dmi_scan_machine();
> +	dmi_memdev_walk();
>  	dmi_set_dump_stack_arch_desc();
>  	return 0;
>  }
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index f0de629..918d489 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -993,6 +993,7 @@ void __init setup_arch(char **cmdline_p)
>  		efi_init();
>  
>  	dmi_scan_machine();
> +	dmi_memdev_walk();
>  	dmi_set_dump_stack_arch_desc();
>  
>  	/*
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index fa0affb..59579a7 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -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;
> +
> +	if (dm->type != DMI_ENTRY_MEM_DEVICE)
> +		return;
> +	if (nr >= dmi_memdev_nr) {
> +		pr_warn(FW_BUG "Too many DIMM entries in SMBIOS table\n");
> +		return;
> +	}
> +	dmi_memdev[nr].handle = dm->handle;
> +	dmi_memdev[nr].device = dmi_string(dm, d[0x10]);
> +	dmi_memdev[nr].bank = dmi_string(dm, d[0x11]);
> +	nr++;
> +}
> +
> +void __init dmi_memdev_walk(void)
> +{
> +	if (!dmi_available)
> +		return;
> +
> +	if (dmi_walk_early(count_mem_devices) == 0 && dmi_memdev_nr) {
> +		dmi_memdev = dmi_alloc(sizeof(*dmi_memdev) * dmi_memdev_nr);
> +		if (dmi_memdev)
> +			dmi_walk_early(save_mem_devices);
> +	}
> +}
> +
>  /*
>   *	Process a DMI table entry. Right now all we care about are the BIOS
>   *	and machine entries. For 2.5 we should pull the smbus controller info
> @@ -815,3 +858,20 @@ bool dmi_match(enum dmi_field f, const char *str)
>  	return !strcmp(info, str);
>  }
>  EXPORT_SYMBOL_GPL(dmi_match);
> +
> +void dmi_memdev_name(u16 handle, const char **bank, const char **device)
> +{
> +	int n;
> +
> +	if (dmi_memdev == NULL)
> +		return;
> +
> +	for (n = 0; n < dmi_memdev_nr; n++) {
> +		if (handle == dmi_memdev[n].handle) {
> +			*bank = dmi_memdev[n].bank;
> +			*device = dmi_memdev[n].device;
> +			break;
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(dmi_memdev_name);
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index b6eb7a0..f820f0a 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -99,6 +99,7 @@ extern const char * dmi_get_system_info(int field);
>  extern const struct dmi_device * dmi_find_device(int type, const char *name,
>  	const struct dmi_device *from);
>  extern void dmi_scan_machine(void);
> +extern void dmi_memdev_walk(void);
>  extern void dmi_set_dump_stack_arch_desc(void);
>  extern bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp);
>  extern int dmi_name_in_vendors(const char *str);
> @@ -107,6 +108,7 @@ extern int dmi_available;
>  extern int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>  	void *private_data);
>  extern bool dmi_match(enum dmi_field f, const char *str);
> +extern void dmi_memdev_name(u16 handle, const char **bank, const char **device);
>  
>  #else
>  
> @@ -115,6 +117,7 @@ static inline const char * dmi_get_system_info(int field) { return NULL; }
>  static inline const struct dmi_device * dmi_find_device(int type, const char *name,
>  	const struct dmi_device *from) { return NULL; }
>  static inline void dmi_scan_machine(void) { return; }
> +static inline void dmi_memdev_walk(void) { }
>  static inline void dmi_set_dump_stack_arch_desc(void) { }
>  static inline bool dmi_get_date(int field, int *yearp, int *monthp, int *dayp)
>  {
> @@ -133,6 +136,8 @@ static inline int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>  	void *private_data) { return -1; }
>  static inline bool dmi_match(enum dmi_field f, const char *str)
>  	{ return false; }
> +static inline void dmi_memdev_name(u16 handle, const char **bank,
> +		const char **device) { }
>  static inline const struct dmi_system_id *
>  	dmi_first_match(const struct dmi_system_id *list) { return NULL; }
>  


-- 

Cheers,
Mauro

[PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error

[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 <tony.luck@intel.com>
Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/mcheck/mce-apei.c |  3 +--
 drivers/acpi/apei/cper.c              |  7 ++++---
 drivers/acpi/apei/ghes.c              |  4 ++--
 drivers/edac/ghes_edac.c              |  5 ++---
 include/linux/cper.h                  | 11 +++++++++--
 5 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mcheck/mce-apei.c
index cd8b166..de8b60a 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-apei.c
@@ -42,8 +42,7 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
 	struct mce m;
 
 	/* Only corrected MC is reported */
-	if (!corrected || !(mem_err->validation_bits &
-				CPER_MEM_VALID_PHYSICAL_ADDRESS))
+	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
 
 	mce_setup(&m);
diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index eb5f6d6..946ef52 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -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
@@ -191,16 +191,17 @@ static const char *cper_mem_err_type_strs[] = {
 	"memory sparing",
 	"scrub corrected error",
 	"scrub uncorrected error",
+	"physical memory map-out event",
 };
 
 static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 {
 	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
 		printk("%s""error_status: 0x%016llx\n", pfx, mem->error_status);
-	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)
+	if (mem->validation_bits & CPER_MEM_VALID_PA)
 		printk("%s""physical_address: 0x%016llx\n",
 		       pfx, mem->physical_addr);
-	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK)
+	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
 		printk("%s""physical_address_mask: 0x%016llx\n",
 		       pfx, mem->physical_addr_mask);
 	if (mem->validation_bits & CPER_MEM_VALID_NODE)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 0db6e4f..a30bc31 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -419,7 +419,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
 
 	if (sec_sev == GHES_SEV_CORRECTED &&
 	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
-	    (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)) {
+	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
 		pfn = mem_err->physical_addr >> PAGE_SHIFT;
 		if (pfn_valid(pfn))
 			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
@@ -430,7 +430,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
 	}
 	if (sev == GHES_SEV_RECOVERABLE &&
 	    sec_sev == GHES_SEV_RECOVERABLE &&
-	    mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
+	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
 		pfn = mem_err->physical_addr >> PAGE_SHIFT;
 		memory_failure_queue(pfn, 0, 0);
 	}
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index bb53467..0ad797b 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -297,15 +297,14 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
 	}
 
 	/* Error address */
-	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
+	if (mem_err->validation_bits & CPER_MEM_VALID_PA) {
 		e->page_frame_number = mem_err->physical_addr >> PAGE_SHIFT;
 		e->offset_in_page = mem_err->physical_addr & ~PAGE_MASK;
 	}
 
 	/* Error grain */
-	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK) {
+	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
 		e->grain = ~(mem_err->physical_addr_mask & ~PAGE_MASK);
-	}
 
 	/* Memory error location, mapped on e->location */
 	p = e->location;
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 09ebe21..2fc0ec3 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -218,8 +218,8 @@ enum {
 #define CPER_PROC_VALID_IP			0x1000
 
 #define CPER_MEM_VALID_ERROR_STATUS		0x0001
-#define CPER_MEM_VALID_PHYSICAL_ADDRESS		0x0002
-#define CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK	0x0004
+#define CPER_MEM_VALID_PA			0x0002
+#define CPER_MEM_VALID_PA_MASK			0x0004
 #define CPER_MEM_VALID_NODE			0x0008
 #define CPER_MEM_VALID_CARD			0x0010
 #define CPER_MEM_VALID_MODULE			0x0020
@@ -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
@@ -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;	/* card handle in UEFI 2.4 */
+	__u16	mem_dev_handle;		/* module handle in UEFI 2.4 */
 };
 
 struct cper_sec_pcie {
-- 
1.8.4.rc3

Re: [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:56:03 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> 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 <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Reviewed-by: Borislav Petkov <bp@suse.de>

Reviewed-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> ---
>  arch/x86/kernel/cpu/mcheck/mce-apei.c |  3 +--
>  drivers/acpi/apei/cper.c              |  7 ++++---
>  drivers/acpi/apei/ghes.c              |  4 ++--
>  drivers/edac/ghes_edac.c              |  5 ++---
>  include/linux/cper.h                  | 11 +++++++++--
>  5 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mcheck/mce-apei.c
> index cd8b166..de8b60a 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce-apei.c
> @@ -42,8 +42,7 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
>  	struct mce m;
>  
>  	/* Only corrected MC is reported */
> -	if (!corrected || !(mem_err->validation_bits &
> -				CPER_MEM_VALID_PHYSICAL_ADDRESS))
> +	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
>  		return;
>  
>  	mce_setup(&m);
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index eb5f6d6..946ef52 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -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
> @@ -191,16 +191,17 @@ static const char *cper_mem_err_type_strs[] = {
>  	"memory sparing",
>  	"scrub corrected error",
>  	"scrub uncorrected error",
> +	"physical memory map-out event",
>  };
>  
>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
>  {
>  	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
>  		printk("%s""error_status: 0x%016llx\n", pfx, mem->error_status);
> -	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)
> +	if (mem->validation_bits & CPER_MEM_VALID_PA)
>  		printk("%s""physical_address: 0x%016llx\n",
>  		       pfx, mem->physical_addr);
> -	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK)
> +	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
>  		printk("%s""physical_address_mask: 0x%016llx\n",
>  		       pfx, mem->physical_addr_mask);
>  	if (mem->validation_bits & CPER_MEM_VALID_NODE)
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 0db6e4f..a30bc31 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -419,7 +419,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
>  
>  	if (sec_sev == GHES_SEV_CORRECTED &&
>  	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
> -	    (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)) {
> +	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
>  		pfn = mem_err->physical_addr >> PAGE_SHIFT;
>  		if (pfn_valid(pfn))
>  			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
> @@ -430,7 +430,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
>  	}
>  	if (sev == GHES_SEV_RECOVERABLE &&
>  	    sec_sev == GHES_SEV_RECOVERABLE &&
> -	    mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
> +	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
>  		pfn = mem_err->physical_addr >> PAGE_SHIFT;
>  		memory_failure_queue(pfn, 0, 0);
>  	}
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index bb53467..0ad797b 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -297,15 +297,14 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
>  	}
>  
>  	/* Error address */
> -	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA) {
>  		e->page_frame_number = mem_err->physical_addr >> PAGE_SHIFT;
>  		e->offset_in_page = mem_err->physical_addr & ~PAGE_MASK;
>  	}
>  
>  	/* Error grain */
> -	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK) {
> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
>  		e->grain = ~(mem_err->physical_addr_mask & ~PAGE_MASK);
> -	}
>  
>  	/* Memory error location, mapped on e->location */
>  	p = e->location;
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index 09ebe21..2fc0ec3 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -218,8 +218,8 @@ enum {
>  #define CPER_PROC_VALID_IP			0x1000
>  
>  #define CPER_MEM_VALID_ERROR_STATUS		0x0001
> -#define CPER_MEM_VALID_PHYSICAL_ADDRESS		0x0002
> -#define CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK	0x0004
> +#define CPER_MEM_VALID_PA			0x0002
> +#define CPER_MEM_VALID_PA_MASK			0x0004
>  #define CPER_MEM_VALID_NODE			0x0008
>  #define CPER_MEM_VALID_CARD			0x0010
>  #define CPER_MEM_VALID_MODULE			0x0020
> @@ -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
> @@ -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;	/* card handle in UEFI 2.4 */
> +	__u16	mem_dev_handle;		/* module handle in UEFI 2.4 */
>  };
>  
>  struct cper_sec_pcie {


-- 

Cheers,
Mauro

Re: [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:56:03 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> 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 <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Reviewed-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/kernel/cpu/mcheck/mce-apei.c |  3 +--
>  drivers/acpi/apei/cper.c              |  7 ++++---
>  drivers/acpi/apei/ghes.c              |  4 ++--
>  drivers/edac/ghes_edac.c              |  5 ++---
>  include/linux/cper.h                  | 11 +++++++++--
>  5 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mcheck/mce-apei.c
> index cd8b166..de8b60a 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce-apei.c
> @@ -42,8 +42,7 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
>  	struct mce m;
>  
>  	/* Only corrected MC is reported */
> -	if (!corrected || !(mem_err->validation_bits &
> -				CPER_MEM_VALID_PHYSICAL_ADDRESS))
> +	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
>  		return;
>  
>  	mce_setup(&m);
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index eb5f6d6..946ef52 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -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
> @@ -191,16 +191,17 @@ static const char *cper_mem_err_type_strs[] = {
>  	"memory sparing",
>  	"scrub corrected error",
>  	"scrub uncorrected error",
> +	"physical memory map-out event",
>  };
>  
>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
>  {
>  	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
>  		printk("%s""error_status: 0x%016llx\n", pfx, mem->error_status);
> -	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)
> +	if (mem->validation_bits & CPER_MEM_VALID_PA)
>  		printk("%s""physical_address: 0x%016llx\n",
>  		       pfx, mem->physical_addr);
> -	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK)
> +	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
>  		printk("%s""physical_address_mask: 0x%016llx\n",
>  		       pfx, mem->physical_addr_mask);
>  	if (mem->validation_bits & CPER_MEM_VALID_NODE)
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 0db6e4f..a30bc31 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -419,7 +419,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
>  
>  	if (sec_sev == GHES_SEV_CORRECTED &&
>  	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
> -	    (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)) {
> +	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
>  		pfn = mem_err->physical_addr >> PAGE_SHIFT;
>  		if (pfn_valid(pfn))
>  			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
> @@ -430,7 +430,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
>  	}
>  	if (sev == GHES_SEV_RECOVERABLE &&
>  	    sec_sev == GHES_SEV_RECOVERABLE &&
> -	    mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
> +	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
>  		pfn = mem_err->physical_addr >> PAGE_SHIFT;
>  		memory_failure_queue(pfn, 0, 0);
>  	}
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index bb53467..0ad797b 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -297,15 +297,14 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
>  	}
>  
>  	/* Error address */
> -	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA) {
>  		e->page_frame_number = mem_err->physical_addr >> PAGE_SHIFT;
>  		e->offset_in_page = mem_err->physical_addr & ~PAGE_MASK;
>  	}
>  
>  	/* Error grain */
> -	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK) {
> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
>  		e->grain = ~(mem_err->physical_addr_mask & ~PAGE_MASK);
> -	}
>  
>  	/* Memory error location, mapped on e->location */
>  	p = e->location;
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index 09ebe21..2fc0ec3 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -218,8 +218,8 @@ enum {
>  #define CPER_PROC_VALID_IP			0x1000
>  
>  #define CPER_MEM_VALID_ERROR_STATUS		0x0001
> -#define CPER_MEM_VALID_PHYSICAL_ADDRESS		0x0002
> -#define CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK	0x0004
> +#define CPER_MEM_VALID_PA			0x0002
> +#define CPER_MEM_VALID_PA_MASK			0x0004
>  #define CPER_MEM_VALID_NODE			0x0008
>  #define CPER_MEM_VALID_CARD			0x0010
>  #define CPER_MEM_VALID_MODULE			0x0020
> @@ -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
> @@ -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;	/* card handle in UEFI 2.4 */
> +	__u16	mem_dev_handle;		/* module handle in UEFI 2.4 */

Hmm... you're adding 3 new types here and the corresponding space inside the
structure (rank, card_handle and module_handle), but the code that parses and
prints it is missing, at apei_mce_report_mem_error(), cper_print_mem(),
ghes_handle_memory_failure() and ghes_edac_report_mem_error().


>  };
>  
>  struct cper_sec_pcie {


-- 

Cheers,
Mauro

Re: [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error

[Chen Gong]

[-- Attachment #1: Type: text/plain, Size: 7609 bytes --]

On Thu, Oct 17, 2013 at 07:23:06AM -0300, Mauro Carvalho Chehab wrote:
> Date: Thu, 17 Oct 2013 07:23:06 -0300
> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, bp@alien8.de, joe@perches.com,
>  naveen.n.rao@linux.vnet.ibm.com, arozansk@redhat.com,
>  linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for
>  memory error
> X-Mailer: Claws Mail 3.9.2 (GTK+ 2.24.19; x86_64-redhat-linux-gnu)
> 
> Em Wed, 16 Oct 2013 10:56:03 -0400
> "Chen, Gong" <gong.chen@linux.intel.com> escreveu:
> 
> > 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 <tony.luck@intel.com>
> > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> > Reviewed-by: Borislav Petkov <bp@suse.de>
> > ---
> >  arch/x86/kernel/cpu/mcheck/mce-apei.c |  3 +--
> >  drivers/acpi/apei/cper.c              |  7 ++++---
> >  drivers/acpi/apei/ghes.c              |  4 ++--
> >  drivers/edac/ghes_edac.c              |  5 ++---
> >  include/linux/cper.h                  | 11 +++++++++--
> >  5 files changed, 18 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mcheck/mce-apei.c
> > index cd8b166..de8b60a 100644
> > --- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
> > +++ b/arch/x86/kernel/cpu/mcheck/mce-apei.c
> > @@ -42,8 +42,7 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
> >  	struct mce m;
> >  
> >  	/* Only corrected MC is reported */
> > -	if (!corrected || !(mem_err->validation_bits &
> > -				CPER_MEM_VALID_PHYSICAL_ADDRESS))
> > +	if (!corrected || !(mem_err->validation_bits & CPER_MEM_VALID_PA))
> >  		return;
> >  
> >  	mce_setup(&m);
> > diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> > index eb5f6d6..946ef52 100644
> > --- a/drivers/acpi/apei/cper.c
> > +++ b/drivers/acpi/apei/cper.c
> > @@ -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
> > @@ -191,16 +191,17 @@ static const char *cper_mem_err_type_strs[] = {
> >  	"memory sparing",
> >  	"scrub corrected error",
> >  	"scrub uncorrected error",
> > +	"physical memory map-out event",
> >  };
> >  
> >  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
> >  {
> >  	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
> >  		printk("%s""error_status: 0x%016llx\n", pfx, mem->error_status);
> > -	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)
> > +	if (mem->validation_bits & CPER_MEM_VALID_PA)
> >  		printk("%s""physical_address: 0x%016llx\n",
> >  		       pfx, mem->physical_addr);
> > -	if (mem->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK)
> > +	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> >  		printk("%s""physical_address_mask: 0x%016llx\n",
> >  		       pfx, mem->physical_addr_mask);
> >  	if (mem->validation_bits & CPER_MEM_VALID_NODE)
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> > index 0db6e4f..a30bc31 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -419,7 +419,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
> >  
> >  	if (sec_sev == GHES_SEV_CORRECTED &&
> >  	    (gdata->flags & CPER_SEC_ERROR_THRESHOLD_EXCEEDED) &&
> > -	    (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)) {
> > +	    (mem_err->validation_bits & CPER_MEM_VALID_PA)) {
> >  		pfn = mem_err->physical_addr >> PAGE_SHIFT;
> >  		if (pfn_valid(pfn))
> >  			memory_failure_queue(pfn, 0, MF_SOFT_OFFLINE);
> > @@ -430,7 +430,7 @@ static void ghes_handle_memory_failure(struct acpi_generic_data *gdata, int sev)
> >  	}
> >  	if (sev == GHES_SEV_RECOVERABLE &&
> >  	    sec_sev == GHES_SEV_RECOVERABLE &&
> > -	    mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
> > +	    mem_err->validation_bits & CPER_MEM_VALID_PA) {
> >  		pfn = mem_err->physical_addr >> PAGE_SHIFT;
> >  		memory_failure_queue(pfn, 0, 0);
> >  	}
> > diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> > index bb53467..0ad797b 100644
> > --- a/drivers/edac/ghes_edac.c
> > +++ b/drivers/edac/ghes_edac.c
> > @@ -297,15 +297,14 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
> >  	}
> >  
> >  	/* Error address */
> > -	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS) {
> > +	if (mem_err->validation_bits & CPER_MEM_VALID_PA) {
> >  		e->page_frame_number = mem_err->physical_addr >> PAGE_SHIFT;
> >  		e->offset_in_page = mem_err->physical_addr & ~PAGE_MASK;
> >  	}
> >  
> >  	/* Error grain */
> > -	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK) {
> > +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
> >  		e->grain = ~(mem_err->physical_addr_mask & ~PAGE_MASK);
> > -	}
> >  
> >  	/* Memory error location, mapped on e->location */
> >  	p = e->location;
> > diff --git a/include/linux/cper.h b/include/linux/cper.h
> > index 09ebe21..2fc0ec3 100644
> > --- a/include/linux/cper.h
> > +++ b/include/linux/cper.h
> > @@ -218,8 +218,8 @@ enum {
> >  #define CPER_PROC_VALID_IP			0x1000
> >  
> >  #define CPER_MEM_VALID_ERROR_STATUS		0x0001
> > -#define CPER_MEM_VALID_PHYSICAL_ADDRESS		0x0002
> > -#define CPER_MEM_VALID_PHYSICAL_ADDRESS_MASK	0x0004
> > +#define CPER_MEM_VALID_PA			0x0002
> > +#define CPER_MEM_VALID_PA_MASK			0x0004
> >  #define CPER_MEM_VALID_NODE			0x0008
> >  #define CPER_MEM_VALID_CARD			0x0010
> >  #define CPER_MEM_VALID_MODULE			0x0020
> > @@ -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
> > @@ -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;	/* card handle in UEFI 2.4 */
> > +	__u16	mem_dev_handle;		/* module handle in UEFI 2.4 */
> 
> Hmm... you're adding 3 new types here and the corresponding space inside the
> structure (rank, card_handle and module_handle), but the code that parses and
> prints it is missing, at apei_mce_report_mem_error(), cper_print_mem(),
> ghes_handle_memory_failure() and ghes_edac_report_mem_error().
> 
> 

1. This patch is just for definition update. 
2. apei_mce_report_mem_error/cper_print_mem/ghes_handle_memory_failure
   finally point to apei/cper. So patch [8/9] can cover it. As for 
   EDAC part (ghes_edac_report_mem_error), I can add a new separate
   patch to fix missed part.

> >  };
> >  
> >  struct cper_sec_pcie {
> 
> 
> -- 
> 
> Cheers,
> Mauro

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error

[Naveen N. Rao]

On 10/16/2013 08:26 PM, Chen, Gong wrote:
> 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 <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Reviewed-by: Borislav Petkov <bp@suse.de>
> ---
>   arch/x86/kernel/cpu/mcheck/mce-apei.c |  3 +--
>   drivers/acpi/apei/cper.c              |  7 ++++---
>   drivers/acpi/apei/ghes.c              |  4 ++--
>   drivers/edac/ghes_edac.c              |  5 ++---
>   include/linux/cper.h                  | 11 +++++++++--
>   5 files changed, 18 insertions(+), 12 deletions(-)

Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>


Regards,
Naveen

[PATCH v2 7/9] ACPI, APEI, CPER: Enhance memory reporting capability

[Chen, Gong]

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 <tony.luck@intel.com>
Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 drivers/acpi/apei/cper.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index 946ef52..b1a8a55 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -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[] = {
-- 
1.8.4.rc3

Re: [PATCH v2 7/9] ACPI, APEI, CPER: Enhance memory reporting capability

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:56:04AM -0400, Chen, Gong wrote:
> 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 <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 7/9] ACPI, APEI, CPER: Enhance memory reporting capability

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:56:04 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> 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 <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Reviewed-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> ---
>  drivers/acpi/apei/cper.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index 946ef52..b1a8a55 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -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[] = {


-- 

Cheers,
Mauro

[PATCH v2 8/9] ACPI, APEI, CPER: Cleanup CPER memory error output format

[Chen, 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 <tony.luck@intel.com>
Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/acpi/apei/cper.c | 69 +++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index b1a8a55..f5bc227 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -33,6 +33,9 @@
 #include <linux/pci.h>
 #include <linux/aer.h>
 
+#define INDENT_SP	" "
+#define HW_CE_INFO \
+	"Above error has been corrected by h/w and no further action required"
 /*
  * CPER record ID need to be unique even after reboot, because record
  * ID is used as index for ERST storage, while CPER records from
@@ -206,29 +209,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);
+		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,
@@ -296,55 +299,45 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
 }
 
-static const char * const cper_estatus_section_flag_strs[] = {
-	"primary",
-	"containment warning",
-	"reset",
-	"error threshold exceeded",
-	"resource not accessible",
-	"latent error",
-};
-
 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;
+	char newpfx[64];
 
 	severity = gdata->error_severity;
-	printk("%s""section: %d, severity: %d, %s\n", pfx, sec_no, severity,
+	printk("%s""Error %d, type: %s\n", pfx, sec_no,
 	       cper_severity_str(severity));
-	printk("%s""flags: 0x%02x\n", pfx, gdata->flags);
-	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)
 		printk("%s""fru_text: %.20s\n", pfx, gdata->fru_text);
 
+	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
 	if (!uuid_le_cmp(*sec_type, CPER_SEC_PROC_GENERIC)) {
 		struct cper_sec_proc_generic *proc_err = (void *)(gdata + 1);
-		printk("%s""section_type: general processor error\n", pfx);
+		printk("%s""section_type: general processor error\n", newpfx);
 		if (gdata->error_data_length >= sizeof(*proc_err))
-			cper_print_proc_generic(pfx, proc_err);
+			cper_print_proc_generic(newpfx, proc_err);
 		else
 			goto err_section_too_small;
 	} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
 		struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
-		printk("%s""section_type: memory error\n", pfx);
+		printk("%s""section_type: memory error\n", newpfx);
 		if (gdata->error_data_length >= sizeof(*mem_err))
-			cper_print_mem(pfx, mem_err);
+			cper_print_mem(newpfx, mem_err);
 		else
 			goto err_section_too_small;
 	} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PCIE)) {
 		struct cper_sec_pcie *pcie = (void *)(gdata + 1);
-		printk("%s""section_type: PCIe error\n", pfx);
+		printk("%s""section_type: PCIe error\n", newpfx);
 		if (gdata->error_data_length >= sizeof(*pcie))
-			cper_print_pcie(pfx, pcie, gdata);
+			cper_print_pcie(newpfx, pcie, gdata);
 		else
 			goto err_section_too_small;
 	} else
-		printk("%s""section type: unknown, %pUl\n", pfx, sec_type);
+		printk("%s""section type: unknown, %pUl\n", newpfx, sec_type);
 
 	return;
 
@@ -358,21 +351,25 @@ void cper_estatus_print(const char *pfx,
 	struct acpi_generic_data *gdata;
 	unsigned int data_len, gedata_len;
 	int sec_no = 0;
+	char newpfx[64];
 	__u16 severity;
 
-	printk("%s""Generic Hardware Error Status\n", pfx);
 	severity = estatus->error_severity;
-	printk("%s""severity: %d, %s\n", pfx, severity,
-	       cper_severity_str(severity));
+	printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
 	data_len = estatus->data_length;
 	gdata = (struct acpi_generic_data *)(estatus + 1);
+	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
 	while (data_len >= sizeof(*gdata)) {
 		gedata_len = gdata->error_data_length;
-		cper_estatus_print_section(pfx, gdata, sec_no);
+		cper_estatus_print_section(newpfx, gdata, sec_no);
 		data_len -= gedata_len + sizeof(*gdata);
 		gdata = (void *)(gdata + 1) + gedata_len;
 		sec_no++;
 	}
+	if (severity != CPER_SEV_FATAL)
+		printk("%s%s\n", pfx,
+		       "Above error has been corrected by h/w "
+		       "and require no further action");
 }
 EXPORT_SYMBOL_GPL(cper_estatus_print);
 
-- 
1.8.4.rc3

Re: [PATCH v2 8/9] ACPI, APEI, CPER: Cleanup CPER memory error output format

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:56:05AM -0400, Chen, Gong wrote:
> 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 <tony.luck@intel.com>
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> ---
>  drivers/acpi/apei/cper.c | 69 +++++++++++++++++++++++-------------------------
>  1 file changed, 33 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index b1a8a55..f5bc227 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -33,6 +33,9 @@
>  #include <linux/pci.h>
>  #include <linux/aer.h>
>  
> +#define INDENT_SP	" "
> +#define HW_CE_INFO \
> +	"Above error has been corrected by h/w and no further action required"

Leftover?


> +	if (severity != CPER_SEV_FATAL)
> +		printk("%s%s\n", pfx,
> +		       "Above error has been corrected by h/w "
> +		       "and require no further action");

Let's write it out and correct grammar:

"Above error has been corrected by the hardware and requires no further action."

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 8/9] ACPI, APEI, CPER: Cleanup CPER memory error output format

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 19:24:10 +0200
Borislav Petkov <bp@alien8.de> escreveu:

> On Wed, Oct 16, 2013 at 10:56:05AM -0400, Chen, Gong wrote:
> > 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 <tony.luck@intel.com>
> > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>

Considering the suggested fixes below:

Reviewed-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> > ---
> >  drivers/acpi/apei/cper.c | 69 +++++++++++++++++++++++-------------------------
> >  1 file changed, 33 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> > index b1a8a55..f5bc227 100644
> > --- a/drivers/acpi/apei/cper.c
> > +++ b/drivers/acpi/apei/cper.c
> > @@ -33,6 +33,9 @@
> >  #include <linux/pci.h>
> >  #include <linux/aer.h>
> >  
> > +#define INDENT_SP	" "
> > +#define HW_CE_INFO \
> > +	"Above error has been corrected by h/w and no further action required"
> 
> Leftover?
> 
> 
> > +	if (severity != CPER_SEV_FATAL)
> > +		printk("%s%s\n", pfx,
> > +		       "Above error has been corrected by h/w "
> > +		       "and require no further action");
> 
> Let's write it out and correct grammar:
> 
> "Above error has been corrected by the hardware and requires no further action."
> 
> Thanks.
> 


-- 

Cheers,
Mauro

[PATCH v2 9/9] ACPI / trace: Add trace interface for eMCA driver

[Chen, Gong]

Use trace interface to elaborate all H/W error related
information.

v2 -> v1: move trace interface into ras_event.h

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/acpi/Kconfig        |   7 ++-
 drivers/acpi/Makefile       |   4 ++
 drivers/acpi/acpi_extlog.c  |  28 +++++++++++-
 drivers/acpi/apei/cper.c    |  13 ++++--
 drivers/acpi/extlog_trace.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/extlog_trace.h |  16 +++++++
 include/linux/cper.h        |   2 +
 include/ras/ras_event.h     |  61 +++++++++++++++++++++++++
 8 files changed, 232 insertions(+), 6 deletions(-)
 create mode 100644 drivers/acpi/extlog_trace.c
 create mode 100644 drivers/acpi/extlog_trace.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 819c06b..0cc8fc8 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -372,13 +372,18 @@ config ACPI_BGRT
 
 source "drivers/acpi/apei/Kconfig"
 
+config EXTLOG_TRACE
+	def_bool n
+
 config ACPI_EXTLOG
 	tristate "Extended Error Log support"
 	depends on X86_MCE
+	select EXTLOG_TRACE
 	default n
 	help
 	  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.
+	  driver adds support for that functionality plus an additional special
+	  tracepoint which carries that information to userspace.
 
 endif	# ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index bce34af..a6e41b7 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -83,4 +83,8 @@ obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
 
 obj-$(CONFIG_ACPI_APEI)		+= apei/
 
+# extended log support
+acpi-$(CONFIG_EXTLOG_TRACE)	+= extlog_trace.o
+CFLAGS_extlog_trace.o := -I$(src)
+
 obj-$(CONFIG_ACPI_EXTLOG)	+= acpi_extlog.o
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index d55b072..01e6a89 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -15,6 +15,7 @@
 #include <asm/mce.h>
 
 #include "apei/apei-internal.h"
+#include "extlog_trace.h"
 
 #define EXT_ELOG_ENTRY_MASK	GENMASK_ULL(52, 0) /* elog entry address mask */
 
@@ -44,6 +45,8 @@ struct extlog_l1_head {
 
 static u8 extlog_dsm_uuid[] = "663E35AF-CC10-41A4-88EA-5470AF055295";
 
+static const uuid_le invalid_uuid = NULL_UUID_LE;
+
 /* L1 table related physical address */
 static u64 elog_base;
 static size_t elog_size;
@@ -132,7 +135,12 @@ static int print_extlog_rcd(const char *pfx,
 
 static int extlog_print(const char *pfx, int cpu, int bank)
 {
-	struct acpi_generic_status *estatus;
+	struct acpi_generic_status *estatus, *tmp;
+	struct acpi_generic_data *gdata;
+	const uuid_le *fru_id = &invalid_uuid;
+	char *fru_text = "";
+	uuid_le *sec_type;
+	static u64 err_count;
 	int rc;
 
 	estatus = extlog_elog_entry_check(cpu, bank);
@@ -143,7 +151,23 @@ static int extlog_print(const char *pfx, int cpu, int bank)
 	/* 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);
+	tmp = (struct acpi_generic_status *)elog_buf;
+	gdata = (struct acpi_generic_data *)(tmp + 1);
+	rc = print_extlog_rcd(pfx, tmp, cpu);
+
+	/* trace extended error log */
+	err_count++;
+	if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
+		fru_id = (uuid_le *)gdata->fru_id;
+	if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
+		fru_text = gdata->fru_text;
+	sec_type = (uuid_le *)gdata->section_type;
+	if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
+		struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
+		if (gdata->error_data_length >= sizeof(*mem_err))
+			trace_mem_error(fru_id, fru_text, err_count,
+					gdata->error_severity, mem_err);
+	}
 
 	return rc;
 }
diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index f5bc227..44bde6a 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -59,11 +59,12 @@ static const char *cper_severity_strs[] = {
 	"info",
 };
 
-static const char *cper_severity_str(unsigned int severity)
+const char *cper_severity_str(unsigned int severity)
 {
 	return severity < ARRAY_SIZE(cper_severity_strs) ?
 		cper_severity_strs[severity] : "unknown";
 }
+EXPORT_SYMBOL_GPL(cper_severity_str);
 
 /*
  * cper_print_bits - print strings for set bits
@@ -198,6 +199,13 @@ static const char *cper_mem_err_type_strs[] = {
 	"physical memory map-out event",
 };
 
+const char *cper_mem_err_type_str(unsigned int etype)
+{
+	return etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
+		cper_mem_err_type_strs[etype] : "unknown";
+}
+EXPORT_SYMBOL_GPL(cper_mem_err_type_str);
+
 static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 {
 	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
@@ -235,8 +243,7 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE) {
 		u8 etype = mem->error_type;
 		printk("%s""error_type: %d, %s\n", pfx, etype,
-		       etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
-		       cper_mem_err_type_strs[etype] : "unknown");
+			cper_mem_err_type_str(etype));
 	}
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE) {
 		const char *bank = NULL, *device = NULL;
diff --git a/drivers/acpi/extlog_trace.c b/drivers/acpi/extlog_trace.c
new file mode 100644
index 0000000..2864080
--- /dev/null
+++ b/drivers/acpi/extlog_trace.c
@@ -0,0 +1,107 @@
+#include <linux/export.h>
+#include <linux/dmi.h>
+#include "extlog_trace.h"
+
+#define CREATE_TRACE_POINTS
+#define TRACE_INCLUDE_PATH ../../include/ras
+#include <ras/ras_event.h>
+
+static char mem_location[LOC_LEN];
+static char dimm_location[LOC_LEN];
+
+static void mem_err_location(struct cper_sec_mem_err *mem)
+{
+	char *p;
+	u32 n = 0;
+
+	memset(mem_location, 0, LOC_LEN);
+	p = mem_location;
+	if (mem->validation_bits & CPER_MEM_VALID_NODE)
+		n += sprintf(p + n, " node: %d", mem->node);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_CARD)
+		n += sprintf(p + n, " card: %d", mem->card);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
+		n += sprintf(p + n, " module: %d", mem->module);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
+		n += sprintf(p + n, " rank: %d", mem->rank);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_BANK)
+		n += sprintf(p + n, " bank: %d", mem->bank);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)
+		n += sprintf(p + n, " device: %d", mem->device);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_ROW)
+		n += sprintf(p + n, " row: %d", mem->row);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_COLUMN)
+		n += sprintf(p + n, " column: %d", mem->column);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_BIT_POSITION)
+		n += sprintf(p + n, " bit_position: %d", mem->bit_pos);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_REQUESTOR_ID)
+		n += sprintf(p + n, " requestor_id: 0x%016llx",
+				mem->requestor_id);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_RESPONDER_ID)
+		n += sprintf(p + n, " responder_id: 0x%016llx",
+				mem->responder_id);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_TARGET_ID)
+		n += sprintf(p + n, " target_id: 0x%016llx", mem->target_id);
+end:
+	return;
+}
+
+static void dimm_err_location(struct cper_sec_mem_err *mem)
+{
+	const char *bank = NULL, *device = NULL;
+
+	memset(dimm_location, 0, LOC_LEN);
+	if (!(mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE))
+		return;
+
+	dmi_memdev_name(mem->mem_dev_handle, &bank, &device);
+	if (bank != NULL && device != NULL)
+		snprintf(dimm_location, LOC_LEN - 1, "%s %s", bank, device);
+	else
+		snprintf(dimm_location, LOC_LEN - 1, "DMI handle: 0x%.4x",
+			 mem->mem_dev_handle);
+}
+
+void trace_mem_error(const uuid_le *fru_id, char *fru_text,
+			    u64 err_count, u32 severity,
+			    struct cper_sec_mem_err *mem)
+{
+	u32 etype = ~0U;
+	u64 phy_addr = 0;
+
+	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
+		etype = mem->error_type;
+	if (mem->validation_bits & CPER_MEM_VALID_PA) {
+		phy_addr = mem->physical_addr;
+		if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
+			phy_addr &= mem->physical_addr_mask;
+	}
+	mem_err_location(mem);
+	dimm_err_location(mem);
+
+	trace_extlog_mem_event(etype, dimm_location, fru_id, fru_text,
+			       err_count, severity, phy_addr, mem_location);
+}
+EXPORT_SYMBOL_GPL(trace_mem_error);
diff --git a/drivers/acpi/extlog_trace.h b/drivers/acpi/extlog_trace.h
new file mode 100644
index 0000000..67bb2c5
--- /dev/null
+++ b/drivers/acpi/extlog_trace.h
@@ -0,0 +1,16 @@
+#ifndef __DEBUG_EXTLOG_H
+#define __DEBUG_EXTLOG_H
+
+#include <linux/cper.h>
+
+#ifdef CONFIG_EXTLOG_TRACE
+extern void trace_mem_error(const uuid_le *fru_id, char *fru_text,
+		u64 err_count, u32 severity, struct cper_sec_mem_err *mem);
+#else
+void trace_mem_error(const uuid_le *fru_id, char *fru_text,
+		u64 err_count, u32 severity, struct cper_sec_mem_err *mem)
+{
+}
+#endif
+
+#endif
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 2fc0ec3..c6d87fc 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -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 * const strs[], unsigned int strs_size);
 
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 21cdb0b..fce14b6 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -8,6 +8,67 @@
 #include <linux/tracepoint.h>
 #include <linux/edac.h>
 #include <linux/ktime.h>
+#include <linux/cper.h>
+
+/*
+ * MCE Extended Error Log trace event
+ *
+ * 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))
+);
 
 /*
  * Hardware Events Report
-- 
1.8.4.rc3

Re: [PATCH v2 9/9] ACPI / trace: Add trace interface for eMCA driver

[Mauro Carvalho Chehab]

Em Wed, 16 Oct 2013 10:56:06 -0400
"Chen, Gong" <gong.chen@linux.intel.com> escreveu:

> Use trace interface to elaborate all H/W error related
> information.
> 
> v2 -> v1: move trace interface into ras_event.h
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>

For the already explained reasons:

Nacked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

You should re-use the existing tracepoint and better integrate it with
EDAC instead of reinventing the wheel.

Regards,
Mauro

> ---
>  drivers/acpi/Kconfig        |   7 ++-
>  drivers/acpi/Makefile       |   4 ++
>  drivers/acpi/acpi_extlog.c  |  28 +++++++++++-
>  drivers/acpi/apei/cper.c    |  13 ++++--
>  drivers/acpi/extlog_trace.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/extlog_trace.h |  16 +++++++
>  include/linux/cper.h        |   2 +
>  include/ras/ras_event.h     |  61 +++++++++++++++++++++++++
>  8 files changed, 232 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/acpi/extlog_trace.c
>  create mode 100644 drivers/acpi/extlog_trace.h
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 819c06b..0cc8fc8 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -372,13 +372,18 @@ config ACPI_BGRT
>  
>  source "drivers/acpi/apei/Kconfig"
>  
> +config EXTLOG_TRACE
> +	def_bool n
> +
>  config ACPI_EXTLOG
>  	tristate "Extended Error Log support"
>  	depends on X86_MCE
> +	select EXTLOG_TRACE
>  	default n
>  	help
>  	  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.
> +	  driver adds support for that functionality plus an additional special
> +	  tracepoint which carries that information to userspace.
>  
>  endif	# ACPI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index bce34af..a6e41b7 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -83,4 +83,8 @@ obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
>  
>  obj-$(CONFIG_ACPI_APEI)		+= apei/
>  
> +# extended log support
> +acpi-$(CONFIG_EXTLOG_TRACE)	+= extlog_trace.o
> +CFLAGS_extlog_trace.o := -I$(src)
> +
>  obj-$(CONFIG_ACPI_EXTLOG)	+= acpi_extlog.o
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index d55b072..01e6a89 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -15,6 +15,7 @@
>  #include <asm/mce.h>
>  
>  #include "apei/apei-internal.h"
> +#include "extlog_trace.h"
>  
>  #define EXT_ELOG_ENTRY_MASK	GENMASK_ULL(52, 0) /* elog entry address mask */
>  
> @@ -44,6 +45,8 @@ struct extlog_l1_head {
>  
>  static u8 extlog_dsm_uuid[] = "663E35AF-CC10-41A4-88EA-5470AF055295";
>  
> +static const uuid_le invalid_uuid = NULL_UUID_LE;
> +
>  /* L1 table related physical address */
>  static u64 elog_base;
>  static size_t elog_size;
> @@ -132,7 +135,12 @@ static int print_extlog_rcd(const char *pfx,
>  
>  static int extlog_print(const char *pfx, int cpu, int bank)
>  {
> -	struct acpi_generic_status *estatus;
> +	struct acpi_generic_status *estatus, *tmp;
> +	struct acpi_generic_data *gdata;
> +	const uuid_le *fru_id = &invalid_uuid;
> +	char *fru_text = "";
> +	uuid_le *sec_type;
> +	static u64 err_count;
>  	int rc;
>  
>  	estatus = extlog_elog_entry_check(cpu, bank);
> @@ -143,7 +151,23 @@ static int extlog_print(const char *pfx, int cpu, int bank)
>  	/* 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);
> +	tmp = (struct acpi_generic_status *)elog_buf;
> +	gdata = (struct acpi_generic_data *)(tmp + 1);
> +	rc = print_extlog_rcd(pfx, tmp, cpu);
> +
> +	/* trace extended error log */
> +	err_count++;
> +	if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
> +		fru_id = (uuid_le *)gdata->fru_id;
> +	if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
> +		fru_text = gdata->fru_text;
> +	sec_type = (uuid_le *)gdata->section_type;
> +	if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
> +		struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
> +		if (gdata->error_data_length >= sizeof(*mem_err))
> +			trace_mem_error(fru_id, fru_text, err_count,
> +					gdata->error_severity, mem_err);
> +	}
>  
>  	return rc;
>  }
> diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
> index f5bc227..44bde6a 100644
> --- a/drivers/acpi/apei/cper.c
> +++ b/drivers/acpi/apei/cper.c
> @@ -59,11 +59,12 @@ static const char *cper_severity_strs[] = {
>  	"info",
>  };
>  
> -static const char *cper_severity_str(unsigned int severity)
> +const char *cper_severity_str(unsigned int severity)
>  {
>  	return severity < ARRAY_SIZE(cper_severity_strs) ?
>  		cper_severity_strs[severity] : "unknown";
>  }
> +EXPORT_SYMBOL_GPL(cper_severity_str);
>  
>  /*
>   * cper_print_bits - print strings for set bits
> @@ -198,6 +199,13 @@ static const char *cper_mem_err_type_strs[] = {
>  	"physical memory map-out event",
>  };
>  
> +const char *cper_mem_err_type_str(unsigned int etype)
> +{
> +	return etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
> +		cper_mem_err_type_strs[etype] : "unknown";
> +}
> +EXPORT_SYMBOL_GPL(cper_mem_err_type_str);
> +
>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
>  {
>  	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
> @@ -235,8 +243,7 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
>  	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE) {
>  		u8 etype = mem->error_type;
>  		printk("%s""error_type: %d, %s\n", pfx, etype,
> -		       etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
> -		       cper_mem_err_type_strs[etype] : "unknown");
> +			cper_mem_err_type_str(etype));
>  	}
>  	if (mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE) {
>  		const char *bank = NULL, *device = NULL;
> diff --git a/drivers/acpi/extlog_trace.c b/drivers/acpi/extlog_trace.c
> new file mode 100644
> index 0000000..2864080
> --- /dev/null
> +++ b/drivers/acpi/extlog_trace.c
> @@ -0,0 +1,107 @@
> +#include <linux/export.h>
> +#include <linux/dmi.h>
> +#include "extlog_trace.h"
> +
> +#define CREATE_TRACE_POINTS
> +#define TRACE_INCLUDE_PATH ../../include/ras
> +#include <ras/ras_event.h>
> +
> +static char mem_location[LOC_LEN];
> +static char dimm_location[LOC_LEN];
> +
> +static void mem_err_location(struct cper_sec_mem_err *mem)
> +{
> +	char *p;
> +	u32 n = 0;
> +
> +	memset(mem_location, 0, LOC_LEN);
> +	p = mem_location;
> +	if (mem->validation_bits & CPER_MEM_VALID_NODE)
> +		n += sprintf(p + n, " node: %d", mem->node);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_CARD)
> +		n += sprintf(p + n, " card: %d", mem->card);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
> +		n += sprintf(p + n, " module: %d", mem->module);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
> +		n += sprintf(p + n, " rank: %d", mem->rank);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_BANK)
> +		n += sprintf(p + n, " bank: %d", mem->bank);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)
> +		n += sprintf(p + n, " device: %d", mem->device);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_ROW)
> +		n += sprintf(p + n, " row: %d", mem->row);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_COLUMN)
> +		n += sprintf(p + n, " column: %d", mem->column);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_BIT_POSITION)
> +		n += sprintf(p + n, " bit_position: %d", mem->bit_pos);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_REQUESTOR_ID)
> +		n += sprintf(p + n, " requestor_id: 0x%016llx",
> +				mem->requestor_id);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_RESPONDER_ID)
> +		n += sprintf(p + n, " responder_id: 0x%016llx",
> +				mem->responder_id);
> +	if (n >= LOC_LEN)
> +		goto end;
> +	if (mem->validation_bits & CPER_MEM_VALID_TARGET_ID)
> +		n += sprintf(p + n, " target_id: 0x%016llx", mem->target_id);
> +end:
> +	return;
> +}
> +
> +static void dimm_err_location(struct cper_sec_mem_err *mem)
> +{
> +	const char *bank = NULL, *device = NULL;
> +
> +	memset(dimm_location, 0, LOC_LEN);
> +	if (!(mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE))
> +		return;
> +
> +	dmi_memdev_name(mem->mem_dev_handle, &bank, &device);
> +	if (bank != NULL && device != NULL)
> +		snprintf(dimm_location, LOC_LEN - 1, "%s %s", bank, device);
> +	else
> +		snprintf(dimm_location, LOC_LEN - 1, "DMI handle: 0x%.4x",
> +			 mem->mem_dev_handle);
> +}
> +
> +void trace_mem_error(const uuid_le *fru_id, char *fru_text,
> +			    u64 err_count, u32 severity,
> +			    struct cper_sec_mem_err *mem)
> +{
> +	u32 etype = ~0U;
> +	u64 phy_addr = 0;
> +
> +	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
> +		etype = mem->error_type;
> +	if (mem->validation_bits & CPER_MEM_VALID_PA) {
> +		phy_addr = mem->physical_addr;
> +		if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> +			phy_addr &= mem->physical_addr_mask;
> +	}
> +	mem_err_location(mem);
> +	dimm_err_location(mem);
> +
> +	trace_extlog_mem_event(etype, dimm_location, fru_id, fru_text,
> +			       err_count, severity, phy_addr, mem_location);
> +}
> +EXPORT_SYMBOL_GPL(trace_mem_error);
> diff --git a/drivers/acpi/extlog_trace.h b/drivers/acpi/extlog_trace.h
> new file mode 100644
> index 0000000..67bb2c5
> --- /dev/null
> +++ b/drivers/acpi/extlog_trace.h
> @@ -0,0 +1,16 @@
> +#ifndef __DEBUG_EXTLOG_H
> +#define __DEBUG_EXTLOG_H
> +
> +#include <linux/cper.h>
> +
> +#ifdef CONFIG_EXTLOG_TRACE
> +extern void trace_mem_error(const uuid_le *fru_id, char *fru_text,
> +		u64 err_count, u32 severity, struct cper_sec_mem_err *mem);
> +#else
> +void trace_mem_error(const uuid_le *fru_id, char *fru_text,
> +		u64 err_count, u32 severity, struct cper_sec_mem_err *mem)
> +{
> +}
> +#endif
> +
> +#endif
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index 2fc0ec3..c6d87fc 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -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 * const strs[], unsigned int strs_size);
>  
> diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
> index 21cdb0b..fce14b6 100644
> --- a/include/ras/ras_event.h
> +++ b/include/ras/ras_event.h
> @@ -8,6 +8,67 @@
>  #include <linux/tracepoint.h>
>  #include <linux/edac.h>
>  #include <linux/ktime.h>
> +#include <linux/cper.h>
> +
> +/*
> + * MCE Extended Error Log trace event
> + *
> + * 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))
> +);
>  
>  /*
>   * Hardware Events Report


-- 

Cheers,
Mauro

Re: [PATCH v2 9/9] ACPI / trace: Add trace interface for eMCA driver

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:56:06AM -0400, Chen, Gong wrote:
> Use trace interface to elaborate all H/W error related
> information.
> 
> v2 -> v1: move trace interface into ras_event.h
> 
> Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
> ---

Just a nitpick below. Other than that:

Acked-by: Borislav Petkov <bp@suse.de>

> diff --git a/drivers/acpi/extlog_trace.h b/drivers/acpi/extlog_trace.h
> new file mode 100644
> index 0000000..67bb2c5
> --- /dev/null
> +++ b/drivers/acpi/extlog_trace.h
> @@ -0,0 +1,16 @@
> +#ifndef __DEBUG_EXTLOG_H
> +#define __DEBUG_EXTLOG_H
> +
> +#include <linux/cper.h>
> +
> +#ifdef CONFIG_EXTLOG_TRACE
> +extern void trace_mem_error(const uuid_le *fru_id, char *fru_text,
> +		u64 err_count, u32 severity, struct cper_sec_mem_err *mem);

Applying: ACPI / trace: Add trace interface for eMCA driver
CHECK: extern prototypes should be avoided in .h files
#265: FILE: drivers/acpi/extlog_trace.h:7:
+extern void trace_mem_error(const uuid_le *fru_id, char *fru_text,

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 0/9] Extended H/W error log driver

[Chen Gong]

[-- Attachment #1: Type: text/plain, Size: 10841 bytes --]

[...]
> 
> dmesg output format has been updated based on the suggestion from Boris.
> For trace output format we still need further discussion. In the last
> patch(support trace interface) I have to reserve previous Kconfig format
> because I find once I put trace_event interface in the module, it will
> not work. I will paste another trace patch(it only works when acpi_extlog is
> builtin) for your answer.
> --

I put my bogus trace patch here.


=========================8<==========================

Subject: ACPI / trace: Add trace interface for eMCA driver (bogus patch)

Use trace interface to elaborate all H/W error related
information.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/acpi/Kconfig       |   3 +-
 drivers/acpi/acpi_extlog.c | 131 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/apei/cper.c   |  13 +++--
 include/linux/cper.h       |   2 +
 include/ras/ras_event.h    |  61 +++++++++++++++++++++
 5 files changed, 204 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 819c06b..eee0258 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -379,6 +379,7 @@ config ACPI_EXTLOG
 	help
 	  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.
+	  driver adds support for that functionality plus an additional special
+	  tracepoint which carries that information to userspace.
 
 endif	# ACPI
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index d55b072..108f4ae 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -11,11 +11,19 @@
 #include <linux/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <linux/cper.h>
+#include <linux/dmi.h>
 #include <linux/ratelimit.h>
 #include <asm/mce.h>
 
 #include "apei/apei-internal.h"
 
+#define CREATE_TRACE_POINTS
+#define TRACE_INCLUDE_PATH ../../include/ras
+#include <ras/ras_event.h>
+
+static char mem_location[LOC_LEN];
+static char dimm_location[LOC_LEN];
+
 #define EXT_ELOG_ENTRY_MASK	GENMASK_ULL(52, 0) /* elog entry address mask */
 
 #define EXTLOG_DSM_REV		0x0
@@ -44,6 +52,8 @@ struct extlog_l1_head {
 
 static u8 extlog_dsm_uuid[] = "663E35AF-CC10-41A4-88EA-5470AF055295";
 
+static const uuid_le invalid_uuid = NULL_UUID_LE;
+
 /* L1 table related physical address */
 static u64 elog_base;
 static size_t elog_size;
@@ -130,9 +140,110 @@ static int print_extlog_rcd(const char *pfx,
 	return 1;
 }
 
+static void mem_err_location(struct cper_sec_mem_err *mem)
+{
+	char *p;
+	u32 n = 0;
+
+	memset(mem_location, 0, LOC_LEN);
+	p = mem_location;
+	if (mem->validation_bits & CPER_MEM_VALID_NODE)
+		n += sprintf(p + n, " node: %d", mem->node);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_CARD)
+		n += sprintf(p + n, " card: %d", mem->card);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
+		n += sprintf(p + n, " module: %d", mem->module);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
+		n += sprintf(p + n, " rank: %d", mem->rank);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_BANK)
+		n += sprintf(p + n, " bank: %d", mem->bank);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)
+		n += sprintf(p + n, " device: %d", mem->device);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_ROW)
+		n += sprintf(p + n, " row: %d", mem->row);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_COLUMN)
+		n += sprintf(p + n, " column: %d", mem->column);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_BIT_POSITION)
+		n += sprintf(p + n, " bit_position: %d", mem->bit_pos);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_REQUESTOR_ID)
+		n += sprintf(p + n, " requestor_id: 0x%016llx",
+				mem->requestor_id);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_RESPONDER_ID)
+		n += sprintf(p + n, " responder_id: 0x%016llx",
+				mem->responder_id);
+	if (n >= LOC_LEN)
+		goto end;
+	if (mem->validation_bits & CPER_MEM_VALID_TARGET_ID)
+		n += sprintf(p + n, " target_id: 0x%016llx", mem->target_id);
+end:
+	return;
+}
+
+static void dimm_err_location(struct cper_sec_mem_err *mem)
+{
+	const char *bank = NULL, *device = NULL;
+
+	memset(dimm_location, 0, LOC_LEN);
+	if (!(mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE))
+		return;
+
+	dmi_memdev_name(mem->mem_dev_handle, &bank, &device);
+	if (bank != NULL && device != NULL)
+		snprintf(dimm_location, LOC_LEN - 1, "%s %s", bank, device);
+	else
+		snprintf(dimm_location, LOC_LEN - 1, "DMI handle: 0x%.4x",
+			 mem->mem_dev_handle);
+}
+
+static void trace_mem_error(const uuid_le *fru_id, char *fru_text,
+			    u64 err_count, u32 severity,
+			    struct cper_sec_mem_err *mem)
+{
+	u32 etype = ~0U;
+	u64 phy_addr = 0;
+
+	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
+		etype = mem->error_type;
+	if (mem->validation_bits & CPER_MEM_VALID_PA) {
+		phy_addr = mem->physical_addr;
+		if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
+			phy_addr &= mem->physical_addr_mask;
+	}
+	mem_err_location(mem);
+	dimm_err_location(mem);
+
+	trace_extlog_mem_event(etype, dimm_location, fru_id, fru_text,
+			       err_count, severity, phy_addr, mem_location);
+}
+
 static int extlog_print(const char *pfx, int cpu, int bank)
 {
-	struct acpi_generic_status *estatus;
+	struct acpi_generic_status *estatus, *tmp;
+	struct acpi_generic_data *gdata;
+	const uuid_le *fru_id = &invalid_uuid;
+	char *fru_text = "";
+	uuid_le *sec_type;
+	static u64 err_count;
 	int rc;
 
 	estatus = extlog_elog_entry_check(cpu, bank);
@@ -143,7 +254,23 @@ static int extlog_print(const char *pfx, int cpu, int bank)
 	/* 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);
+	tmp = (struct acpi_generic_status *)elog_buf;
+	gdata = (struct acpi_generic_data *)(tmp + 1);
+	rc = print_extlog_rcd(pfx, tmp, cpu);
+
+	/* trace extended error log */
+	err_count++;
+	if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
+		fru_id = (uuid_le *)gdata->fru_id;
+	if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
+		fru_text = gdata->fru_text;
+	sec_type = (uuid_le *)gdata->section_type;
+	if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
+		struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
+		if (gdata->error_data_length >= sizeof(*mem_err))
+			trace_mem_error(fru_id, fru_text, err_count,
+					gdata->error_severity, mem_err);
+	}
 
 	return rc;
 }
diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index f5bc227..44bde6a 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -59,11 +59,12 @@ static const char *cper_severity_strs[] = {
 	"info",
 };
 
-static const char *cper_severity_str(unsigned int severity)
+const char *cper_severity_str(unsigned int severity)
 {
 	return severity < ARRAY_SIZE(cper_severity_strs) ?
 		cper_severity_strs[severity] : "unknown";
 }
+EXPORT_SYMBOL_GPL(cper_severity_str);
 
 /*
  * cper_print_bits - print strings for set bits
@@ -198,6 +199,13 @@ static const char *cper_mem_err_type_strs[] = {
 	"physical memory map-out event",
 };
 
+const char *cper_mem_err_type_str(unsigned int etype)
+{
+	return etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
+		cper_mem_err_type_strs[etype] : "unknown";
+}
+EXPORT_SYMBOL_GPL(cper_mem_err_type_str);
+
 static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 {
 	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
@@ -235,8 +243,7 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
 	if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE) {
 		u8 etype = mem->error_type;
 		printk("%s""error_type: %d, %s\n", pfx, etype,
-		       etype < ARRAY_SIZE(cper_mem_err_type_strs) ?
-		       cper_mem_err_type_strs[etype] : "unknown");
+			cper_mem_err_type_str(etype));
 	}
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE) {
 		const char *bank = NULL, *device = NULL;
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 2fc0ec3..c6d87fc 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -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 * const strs[], unsigned int strs_size);
 
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 21cdb0b..579dbb0 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -8,6 +8,67 @@
 #include <linux/tracepoint.h>
 #include <linux/edac.h>
 #include <linux/ktime.h>
+#include <linux/cper.h>
+
+/*
+ * MCE Extended Error Log Trace event
+ *
+ * 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))
+);
 
 /*
  * Hardware Events Report
-- 
1.8.4.rc3

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 0/9] Extended H/W error log driver

[Borislav Petkov]

On Wed, Oct 16, 2013 at 10:55:57AM -0400, Chen, Gong wrote:
> [PATCH v2 1/9] ACPI, APEI, CPER: Fix status check during error printing
> [PATCH v2 2/9] ACPI, CPER: Update cper info
> [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro
> [PATCH v2 4/9] ACPI, x86: Extended error log driver for x86 platform
> [PATCH v2 5/9] DMI: Parse memory device (type 17) in SMBIOS
> [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error
> [PATCH v2 7/9] ACPI, APEI, CPER: Enhance memory reporting capability
> [PATCH v2 8/9] ACPI, APEI, CPER: Cleanup CPER memory error output format
> [PATCH v2 9/9] ACPI / trace: Add trace interface for eMCA driver
> 
> This patch series adds an enhanced MCA event logging driver provided by Intel.
> Please refer to this link: htpp://www.intel.com/content/www/us/en/architecture-and-technology/enhanced-mca-logging-xeon-paper.html
> 
> Certain usages such as Predictive Failure Analysis (PFA) require more
> information about the error than what can be described in processor
> machine check banks. Most server processors log additional information
> about the error in processor uncore registers. Since the addresses
> and layout of these registers vary widely from one processor to another,
> system software cannot readily make use of them. To complicate matters
> further, some of the additionalerror information cannot be constructed

space between "additional" and "error".

> without detailed knowledge about platform topology. This enhanced MCA
> logging driver allows firmware to provide additional error information
> to MCE/CMCI handler and thus addresses this gap.

This paragraph sounds like a very good description of the feature and
should actually be the Kconfig text in patch 4/9.

> 
> After applying this patch series, when a memory corrected error happens,
> we can get following information:
> 
> dmesg output:
> 
> [  949.545817] {1}Hardware error detected on CPU15
> [  949.549786] {1}event severity: corrected
> [  949.549786] {1} Error 0, type: corrected
> [  949.549786] {1}  section_type: memory error
> [  949.549786] {1}  physical_address: 0x0000001057eb0000
> [  949.549786] {1}  DIMM location: Memriser3 CHANNEL A DIMM 0
> [  949.549786] {1}Above error has been corrected by h/w and require no further action
> [  949.549786] mce: [Hardware Error]: Machine check events logged
> [ 1010.902124] {2}Hardware error detected on CPU15
> [ 1010.906064] {2}event severity: corrected
> [ 1010.906064] {2} Error 0, type: corrected
> [ 1010.906064] {2}  section_type: memory error
> [ 1010.906064] {2}  physical_address: 0x0000001057eb0000
> [ 1010.906064] {2}  DIMM location: Memriser3 CHANNEL A DIMM 0
> [ 1010.906064] {2}Above error has been corrected by h/w and require no further action
> [ 1010.906064] mce: [Hardware Error]: Machine check events logged

Yep, looks almost very good. One nit: can you raise the action line
higher, like this:

> [  949.545817] {1}Hardware error detected on CPU15
> [  949.549786] {1}It has been corrected by h/w and requires no further action

<here come the error details>

I mean, this is only the printk output and with a userspace consumer of
the tracepoint, none of this will go to dmesg but in cases when there's
no userspace consumer, it is still readable and understandable.

> For trace output format we still need further discussion. In the last
> patch(support trace interface) I have to reserve previous Kconfig
> format because I find once I put trace_event interface in the module,
> it will not work. I will paste another trace patch(it only works when
> acpi_extlog is builtin) for your answer.

I think to be able to define TRACE_EVENTs in modules, you need
https://lwn.net/Articles/383362/

Steve, that still true?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 0/9] Extended H/W error log driver

[Joe Perches]

On Wed, 2013-10-16 at 18:05 +0200, Borislav Petkov wrote:
> On Wed, Oct 16, 2013 at 10:55:57AM -0400, Chen, Gong wrote:
[]
> > After applying this patch series, when a memory corrected error happens,
> > we can get following information:
> > 
> > dmesg output:
> > 
> > [  949.545817] {1}Hardware error detected on CPU15
> > [  949.549786] {1}event severity: corrected
> > [  949.549786] {1} Error 0, type: corrected
> > [  949.549786] {1}  section_type: memory error
> > [  949.549786] {1}  physical_address: 0x0000001057eb0000
> > [  949.549786] {1}  DIMM location: Memriser3 CHANNEL A DIMM 0
> > [  949.549786] {1}Above error has been corrected by h/w and require no further action
> > [  949.549786] mce: [Hardware Error]: Machine check events logged

> Yep, looks almost very good. One nit: can you raise the action line
> higher, like this:
> 
> > [  949.545817] {1}Hardware error detected on CPU15
> > [  949.549786] {1}It has been corrected by h/w and requires no further action
> 
> <here come the error details>

Perhaps this would be nicer still with the "mce:" prefix
on all the log lines with the overall description emitted
first.  It could help make grepping the log a bit easier.

[  xxx.xxxxxx] mce: [Hardware Error]: Machine check events logged
[  xxx.xxxxxx] mce: {1}Hardware error detected on CPU15
[  xxx.xxxxxx] mce: {1}Above error has been corrected by h/w and require no further action
[  xxx.xxxxxx] mce: {1}event severity: corrected
[  xxx.xxxxxx] mce: {1} Error 0, type: corrected
[  xxx.xxxxxx] mce: {1}  section_type: memory error
[  xxx.xxxxxx] mce: {1}  physical_address: 0x0000001057eb0000
[  xxx.xxxxxx] mce: {1}  DIMM location: Memriser3 CHANNEL A DIMM 0

grammar: s/require/requires or maybe required

Re: [PATCH v2 0/9] Extended H/W error log driver

[Steven Rostedt]

On Wed, 16 Oct 2013 18:05:50 +0200
Borislav Petkov <bp@alien8.de> wrote:

 
> > For trace output format we still need further discussion. In the last
> > patch(support trace interface) I have to reserve previous Kconfig
> > format because I find once I put trace_event interface in the module,
> > it will not work. I will paste another trace patch(it only works when

What did not work?

> > acpi_extlog is builtin) for your answer.
> 
> I think to be able to define TRACE_EVENTs in modules, you need
> https://lwn.net/Articles/383362/
> 
> Steve, that still true?
> 

Take a look at samples/trace_events/

That's a module that uses TRACE_EVENTs.

What exactly is the problem here?


-- Steve

Re: [PATCH v2 0/9] Extended H/W error log driver

[Borislav Petkov]

On Wed, Oct 16, 2013 at 12:56:46PM -0400, Steven Rostedt wrote:
> On Wed, 16 Oct 2013 18:05:50 +0200
> Borislav Petkov <bp@alien8.de> wrote:
> 
>  
> > > For trace output format we still need further discussion. In the last
> > > patch(support trace interface) I have to reserve previous Kconfig
> > > format because I find once I put trace_event interface in the module,
> > > it will not work. I will paste another trace patch(it only works when
> 
> What did not work?
> 
> > > acpi_extlog is builtin) for your answer.
> > 
> > I think to be able to define TRACE_EVENTs in modules, you need
> > https://lwn.net/Articles/383362/
> > 
> > Steve, that still true?
> > 
> 
> Take a look at samples/trace_events/
> 
> That's a module that uses TRACE_EVENTs.
> 
> What exactly is the problem here?

Right, the only difference I can see is that include/ras/ras_event.h
doesn't have those below:

#undef TRACE_INCLUDE_PATH
#undef TRACE_INCLUDE_FILE
#define TRACE_INCLUDE_PATH .

Perhaps that is the problem?

Gong, what is exactly the issue you're observing?

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 0/9] Extended H/W error log driver

[Borislav Petkov]

On Wed, Oct 16, 2013 at 08:00:38PM +0200, Borislav Petkov wrote:
> Right, the only difference I can see is that include/ras/ras_event.h
> doesn't have those below:
> 
> #undef TRACE_INCLUDE_PATH
> #undef TRACE_INCLUDE_FILE
> #define TRACE_INCLUDE_PATH .
> 
> Perhaps that is the problem?
> 
> Gong, what is exactly the issue you're observing?

Ok, I think I know what the issue is:

Gong has

diff --git a/drivers/acpi/extlog_trace.c b/drivers/acpi/extlog_trace.c
new file mode 100644
index 000000000000..28640807fb09
--- /dev/null
+++ b/drivers/acpi/extlog_trace.c
@@ -0,0 +1,107 @@
+#include <linux/export.h>
+#include <linux/dmi.h>
+#include "extlog_trace.h"
+
+#define CREATE_TRACE_POINTS
+#define TRACE_INCLUDE_PATH ../../include/ras
+#include <ras/ras_event.h>

for the ras tracepoint although this is done already in
drivers/edac/edac_mc.c

Gong, can you try moving the CREATE_TRACE_POINTS line to a new file -
arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move
it away from edac_mc.c. Does that help?

Also, see Documentation/trace/tracepoints.txt for more info.

HTH.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 0/9] Extended H/W error log driver

[Chen Gong]

[-- Attachment #1: Type: text/plain, Size: 3044 bytes --]

On Wed, Oct 16, 2013 at 08:11:17PM +0200, Borislav Petkov wrote:
> Date: Wed, 16 Oct 2013 20:11:17 +0200
> From: Borislav Petkov <bp@alien8.de>
> To: Steven Rostedt <rostedt@goodmis.org>, "Chen, Gong"
>  <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, joe@perches.com, naveen.n.rao@linux.vnet.ibm.com,
>  arozansk@redhat.com, linux-acpi@vger.kernel.org,
>  linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 0/9] Extended H/W error log driver
> User-Agent: Mutt/1.5.21 (2010-09-15)
> 
> On Wed, Oct 16, 2013 at 08:00:38PM +0200, Borislav Petkov wrote:
> > Right, the only difference I can see is that include/ras/ras_event.h
> > doesn't have those below:
> > 
> > #undef TRACE_INCLUDE_PATH
> > #undef TRACE_INCLUDE_FILE
> > #define TRACE_INCLUDE_PATH .
> > 
> > Perhaps that is the problem?
> > 
> > Gong, what is exactly the issue you're observing?
> 
> Ok, I think I know what the issue is:
> 
> Gong has
> 
> diff --git a/drivers/acpi/extlog_trace.c b/drivers/acpi/extlog_trace.c
> new file mode 100644
> index 000000000000..28640807fb09
> --- /dev/null
> +++ b/drivers/acpi/extlog_trace.c
> @@ -0,0 +1,107 @@
> +#include <linux/export.h>
> +#include <linux/dmi.h>
> +#include "extlog_trace.h"
> +
> +#define CREATE_TRACE_POINTS
> +#define TRACE_INCLUDE_PATH ../../include/ras
> +#include <ras/ras_event.h>
> 
> for the ras tracepoint although this is done already in
> drivers/edac/edac_mc.c
> 

Sorry I don't express clearly enough. The patch [v2 9/9] in this patch
seris can work well. The bogus one is in myself reply for patch [v2 0/9].
In this patch series I keep trace interface always builtin, so it can
work for module & builtin, whether CREATE_TRACE_POINTS is defined
multi-times or not.

The weird thing for bogus patch is if it is compiled as a module, I can
find the trace_xxx function is called definitely and paramerters are
expected but nothing output via trace interface, just like below:

# tracer: nop
#
# entries-in-buffer/entries-written: 0/0   #P:120
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |


I highly suspect my trace_xxx function is compiled as an empty function
if following my bogus patch.

> Gong, can you try moving the CREATE_TRACE_POINTS line to a new file -
> arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move
> it away from edac_mc.c. Does that help?

In current kernel we haven't arch/x86/ras/ras.c. You mean I create
a new one there and just add some trace macro definition?

> 
> Also, see Documentation/trace/tracepoints.txt for more info.
> 
> HTH.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Sent from a fat crate under my desk. Formatting is fine.
> --

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 0/9] Extended H/W error log driver

[Steven Rostedt]

On Thu, 17 Oct 2013 10:33:48 -0400
Chen Gong <gong.chen@linux.intel.com> wrote:


> > Gong, can you try moving the CREATE_TRACE_POINTS line to a new file -
> > arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move
> > it away from edac_mc.c. Does that help?
> 
> In current kernel we haven't arch/x86/ras/ras.c. You mean I create
> a new one there and just add some trace macro definition?

The CREATE_TRACE_POINTS will cause the TRACE_EVENT() macro to define
the functions used for tracing. If you have it defined more than once,
it will either cause a kernel compile error (variables declared more
than once), or if used in modules, will cause confusion (as well as
bloat) because different functions and variables will be defined for
the same tracepoint.

If you need to create a separate file that you can have it defined in a
single place, then please do so!

-- Steve

Re: [PATCH v2 0/9] Extended H/W error log driver

[Borislav Petkov]

On Thu, Oct 17, 2013 at 11:25:41AM -0400, Steven Rostedt wrote:
> On Thu, 17 Oct 2013 10:33:48 -0400
> Chen Gong <gong.chen@linux.intel.com> wrote:
> 
> 
> > > Gong, can you try moving the CREATE_TRACE_POINTS line to a new file -
> > > arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move
> > > it away from edac_mc.c. Does that help?
> > 
> > In current kernel we haven't arch/x86/ras/ras.c. You mean I create
> > a new one there and just add some trace macro definition?
> 
> The CREATE_TRACE_POINTS will cause the TRACE_EVENT() macro to define
> the functions used for tracing. If you have it defined more than once,
> it will either cause a kernel compile error (variables declared more
> than once), or if used in modules, will cause confusion (as well as
> bloat) because different functions and variables will be defined for
> the same tracepoint.
> 
> If you need to create a separate file that you can have it defined in a
> single place, then please do so!

Yes, the plan was this all along to have arch/x86/ras/ras.c or
arch/x86/ras/core.c where CREATE_TRACE_POINTS is done, among other
things.

Btw, Gong, there's an easier way to test this, if you wanna:

simply make sure EDAC is disabled in the kernel with your patches -
i.e., CONFIG_EDAC is not set in your config. It should work then.

But, the final solution should be what Steve says with the file I'm
suggesting :)

Thanks.

-- 
Regards/Gruss,
    Boris.

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