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

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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 <bp@suse.de>
---
 arch/x86/include/asm/mce.h           |  7 ++++---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 10 +++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 9bd7ff5ffbcc..4d97875d9543 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 9b5403462936..ac2f4f2cad18 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;
 }
 
-- 
2.10.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

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

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 4d97875d9543..ccc801a0da0f 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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;
 };
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index ac2f4f2cad18..ff81667af2f2 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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))
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index daaac2c79ca7..8e96c6ddf272 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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]);
 	}
-- 
2.10.0

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

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Make it differ more from struct smca_bank_name for better readability.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index ccc801a0da0f..8ffd21596dd7 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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)
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index ff81667af2f2..afeb02b87127 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;
 }
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 8e96c6ddf272..3dee58583d25 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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);
 
-- 
2.10.0

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

[Yazen Ghannam]

> 
> 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 <bp@suse.de>

Looks good to me.

Please add:
Tested-by: Yazen Ghannam <yazen.ghannam@amd.com>

Ditto for the others.

Thanks,
Yazen

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

[Borislav Petkov]

On Fri, Nov 04, 2016 at 10:44:47AM -0400, Yazen Ghannam wrote:
> Please add:
> Tested-by: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> Ditto for the others.

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

---
From: Borislav Petkov <bp@suse.de>
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 <bp@suse.de>
---
 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(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 8ffd21596dd7..748b8da8e627 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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 */
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index afeb02b87127..e68a305ff666 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;
 }
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 3dee58583d25..80762acd8cc8 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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);
 
-- 
2.10.0

-- 
Regards/Gruss,
    Boris.

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

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

[tip-bot for Borislav Petkov]

Commit-ID:  79349f529ab1a629b9e43e81b4a5b2c22d1e9a65
Gitweb:     http://git.kernel.org/tip/79349f529ab1a629b9e43e81b4a5b2c22d1e9a65
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Tue, 1 Nov 2016 17:33:00 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <bp@suse.de>
Tested-by: Yazen Ghannam <yazen.ghannam@amd.com>
Link: http://lkml.kernel.org/r/20161103125556.15482-1-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/mce.h           |  7 ++++---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 10 +++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 9bd7ff5..4d97875 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 9b54034..ac2f4f2 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;
 }
 

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

[tip-bot for Borislav Petkov]

Commit-ID:  1ce9cd7f9f0b71af7c496b816734bc2dc699363a
Gitweb:     http://git.kernel.org/tip/1ce9cd7f9f0b71af7c496b816734bc2dc699363a
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 2 Nov 2016 12:48:01 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <bp@suse.de>
Tested-by: Yazen Ghannam <yazen.ghannam@amd.com>
Link: http://lkml.kernel.org/r/20161103125556.15482-2-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 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(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 4d97875..ccc801a 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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;
 };
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index ac2f4f2..ff81667 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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))
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index daaac2c..8e96c6dd 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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]);
 	}

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

[tip-bot for Borislav Petkov]

Commit-ID:  a9a1c0ee04aa771e5523ae33e458c702261ab547
Gitweb:     http://git.kernel.org/tip/a9a1c0ee04aa771e5523ae33e458c702261ab547
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Wed, 2 Nov 2016 13:24:47 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <bp@suse.de>
Tested-by: Yazen Ghannam <yazen.ghannam@amd.com>
Link: http://lkml.kernel.org/r/20161103125556.15482-3-bp@alien8.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 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(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index ccc801a..8ffd215 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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)
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index ff81667..afeb02b8 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;
 }
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 8e96c6dd..3dee585 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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);
 

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

[tip-bot for Borislav Petkov]

Commit-ID:  c09a8c40e0a0b4994925ac8eba91b85d76f440a3
Gitweb:     http://git.kernel.org/tip/c09a8c40e0a0b4994925ac8eba91b85d76f440a3
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 3 Nov 2016 21:12:33 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <bp@suse.de>
Link: http://lkml.kernel.org/r/20161104152317.5r276t35df53qk76@pd.tnic
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 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(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 8ffd215..748b8da 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -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 */
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index afeb02b8..e68a305 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;
 }
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 3dee585..80762ac 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -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);
 

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

[Yazen Ghannam]

>  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);
>

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.

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

Thanks,
Yazen

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

[Borislav Petkov]

On Thu, Nov 10, 2016 at 12:50:04PM -0500, Yazen Ghannam wrote:
> Adding extra parentheses in HWID_MCATYPE() gives the same assembly as the
> original code and fixes the behavior.
> 
> > +	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.

-- 
Regards/Gruss,
    Boris.

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

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

[Thomas Gleixner]

On Thu, 10 Nov 2016, Borislav Petkov wrote:
> On Thu, Nov 10, 2016 at 12:50:04PM -0500, Yazen Ghannam wrote:
> > Adding extra parentheses in HWID_MCATYPE() gives the same assembly as the
> > original code and fixes the behavior.
> > 
> > > +	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.

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

[Yazen Ghannam]

> > 
> > 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.
 
> Patch please.

Will do.

Thanks,
Yazen