[PATCH V2 0/5] Updates to AMD MCE driver per Scalable MCA spec

[PATCH V2 0/5] Updates to AMD MCE driver per Scalable MCA spec

[Aravind Gopalakrishnan]

The patchset contains updates to the MCE driver based
on the Scalable MCA specification.

Patches 1-3 include some minor changes to existing code
and have been tested for regressions on older families.

Patches 4-5 is new code and only runs on processors
with ScalableMCA feature enabled (for future)

Patch 1: Order of mce_amd_feature_init() was incorrect as
	 it should be called after we gather features from
	 cpuid bits. Fixing that in this patch
Patch 2: We do not require shared bank verification on ZP.
	 Modifying code here to return early if we are on a processor
	 that supports SMCA feature.
Patch 3: The number of blocks per bank is reduced from Fam17h onwards.
	 Fixing code to reflect this architectural change
Patch 4: LVT offset for thresholding is now programmed in different MSR
	 as opposed to per-bank MISC register in earlier processors.
	 Fixing code here to obtain LVT offset from correct MSR.
Patch 5: OS is required to set MCAXEn bit in the per-bank CONFIG MSR
	 to acknowledge the use of new MSR range for MCA.
	 Doing that here and also creating definitions for the new
	 MSR range in msr-index.

Changes in V2 (per Boris)
  - Include only definitions on header file that we are using here
  - Add comments around macros that belong to SMCA

Aravind Gopalakrishnan (5):
  x86, mce: Fix order of AMD MCE init function call
  x86/mcheck/AMD: Do not perform shared bank check for future processors
  x86/mcheck/AMD: Reduce number of blocks scanned per bank
  x86/mcheck/AMD: Fix LVT offset configuration for thresholding
  x86/mcheck/AMD: Set MCAX Enable bit

 arch/x86/include/asm/msr-index.h     |  5 +++
 arch/x86/kernel/cpu/mcheck/mce.c     |  2 +-
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 63 ++++++++++++++++++++++++++++++++++--
 3 files changed, 67 insertions(+), 3 deletions(-)

-- 
2.7.0

[PATCH V2 1/5] x86, mce: Fix order of AMD MCE init function call

[Aravind Gopalakrishnan]

In mce_amd_feature_init() we take decisions based
on mce_flags being set or not. So the feature
detection using cpuid should naturally be ordered before
we call mce_amd_feature_init()

Fixing that here.

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index a006f4c..b718080 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1617,10 +1617,10 @@ static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c)
 	case X86_VENDOR_AMD: {
 		u32 ebx = cpuid_ebx(0x80000007);
 
-		mce_amd_feature_init(c);
 		mce_flags.overflow_recov = !!(ebx & BIT(0));
 		mce_flags.succor	 = !!(ebx & BIT(1));
 		mce_flags.smca		 = !!(ebx & BIT(3));
+		mce_amd_feature_init(c);
 
 		break;
 		}
-- 
2.7.0

[PATCH V2 2/5] x86/mcheck/AMD: Do not perform shared bank check for future processors

[Aravind Gopalakrishnan]

Fam17h and above should not require a check to see if a bank
is shared or not. For shared banks, there will always be only
one core that has visibility over the MSRs and only that
particular core will be allowed to write to the MSRs

Fixing the code to return early if we detect Fam17h or above.
No change in functionality for earlier processors

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index e99b150..da570a8 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -84,6 +84,14 @@ struct thresh_restart {
 
 static inline bool is_shared_bank(int bank)
 {
+	/*
+	 * For Fam17h and above, we shouldn't require this check.
+	 * Only the core that can see valid values on the MSRs has
+	 * control over the respective MCA bank
+	 */
+	if (mce_flags.smca)
+		return 0;
+
 	/* Bank 4 is for northbridge reporting and is thus shared */
 	return (bank == 4);
 }
-- 
2.7.0

Re: [PATCH V2 2/5] x86/mcheck/AMD: Do not perform shared bank check for future processors

[kbuild test robot]

Hi Aravind,

[auto build test WARNING on v4.4-rc8]
[also build test WARNING on next-20160115]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Aravind-Gopalakrishnan/x86-mce-Fix-order-of-AMD-MCE-init-function-call/20160116-074701


coccinelle warnings: (new ones prefixed by >>)

>> arch/x86/kernel/cpu/mcheck/mce_amd.c:93:9-10: WARNING: return of 0/1 in function 'is_shared_bank' with return type bool

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] x86/mcheck/AMD: fix boolreturn.cocci warnings

[kbuild test robot]

