[PATCH V2 0/2] MCE, AMD: MCE decoding support for AMD Family 16h

[PATCH V2 0/2] MCE, AMD: MCE decoding support for AMD Family 16h

[Jacob Shin]

The following patchset enables MCE decoding support for AMD Family 16h
processors.

Changes in V2:
* Changed if/else style and pr_cont usage per feedback from:
  https://lkml.org/lkml/2012/12/17/365

* Merged f14h and f16h mc0 and mc2 decoding into common function per
  feedback from: https://lkml.org/lkml/2012/12/18/269

* Fixed typo in INT_ERROR decoding.

Jacob Shin (2):
  MCE, AMD: Make MC2 decoding part of amd_decoder_ops as well
  MCE, AMD: MCE decoding support for AMD Family 16h

 drivers/edac/mce_amd.c |  139 +++++++++++++++++++++++++++++++++++-------------
 drivers/edac/mce_amd.h |    4 ++
 2 files changed, 105 insertions(+), 38 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] MCE, AMD: Make MC2 decoding part of amd_decoder_ops as well

[Jacob Shin]

Currently only AMD Family 15h processors have special handling for MC2
errors, since upcoming Family 16h will also need unique handling,
let's make MC2 handling part of amd_decoder_ops.

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 drivers/edac/mce_amd.c |   56 +++++++++++++++++++++++++-----------------------
 drivers/edac/mce_amd.h |    1 +
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index ad63757..e4752be 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -399,12 +399,9 @@ static void decode_mc1_mce(struct mce *m)
 		pr_emerg(HW_ERR "Corrupted MC1 MCE info?\n");
 }
 
-static void decode_mc2_mce(struct mce *m)
+static bool k8_mc2_mce(u16 ec, u8 xec)
 {
-	u16 ec = EC(m->status);
-	u8 xec = XEC(m->status, xec_mask);
-
-	pr_emerg(HW_ERR "MC2 Error");
+	bool ret = true;
 
 	if (xec == 0x1)
 		pr_cont(" in the write data buffers.\n");
@@ -429,24 +426,18 @@ static void decode_mc2_mce(struct mce *m)
 				pr_cont(": %s parity/ECC error during data "
 					"access from L2.\n", R4_MSG(ec));
 			else
-				goto wrong_mc2_mce;
+				ret = false;
 		} else
-			goto wrong_mc2_mce;
+			ret = false;
 	} else
-		goto wrong_mc2_mce;
-
-	return;
+		ret = false;
 
- wrong_mc2_mce:
-	pr_emerg(HW_ERR "Corrupted MC2 MCE info?\n");
+	return ret;
 }
 
-static void decode_f15_mc2_mce(struct mce *m)
+static bool f15h_mc2_mce(u16 ec, u8 xec)
 {
-	u16 ec = EC(m->status);
-	u8 xec = XEC(m->status, xec_mask);
-
-	pr_emerg(HW_ERR "MC2 Error: ");
+	bool ret = true;
 
 	if (TLB_ERROR(ec)) {
 		if (xec == 0x0)
@@ -454,10 +445,10 @@ static void decode_f15_mc2_mce(struct mce *m)
 		else if (xec == 0x1)
 			pr_cont("Poison data provided for TLB fill.\n");
 		else
-			goto wrong_f15_mc2_mce;
+			ret = false;
 	} else if (BUS_ERROR(ec)) {
 		if (xec > 2)
-			goto wrong_f15_mc2_mce;
+			ret = false;
 
 		pr_cont("Error during attempted NB data read.\n");
 	} else if (MEM_ERROR(ec)) {
@@ -471,14 +462,22 @@ static void decode_f15_mc2_mce(struct mce *m)
 			break;
 
 		default:
-			goto wrong_f15_mc2_mce;
+			ret = false;
 		}
 	}
 
