x86/RAS: Simplify SMCA bank descriptor struct

PATCH In review Borislav Petkov 5 participants 13 messages Nov 3, 2016 – Nov 10, 2016

Subsystems (3)

x86 EDAC-core EDAC-AMD64

lore

Share & tools

lore

Borislav Petkov

author maintainer

2016-11-03 12:56 UTC #0 lore raw

PATCH 1/3 x86/RAS: Simplify SMCA bank descriptor struct

From: Borislav Petkov <[email protected]>

Call the struct simply smca_bank, it's instance ID can be simply ->id. Makes the code much more readable.

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/include/asm/mce.h           |  7 ++++---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 10 +++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

arch/x86/include/asm/mce.h +4 -3

index 9bd7ff5ffbcc..4d97875d9543 100644
@@ -371,12 +371,13 @@ struct smca_hwid_mcatype {
 	u32 xec_bitmap;		/* Bitmap of valid ExtErrorCodes; current max is 21. */
 };
 
-struct smca_bank_info {
+struct smca_bank {
 	struct smca_hwid_mcatype *type;
-	u32 type_instance;
+	/* Instance ID */
+	u32 id;
 };
 
-extern struct smca_bank_info smca_banks[MAX_NR_BANKS];
+extern struct smca_bank smca_banks[MAX_NR_BANKS];
 
 #endif

arch/x86/kernel/cpu/mcheck/mce_amd.c +5 -5

index 9b5403462936..ac2f4f2cad18 100644
@@ -116,7 +116,7 @@ static struct smca_hwid_mcatype smca_hwid_mcatypes[] = {
 	{ SMCA_SMU,	 HWID_MCATYPE(0x01, 0x0), 0x1 },
 };
 
-struct smca_bank_info smca_banks[MAX_NR_BANKS];
+struct smca_bank smca_banks[MAX_NR_BANKS];
 EXPORT_SYMBOL_GPL(smca_banks);
 
 /*

@@ -150,14 +150,14 @@ static void get_smca_bank_info(unsigned int bank)
 {
 	unsigned int i, hwid_mcatype, cpu = smp_processor_id();
 	struct smca_hwid_mcatype *type;
-	u32 high, instanceId;
+	u32 high, instance_id;
 	u16 hwid, mcatype;
 
 	/* Collect bank_info using CPU 0 for now. */
 	if (cpu)
 		return;
 
-	if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), &instanceId, &high)) {
+	if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), &instance_id, &high)) {
 		pr_warn("Failed to read MCA_IPID for bank %d\n", bank);
 		return;
 	}

@@ -170,7 +170,7 @@ static void get_smca_bank_info(unsigned int bank)
 		type = &smca_hwid_mcatypes[i];
 		if (hwid_mcatype == type->hwid_mcatype) {
 			smca_banks[bank].type = type;
-			smca_banks[bank].type_instance = instanceId;
+			smca_banks[bank].id = instance_id;
 			break;
 		}
 	}

@@ -839,7 +839,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 
 	snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
 		 "%s_%x", smca_bank_names[bank_type].name,
-			  smca_banks[bank].type_instance);
+			  smca_banks[bank].id);
 	return buf_mcatype;
 }

Signature · 1 line

2.10.0

Borislav Petkov

author maintainer

2016-11-03 12:56 UTC #1 lore raw

PATCH 2/3 x86/RAS: Simplify SMCA HWID descriptor struct

From: Borislav Petkov <[email protected]>

Call it simply smca_hwid and call local variables "hwid". More readable.

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/include/asm/mce.h           |  4 ++--
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 24 +++++++++---------------
 drivers/edac/mce_amd.c               | 10 +++++-----
 3 files changed, 16 insertions(+), 22 deletions(-)

arch/x86/include/asm/mce.h +2 -2

index 4d97875d9543..ccc801a0da0f 100644
@@ -365,14 +365,14 @@ extern struct smca_bank_name smca_bank_names[N_SMCA_BANK_TYPES];
 
 #define HWID_MCATYPE(hwid, mcatype) ((hwid << 16) | mcatype)
 