arch/x86/kernel/cpu/mcheck/mce_amd.c:93:9-10: WARNING: return of 0/1 in function 'is_shared_bank' with return type bool

 Return statements in functions returning bool should use
 true/false instead of 1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci

CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 mce_amd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -90,7 +90,7 @@ static inline bool is_shared_bank(int ba
 	 * control over the respective MCA bank
 	 */
 	if (mce_flags.smca)
-		return 0;
+		return false;
 
 	/* Bank 4 is for northbridge reporting and is thus shared */
 	return (bank == 4);

[PATCH V2 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per bank

[Aravind Gopalakrishnan]

>From Fam17h onwards, the number of extended MISC register
blocks is reduced to 4. It is an architectural change
from what we had on earlier processors.

Changing the value of NRBLOCKS here to reflect that change.

Although theoritically the total number of extended MCx_MISC
registers was 8 in earlier processor families, in practice
we only had to use the extra registers for MC4. And only 2 of
those were used. So this change does not affect older processors.
Tested it on Fam10h, Fam15h systems and works fine.

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index da570a8..e650fdc 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -28,7 +28,7 @@
 #include <asm/msr.h>
 #include <asm/trace/irq_vectors.h>
 
-#define NR_BLOCKS         9
+#define NR_BLOCKS         5
 #define THRESHOLD_MAX     0xFFF
 #define INT_TYPE_APIC     0x00020000
 #define MASK_VALID_HI     0x80000000
-- 
2.7.0

[PATCH V2 4/5] x86/mcheck/AMD: Fix LVT offset configuration for thresholding

[Aravind Gopalakrishnan]

For processor families with  SMCA feature, the LVT offset
for threshold interrupts is configured only in MSR 0xC0000410
and not in each per bank MISC register as was done in earlier
families.

Fixing the code here to obtain the LVT offset from the correct
MSR for those families which have SMCA feature enabled.

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index e650fdc..29a7688 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -49,6 +49,15 @@
 #define DEF_LVT_OFF		0x2
 #define DEF_INT_TYPE_APIC	0x2
 
+/*
+ * SMCA settings:
+ * The following defines provide masks or bit positions of
+ * MSRs that are applicable only to SMCA enabled processors
+ */
+
+/* Threshold LVT offset is at MSR0xC0000410[15:12] */
+#define SMCA_THR_LVT_OFF	0xF000
+
 static const char * const th_names[] = {
 	"load_store",
 	"insn_fetch",
@@ -143,6 +152,15 @@ static int lvt_off_valid(struct threshold_block *b, int apic, u32 lo, u32 hi)
 	}
 
 	if (apic != msr) {
+		/*
+		 * For SMCA enabled processors, LVT offset is programmed at
+		 * different MSR and BIOS provides the value.
+		 * The original field where LVT offset was set is Reserved.
+		 * So, return early here.
+		 */
+		if (mce_flags.smca)
+			return 0;
+
 		pr_err(FW_BUG "cpu %d, invalid threshold interrupt offset %d "
 		       "for bank %d, block %d (MSR%08X=0x%x%08x)\n",
 		       b->cpu, apic, b->bank, b->block, b->address, hi, lo);
@@ -301,7 +319,21 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
 				goto init;
 
 			b.interrupt_enable = 1;
-			new	= (high & MASK_LVTOFF_HI) >> 20;
+
+			if (mce_flags.smca) {
+				u32 smca_low = 0, smca_high = 0;
+
+				/* Gather LVT offset for thresholding */
+				if (rdmsr_safe(MSR_CU_DEF_ERR,
+					       &smca_low,
+					       &smca_high))
+					break;
+
+				new = (smca_low & SMCA_THR_LVT_OFF) >> 12;
+			} else {
+				new	= (high & MASK_LVTOFF_HI) >> 20;
+			}
+
 			offset  = setup_APIC_mce_threshold(offset, new);
 
 			if ((offset == new) &&
-- 
2.7.0

Re: [PATCH V2 4/5] x86/mcheck/AMD: Fix LVT offset configuration for thresholding

[Borislav Petkov]

On Fri, Jan 15, 2016 at 05:50:35PM -0600, Aravind Gopalakrishnan wrote:
> For processor families with  SMCA feature, the LVT offset
> for threshold interrupts is configured only in MSR 0xC0000410
> and not in each per bank MISC register as was done in earlier
> families.
> 
> Fixing the code here to obtain the LVT offset from the correct
> MSR for those families which have SMCA feature enabled.
> 
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)

Did a small cleanup ontop:

---
From: Borislav Petkov <bp@suse.de>
Subject: [PATCH] x86/mce/AMD: Carve out threshold block preparation

mce_amd_feature_init() was getting pretty fat, carve out the
threshold_block setup into a separate function on order to simplify
flow and make it more understandable.

No functionality change.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 88 ++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index b9739b9b9341..57c7b296d1c5 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -267,14 +267,60 @@ static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c)
 	wrmsr(MSR_CU_DEF_ERR, low, high);
 }
 
+static int
+prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
+			int offset, u32 misc_high)
+{
+	unsigned int cpu = smp_processor_id();
+	struct threshold_block b;
+	int new;
+
+	if (!block)
+		per_cpu(bank_map, cpu) |= (1 << bank);
+
+	memset(&b, 0, sizeof(b));
+	b.cpu			= cpu;
+	b.bank			= bank;
+	b.block			= block;
+	b.address		= addr;
+	b.interrupt_capable	= lvt_interrupt_supported(bank, misc_high);
+
+	if (!b.interrupt_capable)
+		goto done;
+
+	b.interrupt_enable = 1;
+
+	if (mce_flags.smca) {
+		u32 smca_low = 0, smca_high = 0;
+
+		/* Gather LVT offset for thresholding */
+		if (rdmsr_safe(MSR_CU_DEF_ERR, &smca_low, &smca_high))
+			goto out;
+
+		new = (smca_low & SMCA_THR_LVT_OFF) >> 12;
+	} else {
+		new = (misc_high & MASK_LVTOFF_HI) >> 20;
+	}
+
+	offset = setup_APIC_mce_threshold(offset, new);
+
+	if ((offset == new) &&
+	    (mce_threshold_vector != amd_threshold_interrupt))
+		mce_threshold_vector = amd_threshold_interrupt;
+
+done:
+	mce_threshold_block_init(&b, offset);
+
+out:
+	return offset;
+}
+
 /* cpu init entry point, called from mce.c with preempt off */
 void mce_amd_feature_init(struct cpuinfo_x86 *c)
 {
-	struct threshold_block b;
-	unsigned int cpu = smp_processor_id();
 	u32 low = 0, high = 0, address = 0;
 	unsigned int bank, block;
-	int offset = -1, new;
+	int offset = -1;
 
 	for (bank = 0; bank < mca_cfg.banks; ++bank) {
 		for (block = 0; block < NR_BLOCKS; ++block) {
@@ -299,41 +345,7 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
 			     (high & MASK_LOCKED_HI))
 				continue;
 
-			if (!block)
-				per_cpu(bank_map, cpu) |= (1 << bank);
-
-			memset(&b, 0, sizeof(b));
-			b.cpu			= cpu;
-			b.bank			= bank;
-			b.block			= block;
-			b.address		= address;
-			b.interrupt_capable	= lvt_interrupt_supported(bank, high);
-
-			if (!b.interrupt_capable)
-				goto init;
-
-			b.interrupt_enable = 1;
-
-			if (mce_flags.smca) {
-				u32 smca_low = 0, smca_high = 0;
-
-				/* Gather LVT offset for thresholding */
-				if (rdmsr_safe(MSR_CU_DEF_ERR, &smca_low, &smca_high))
-					break;
-
-				new = (smca_low & SMCA_THR_LVT_OFF) >> 12;
-			} else {
-				new = (high & MASK_LVTOFF_HI) >> 20;
-			}
-
-			offset  = setup_APIC_mce_threshold(offset, new);
-
-			if ((offset == new) &&
-			    (mce_threshold_vector != amd_threshold_interrupt))
-				mce_threshold_vector = amd_threshold_interrupt;
-
-init:
-			mce_threshold_block_init(&b, offset);
+			offset = prepare_threshold_block(bank, block, address, offset, high);
 		}
 	}
 
