Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1309723 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] Updates to AMD MCE driver per Scalable MCA spec Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> - 2016-01-14 23:40 +0100
    Re: [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per  bank Borislav Petkov <bp@alien8.de> - 2016-01-14 23:40 +0100
      Re: [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per  bank Borislav Petkov <bp@alien8.de> - 2016-01-15 00:00 +0100
        Re: [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per  bank Borislav Petkov <bp@alien8.de> - 2016-01-15 12:20 +0100
    [PATCH 1/5] x86, mce: Fix order of AMD MCE init function call Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> - 2016-01-14 23:40 +0100
    [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per bank Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> - 2016-01-14 23:40 +0100
    [PATCH 2/5] x86/mcheck/AMD: Do not perform shared bank check for future processors Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> - 2016-01-14 23:40 +0100

#1309723 — [PATCH 0/5] Updates to AMD MCE driver per Scalable MCA spec

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.
	 
Note: checkpatch generates warnings for Patch 5. But I have not
wrapped text around the character limit as it looked ugly.
(I overshot it by only a character or two)

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     | 23 +++++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce.c     |  2 +-
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 50 ++++++++++++++++++++++++++++++++++--
 3 files changed, 72 insertions(+), 3 deletions(-)

-- 
2.7.0

[toc] | [next] | [standalone]

#1309724 — Re: [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per bank

On Thu, Jan 14, 2016 at 04:05:38PM -0600, Aravind Gopalakrishnan wrote:
> 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

This doesn't look necessary to me. We do check MCi_MISC[BlkPtr] before
accessing that MSR.

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

#1309745 — Re: [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per bank

On Thu, Jan 14, 2016 at 04:48:22PM -0600, Aravind Gopalakrishnan wrote:
> True. But that BlkPtr logic also will undergo changes as it's interpretation
> for future processors is different.

But there still must be a bit there which says "this register is valid",
like MCi_MISC[63].

And so I'd very much prefer checking a bit (or bits) instead of relying
on defines.

> We are guaranteed to have all the MISC registers (all 5 of them) going
> forward.

Guarantees are worth nothing.

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

#1310057 — Re: [PATCH 3/5] x86/mcheck/AMD: Reduce number of blocks scanned per bank

On Thu, Jan 14, 2016 at 05:08:30PM -0600, Aravind Gopalakrishnan wrote:
> In the same manner, we'd still have to know the last possible MISC
> register for future processors..

I was going to suggest that we should probably *count* the MISC
registers upfront so that we know exactly how many are we dealing with
instead of relying on macros but that would be overengineering it for no
good reason. And we're checking the valid bits and so on, so we're good.

So ok, I'm persuaded.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web