-struct smca_hwid_mcatype {
+struct smca_hwid {
 	unsigned int bank_type;	/* Use with smca_bank_types for easy indexing. */
 	u32 hwid_mcatype;	/* (hwid,mcatype) tuple */
 	u32 xec_bitmap;		/* Bitmap of valid ExtErrorCodes; current max is 21. */
 };
 
 struct smca_bank {
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *hwid;
 	/* Instance ID */
 	u32 id;
 };

arch/x86/kernel/cpu/mcheck/mce_amd.c +9 ~~-15~~

index ac2f4f2cad18..ff81667af2f2 100644
@@ -86,7 +86,7 @@ struct smca_bank_name smca_bank_names[] = {
 };
 EXPORT_SYMBOL_GPL(smca_bank_names);
 
-static struct smca_hwid_mcatype smca_hwid_mcatypes[] = {
+static struct smca_hwid smca_hwid_mcatypes[] = {
 	/* { bank_type, hwid_mcatype, xec_bitmap } */
 
 	/* ZN Core (HWID=0xB0) MCA types */

@@ -142,16 +142,11 @@ static void default_deferred_error_interrupt(void)
 }
 void (*deferred_error_int_vector)(void) = default_deferred_error_interrupt;
 
-/*
- * CPU Initialization
- */
-
 static void get_smca_bank_info(unsigned int bank)
 {
 	unsigned int i, hwid_mcatype, cpu = smp_processor_id();
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *s_hwid;
 	u32 high, instance_id;
-	u16 hwid, mcatype;
 
 	/* Collect bank_info using CPU 0 for now. */
 	if (cpu)

@@ -162,14 +157,13 @@ static void get_smca_bank_info(unsigned int bank)
 		return;
 	}
 
-	hwid = high & MCI_IPID_HWID;
-	mcatype = (high & MCI_IPID_MCATYPE) >> 16;
-	hwid_mcatype = HWID_MCATYPE(hwid, mcatype);
+	hwid_mcatype = HWID_MCATYPE(high & MCI_IPID_HWID,
+				    (high & MCI_IPID_MCATYPE) >> 16);
 
 	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
-		type = &smca_hwid_mcatypes[i];
-		if (hwid_mcatype == type->hwid_mcatype) {
-			smca_banks[bank].type = type;
+		s_hwid = &smca_hwid_mcatypes[i];
+		if (hwid_mcatype == s_hwid->hwid_mcatype) {
+			smca_banks[bank].hwid = s_hwid;
 			smca_banks[bank].id = instance_id;
 			break;
 		}

@@ -826,10 +820,10 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 		return th_names[bank];
 	}
 
-	if (!smca_banks[bank].type)
+	if (!smca_banks[bank].hwid)
 		return NULL;
 
-	bank_type = smca_banks[bank].type->bank_type;
+	bank_type = smca_banks[bank].hwid->bank_type;
 
 	if (b && bank_type == SMCA_UMC) {
 		if (b->block < ARRAY_SIZE(smca_umc_block_names))

drivers/edac/mce_amd.c +5 -5

index daaac2c79ca7..8e96c6ddf272 100644
@@ -851,7 +851,7 @@ static void decode_mc6_mce(struct mce *m)
 /* Decode errors according to Scalable MCA specification */
 static void decode_smca_errors(struct mce *m)
 {
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *hwid;
 	unsigned int bank_type;
 	const char *ip_name;
 	u8 xec = XEC(m->status, xec_mask);

@@ -862,18 +862,18 @@ static void decode_smca_errors(struct mce *m)
 	if (boot_cpu_data.x86 >= 0x17 && m->bank == 4)
 		pr_emerg(HW_ERR "Bank 4 is reserved on Fam17h.\n");
 
-	type = smca_banks[m->bank].type;
-	if (!type)
+	hwid = smca_banks[m->bank].hwid;
+	if (!hwid)
 		return;
 
-	bank_type = type->bank_type;
+	bank_type = hwid->bank_type;
 	ip_name = smca_bank_names[bank_type].long_name;
 
 	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
 
 	/* Only print the decode of valid error codes */
 	if (xec < smca_mce_descs[bank_type].num_descs &&
-			(type->xec_bitmap & BIT_ULL(xec))) {
+			(hwid->xec_bitmap & BIT_ULL(xec))) {
 		pr_emerg(HW_ERR "%s Error: ", ip_name);
 		pr_cont("%s.\n", smca_mce_descs[bank_type].descs[xec]);
 	}

Signature · 1 line

2.10.0

tip-bot for Borislav Petkov

merge bot

2016-11-08 16:21 UTC #2 lore raw

[tip:ras/core] x86/RAS: Simplify SMCA HWID descriptor struct

x86/RAS: Simplify SMCA HWID descriptor struct

Commit-ID:  1ce9cd7f9f0b71af7c496b816734bc2dc699363a
Gitweb:     http://git.kernel.org/tip/1ce9cd7f9f0b71af7c496b816734bc2dc699363a
Author:     Borislav Petkov <[email protected]>
AuthorDate: Wed, 2 Nov 2016 12:48:01 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 8 Nov 2016 17:10:14 +0100

x86/RAS: Simplify SMCA HWID descriptor struct

Call it simply smca_hwid and call local variables "hwid". More readable.

Signed-off-by Borislav Petkov <[email protected]>

Tested-by Yazen Ghannam <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/include/asm/mce.h           |  4 ++--
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 24 +++++++++---------------
 drivers/edac/mce_amd.c               | 10 +++++-----
 3 files changed, 16 insertions(+), 22 deletions(-)

arch/x86/include/asm/mce.h +2 -2

index 4d97875..ccc801a 100644
@@ -365,14 +365,14 @@ extern struct smca_bank_name smca_bank_names[N_SMCA_BANK_TYPES];
 
 #define HWID_MCATYPE(hwid, mcatype) ((hwid << 16) | mcatype)
 
-struct smca_hwid_mcatype {
+struct smca_hwid {
 	unsigned int bank_type;	/* Use with smca_bank_types for easy indexing. */
 	u32 hwid_mcatype;	/* (hwid,mcatype) tuple */
 	u32 xec_bitmap;		/* Bitmap of valid ExtErrorCodes; current max is 21. */
 };
 
 struct smca_bank {
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *hwid;
 	/* Instance ID */
 	u32 id;
 };

arch/x86/kernel/cpu/mcheck/mce_amd.c +9 ~~-15~~

index ac2f4f2..ff81667 100644
@@ -86,7 +86,7 @@ struct smca_bank_name smca_bank_names[] = {
 };
 EXPORT_SYMBOL_GPL(smca_bank_names);
 
-static struct smca_hwid_mcatype smca_hwid_mcatypes[] = {
+static struct smca_hwid smca_hwid_mcatypes[] = {
 	/* { bank_type, hwid_mcatype, xec_bitmap } */
 
 	/* ZN Core (HWID=0xB0) MCA types */

@@ -142,16 +142,11 @@ static void default_deferred_error_interrupt(void)
 }
 void (*deferred_error_int_vector)(void) = default_deferred_error_interrupt;
 
-/*
- * CPU Initialization
- */
-
 static void get_smca_bank_info(unsigned int bank)
 {
 	unsigned int i, hwid_mcatype, cpu = smp_processor_id();
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *s_hwid;
 	u32 high, instance_id;
-	u16 hwid, mcatype;
 
 	/* Collect bank_info using CPU 0 for now. */
 	if (cpu)

@@ -162,14 +157,13 @@ static void get_smca_bank_info(unsigned int bank)
 		return;
 	}
 
-	hwid = high & MCI_IPID_HWID;
-	mcatype = (high & MCI_IPID_MCATYPE) >> 16;
-	hwid_mcatype = HWID_MCATYPE(hwid, mcatype);
+	hwid_mcatype = HWID_MCATYPE(high & MCI_IPID_HWID,
+				    (high & MCI_IPID_MCATYPE) >> 16);
 
 	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
-		type = &smca_hwid_mcatypes[i];
-		if (hwid_mcatype == type->hwid_mcatype) {
-			smca_banks[bank].type = type;
+		s_hwid = &smca_hwid_mcatypes[i];
+		if (hwid_mcatype == s_hwid->hwid_mcatype) {
+			smca_banks[bank].hwid = s_hwid;
 			smca_banks[bank].id = instance_id;
 			break;
 		}

@@ -826,10 +820,10 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 		return th_names[bank];
 	}
 
-	if (!smca_banks[bank].type)
+	if (!smca_banks[bank].hwid)
 		return NULL;
 
-	bank_type = smca_banks[bank].type->bank_type;
+	bank_type = smca_banks[bank].hwid->bank_type;
 
 	if (b && bank_type == SMCA_UMC) {
 		if (b->block < ARRAY_SIZE(smca_umc_block_names))

drivers/edac/mce_amd.c +5 -5

index daaac2c..8e96c6dd 100644
@@ -851,7 +851,7 @@ static void decode_mc6_mce(struct mce *m)
 /* Decode errors according to Scalable MCA specification */
 static void decode_smca_errors(struct mce *m)
 {
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *hwid;
 	unsigned int bank_type;
 	const char *ip_name;
 	u8 xec = XEC(m->status, xec_mask);

@@ -862,18 +862,18 @@ static void decode_smca_errors(struct mce *m)
 	if (boot_cpu_data.x86 >= 0x17 && m->bank == 4)
 		pr_emerg(HW_ERR "Bank 4 is reserved on Fam17h.\n");
 
-	type = smca_banks[m->bank].type;
-	if (!type)
+	hwid = smca_banks[m->bank].hwid;
+	if (!hwid)
 		return;
 
-	bank_type = type->bank_type;
+	bank_type = hwid->bank_type;
 	ip_name = smca_bank_names[bank_type].long_name;
 
 	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);
 
 	/* Only print the decode of valid error codes */
 	if (xec < smca_mce_descs[bank_type].num_descs &&
-			(type->xec_bitmap & BIT_ULL(xec))) {
+			(hwid->xec_bitmap & BIT_ULL(xec))) {
 		pr_emerg(HW_ERR "%s Error: ", ip_name);
 		pr_cont("%s.\n", smca_mce_descs[bank_type].descs[xec]);
 	}

Yazen Ghannam

maintainer

2016-11-10 19:21 UTC #3 lore raw

Re: [tip:ras/core] x86/RAS: Simplify SMCA HWID descriptor struct

Quoted 11 lines

static void get_smca_bank_info(unsigned int bank) {

arch/x86/kernel/cpu/mcheck/mce_amd.c +1 -2

 	unsigned int i, hwid_mcatype, cpu = smp_processor_id();
-	struct smca_hwid_mcatype *type;
+	struct smca_hwid *s_hwid;
 	u32 high, instance_id;
-	u16 hwid, mcatype;
 
 	/* Collect bank_info using CPU 0 for now. */
 	if (cpu)

arch/x86/kernel/cpu/mcheck/mce_amd.c static void get_smca_bank_info(unsigned int bank) · line 162

Quoted 9 lines

arch/x86/kernel/cpu/mcheck/mce_amd.c +2 -3

@@ -162,14 +157,13 @@ static void get_smca_bank_info(unsigned int bank)
 		return;
 	}
 
-	hwid = high & MCI_IPID_HWID;
-	mcatype = (high & MCI_IPID_MCATYPE) >> 16;
-	hwid_mcatype = HWID_MCATYPE(hwid, mcatype);
+	hwid_mcatype = HWID_MCATYPE(high & MCI_IPID_HWID,
+				    (high & MCI_IPID_MCATYPE) >> 16);

Sorry for catching this late, but it seems this change doesn't compile correctly. This causes the value of hwid_mcatype to be incorrect, so we will never match a bank to its type.

I see this with GCC 4.8.5 and 5.4.0.

There are no warnings or issues when building or booting just that the behavior is incorrect.

Disassembly of above change:

      db:       8b 45 e0                mov    -0x20(%rbp),%eax
      de:       41 89 c4                mov    %eax,%r12d
      e1:       25 00 00 ff 0f          and    $0xfff0000,%eax
      e6:       41 c1 ec 10             shr    $0x10,%r12d
      ea:       41 09 c4                or     %eax,%r12d

Disassembly of original code:

     286:       8b 45 d0                mov    -0x30(%rbp),%eax
     289:       41 89 c5                mov    %eax,%r13d
     28c:       c1 e8 10                shr    $0x10,%eax
     28f:       41 81 e5 ff 0f 00 00    and    $0xfff,%r13d
     296:       41 c1 e5 10             shl    $0x10,%r13d
     29a:       41 09 c5                or     %eax,%r13d

Adding extra parentheses in HWID_MCATYPE() gives the same assembly as the original code and fixes the behavior.

Review static void get_smca_bank_info(unsigned int bank) · line 164

Quoted 2 lines

(quoted diff) +2 -0

+	hwid_mcatype = HWID_MCATYPE((high & MCI_IPID_HWID)),
+				    ((high & MCI_IPID_MCATYPE) >> 16));

Thanks,
Yazen

Borislav Petkov

2016-11-10 17:57 UTC #4 lore raw

Re: [tip:ras/core] x86/RAS: Simplify SMCA HWID descriptor struct

Quoted 5 lines · Yazen Ghannam

Adding extra parentheses in HWID_MCATYPE() gives the same assembly as the original code and fixes the behavior.
Review
Quoted 2 lines
(quoted diff) +2 -0
+	hwid_mcatype = HWID_MCATYPE((high & MCI_IPID_HWID)),
+				    ((high & MCI_IPID_MCATYPE) >> 16));

Argh, the macro should be adding the additional parentheses:

#define HWID_MCATYPE(hwid, mcatype) (((hwid) << 16) | (mcatype))

That should fix the issue too.

Signature · 5 lines

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

Thomas Gleixner

2016-11-10 19:56 UTC #5 lore raw

Re: [tip:ras/core] x86/RAS: Simplify SMCA HWID descriptor struct

Quoted 4 lines · Borislav Petkov

Quoted 3 lines · Yazen Ghannam
Adding extra parentheses in HWID_MCATYPE() gives the same assembly as the original code and fixes the behavior.

Quoted 8 lines

Quoted 2 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+	hwid_mcatype = HWID_MCATYPE((high & MCI_IPID_HWID)),
+				    ((high & MCI_IPID_MCATYPE) >> 16));
Argh, the macro should be adding the additional parentheses:
#define HWID_MCATYPE(hwid, mcatype) (((hwid) << 16) | (mcatype))
That should fix the issue too.

Patch please.

Yazen Ghannam

maintainer

2016-11-10 20:12 UTC #6 lore raw

Re: [tip:ras/core] x86/RAS: Simplify SMCA HWID descriptor struct

Quoted 7 lines

Quoted 6 lines
Argh, the macro should be adding the additional parentheses:
#define HWID_MCATYPE(hwid, mcatype) (((hwid) << 16) | (mcatype))
That should fix the issue too.

Yep, sure does.

Quoted 1 line

Patch please.

Will do.

Thanks,
Yazen

Borislav Petkov

author maintainer

2016-11-03 12:56 UTC #7 lore raw

PATCH 3/3 x86/RAS: Rename smca_bank_names to smca_names

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Make it differ more from struct smca_bank_name for better readability.

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/include/asm/mce.h           | 2 +-
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 6 +++---
 drivers/edac/mce_amd.c               | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

arch/x86/include/asm/mce.h +1 -1

index ccc801a0da0f..8ffd21596dd7 100644
@@ -361,7 +361,7 @@ struct smca_bank_name {
 	const char *long_name;	/* Long name for pretty-printing */
 };
 
-extern struct smca_bank_name smca_bank_names[N_SMCA_BANK_TYPES];
+extern struct smca_bank_name smca_names[N_SMCA_BANK_TYPES];
 
 #define HWID_MCATYPE(hwid, mcatype) ((hwid << 16) | mcatype)

arch/x86/kernel/cpu/mcheck/mce_amd.c +3 -3

index ff81667af2f2..afeb02b87127 100644
@@ -69,7 +69,7 @@ static const char * const smca_umc_block_names[] = {
 	"misc_umc"
 };
 
-struct smca_bank_name smca_bank_names[] = {
+struct smca_bank_name smca_names[] = {
 	[SMCA_LS]	= { "load_store",	"Load Store Unit" },
 	[SMCA_IF]	= { "insn_fetch",	"Instruction Fetch Unit" },
 	[SMCA_L2_CACHE]	= { "l2_cache",		"L2 Cache" },

@@ -84,7 +84,7 @@ struct smca_bank_name smca_bank_names[] = {
 	[SMCA_PSP]	= { "psp",		"Platform Security Processor" },
 	[SMCA_SMU]	= { "smu",		"System Management Unit" },
 };
-EXPORT_SYMBOL_GPL(smca_bank_names);
+EXPORT_SYMBOL_GPL(smca_names);
 
 static struct smca_hwid smca_hwid_mcatypes[] = {
 	/* { bank_type, hwid_mcatype, xec_bitmap } */

@@ -832,7 +832,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 	}
 
 	snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
-		 "%s_%x", smca_bank_names[bank_type].name,
+		 "%s_%x", smca_names[bank_type].name,
 			  smca_banks[bank].id);
 	return buf_mcatype;
 }

drivers/edac/mce_amd.c +1 -1

index 8e96c6ddf272..3dee58583d25 100644
@@ -867,7 +867,7 @@ static void decode_smca_errors(struct mce *m)
 		return;
 
 	bank_type = hwid->bank_type;
-	ip_name = smca_bank_names[bank_type].long_name;
+	ip_name = smca_names[bank_type].long_name;
 
 	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);

Signature · 1 line

2.10.0

tip-bot for Borislav Petkov

merge bot

2016-11-08 16:21 UTC #8 lore raw

[tip:ras/core] x86/RAS: Rename smca_bank_names to smca_names

x86/RAS: Rename smca_bank_names to smca_names

Commit-ID:  a9a1c0ee04aa771e5523ae33e458c702261ab547
Gitweb:     http://git.kernel.org/tip/a9a1c0ee04aa771e5523ae33e458c702261ab547
Author:     Borislav Petkov <[email protected]>
AuthorDate: Wed, 2 Nov 2016 13:24:47 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 8 Nov 2016 17:10:14 +0100

x86/RAS: Rename smca_bank_names to smca_names

Make it differ more from struct smca_bank_name for better readability.

Signed-off-by Borislav Petkov <[email protected]>

Tested-by Yazen Ghannam <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/include/asm/mce.h           | 2 +-
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 6 +++---
 drivers/edac/mce_amd.c               | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

arch/x86/include/asm/mce.h +1 -1

index ccc801a..8ffd215 100644
@@ -361,7 +361,7 @@ struct smca_bank_name {
 	const char *long_name;	/* Long name for pretty-printing */
 };
 
-extern struct smca_bank_name smca_bank_names[N_SMCA_BANK_TYPES];
+extern struct smca_bank_name smca_names[N_SMCA_BANK_TYPES];
 
 #define HWID_MCATYPE(hwid, mcatype) ((hwid << 16) | mcatype)

arch/x86/kernel/cpu/mcheck/mce_amd.c +3 -3

index ff81667..afeb02b8 100644
@@ -69,7 +69,7 @@ static const char * const smca_umc_block_names[] = {
 	"misc_umc"
 };
 
-struct smca_bank_name smca_bank_names[] = {
+struct smca_bank_name smca_names[] = {
 	[SMCA_LS]	= { "load_store",	"Load Store Unit" },
 	[SMCA_IF]	= { "insn_fetch",	"Instruction Fetch Unit" },
 	[SMCA_L2_CACHE]	= { "l2_cache",		"L2 Cache" },

@@ -84,7 +84,7 @@ struct smca_bank_name smca_bank_names[] = {
 	[SMCA_PSP]	= { "psp",		"Platform Security Processor" },
 	[SMCA_SMU]	= { "smu",		"System Management Unit" },
 };
-EXPORT_SYMBOL_GPL(smca_bank_names);
+EXPORT_SYMBOL_GPL(smca_names);
 
 static struct smca_hwid smca_hwid_mcatypes[] = {
 	/* { bank_type, hwid_mcatype, xec_bitmap } */

@@ -832,7 +832,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 	}
 
 	snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
-		 "%s_%x", smca_bank_names[bank_type].name,
+		 "%s_%x", smca_names[bank_type].name,
 			  smca_banks[bank].id);
 	return buf_mcatype;
 }

drivers/edac/mce_amd.c +1 -1

index 8e96c6dd..3dee585 100644
@@ -867,7 +867,7 @@ static void decode_smca_errors(struct mce *m)
 		return;
 
 	bank_type = hwid->bank_type;
-	ip_name = smca_bank_names[bank_type].long_name;
+	ip_name = smca_names[bank_type].long_name;
 
 	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);

Yazen Ghannam

maintainer Tested-by on v1 1/3

2016-11-04 15:17 UTC #9 lore raw

In reply to Borislav Petkov

Quoted 5 lines

Call the struct simply smca_bank, it's instance ID can be simply ->id. Makes the code much more readable.
Signed-off-by Borislav Petkov <[email protected]>

Looks good to me.

Please add:

Tested-by Yazen Ghannam <[email protected]>

Ditto for the others.

Thanks,
Yazen

Borislav Petkov

author maintainer

2016-11-04 15:23 UTC #10 lore raw

Quoted 4 lines · Yazen Ghannam

Please add:
Tested-by Yazen Ghannam <[email protected]>
Ditto for the others.

Thanks, here's one more which I've been meaning to do:

---
From: Borislav Petkov <[email protected]>
Date: Thu, 3 Nov 2016 21:12:33 +0100
Subject: [PATCH] x86/RAS: Hide SMCA bank names

Add accessor functions and hide the smca_names array. Also, add a sanity-check to bank HWID assignment in get_smca_bank_info().

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/include/asm/mce.h           |  8 +-------
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 32 +++++++++++++++++++++++++++++---
 drivers/edac/mce_amd.c               |  2 +-
 3 files changed, 31 insertions(+), 11 deletions(-)

arch/x86/include/asm/mce.h +1 -7

index 8ffd21596dd7..748b8da8e627 100644
@@ -356,13 +356,6 @@ enum smca_bank_types {
 	N_SMCA_BANK_TYPES
 };
 
-struct smca_bank_name {
-	const char *name;	/* Short name for sysfs */
-	const char *long_name;	/* Long name for pretty-printing */
-};
-
-extern struct smca_bank_name smca_names[N_SMCA_BANK_TYPES];
-
 #define HWID_MCATYPE(hwid, mcatype) ((hwid << 16) | mcatype)
 
 struct smca_hwid {

@@ -379,6 +372,7 @@ struct smca_bank {
 
 extern struct smca_bank smca_banks[MAX_NR_BANKS];
 
+extern const char *smca_get_long_name(enum smca_bank_types t);
 #endif
 
 #endif /* _ASM_X86_MCE_H */

arch/x86/kernel/cpu/mcheck/mce_amd.c +29 -3

index afeb02b87127..e68a305ff666 100644
@@ -69,7 +69,12 @@ static const char * const smca_umc_block_names[] = {
 	"misc_umc"
 };
 
-struct smca_bank_name smca_names[] = {
+struct smca_bank_name {
+	const char *name;	/* Short name for sysfs */
+	const char *long_name;	/* Long name for pretty-printing */
+};
+
+static struct smca_bank_name smca_names[] = {
 	[SMCA_LS]	= { "load_store",	"Load Store Unit" },
 	[SMCA_IF]	= { "insn_fetch",	"Instruction Fetch Unit" },
 	[SMCA_L2_CACHE]	= { "l2_cache",		"L2 Cache" },

@@ -84,7 +89,23 @@ struct smca_bank_name smca_names[] = {
 	[SMCA_PSP]	= { "psp",		"Platform Security Processor" },
 	[SMCA_SMU]	= { "smu",		"System Management Unit" },
 };
-EXPORT_SYMBOL_GPL(smca_names);
+
+const char *smca_get_name(enum smca_bank_types t)
+{
+	if (t >= N_SMCA_BANK_TYPES)
+		return NULL;
+
+	return smca_names[t].name;
+}
+
+const char *smca_get_long_name(enum smca_bank_types t)
+{
+	if (t >= N_SMCA_BANK_TYPES)
+		return NULL;
+
+	return smca_names[t].long_name;
+}
+EXPORT_SYMBOL_GPL(smca_get_long_name);
 
 static struct smca_hwid smca_hwid_mcatypes[] = {
 	/* { bank_type, hwid_mcatype, xec_bitmap } */

@@ -163,6 +184,11 @@ static void get_smca_bank_info(unsigned int bank)
 	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
 		s_hwid = &smca_hwid_mcatypes[i];
 		if (hwid_mcatype == s_hwid->hwid_mcatype) {
+
+			WARN(smca_banks[bank].hwid,
+			     "Bank %s already initialized!\n",
+			     smca_get_name(s_hwid->bank_type));
+
 			smca_banks[bank].hwid = s_hwid;
 			smca_banks[bank].id = instance_id;
 			break;

@@ -832,7 +858,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 	}
 
 	snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
-		 "%s_%x", smca_names[bank_type].name,
+		 "%s_%x", smca_get_name(bank_type),
 			  smca_banks[bank].id);
 	return buf_mcatype;
 }

drivers/edac/mce_amd.c +1 -1

index 3dee58583d25..80762acd8cc8 100644
@@ -867,7 +867,7 @@ static void decode_smca_errors(struct mce *m)
 		return;
 
 	bank_type = hwid->bank_type;
-	ip_name = smca_names[bank_type].long_name;
+	ip_name = smca_get_long_name(bank_type);
 
 	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);

Signature · 7 lines

2.10.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

tip-bot for Borislav Petkov

merge bot

2016-11-08 16:22 UTC #11 lore raw

[tip:ras/core] x86/RAS: Hide SMCA bank names

x86/RAS: Hide SMCA bank names

Commit-ID:  c09a8c40e0a0b4994925ac8eba91b85d76f440a3
Gitweb:     http://git.kernel.org/tip/c09a8c40e0a0b4994925ac8eba91b85d76f440a3
Author:     Borislav Petkov <[email protected]>
AuthorDate: Thu, 3 Nov 2016 21:12:33 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 8 Nov 2016 17:10:15 +0100

x86/RAS: Hide SMCA bank names

Add accessor functions and hide the smca_names array. Also, add a sanity-check to bank HWID assignment in get_smca_bank_info().

Signed-off-by Borislav Petkov <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/include/asm/mce.h           |  8 +-------
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 32 +++++++++++++++++++++++++++++---
 drivers/edac/mce_amd.c               |  2 +-
 3 files changed, 31 insertions(+), 11 deletions(-)

arch/x86/include/asm/mce.h +1 -7

index 8ffd215..748b8da 100644
@@ -356,13 +356,6 @@ enum smca_bank_types {
 	N_SMCA_BANK_TYPES
 };
 
-struct smca_bank_name {
-	const char *name;	/* Short name for sysfs */
-	const char *long_name;	/* Long name for pretty-printing */
-};
-
-extern struct smca_bank_name smca_names[N_SMCA_BANK_TYPES];
-
 #define HWID_MCATYPE(hwid, mcatype) ((hwid << 16) | mcatype)
 
 struct smca_hwid {

@@ -379,6 +372,7 @@ struct smca_bank {
 
 extern struct smca_bank smca_banks[MAX_NR_BANKS];
 
+extern const char *smca_get_long_name(enum smca_bank_types t);
 #endif
 
 #endif /* _ASM_X86_MCE_H */

arch/x86/kernel/cpu/mcheck/mce_amd.c +29 -3

index afeb02b8..e68a305 100644
@@ -69,7 +69,12 @@ static const char * const smca_umc_block_names[] = {
 	"misc_umc"
 };
 
-struct smca_bank_name smca_names[] = {
+struct smca_bank_name {
+	const char *name;	/* Short name for sysfs */
+	const char *long_name;	/* Long name for pretty-printing */
+};
+
+static struct smca_bank_name smca_names[] = {
 	[SMCA_LS]	= { "load_store",	"Load Store Unit" },
 	[SMCA_IF]	= { "insn_fetch",	"Instruction Fetch Unit" },
 	[SMCA_L2_CACHE]	= { "l2_cache",		"L2 Cache" },

@@ -84,7 +89,23 @@ struct smca_bank_name smca_names[] = {
 	[SMCA_PSP]	= { "psp",		"Platform Security Processor" },
 	[SMCA_SMU]	= { "smu",		"System Management Unit" },
 };
-EXPORT_SYMBOL_GPL(smca_names);
+
+const char *smca_get_name(enum smca_bank_types t)
+{
+	if (t >= N_SMCA_BANK_TYPES)
+		return NULL;
+
+	return smca_names[t].name;
+}
+
+const char *smca_get_long_name(enum smca_bank_types t)
+{
+	if (t >= N_SMCA_BANK_TYPES)
+		return NULL;
+
+	return smca_names[t].long_name;
+}
+EXPORT_SYMBOL_GPL(smca_get_long_name);
 
 static struct smca_hwid smca_hwid_mcatypes[] = {
 	/* { bank_type, hwid_mcatype, xec_bitmap } */

@@ -163,6 +184,11 @@ static void get_smca_bank_info(unsigned int bank)
 	for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
 		s_hwid = &smca_hwid_mcatypes[i];
 		if (hwid_mcatype == s_hwid->hwid_mcatype) {
+
+			WARN(smca_banks[bank].hwid,
+			     "Bank %s already initialized!\n",
+			     smca_get_name(s_hwid->bank_type));
+
 			smca_banks[bank].hwid = s_hwid;
 			smca_banks[bank].id = instance_id;
 			break;

@@ -832,7 +858,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 	}
 
 	snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
-		 "%s_%x", smca_names[bank_type].name,
+		 "%s_%x", smca_get_name(bank_type),
 			  smca_banks[bank].id);
 	return buf_mcatype;
 }

drivers/edac/mce_amd.c +1 -1

index 3dee585..80762ac 100644
@@ -867,7 +867,7 @@ static void decode_smca_errors(struct mce *m)
 		return;
 
 	bank_type = hwid->bank_type;
-	ip_name = smca_names[bank_type].long_name;
+	ip_name = smca_get_long_name(bank_type);
 
 	pr_emerg(HW_ERR "%s Extended Error Code: %d\n", ip_name, xec);

tip-bot for Borislav Petkov

merge bot

2016-11-08 16:20 UTC #12 lore raw

In reply to Borislav Petkov

x86/RAS: Simplify SMCA bank descriptor struct

Commit-ID:  79349f529ab1a629b9e43e81b4a5b2c22d1e9a65
Gitweb:     http://git.kernel.org/tip/79349f529ab1a629b9e43e81b4a5b2c22d1e9a65
Author:     Borislav Petkov <[email protected]>
AuthorDate: Tue, 1 Nov 2016 17:33:00 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 8 Nov 2016 17:10:14 +0100

x86/RAS: Simplify SMCA bank descriptor struct

Call the struct simply smca_bank, it's instance ID can be simply ->id. Makes the code much more readable.

Signed-off-by Borislav Petkov <[email protected]>

Tested-by Yazen Ghannam <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/include/asm/mce.h           |  7 ++++---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 10 +++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

arch/x86/include/asm/mce.h +4 -3

index 9bd7ff5..4d97875 100644
@@ -371,12 +371,13 @@ struct smca_hwid_mcatype {
 	u32 xec_bitmap;		/* Bitmap of valid ExtErrorCodes; current max is 21. */
 };
 
-struct smca_bank_info {
+struct smca_bank {
 	struct smca_hwid_mcatype *type;
-	u32 type_instance;
+	/* Instance ID */
+	u32 id;
 };
 
-extern struct smca_bank_info smca_banks[MAX_NR_BANKS];
+extern struct smca_bank smca_banks[MAX_NR_BANKS];
 
 #endif

arch/x86/kernel/cpu/mcheck/mce_amd.c +5 -5

index 9b54034..ac2f4f2 100644
@@ -116,7 +116,7 @@ static struct smca_hwid_mcatype smca_hwid_mcatypes[] = {
 	{ SMCA_SMU,	 HWID_MCATYPE(0x01, 0x0), 0x1 },
 };
 
-struct smca_bank_info smca_banks[MAX_NR_BANKS];
+struct smca_bank smca_banks[MAX_NR_BANKS];
 EXPORT_SYMBOL_GPL(smca_banks);
 
 /*

@@ -150,14 +150,14 @@ static void get_smca_bank_info(unsigned int bank)
 {
 	unsigned int i, hwid_mcatype, cpu = smp_processor_id();
 	struct smca_hwid_mcatype *type;
-	u32 high, instanceId;
+	u32 high, instance_id;
 	u16 hwid, mcatype;
 
 	/* Collect bank_info using CPU 0 for now. */
 	if (cpu)
 		return;
 
-	if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), &instanceId, &high)) {
+	if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), &instance_id, &high)) {
 		pr_warn("Failed to read MCA_IPID for bank %d\n", bank);
 		return;
 	}

@@ -170,7 +170,7 @@ static void get_smca_bank_info(unsigned int bank)
 		type = &smca_hwid_mcatypes[i];
 		if (hwid_mcatype == type->hwid_mcatype) {
 			smca_banks[bank].type = type;
-			smca_banks[bank].type_instance = instanceId;
+			smca_banks[bank].id = instance_id;
 			break;
 		}
 	}

@@ -839,7 +839,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
 
 	snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
 		 "%s_%x", smca_bank_names[bank_type].name,
-			  smca_banks[bank].type_instance);
+			  smca_banks[bank].id);
 	return buf_mcatype;
 }