-	return;
+	return ret;
+}
+
+static void decode_mc2_mce(struct mce *m)
+{
+	u16 ec = EC(m->status);
+	u8 xec = XEC(m->status, xec_mask);
 
- wrong_f15_mc2_mce:
-	pr_emerg(HW_ERR "Corrupted MC2 MCE info?\n");
+	pr_emerg(HW_ERR "MC2 Error: ");
+
+	if (!fam_ops->mc2_mce(ec, xec))
+		pr_cont(HW_ERR "Corrupted MC2 MCE info?\n");
 }
 
 static void decode_mc3_mce(struct mce *m)
@@ -702,10 +701,7 @@ int amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 		break;
 
 	case 2:
-		if (c->x86 == 0x15)
-			decode_f15_mc2_mce(m);
-		else
-			decode_mc2_mce(m);
+		decode_mc2_mce(m);
 		break;
 
 	case 3:
@@ -783,33 +779,39 @@ static int __init mce_amd_init(void)
 	case 0xf:
 		fam_ops->mc0_mce = k8_mc0_mce;
 		fam_ops->mc1_mce = k8_mc1_mce;
+		fam_ops->mc2_mce = k8_mc2_mce;
 		break;
 
 	case 0x10:
 		fam_ops->mc0_mce = f10h_mc0_mce;
 		fam_ops->mc1_mce = k8_mc1_mce;
+		fam_ops->mc2_mce = k8_mc2_mce;
 		break;
 
 	case 0x11:
 		fam_ops->mc0_mce = k8_mc0_mce;
 		fam_ops->mc1_mce = k8_mc1_mce;
+		fam_ops->mc2_mce = k8_mc2_mce;
 		break;
 
 	case 0x12:
 		fam_ops->mc0_mce = f12h_mc0_mce;
 		fam_ops->mc1_mce = k8_mc1_mce;
+		fam_ops->mc2_mce = k8_mc2_mce;
 		break;
 
 	case 0x14:
 		nb_err_cpumask  = 0x3;
 		fam_ops->mc0_mce = f14h_mc0_mce;
 		fam_ops->mc1_mce = f14h_mc1_mce;
+		fam_ops->mc2_mce = k8_mc2_mce;
 		break;
 
 	case 0x15:
 		xec_mask = 0x1f;
 		fam_ops->mc0_mce = f15h_mc0_mce;
 		fam_ops->mc1_mce = f15h_mc1_mce;
+		fam_ops->mc2_mce = f15h_mc2_mce;
 		break;
 
 	default:
diff --git a/drivers/edac/mce_amd.h b/drivers/edac/mce_amd.h
index 6796799..000f6e2 100644
--- a/drivers/edac/mce_amd.h
+++ b/drivers/edac/mce_amd.h
@@ -78,6 +78,7 @@ extern const char * const ii_msgs[];
 struct amd_decoder_ops {
 	bool (*mc0_mce)(u16, u8);
 	bool (*mc1_mce)(u16, u8);
+	bool (*mc2_mce)(u16, u8);
 };
 
 void amd_report_gart_errors(bool);
-- 
1.7.9.5

[PATCH 2/2] MCE, AMD: MCE decoding support for AMD Family 16h

[Jacob Shin]

Add MCE decoding logic for AMD Family 16h processors.

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 drivers/edac/mce_amd.c |   83 +++++++++++++++++++++++++++++++++++++++++-------
 drivers/edac/mce_amd.h |    3 ++
 2 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index e4752be..ce4c8b7 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -64,6 +64,10 @@ EXPORT_SYMBOL_GPL(to_msgs);
 const char * const ii_msgs[] = { "MEM", "RESV", "IO", "GEN" };
 EXPORT_SYMBOL_GPL(ii_msgs);
 
+/* internal error type */
+const char * const uu_msgs[] = { "RESV", "RESV", "HWA", "RESV" };
+EXPORT_SYMBOL_GPL(uu_msgs);
+
 static const char * const f15h_mc1_mce_desc[] = {
 	"UC during a demand linefill from L2",
 	"Parity error during data load from IC",
@@ -176,7 +180,7 @@ static bool k8_mc0_mce(u16 ec, u8 xec)
 	return f10h_mc0_mce(ec, xec);
 }
 