-- 
2.3.5

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V2 4/5] x86/mcheck/AMD: Fix LVT offset configuration for thresholding

[Borislav Petkov]

On Fri, Jan 15, 2016 at 05:50:35PM -0600, Aravind Gopalakrishnan wrote:
> For processor families with  SMCA feature, the LVT offset
> for threshold interrupts is configured only in MSR 0xC0000410
> and not in each per bank MISC register as was done in earlier
> families.
> 
> Fixing the code here to obtain the LVT offset from the correct
> MSR for those families which have SMCA feature enabled.
> 
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index e650fdc..29a7688 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -49,6 +49,15 @@
>  #define DEF_LVT_OFF		0x2
>  #define DEF_INT_TYPE_APIC	0x2
>  
> +/*
> + * SMCA settings:
> + * The following defines provide masks or bit positions of
> + * MSRs that are applicable only to SMCA enabled processors
> + */
> +
> +/* Threshold LVT offset is at MSR0xC0000410[15:12] */
> +#define SMCA_THR_LVT_OFF	0xF000
> +
>  static const char * const th_names[] = {
>  	"load_store",
>  	"insn_fetch",
> @@ -143,6 +152,15 @@ static int lvt_off_valid(struct threshold_block *b, int apic, u32 lo, u32 hi)
>  	}
>  
>  	if (apic != msr) {
> +		/*
> +		 * For SMCA enabled processors, LVT offset is programmed at
> +		 * different MSR and BIOS provides the value.
> +		 * The original field where LVT offset was set is Reserved.
> +		 * So, return early here.
> +		 */
> +		if (mce_flags.smca)
> +			return 0;
> +
>  		pr_err(FW_BUG "cpu %d, invalid threshold interrupt offset %d "
>  		       "for bank %d, block %d (MSR%08X=0x%x%08x)\n",
>  		       b->cpu, apic, b->bank, b->block, b->address, hi, lo);
> @@ -301,7 +319,21 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
>  				goto init;
>  
>  			b.interrupt_enable = 1;
> -			new	= (high & MASK_LVTOFF_HI) >> 20;
> +
> +			if (mce_flags.smca) {
> +				u32 smca_low = 0, smca_high = 0;

Those variables don't need to be initialized to 0 since you're reading
into them right afterwards.

I fixed that up.



> +
> +				/* Gather LVT offset for thresholding */
> +				if (rdmsr_safe(MSR_CU_DEF_ERR,
> +					       &smca_low,
> +					       &smca_high))
> +					break;
> +

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[PATCH V2 5/5] x86/mcheck/AMD: Set MCAX Enable bit

[Aravind Gopalakrishnan]

It is required for OS to acknowledge that it is using
the MCAX register set and its associated fields by setting
the 'McaXEnable' bit in each bank's MCi_CONFIG register. If
it is not set, then all UC errors will cause a system panic.

So setting the bit here and also defining the new MSR range for
SMCA enabled proccessors in msr-index

Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 arch/x86/include/asm/msr-index.h     |  5 +++++
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index b05402e..088b5a7 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -264,6 +264,11 @@
 #define MSR_IA32_MC0_CTL2		0x00000280
 #define MSR_IA32_MCx_CTL2(x)		(MSR_IA32_MC0_CTL2 + (x))
 
+/* SMCA defined MSR register set for AMD64 */
+#define MSR_AMD64_SMCA_MC0_CONFIG	0xc0002004
+
+#define MSR_AMD64_SMCA_MCx_CONFIG(x)	(MSR_AMD64_SMCA_MC0_CONFIG + 0x10*(x))
+
 #define MSR_P6_PERFCTR0			0x000000c1
 #define MSR_P6_PERFCTR1			0x000000c2
 #define MSR_P6_EVNTSEL0			0x00000186
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 29a7688..158ea86 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -57,6 +57,14 @@
 
 /* Threshold LVT offset is at MSR0xC0000410[15:12] */
 #define SMCA_THR_LVT_OFF	0xF000
+/*
+ * OS is required to set the MCAX bit to acknowledge
+ * that it is now using the new MSR ranges and new registers
+ * under each bank. It also means that OS will configure
+ * Deferred errors in the new MCx_CONFIG register.
+ * If the bit is not set, UC errors will cause a system panic
+ */
+#define SMCA_MCAX_EN_OFF	0x1
 
 static const char * const th_names[] = {
 	"load_store",
@@ -322,6 +330,17 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
 
 			if (mce_flags.smca) {
 				u32 smca_low = 0, smca_high = 0;
+				u32 smca_addr = 0;
+
+				/* Set MCAXEnable bit for each bank */
+				smca_addr = MSR_AMD64_SMCA_MCx_CONFIG(bank);
+				if (rdmsr_safe(smca_addr,
+					       &smca_low,
+					       &smca_high))
+					continue;
+
+				smca_high |= SMCA_MCAX_EN_OFF;
+				wrmsr(smca_addr, smca_low, smca_high);
 
 				/* Gather LVT offset for thresholding */
 				if (rdmsr_safe(MSR_CU_DEF_ERR,
-- 
2.7.0

Re: [PATCH V2 0/5] Updates to AMD MCE driver per Scalable MCA spec

[Borislav Petkov]

On Fri, Jan 15, 2016 at 05:50:31PM -0600, Aravind Gopalakrishnan wrote:
> The patchset contains updates to the MCE driver based
> on the Scalable MCA specification.

Applied and pushed here:

http://git.kernel.org/cgit/linux/kernel/git/bp/bp.git/log/?h=tip-ras

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.