diff mbox

[3/7,v4] CPER: Adjust code flow of some functions

Message ID 1400142646-10127-4-git-send-email-gong.chen@linux.intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Chen Gong May 15, 2014, 8:30 a.m. UTC
Some codes can be reorganzied as a common function for other usages.

v4 -> v3: minor adjustment to make output format more gracefully.
v3 -> v2: Fix a bug when calculating string length & minor fix.
v2 -> v1: Use scnprintf to simplify codes.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/firmware/efi/cper.c | 158 +++++++++++++++++++++++++++++---------------
 include/linux/cper.h        |  11 +++
 2 files changed, 114 insertions(+), 55 deletions(-)

Comments

Borislav Petkov May 21, 2014, 11:05 a.m. UTC | #1
On Thu, May 15, 2014 at 04:30:42AM -0400, Chen, Gong wrote:
> +const char *cper_mem_err_type_str(unsigned int etype)
>  {
> -	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_PA)
> -		printk("%s""physical_address: 0x%016llx\n",
> -		       pfx, mem->physical_addr);
> -	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> -		printk("%s""physical_address_mask: 0x%016llx\n",

The physical address mask is still not part of the tracepoint as a u8 as
we talked.

> -		       pfx, mem->physical_addr_mask);
> +	return etype < ARRAY_SIZE(mem_err_type_strs) ?
> +		mem_err_type_strs[etype] : "unknown";
> +}
> +EXPORT_SYMBOL_GPL(cper_mem_err_type_str);
Chen Gong May 21, 2014, 11:51 p.m. UTC | #2
On Wed, May 21, 2014 at 01:05:21PM +0200, Borislav Petkov wrote:
> > +const char *cper_mem_err_type_str(unsigned int etype)
> >  {
> > -	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_PA)
> > -		printk("%s""physical_address: 0x%016llx\n",
> > -		       pfx, mem->physical_addr);
> > -	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> > -		printk("%s""physical_address_mask: 0x%016llx\n",
> 
> The physical address mask is still not part of the tracepoint as a u8 as
> we talked.
> 
I thought out discussion is only for trace part. But it is OK to me to
make whole style aligned.
Borislav Petkov May 22, 2014, 10:52 a.m. UTC | #3
On Wed, May 21, 2014 at 07:51:59PM -0400, Chen, Gong wrote:
> On Wed, May 21, 2014 at 01:05:21PM +0200, Borislav Petkov wrote:
> > > +const char *cper_mem_err_type_str(unsigned int etype)
> > >  {
> > > -	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_PA)
> > > -		printk("%s""physical_address: 0x%016llx\n",
> > > -		       pfx, mem->physical_addr);
> > > -	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> > > -		printk("%s""physical_address_mask: 0x%016llx\n",
> > 
> > The physical address mask is still not part of the tracepoint as a u8 as
> > we talked.
> > 
> I thought out discussion is only for trace part. But it is OK to me to
> make whole style aligned.

No, I'm not talking about style - I'm talking about adding the physical address
mask to the tracepoint call:

+TRACE_EVENT(extlog_mem_event,
+       TP_PROTO(u32 etype,
+                const uuid_le *fru_id,
+                u64 error_count,

Btw, is that the error_count we're reporting?? You surely can't claim
that we'll ever report 2^64-1 errors, right?

I'd make that u32 and I'd call it

		u32 error_number;

as it is a counter we're incrementing.

+                u32 severity,

That severity can surely be u8 - we can't have 2^32-1 severities in any
normal case - I see only 5. I'm sure 256 is plenty.

And now that we slimmed some of those insanely-sized members, we can add

		u8 pa_mask_lsb

or something to that effect.

Makes sense?

+                char *dimm_info,
+                char *pa_info,
+                char *mem_loc,
+                char *fru_text),

+TRACE_EVENT(extlog_mem_event,
Chen Gong May 23, 2014, 1:49 a.m. UTC | #4
On Thu, May 22, 2014 at 12:52:42PM +0200, Borislav Petkov wrote:
> > > > +const char *cper_mem_err_type_str(unsigned int etype)
> > > >  {
> > > > -	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_PA)
> > > > -		printk("%s""physical_address: 0x%016llx\n",
> > > > -		       pfx, mem->physical_addr);
> > > > -	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> > > > -		printk("%s""physical_address_mask: 0x%016llx\n",
> > > 
> > > The physical address mask is still not part of the tracepoint as a u8 as
> > > we talked.
> > > 
> > I thought out discussion is only for trace part. But it is OK to me to
> > make whole style aligned.
> 
> No, I'm not talking about style - I'm talking about adding the physical address
> mask to the tracepoint call:

If so, it has been there already. Maybe you should check patch 5/7. I merge
pa/pa_mask into pa_info as a whole to avoid too much calculation/logic in
trace.

> +TRACE_EVENT(extlog_mem_event,
> +       TP_PROTO(u32 etype,
> +                const uuid_le *fru_id,
> +                u64 error_count,
> 
> Btw, is that the error_count we're reporting?? You surely can't claim
> that we'll ever report 2^64-1 errors, right?
> 
> I'd make that u32 and I'd call it
> 
> 		u32 error_number;
Fine.
> 
> as it is a counter we're incrementing.
> 
> +                u32 severity,
Fine.
Borislav Petkov May 23, 2014, 9:37 a.m. UTC | #5
On Thu, May 22, 2014 at 09:49:10PM -0400, Chen, Gong wrote:
> If so, it has been there already. Maybe you should check patch
> 5/7. I merge pa/pa_mask into pa_info as a whole to avoid too much
> calculation/logic in trace.

My bad, how did I miss that:

> +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;
> +       char pa_info[64];
> +       u8 n = 0;
> +
> +       if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
> +               etype = mem->error_type;

More SNAFU: mem->error_type is u8 and you're saving it into a u32. What
possible reason can you have for that?

> +
> +       memset(pa_info, 0, 64);
> +       if (mem->validation_bits & CPER_MEM_VALID_PA)
> +               n = snprintf(pa_info, 63, "physical addr: 0x%016llx ",
> +                            mem->physical_addr);
> +
> +       if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> +               snprintf(pa_info + n, 63 - n, "addr LSB: 0x%x ",
> +                        (u8)__ffs64(mem->physical_addr_mask));

So pa_info is 64 bytes!!! For what, a u64 and a u8? That's 9 bytes.

You don't seem to get the idea that we cannot allow ourselves to waste
bytes in an error record like that. How hard it is to comprehend? Am I
telling it wrong or do you need someone else to explain it to you?

Ok, here's how the tracepoint should look like:

	TP_PROTO(u32 error_number,
		 u8 etype,
		 u8 severity,
		 u64 pa,
		 u8 pa_mask_lsb,
		 const uuid_le *fru_id,
		 char *dimm_info,
		 char *mem_loc,
		 char *fru_text)

Now if you have valid technical reasons why it shouldn't be done
that way, do tell. Otherwise do it exactly this way without changing
anything.

Or if you don't wanna, I can do it instead - it'll be much easier for me
than reviewing it again.
Chen Gong May 26, 2014, 2:07 a.m. UTC | #6
On Fri, May 23, 2014 at 11:37:03AM +0200, Borislav Petkov wrote:
> Date: Fri, 23 May 2014 11:37:03 +0200
> From: Borislav Petkov <bp@alien8.de>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, m.chehab@samsung.com, linux-acpi@vger.kernel.org
> Subject: Re: [PATCH 3/7 v4] CPER: Adjust code flow of some functions
> User-Agent: Mutt/1.5.23 (2014-03-12)
> 
> On Thu, May 22, 2014 at 09:49:10PM -0400, Chen, Gong wrote:
> > If so, it has been there already. Maybe you should check patch
> > 5/7. I merge pa/pa_mask into pa_info as a whole to avoid too much
> > calculation/logic in trace.
> 
> My bad, how did I miss that:
> 
> > +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;
> > +       char pa_info[64];
> > +       u8 n = 0;
> > +
> > +       if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
> > +               etype = mem->error_type;
> 
> More SNAFU: mem->error_type is u8 and you're saving it into a u32. What
> possible reason can you have for that?
OK, I will fix it.
> 
> > +
> > +       memset(pa_info, 0, 64);
> > +       if (mem->validation_bits & CPER_MEM_VALID_PA)
> > +               n = snprintf(pa_info, 63, "physical addr: 0x%016llx ",
> > +                            mem->physical_addr);
> > +
> > +       if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> > +               snprintf(pa_info + n, 63 - n, "addr LSB: 0x%x ",
> > +                        (u8)__ffs64(mem->physical_addr_mask));
> 
> So pa_info is 64 bytes!!! For what, a u64 and a u8? That's 9 bytes.
> 
Oh, in my subconscious I am always afraid some kind of buffer overflow attach.
You are right, here it is obviously too wasteful. I can shrink it to 10
bytes. Please see my another reply for why I hope to use a string.
Borislav Petkov May 26, 2014, 10:23 a.m. UTC | #7
On Sun, May 25, 2014 at 10:07:45PM -0400, Chen, Gong wrote:
> I can shrink it to 10 bytes. Please see my another reply for why I
> hope to use a string.

No need - just instantiate both the physical address and mask to invalid
values and put that in a comment somewhere stating which values are
invalid.
diff mbox

Patch

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 1491dd4..a53723a 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -34,6 +34,10 @@ 
 #include <linux/aer.h>
 
 #define INDENT_SP	" "
+
+static char mem_location[CPER_REC_LEN];
+static char dimm_location[CPER_REC_LEN];
+
 /*
  * CPER record ID need to be unique even after reboot, because record
  * ID is used as index for ERST storage, while CPER records from
@@ -50,18 +54,19 @@  u64 cper_next_record_id(void)
 }
 EXPORT_SYMBOL_GPL(cper_next_record_id);
 
-static const char *cper_severity_strs[] = {
+static const char * const severity_strs[] = {
 	"recoverable",
 	"fatal",
 	"corrected",
 	"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";
+	return severity < ARRAY_SIZE(severity_strs) ?
+		severity_strs[severity] : "unknown";
 }
+EXPORT_SYMBOL_GPL(cper_severity_str);
 
 /*
  * cper_print_bits - print strings for set bits
@@ -100,32 +105,32 @@  void cper_print_bits(const char *pfx, unsigned int bits,
 		printk("%s\n", buf);
 }
 
-static const char * const cper_proc_type_strs[] = {
+static const char * const proc_type_strs[] = {
 	"IA32/X64",
 	"IA64",
 };
 
-static const char * const cper_proc_isa_strs[] = {
+static const char * const proc_isa_strs[] = {
 	"IA32",
 	"IA64",
 	"X64",
 };
 
-static const char * const cper_proc_error_type_strs[] = {
+static const char * const proc_error_type_strs[] = {
 	"cache error",
 	"TLB error",
 	"bus error",
 	"micro-architectural error",
 };
 
-static const char * const cper_proc_op_strs[] = {
+static const char * const proc_op_strs[] = {
 	"unknown or generic",
 	"data read",
 	"data write",
 	"instruction execution",
 };
 
-static const char * const cper_proc_flag_strs[] = {
+static const char * const proc_flag_strs[] = {
 	"restartable",
 	"precise IP",
 	"overflow",
@@ -137,26 +142,26 @@  static void cper_print_proc_generic(const char *pfx,
 {
 	if (proc->validation_bits & CPER_PROC_VALID_TYPE)
 		printk("%s""processor_type: %d, %s\n", pfx, proc->proc_type,
-		       proc->proc_type < ARRAY_SIZE(cper_proc_type_strs) ?
-		       cper_proc_type_strs[proc->proc_type] : "unknown");
+		       proc->proc_type < ARRAY_SIZE(proc_type_strs) ?
+		       proc_type_strs[proc->proc_type] : "unknown");
 	if (proc->validation_bits & CPER_PROC_VALID_ISA)
 		printk("%s""processor_isa: %d, %s\n", pfx, proc->proc_isa,
-		       proc->proc_isa < ARRAY_SIZE(cper_proc_isa_strs) ?
-		       cper_proc_isa_strs[proc->proc_isa] : "unknown");
+		       proc->proc_isa < ARRAY_SIZE(proc_isa_strs) ?
+		       proc_isa_strs[proc->proc_isa] : "unknown");
 	if (proc->validation_bits & CPER_PROC_VALID_ERROR_TYPE) {
 		printk("%s""error_type: 0x%02x\n", pfx, proc->proc_error_type);
 		cper_print_bits(pfx, proc->proc_error_type,
-				cper_proc_error_type_strs,
-				ARRAY_SIZE(cper_proc_error_type_strs));
+				proc_error_type_strs,
+				ARRAY_SIZE(proc_error_type_strs));
 	}
 	if (proc->validation_bits & CPER_PROC_VALID_OPERATION)
 		printk("%s""operation: %d, %s\n", pfx, proc->operation,
-		       proc->operation < ARRAY_SIZE(cper_proc_op_strs) ?
-		       cper_proc_op_strs[proc->operation] : "unknown");
+		       proc->operation < ARRAY_SIZE(proc_op_strs) ?
+		       proc_op_strs[proc->operation] : "unknown");
 	if (proc->validation_bits & CPER_PROC_VALID_FLAGS) {
 		printk("%s""flags: 0x%02x\n", pfx, proc->flags);
-		cper_print_bits(pfx, proc->flags, cper_proc_flag_strs,
-				ARRAY_SIZE(cper_proc_flag_strs));
+		cper_print_bits(pfx, proc->flags, proc_flag_strs,
+				ARRAY_SIZE(proc_flag_strs));
 	}
 	if (proc->validation_bits & CPER_PROC_VALID_LEVEL)
 		printk("%s""level: %d\n", pfx, proc->level);
@@ -177,7 +182,7 @@  static void cper_print_proc_generic(const char *pfx,
 		printk("%s""IP: 0x%016llx\n", pfx, proc->ip);
 }
 
-static const char *cper_mem_err_type_strs[] = {
+static const char * const mem_err_type_strs[] = {
 	"unknown",
 	"no error",
 	"single-bit ECC",
@@ -196,58 +201,101 @@  static const char *cper_mem_err_type_strs[] = {
 	"physical memory map-out event",
 };
 
-static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
+const char *cper_mem_err_type_str(unsigned int etype)
 {
-	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_PA)
-		printk("%s""physical_address: 0x%016llx\n",
-		       pfx, mem->physical_addr);
-	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
-		printk("%s""physical_address_mask: 0x%016llx\n",
-		       pfx, mem->physical_addr_mask);
+	return etype < ARRAY_SIZE(mem_err_type_strs) ?
+		mem_err_type_strs[etype] : "unknown";
+}
+EXPORT_SYMBOL_GPL(cper_mem_err_type_str);
+
+int cper_mem_err_location(const struct cper_sec_mem_err *mem, char *msg)
+{
+	u32 len, n;
+
+	if (!msg)
+		return 0;
+
+	n = 0;
+	len = CPER_REC_LEN - 1;
 	if (mem->validation_bits & CPER_MEM_VALID_NODE)
-		pr_debug("node: %d\n", mem->node);
+		n += scnprintf(msg + n, len - n, "node: %d ", mem->node);
 	if (mem->validation_bits & CPER_MEM_VALID_CARD)
-		pr_debug("card: %d\n", mem->card);
+		n += scnprintf(msg + n, len - n, "card: %d ", mem->card);
 	if (mem->validation_bits & CPER_MEM_VALID_MODULE)
-		pr_debug("module: %d\n", mem->module);
+		n += scnprintf(msg + n, len - n, "module: %d ", mem->module);
 	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
-		pr_debug("rank: %d\n", mem->rank);
+		n += scnprintf(msg + n, len - n, "rank: %d ", mem->rank);
 	if (mem->validation_bits & CPER_MEM_VALID_BANK)
-		pr_debug("bank: %d\n", mem->bank);
+		n += scnprintf(msg + n, len - n, "bank: %d ", mem->bank);
 	if (mem->validation_bits & CPER_MEM_VALID_DEVICE)
-		pr_debug("device: %d\n", mem->device);
+		n += scnprintf(msg + n, len - n, "device: %d ", mem->device);
 	if (mem->validation_bits & CPER_MEM_VALID_ROW)
-		pr_debug("row: %d\n", mem->row);
+		n += scnprintf(msg + n, len - n, "row: %d ", mem->row);
 	if (mem->validation_bits & CPER_MEM_VALID_COLUMN)
-		pr_debug("column: %d\n", mem->column);
+		n += scnprintf(msg + n, len - n, "column: %d ", mem->column);
 	if (mem->validation_bits & CPER_MEM_VALID_BIT_POSITION)
-		pr_debug("bit_position: %d\n", mem->bit_pos);
+		n += scnprintf(msg + n, len - n, "bit_position: %d ",
+			       mem->bit_pos);
 	if (mem->validation_bits & CPER_MEM_VALID_REQUESTOR_ID)
-		pr_debug("requestor_id: 0x%016llx\n", mem->requestor_id);
+		n += scnprintf(msg + n, len - n, "requestor_id: 0x%016llx ",
+			       mem->requestor_id);
 	if (mem->validation_bits & CPER_MEM_VALID_RESPONDER_ID)
-		pr_debug("responder_id: 0x%016llx\n", mem->responder_id);
+		n += scnprintf(msg + n, len - n, "responder_id: 0x%016llx ",
+			       mem->responder_id);
 	if (mem->validation_bits & CPER_MEM_VALID_TARGET_ID)
-		pr_debug("target_id: 0x%016llx\n", mem->target_id);
+		scnprintf(msg + n, len - n, "target_id: 0x%016llx ",
+			  mem->target_id);
+
+	return n;
+}
+EXPORT_SYMBOL_GPL(cper_mem_err_location);
+
+int cper_dimm_err_location(const struct cper_sec_mem_err *mem, char *msg)
+{
+	u32 len, n;
+	const char *bank = NULL, *device = NULL;
+
+	if (!msg || !(mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE))
+		return 0;
+
+	n = 0;
+	len = CPER_REC_LEN - 1;
+	dmi_memdev_name(mem->mem_dev_handle, &bank, &device);
+	if (bank && device)
+		n = snprintf(msg, len, "DIMM location: %s %s", bank, device);
+	else
+		n = snprintf(msg, len,
+			     "DIMM location: not present. DMI handle: 0x%.4x",
+			     mem->mem_dev_handle);
+
+	return n;
+}
+EXPORT_SYMBOL_GPL(cper_dimm_err_location);
+
+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_PA)
+		printk("%s""physical_address: 0x%016llx\n",
+		       pfx, mem->physical_addr);
+	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
+		printk("%s""physical_address_mask: 0x%016llx\n",
+		       pfx, mem->physical_addr_mask);
+	memset(mem_location, 0, CPER_REC_LEN);
+	if (cper_mem_err_location(mem, mem_location))
+		printk("%s%s\n", pfx, mem_location);
 	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");
-	}
-	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);
+		       cper_mem_err_type_str(etype));
 	}
+	memset(dimm_location, 0, CPER_REC_LEN);
+	if (cper_dimm_err_location(mem, dimm_location))
+		printk("%s%s\n", pfx, dimm_location);
 }
 
-static const char *cper_pcie_port_type_strs[] = {
+static const char * const pcie_port_type_strs[] = {
 	"PCIe end point",
 	"legacy PCI end point",
 	"unknown",
@@ -266,8 +314,8 @@  static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 {
 	if (pcie->validation_bits & CPER_PCIE_VALID_PORT_TYPE)
 		printk("%s""port_type: %d, %s\n", pfx, pcie->port_type,
-		       pcie->port_type < ARRAY_SIZE(cper_pcie_port_type_strs) ?
-		       cper_pcie_port_type_strs[pcie->port_type] : "unknown");
+		       pcie->port_type < ARRAY_SIZE(pcie_port_type_strs) ?
+		       pcie_port_type_strs[pcie->port_type] : "unknown");
 	if (pcie->validation_bits & CPER_PCIE_VALID_VERSION)
 		printk("%s""version: %d.%d\n", pfx,
 		       pcie->version.major, pcie->version.minor);
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 2fc0ec3..dc84337 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -36,6 +36,13 @@ 
 #define CPER_RECORD_REV				0x0100
 
 /*
+ * CPER record length contains the CPER fields which are relevant for further
+ * handling of a memory error in userspace (we don't carry all the fields
+ * defined in the UEFI spec because some of them don't make any sense.)
+ * Currently, a length of 256 should be more than enough.
+ */
+#define CPER_REC_LEN					256
+/*
  * Severity difinition for error_severity in struct cper_record_header
  * and section_severity in struct cper_section_descriptor
  */
@@ -395,7 +402,11 @@  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);
+int cper_mem_err_location(const struct cper_sec_mem_err *mem, char *msg);
+int cper_dimm_err_location(const struct cper_sec_mem_err *mem, char *msg);
 
 #endif