-static bool f14h_mc0_mce(u16 ec, u8 xec)
+static bool cat_mc0_mce(u16 ec, u8 xec)
 {
 	u8 r4	 = R4(ec);
 	bool ret = true;
@@ -330,22 +334,27 @@ static bool k8_mc1_mce(u16 ec, u8 xec)
 	return ret;
 }
 
-static bool f14h_mc1_mce(u16 ec, u8 xec)
+static bool cat_mc1_mce(u16 ec, u8 xec)
 {
 	u8 r4    = R4(ec);
 	bool ret = true;
 
 	if (MEM_ERROR(ec)) {
-		if (TT(ec) != 0 || LL(ec) != 1)
+		if (TT(ec) != TT_INSTR)
 			ret = false;
-
-		if (r4 == R4_IRD)
+		else if (r4 == R4_IRD)
 			pr_cont("Data/tag array parity error for a tag hit.\n");
 		else if (r4 == R4_SNOOP)
 			pr_cont("Tag error during snoop/victimization.\n");
+		else if (xec == 0x0)
+			pr_cont("Tag parity error from victim castout.\n");
+		else if (xec == 0x2)
+			pr_cont("Microcode patch RAM parity error.\n");
 		else
 			ret = false;
-	}
+	} else
+		ret = false;
+
 	return ret;
 }
 
@@ -469,6 +478,47 @@ static bool f15h_mc2_mce(u16 ec, u8 xec)
 	return ret;
 }
 
+static bool f16h_mc2_mce(u16 ec, u8 xec)
+{
+	u8 r4 = R4(ec);
+
+	if (!MEM_ERROR(ec))
+		return false;
+
+	switch (xec) {
+	case 0x04 ... 0x05:
+		pr_cont("%cBUFF parity error.\n", (r4 == R4_RD) ? 'I' : 'O');
+		break;
+
+	case 0x09 ... 0x0b:
+	case 0x0d ... 0x0f:
+		pr_cont("ECC error in L2 tag (%s).\n",
+			((r4 == R4_GEN)   ? "BankReq" :
+			((r4 == R4_SNOOP) ? "Prb"     : "Fill")));
+		break;
+
+	case 0x10 ... 0x19:
+	case 0x1b:
+		pr_cont("ECC error in L2 data array (%s).\n",
+			(((r4 == R4_RD) && !(xec & 0x3)) ? "Hit"  :
+			((r4 == R4_GEN)   ? "Attr" :
+			((r4 == R4_EVICT) ? "Vict" : "Fill"))));
+		break;
+
+	case 0x1c ... 0x1d:
+	case 0x1f:
+		pr_cont("Parity error in L2 attribute bits (%s).\n",
+			((r4 == R4_RD)  ? "Hit"  :
+			((r4 == R4_GEN) ? "Attr" : "Fill")));
+		break;
+
+	default:
+		return false;
+	}
+
+	return true;
+}
+
 static void decode_mc2_mce(struct mce *m)
 {
 	u16 ec = EC(m->status);
@@ -546,7 +596,7 @@ static void decode_mc4_mce(struct mce *m)
 		return;
 
 	case 0x19:
-		if (boot_cpu_data.x86 == 0x15)
+		if (boot_cpu_data.x86 == 0x15 || boot_cpu_data.x86 == 0x16)
 			pr_cont("Compute Unit Data Error.\n");
 		else
 			goto wrong_mc4_mce;
@@ -632,6 +682,10 @@ static void decode_mc6_mce(struct mce *m)
 
 static inline void amd_decode_err_code(u16 ec)
 {
+	if (INT_ERROR(ec)) {
+		pr_emerg(HW_ERR "internal: %s\n", UU_MSG(ec));
+		return;
+	}
 
 	pr_emerg(HW_ERR "cache level: %s", LL_MSG(ec));
 
@@ -736,7 +790,7 @@ int amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
 		((m->status & MCI_STATUS_PCC)	? "PCC"	  : "-"),
 		((m->status & MCI_STATUS_ADDRV)	? "AddrV" : "-"));
 
-	if (c->x86 == 0x15)
+	if (c->x86 == 0x15 || c->x86 == 0x16)
 		pr_cont("|%s|%s",
 			((m->status & MCI_STATUS_DEFERRED) ? "Deferred" : "-"),
 			((m->status & MCI_STATUS_POISON)   ? "Poison"   : "-"));
@@ -768,7 +822,7 @@ static int __init mce_amd_init(void)
 	if (c->x86_vendor != X86_VENDOR_AMD)
 		return 0;
 
-	if (c->x86 < 0xf || c->x86 > 0x15)
+	if (c->x86 < 0xf || c->x86 > 0x16)
 		return 0;
 
 	fam_ops = kzalloc(sizeof(struct amd_decoder_ops), GFP_KERNEL);
@@ -802,8 +856,8 @@ static int __init mce_amd_init(void)
 
 	case 0x14:
 		nb_err_cpumask  = 0x3;
-		fam_ops->mc0_mce = f14h_mc0_mce;
-		fam_ops->mc1_mce = f14h_mc1_mce;
+		fam_ops->mc0_mce = cat_mc0_mce;
+		fam_ops->mc1_mce = cat_mc1_mce;
 		fam_ops->mc2_mce = k8_mc2_mce;
 		break;
 
@@ -814,6 +868,13 @@ static int __init mce_amd_init(void)
 		fam_ops->mc2_mce = f15h_mc2_mce;
 		break;
 
+	case 0x16:
+		xec_mask = 0x1f;
+		fam_ops->mc0_mce = cat_mc0_mce;
+		fam_ops->mc1_mce = cat_mc1_mce;
+		fam_ops->mc2_mce = f16h_mc2_mce;
+		break;
+
 	default:
 		printk(KERN_WARNING "Huh? What family is it: 0x%x?!\n", c->x86);
 		kfree(fam_ops);
diff --git a/drivers/edac/mce_amd.h b/drivers/edac/mce_amd.h
index 000f6e2..2828908 100644
--- a/drivers/edac/mce_amd.h
+++ b/drivers/edac/mce_amd.h
@@ -14,6 +14,7 @@
 #define TLB_ERROR(x)			(((x) & 0xFFF0) == 0x0010)
 #define MEM_ERROR(x)			(((x) & 0xFF00) == 0x0100)
 #define BUS_ERROR(x)			(((x) & 0xF800) == 0x0800)
+#define INT_ERROR(x)			(((x) & 0xF4FF) == 0x0400)
 
 #define TT(x)				(((x) >> 2) & 0x3)
 #define TT_MSG(x)			tt_msgs[TT(x)]
@@ -25,6 +26,8 @@
 #define TO_MSG(x)			to_msgs[TO(x)]
 #define PP(x)				(((x) >> 9) & 0x3)
 #define PP_MSG(x)			pp_msgs[PP(x)]
+#define UU(x)				(((x) >> 8) & 0x3)
+#define UU_MSG(x)			uu_msgs[UU(x)]
 
 #define R4(x)				(((x) >> 4) & 0xf)
 #define R4_MSG(x)			((R4(x) < 9) ?  rrrr_msgs[R4(x)] : "Wrong R4!")
-- 
1.7.9.5

Re: [PATCH 2/2] MCE, AMD: MCE decoding support for AMD Family 16h

[Joe Perches]

On Tue, 2012-12-18 at 15:06 -0600, Jacob Shin wrote:
> Add MCE decoding logic for AMD Family 16h processors.

More trivia:

> diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
[]
> @@ -64,6 +64,10 @@ EXPORT_SYMBOL_GPL(to_msgs);
>  const char * const ii_msgs[] = { "MEM", "RESV", "IO", "GEN" };
>  EXPORT_SYMBOL_GPL(ii_msgs);
>  
> +/* internal error type */
> +const char * const uu_msgs[] = { "RESV", "RESV", "HWA", "RESV" };
> +EXPORT_SYMBOL_GPL(uu_msgs);

Do these really need to be exported?
These don't seem to be used outside of mce_amd.

If these really need export, are these very good names?
Perhaps these should be prefixed with something?

Re: [PATCH 2/2] MCE, AMD: MCE decoding support for AMD Family 16h

[Borislav Petkov]

On Tue, Dec 18, 2012 at 02:10:59PM -0800, Joe Perches wrote:
> On Tue, 2012-12-18 at 15:06 -0600, Jacob Shin wrote:
> > Add MCE decoding logic for AMD Family 16h processors.
> 
> More trivia:
> 
> > diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
> []
> > @@ -64,6 +64,10 @@ EXPORT_SYMBOL_GPL(to_msgs);
> >  const char * const ii_msgs[] = { "MEM", "RESV", "IO", "GEN" };
> >  EXPORT_SYMBOL_GPL(ii_msgs);
> >  
> > +/* internal error type */
> > +const char * const uu_msgs[] = { "RESV", "RESV", "HWA", "RESV" };
> > +EXPORT_SYMBOL_GPL(uu_msgs);
> 
> Do these really need to be exported?
> These don't seem to be used outside of mce_amd.
> 
> If these really need export, are these very good names?
> Perhaps these should be prefixed with something?

Hmm, some of those were used in amd64_edac at some point but then the
whole decoding code got concentrated into mce_amd.c but the exports
remained.

PP() and thus pp_msgs is still used there, for example.

So, if you'd like, you could wait until I apply Jacob's patches and push
that git branch to kernel.org and then write a patch ontop removing all
the exports except pp_msgs (you could add them to the "amd_" namespace
by calling it amd_pp_msgs or something) test that patch and send it to
me.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 2/2] MCE, AMD: MCE decoding support for AMD Family 16h

[Borislav Petkov]

On Tue, Dec 18, 2012 at 11:56:10PM +0100, Borislav Petkov wrote:
> So, if you'd like, you could wait until I apply Jacob's patches and
> push that git branch to kernel.org and then write a patch ontop
> removing all the exports except pp_msgs (you could add them to the
> "amd_" namespace by calling it amd_pp_msgs or something) test that
> patch and send it to me.

Ok, I've applied Jacob's stuff. You can do your patch ontop of

git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git edac-for-3.9

and send it to me.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH V2 0/2] MCE, AMD: MCE decoding support for AMD Family 16h

[Borislav Petkov]

On Tue, Dec 18, 2012 at 03:06:09PM -0600, Jacob Shin wrote:
> The following patchset enables MCE decoding support for AMD Family 16h
> processors.
> 
> Changes in V2:
> * Changed if/else style and pr_cont usage per feedback from:
>   https://lkml.org/lkml/2012/12/17/365
> 
> * Merged f14h and f16h mc0 and mc2 decoding into common function per
>   feedback from: https://lkml.org/lkml/2012/12/18/269
> 
> * Fixed typo in INT_ERROR decoding.
> 
> Jacob Shin (2):
>   MCE, AMD: Make MC2 decoding part of amd_decoder_ops as well
>   MCE, AMD: MCE decoding support for AMD Family 16h

Ok, applied and pushed out to:

git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git edac-for-3.9

Please take a look and give them a run.

Also, I have a python injection script with which you could test
your decoding code on F16h - no real MCEs are getting injected, but
software-only. Let me know if you want it and I'll dust it off and send
it to you along with instructions on how to use it.

Thanks.

-- 
Regards/Gruss,
    Boris.

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