Patch 1, 2: Cleanups to the code. No functional change
Patch 3: Modifying flags attribute to accept string arguments and
have do_inject() recognize these changes before injecting
a 'sw' or 'hw' error
Patch 4: Extend flags attribute to accept values of 'dfr', 'thr'.
These values will be used to trigger deferred error/threshold
error apic interrupts respectively
Patch 5: Include permissions for each file within struct dfs_node dfs_fls[]
Patch 6: Add README file that describes the attributes
Patch 7: Factor out number of nodes calculation from amd_get_topology().
No functional change.
Patch 8: Provide accessor function to obtain the number of nodes per processor
Patch 9: Modify injection mechanism for bank 4 errors. Since they are
typically logged or reported only on NBC, we make sure that
we inject on the correct core here.
Changes in V2: (per Boris)
- Remove arbitrary definition of MAX_FLAG_OPT_SIZE and define it
to sizeof the flag option we are introducing in the patch (Patch 3)
- Increase it later as needed (Patch 4)
- Simplify string comparison loop in __set_inj()
- Move return value assignment in flags_write() to after initial condition
- Remove hardcoded numbers used to inject deferred/threshold interrupts
and use respective macros instead
- Use switch-case to decide between injecting apic interrupts and #MC
- Add permissions to dfs_fls[] array in a separate patch (Patch 5)
and README file in the following patch (Patch 6)
- Remove CONFIG_X86_HT ifdefs and factor out number of nodes calculation
- Provide accessor function in amd.c
Aravind Gopalakrishnan (9):
edac, mce_amd_inj: Use MCE_INJECT_GET for bank
edac, mce_amd_inj: Rework sanity check for inj_bank_set
edac, mce_amd_inj: Modify flags attrigute to use string arguments
edac, mce_amd_inj: Add capability to trigger apic interrupts
edac, mce_amd_inj: Add individual permissions field for dfs_node
edac, mce_amd_inj: Add README file
x86, amd: Factor out number of nodes calculation
x86, amd: Provide accessor for number of nodes
edac, mce_amd_inj: Inject errors on NBC for bank 4 errors
arch/x86/include/asm/processor.h | 1 +
arch/x86/kernel/cpu/amd.c | 42 ++++--
drivers/edac/mce_amd_inj.c | 270 ++++++++++++++++++++++++++++++++++-----
3 files changed, 275 insertions(+), 38 deletions(-)
--
2.4.0
inj_bank_get is generic enough that we can use the
macro in it's place. Doing that here.
No functional change.
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index f7681b5..7f3a97a 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -187,13 +187,7 @@ static int inj_bank_set(void *data, u64 val)
return 0;
}
-static int inj_bank_get(void *data, u64 *val)
-{
- struct mce *m = (struct mce *)data;
-
- *val = m->bank;
- return 0;
-}
+MCE_INJECT_GET(bank);
DEFINE_SIMPLE_ATTRIBUTE(bank_fops, inj_bank_get, inj_bank_set, "%llu\n");
--
2.4.0
The number of banks for a given processor is encoded in
MSR_IA32_MCG_CAP. So, use this to obtain the value and
for sanity checking in inj_bank_set() instead of requiring a
family/model check.
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index 7f3a97a..15f6aa1 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -25,6 +25,8 @@
static struct mce i_mce;
static struct dentry *dfs_inj;
+static u8 n_banks;
+
#define MCE_INJECT_SET(reg) \
static int inj_##reg##_set(void *data, u64 val) \
{ \
@@ -174,11 +176,9 @@ static int inj_bank_set(void *data, u64 val)
{
struct mce *m = (struct mce *)data;
- if (val > 5) {
- if (boot_cpu_data.x86 != 0x15 || val > 6) {
- pr_err("Non-existent MCE bank: %llu\n", val);
- return -EINVAL;
- }
+ if (val >= n_banks) {
+ pr_err("Non-existent MCE bank: %llu\n", val);
+ return -EINVAL;
}
m->bank = val;
@@ -207,6 +207,10 @@ static struct dfs_node {
static int __init init_mce_inject(void)
{
int i;
+ u64 cap;
+
+ rdmsrl(MSR_IA32_MCG_CAP, cap);
+ n_banks = cap & MCG_BANKCNT_MASK;
dfs_inj = debugfs_create_dir("mce-inject", NULL);
if (!dfs_inj)
--
2.4.0
Use char values such as "hw" or "sw" to indicate the type of error
injection to be performed.
Current flags attribute derives the meanings of values that can be
programmed into it from asm/mce.h. Moving to defined strings for the
atribute allows this module to be self sufficient and removes the
dependency. Also, we can introduce new flags as and when needed without
having to worry about conflicting with the flags already defined
in asm/mce.h
Also, modify do_inject() to use the newly defined injection_type enum
to figure out the injection mechanism we need to use
Suggested-by: Borislav Petkov <[email protected]>
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 82 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 72 insertions(+), 10 deletions(-)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index 15f6aa1..c129a8d 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -15,6 +15,8 @@
#include <linux/device.h>
#include <linux/module.h>
#include <linux/cpu.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
#include <asm/mce.h>
#include "mce_amd.h"
@@ -27,6 +29,23 @@ static struct dentry *dfs_inj;
static u8 n_banks;
+#define MAX_FLAG_OPT_SIZE 3
+
+enum injection_type {
+ SW_INJ = 0, /* SW injection, simply decode the error */
+ HW_INJ, /* Trigger a #MC */
+ N_INJ_TYPES,
+};
+
+static const char * const flags_options[] = {
+ [SW_INJ] = "sw",
+ [HW_INJ] = "hw",
+ NULL
+};
+
+/* Set default injection to SW_INJ */
+enum injection_type inj_type = SW_INJ;
+
#define MCE_INJECT_SET(reg) \
static int inj_##reg##_set(void *data, u64 val) \
{ \
@@ -81,24 +100,67 @@ static int toggle_hw_mce_inject(unsigned int cpu, bool enable)
return err;
}
-static int flags_get(void *data, u64 *val)
+static int __set_inj(const char *buf)
{
- struct mce *m = (struct mce *)data;
+ int i;
- *val = m->inject_flags;
+ for (i = 0; i < N_INJ_TYPES; i++) {
+ if (!strncmp(flags_options[i], buf,
+ strlen(flags_options[i]))) {
+ inj_type = i;
+ return 0;
+ }
+ }
+ return -EINVAL;
+}
- return 0;
+static ssize_t flags_read(struct file *filp, char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ char buf[MAX_FLAG_OPT_SIZE];
+ int n;
+
+ n = sprintf(buf, "%s\n", flags_options[inj_type]);
+
+ return simple_read_from_buffer(ubuf, cnt, ppos, buf, n);
}
-static int flags_set(void *data, u64 val)
+static ssize_t flags_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
{
- struct mce *m = (struct mce *)data;
+ char buf[MAX_FLAG_OPT_SIZE];
+ int err;
+ size_t ret;
- m->inject_flags = (u8)val;
- return 0;
+ if (cnt > MAX_FLAG_OPT_SIZE)
+ cnt = MAX_FLAG_OPT_SIZE;
+
+ ret = cnt;
+
+ if (copy_from_user(&buf, ubuf, cnt))
+ return -EFAULT;
+
+ buf[cnt - 1] = 0;
+
+ /* strip whitespaces.. */
+ strstrip(buf);
+
+ err = __set_inj(buf);
+ if (err) {
+ pr_err("%s: Invalid flags value: %s\n", __func__, buf);
+ return err;
+ }
+
+ *ppos += ret;
+
+ return ret;
}
-DEFINE_SIMPLE_ATTRIBUTE(flags_fops, flags_get, flags_set, "%llu\n");
+static const struct file_operations flags_fops = {
+ .read = flags_read,
+ .write = flags_write,
+ .llseek = generic_file_llseek,
+};
/*
* On which CPU to inject?
@@ -130,7 +192,7 @@ static void do_inject(void)
unsigned int cpu = i_mce.extcpu;
u8 b = i_mce.bank;
- if (!(i_mce.inject_flags & MCJ_EXCEPTION)) {
+ if (inj_type == SW_INJ) {
amd_decode_mce(NULL, 0, &i_mce);
return;
}
--
2.4.0
With this extension to the flags attribute, deferred error interrupts
and threshold interrupts can be triggered to test the apic interrupt
handler functionality for these type of errors
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 41 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index c129a8d..377fd6c 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -29,17 +29,21 @@ static struct dentry *dfs_inj;
static u8 n_banks;
-#define MAX_FLAG_OPT_SIZE 3
+#define MAX_FLAG_OPT_SIZE 4
enum injection_type {
SW_INJ = 0, /* SW injection, simply decode the error */
HW_INJ, /* Trigger a #MC */
+ DFR_INT_INJ, /* Trigger Deferred error interrupt */
+ THR_INT_INJ, /* Trigger threshold interrupt */
N_INJ_TYPES,
};
static const char * const flags_options[] = {
[SW_INJ] = "sw",
[HW_INJ] = "hw",
+ [DFR_INT_INJ] = "dfr",
+ [THR_INT_INJ] = "thr",
NULL
};
@@ -186,6 +190,16 @@ static void trigger_mce(void *info)
asm volatile("int $18");
}
+static void trigger_dfr_int(void *info)
+{
+ asm volatile("int %0" :: "i" (DEFERRED_ERROR_VECTOR));
+}
+
+static void trigger_thr_int(void *info)
+{
+ asm volatile("int %0" :: "i" (THRESHOLD_APIC_VECTOR));
+}
+
static void do_inject(void)
{
u64 mcg_status = 0;
@@ -197,6 +211,20 @@ static void do_inject(void)
return;
}
+ if (inj_type == DFR_INT_INJ) {
+ /*
+ * Ensure necessary status bits for deferred errors:
+ * a. MCx_STATUS[Deferred] is set -
+ * This is to ensure the error will be handled by the
+ * interrupt handler
+ * b. unset MCx_STATUS[UC]
+ * As deferred errors are _not_ UC
+ */
+
+ i_mce.status |= MCI_STATUS_DEFERRED;
+ i_mce.status |= (i_mce.status & ~MCI_STATUS_UC);
+ }
+
get_online_cpus();
if (!cpu_online(cpu))
goto err;
@@ -223,7 +251,16 @@ static void do_inject(void)
toggle_hw_mce_inject(cpu, false);
- smp_call_function_single(cpu, trigger_mce, NULL, 0);
+ switch (inj_type) {
+ case DFR_INT_INJ:
+ smp_call_function_single(cpu, trigger_dfr_int, NULL, 0);
+ break;
+ case THR_INT_INJ:
+ smp_call_function_single(cpu, trigger_thr_int, NULL, 0);
+ break;
+ default:
+ smp_call_function_single(cpu, trigger_mce, NULL, 0);
+ }
err:
put_online_cpus();
--
2.4.0
Using dfs_fls[] array to encapsulate the permissions necessary for
each file. While creating the file, we iterate over each of them
and use the perm field.
In a later patch, we will add a README file that needs different
permissions. Hence the move here to add a perm field.
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index 377fd6c..ad18913 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -294,13 +294,20 @@ static struct dfs_node {
char *name;
struct dentry *d;
const struct file_operations *fops;
+ umode_t perm;
} dfs_fls[] = {
- { .name = "status", .fops = &status_fops },
- { .name = "misc", .fops = &misc_fops },
- { .name = "addr", .fops = &addr_fops },
- { .name = "bank", .fops = &bank_fops },
- { .name = "flags", .fops = &flags_fops },
- { .name = "cpu", .fops = &extcpu_fops },
+ { .name = "status", .fops = &status_fops,
+ .perm = S_IRUSR | S_IWUSR },
+ { .name = "misc", .fops = &misc_fops,
+ .perm = S_IRUSR | S_IWUSR },
+ { .name = "addr", .fops = &addr_fops,
+ .perm = S_IRUSR | S_IWUSR },
+ { .name = "bank", .fops = &bank_fops,
+ .perm = S_IRUSR | S_IWUSR },
+ { .name = "flags", .fops = &flags_fops,
+ .perm = S_IRUSR | S_IWUSR },
+ { .name = "cpu", .fops = &extcpu_fops,
+ .perm = S_IRUSR | S_IWUSR },
};
static int __init init_mce_inject(void)
@@ -317,7 +324,7 @@ static int __init init_mce_inject(void)
for (i = 0; i < ARRAY_SIZE(dfs_fls); i++) {
dfs_fls[i].d = debugfs_create_file(dfs_fls[i].name,
- S_IRUSR | S_IWUSR,
+ dfs_fls[i].perm,
dfs_inj,
&i_mce,
dfs_fls[i].fops);
--
2.4.0
Provides information about each file and the usages.
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index ad18913..b7e108c 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -290,6 +290,57 @@ MCE_INJECT_GET(bank);
DEFINE_SIMPLE_ATTRIBUTE(bank_fops, inj_bank_get, inj_bank_set, "%llu\n");
+static const char readme_msg[] =
+ "\nDescription of the files and their usages:\n\n"
+ "status: Set a value to be programmed into MCx_STATUS(bank)\n"
+ "\t The status bits provide insight into the type of\n"
+ "\t error that caused the MCE.\n\n"
+ "misc: Set value of MCx_MISC(bank)\n"
+ "\t misc register provides auxiliary info about the error. This\n"
+ "\t register is typically used for error thresholding purpose and\n"
+ "\t validity of the register is indicated by MCx_STATUS[MiscV]\n\n"
+ "addr: Error address value to be written to MCx_ADDR(bank)\n"
+ "\t This register is used to log address information associated\n"
+ "\t with the error.\n\n"
+ "Note: See respective BKDGs for the exact bit definitions of the\n"
+ "\t above registers as they mirror the MCi_[STATUS | MISC | ADDR]\n"
+ "\t hardware registers.\n\n"
+ "bank: Specify the bank you want to inject the error into.\n"
+ "\t The number of banks in a processor varies and is family/model\n"
+ "\t dependent. So, a sanity check performed while writing.\n"
+ "\t Writing to this file will trigger a #MC or APIC interrupts or\n"
+ "\t invoke the error decoder routines for AMD processors. The value\n"
+ "\t in 'flags' file decides which of above actions is triggered.\n\n"
+ "flags: Write to this file to speficy the error injection policy.\n"
+ "\t Allowed values:\n"
+ "\t\t\"sw\" - SW error injection, Only calls error decoder\n"
+ "\t\t\troutines to print error info in human readable format\n"
+ "\t\t\"hw\" - HW error injection, Forces a #MC,\n"
+ "\t\t\tcauses exception handler to handle the error\n"
+ "\t\t\tif UC or poll handler catches it if CE\n"
+ "\t\t\tWarning: Might cause system panic if MCx_STATUS[PCC]\n"
+ "\t\t\tis set. For debug purposes, consider setting\n"
+ "\t\t\t/<debugfs_mountpoint>/mce/fake_panic\n"
+ "\t\t\"dfr\" - Trigger APIC interrupt for Deferred error\n"
+ "\t\t\tError is handled by deferred error apic handler if\n"
+ "\t\t\tfeature is present in HW.\n"
+ "\t\t\"thr\" - Trigger APIC interrupt for threshold error\n"
+ "\t\t\tError is handled by threshold apic handler\n\n"
+ "cpu: The cpu to inject the error on.\n\n"
+;
+
+static ssize_t
+inj_readme_read(struct file *filp, char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ return simple_read_from_buffer(ubuf, cnt, ppos,
+ readme_msg, strlen(readme_msg));
+}
+
+static const struct file_operations readme_fops = {
+ .read = inj_readme_read,
+};
+
static struct dfs_node {
char *name;
struct dentry *d;
@@ -308,6 +359,8 @@ static struct dfs_node {
.perm = S_IRUSR | S_IWUSR },
{ .name = "cpu", .fops = &extcpu_fops,
.perm = S_IRUSR | S_IWUSR },
+ { .name = "README", .fops = &readme_fops,
+ .perm = S_IRUSR | S_IRGRP | S_IROTH },
};
static int __init init_mce_inject(void)
--
2.4.0
Factoring out number of nodes calculation out of amd_get_topology()
and saving the value in a static variable.
A later patch will introduce a accessor for this value so we
can use the information elsewhere in EDAC. The usage will be
included in a future patch too.
While at it, remove X86_HT #ifdefs around the code block for
amd_get_topology() and it's caller amd_detect_cmp(). Since
CONFIG_X86_HT defaults to Y, this code is always built-in.
Besides, amd_get_topology() extracts necessary info from
cpuid_[eax|ebx](0x8000001e) for platforms that are not 'HT'
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Jacob Shin <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
arch/x86/kernel/cpu/amd.c | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 94e7051..c595669 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -19,6 +19,13 @@
#include "cpu.h"
+/*
+ * nodes_per_processor: Specifies number of nodes per socket
+ * Refer Fam15h Models 00-0fh BKDG-
+ * CPUID Fn8000_001E_ECX Node Identifiers[10:8]
+ */
+static u32 nodes_per_processor = 1;
+
static inline int rdmsrl_amd_safe(unsigned msr, unsigned long long *p)
{
u32 gprs[8] = { 0 };
@@ -282,25 +289,40 @@ static int nearby_node(int apicid)
}
#endif
+static void amd_set_num_nodes(void)
+{
+ if (cpu_has_topoext) {
+ u32 ecx;
+
+ ecx = cpuid_ecx(0x8000001e);
+ nodes_per_processor = ((ecx >> 8) & 7) + 1;
+ } else if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
+ u64 value;
+
+ rdmsrl(MSR_FAM10H_NODE_ID, value);
+ nodes_per_processor = ((value >> 3) & 7) + 1;
+ }
+}
+
/*
* Fixup core topology information for
* (1) AMD multi-node processors
* Assumption: Number of cores in each internal node is the same.
* (2) AMD processors supporting compute units
*/
-#ifdef CONFIG_X86_HT
static void amd_get_topology(struct cpuinfo_x86 *c)
{
- u32 nodes, cores_per_cu = 1;
+ u32 cores_per_cu = 1;
u8 node_id;
int cpu = smp_processor_id();
+ amd_set_num_nodes();
+
/* get information required for multi-node processors */
if (cpu_has_topoext) {
u32 eax, ebx, ecx, edx;
cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
- nodes = ((ecx >> 8) & 7) + 1;
node_id = ecx & 7;
/* get compute unit information */
@@ -311,18 +333,17 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
u64 value;
rdmsrl(MSR_FAM10H_NODE_ID, value);
- nodes = ((value >> 3) & 7) + 1;
node_id = value & 7;
} else
return;
/* fixup multi-node processor information */
- if (nodes > 1) {
+ if (nodes_per_processor > 1) {
u32 cores_per_node;
u32 cus_per_node;
set_cpu_cap(c, X86_FEATURE_AMD_DCM);
- cores_per_node = c->x86_max_cores / nodes;
+ cores_per_node = c->x86_max_cores / nodes_per_processor;
cus_per_node = cores_per_node / cores_per_cu;
/* store NodeID, use llc_shared_map to store sibling info */
@@ -333,7 +354,6 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
c->compute_unit_id %= cus_per_node;
}
}
-#endif
/*
* On a AMD dual core setup the lower bits of the APIC id distinguish the cores.
@@ -341,7 +361,6 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
*/
static void amd_detect_cmp(struct cpuinfo_x86 *c)
{
-#ifdef CONFIG_X86_HT
unsigned bits;
int cpu = smp_processor_id();
@@ -353,7 +372,6 @@ static void amd_detect_cmp(struct cpuinfo_x86 *c)
/* use socket ID also for last level cache */
per_cpu(cpu_llc_id, cpu) = c->phys_proc_id;
amd_get_topology(c);
-#endif
}
u16 amd_get_nb_id(int cpu)
--
2.4.0
Add an accessor function amd_get_nodes_cnt() which returns
the number of nodes per socket.
In a subsequent patch, we will use this info in EDAC
mce_amd_inj module.
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Jacob Shin <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Hector Marco-Gisbert <[email protected]>
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
arch/x86/include/asm/processor.h | 1 +
arch/x86/kernel/cpu/amd.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 8e04f51..34faf24 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -820,6 +820,7 @@ static inline int mpx_disable_management(struct task_struct *tsk)
#endif /* CONFIG_X86_INTEL_MPX */
extern u16 amd_get_nb_id(int cpu);
+extern u32 amd_get_nodes_cnt(void);
static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
{
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index c595669..e91b9bd 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -384,6 +384,12 @@ u16 amd_get_nb_id(int cpu)
}
EXPORT_SYMBOL_GPL(amd_get_nb_id);
+u32 amd_get_nodes_cnt(void)
+{
+ return nodes_per_processor;
+}
+EXPORT_SYMBOL_GPL(amd_get_nodes_cnt);
+
static void srat_detect_node(struct cpuinfo_x86 *c)
{
#ifdef CONFIG_NUMA
--
2.4.0
For bank 4 errors, MCE is logged and reported only on
node base cores. Refer D18F3x44[NbMcaToMstCpuEn] field in
Fam10h and later BKDGs.
This patch ensures that we inject the error on the node base core
for bank 4 errors. Otherwise, triggering #MC or apic interrupts on
a non node base core would not have any effect on the system.
(i.e), we would not see any relevant output on kernel logs for
the error we just injected.
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
drivers/edac/mce_amd_inj.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index b7e108c..45aac4f 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -17,9 +17,12 @@
#include <linux/cpu.h>
#include <linux/string.h>
#include <linux/uaccess.h>
+#include <linux/pci.h>
#include <asm/mce.h>
+#include <asm/amd_nb.h>
#include "mce_amd.h"
+#include "amd64_edac.h"
/*
* Collect all the MCi_XXX settings
@@ -200,6 +203,44 @@ static void trigger_thr_int(void *info)
asm volatile("int %0" :: "i" (THRESHOLD_APIC_VECTOR));
}
+static u32 amd_get_nbc_for_node(int node_id)
+{
+ struct cpuinfo_x86 *c = &boot_cpu_data;
+ u32 cores_per_node;
+
+ cores_per_node = c->x86_max_cores / amd_get_nodes_cnt();
+
+ return cores_per_node * node_id;
+}
+
+static void toggle_nb_mca_mst_cpu(u16 nid)
+{
+ struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
+ u32 val;
+ int err;
+
+ if (!F3)
+ return;
+
+ err = pci_read_config_dword(F3, NBCFG, &val);
+ if (err) {
+ pr_err("%s: Error reading F%dx%03x.\n", __func__,
+ PCI_FUNC(F3->devfn),
+ NBCFG);
+ return;
+ }
+
+ if (!(val & BIT(27))) {
+ pr_err("%s: BIOS not setting D18F3x44[NbMcaToMstCpuEn]. Doing that here\n", __func__);
+ val |= BIT(27);
+ err = pci_write_config_dword(F3, NBCFG, val);
+ if (err)
+ pr_err("%s: Error writing F%dx%03x.\n", __func__,
+ PCI_FUNC(F3->devfn),
+ NBCFG);
+ }
+}
+
static void do_inject(void)
{
u64 mcg_status = 0;
@@ -235,6 +276,20 @@ static void do_inject(void)
if (!(i_mce.status & MCI_STATUS_PCC))
mcg_status |= MCG_STATUS_RIPV;
+ /*
+ * For multi node cpus, logging and reporting of bank == 4 errors
+ * happen only on the node base core. Refer D18F3x44[NbMcaToMstCpuEn]
+ * for Fam10h and later BKDGs
+ */
+ if (static_cpu_has(X86_FEATURE_AMD_DCM) && b == 4) {
+ /*
+ * BIOS sets D18F3x44[NbMcaToMstCpuEn] by default.
+ * But make sure of it here just in case..
+ */
+ toggle_nb_mca_mst_cpu(amd_get_nb_id(cpu));
+ cpu = amd_get_nbc_for_node(amd_get_nb_id(cpu));
+ }
+
toggle_hw_mce_inject(cpu, true);
wrmsr_on_cpu(cpu, MSR_IA32_MCG_STATUS,
--
2.4.0
On Tue, Jun 02, 2015 at 03:36:00PM -0500, Aravind Gopalakrishnan wrote:
> Factoring out number of nodes calculation out of amd_get_topology()
> and saving the value in a static variable.
>
> A later patch will introduce a accessor for this value so we
> can use the information elsewhere in EDAC. The usage will be
> included in a future patch too.
>
> While at it, remove X86_HT #ifdefs around the code block for
> amd_get_topology() and it's caller amd_detect_cmp(). Since
> CONFIG_X86_HT defaults to Y, this code is always built-in.
> Besides, amd_get_topology() extracts necessary info from
> cpuid_[eax|ebx](0x8000001e) for platforms that are not 'HT'
...
> +static void amd_set_num_nodes(void)
> +{
> + if (cpu_has_topoext) {
> + u32 ecx;
> +
> + ecx = cpuid_ecx(0x8000001e);
> + nodes_per_processor = ((ecx >> 8) & 7) + 1;
> + } else if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
> + u64 value;
> +
> + rdmsrl(MSR_FAM10H_NODE_ID, value);
> + nodes_per_processor = ((value >> 3) & 7) + 1;
> + }
> +}
> +
> /*
> * Fixup core topology information for
> * (1) AMD multi-node processors
> * Assumption: Number of cores in each internal node is the same.
> * (2) AMD processors supporting compute units
> */
> -#ifdef CONFIG_X86_HT
> static void amd_get_topology(struct cpuinfo_x86 *c)
> {
> - u32 nodes, cores_per_cu = 1;
> + u32 cores_per_cu = 1;
> u8 node_id;
> int cpu = smp_processor_id();
>
> + amd_set_num_nodes();
When I said, you like to complicate stuff, I wasn't joking. :-)
See below for what I actually meant. Diff is ontop of the X86_HT removal
patch from today:
---
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 5bd3a99dc20b..bdca795743e8 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -19,6 +19,8 @@
#include "cpu.h"
+static u32 nodes_per_processor = 1;
+
static inline int rdmsrl_amd_safe(unsigned msr, unsigned long long *p)
{
u32 gprs[8] = { 0 };
@@ -291,7 +293,7 @@ static int nearby_node(int apicid)
#ifdef CONFIG_SMP
static void amd_get_topology(struct cpuinfo_x86 *c)
{
- u32 nodes, cores_per_cu = 1;
+ u32 cores_per_cu = 1;
u8 node_id;
int cpu = smp_processor_id();
@@ -300,7 +302,7 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
u32 eax, ebx, ecx, edx;
cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
- nodes = ((ecx >> 8) & 7) + 1;
+ nodes_per_processor = ((ecx >> 8) & 7) + 1;
node_id = ecx & 7;
/* get compute unit information */
@@ -311,18 +313,18 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
u64 value;
rdmsrl(MSR_FAM10H_NODE_ID, value);
- nodes = ((value >> 3) & 7) + 1;
+ nodes_per_processor = ((value >> 3) & 7) + 1;
node_id = value & 7;
} else
return;
/* fixup multi-node processor information */
- if (nodes > 1) {
+ if (nodes_per_processor > 1) {
u32 cores_per_node;
u32 cus_per_node;
set_cpu_cap(c, X86_FEATURE_AMD_DCM);
- cores_per_node = c->x86_max_cores / nodes;
+ cores_per_node = c->x86_max_cores / nodes_per_processor;
cus_per_node = cores_per_node / cores_per_cu;
/* store NodeID, use llc_shared_map to store sibling info */
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On Tue, Jun 02, 2015 at 03:35:56PM -0500, Aravind Gopalakrishnan wrote:
> Use char values such as "hw" or "sw" to indicate the type of error
> injection to be performed.
>
> Current flags attribute derives the meanings of values that can be
> programmed into it from asm/mce.h. Moving to defined strings for the
> atribute allows this module to be self sufficient and removes the
> dependency. Also, we can introduce new flags as and when needed without
> having to worry about conflicting with the flags already defined
> in asm/mce.h
>
> Also, modify do_inject() to use the newly defined injection_type enum
> to figure out the injection mechanism we need to use
>
> Suggested-by: Borislav Petkov <[email protected]>
> Signed-off-by: Aravind Gopalakrishnan <[email protected]>
> ---
> drivers/edac/mce_amd_inj.c | 82 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 72 insertions(+), 10 deletions(-)
...
> -static int flags_set(void *data, u64 val)
> +static ssize_t flags_write(struct file *filp, const char __user *ubuf,
> + size_t cnt, loff_t *ppos)
> {
> - struct mce *m = (struct mce *)data;
> + char buf[MAX_FLAG_OPT_SIZE];
> + int err;
> + size_t ret;
>
> - m->inject_flags = (u8)val;
> - return 0;
> + if (cnt > MAX_FLAG_OPT_SIZE)
> + cnt = MAX_FLAG_OPT_SIZE;
> +
> + ret = cnt;
> +
> + if (copy_from_user(&buf, ubuf, cnt))
> + return -EFAULT;
> +
> + buf[cnt - 1] = 0;
> +
> + /* strip whitespaces.. */
> + strstrip(buf);
Didn't your compiler trigger that:
drivers/edac/mce_amd_inj.c: In function ‘flags_write’:
drivers/edac/mce_amd_inj.c:146:2: warning: ignoring return value of ‘strstrip’, declared with attribute warn_unused_result [-Wunused-result]
strstrip(buf);
^
?
Because it is a valid warning. You need to take the return value. I
fixed it up like this:
---
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index c129a8da34b2..5c847fe6e9bd 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -128,7 +128,7 @@ static ssize_t flags_read(struct file *filp, char __user *ubuf,
static ssize_t flags_write(struct file *filp, const char __user *ubuf,
size_t cnt, loff_t *ppos)
{
- char buf[MAX_FLAG_OPT_SIZE];
+ char buf[MAX_FLAG_OPT_SIZE], *__buf;
int err;
size_t ret;
@@ -142,12 +142,12 @@ static ssize_t flags_write(struct file *filp, const char __user *ubuf,
buf[cnt - 1] = 0;
- /* strip whitespaces.. */
- strstrip(buf);
+ /* strip whitespace */
+ __buf = strstrip(buf);
- err = __set_inj(buf);
+ err = __set_inj(__buf);
if (err) {
- pr_err("%s: Invalid flags value: %s\n", __func__, buf);
+ pr_err("%s: Invalid flags value: %s\n", __func__, __buf);
return err;
}
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On Tue, Jun 02, 2015 at 03:35:53PM -0500, Aravind Gopalakrishnan wrote:
> Patch 1, 2: Cleanups to the code. No functional change
> Patch 3: Modifying flags attribute to accept string arguments and
> have do_inject() recognize these changes before injecting
> a 'sw' or 'hw' error
> Patch 4: Extend flags attribute to accept values of 'dfr', 'thr'.
> These values will be used to trigger deferred error/threshold
> error apic interrupts respectively
> Patch 5: Include permissions for each file within struct dfs_node dfs_fls[]
> Patch 6: Add README file that describes the attributes
Applied up to here. The resting is pending for now...
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On 6/3/2015 9:50 AM, Borislav Petkov wrote:
> On Tue, Jun 02, 2015 at 03:35:56PM -0500, Aravind Gopalakrishnan wrote:
>
>
>> +
>> + buf[cnt - 1] = 0;
>> +
>> + /* strip whitespaces.. */
>> + strstrip(buf);
> Didn't your compiler trigger that:
>
> drivers/edac/mce_amd_inj.c: In function ‘flags_write’:
> drivers/edac/mce_amd_inj.c:146:2: warning: ignoring return value of ‘strstrip’, declared with attribute warn_unused_result [-Wunused-result]
> strstrip(buf);
> ^
>
> ?
Oddly, No. The only thing I got was:
arch/x86/kernel/cpu/microcode/intel_early.c: In function
âget_matching_model_microcode.isra.2.constprop.7â:
arch/x86/kernel/cpu/microcode/intel_early.c:348:1: warning: the frame
size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=]
}
^
>
> Because it is a valid warning. You need to take the return value. I
> fixed it up like this:
Thanks for fixing it.
-Aravind.
> ---
> diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
> index c129a8da34b2..5c847fe6e9bd 100644
> --- a/drivers/edac/mce_amd_inj.c
> +++ b/drivers/edac/mce_amd_inj.c
> @@ -128,7 +128,7 @@ static ssize_t flags_read(struct file *filp, char __user *ubuf,
> static ssize_t flags_write(struct file *filp, const char __user *ubuf,
> size_t cnt, loff_t *ppos)
> {
> - char buf[MAX_FLAG_OPT_SIZE];
> + char buf[MAX_FLAG_OPT_SIZE], *__buf;
> int err;
> size_t ret;
>
> @@ -142,12 +142,12 @@ static ssize_t flags_write(struct file *filp, const char __user *ubuf,
>
> buf[cnt - 1] = 0;
>
> - /* strip whitespaces.. */
> - strstrip(buf);
> + /* strip whitespace */
> + __buf = strstrip(buf);
>
> - err = __set_inj(buf);
> + err = __set_inj(__buf);
> if (err) {
> - pr_err("%s: Invalid flags value: %s\n", __func__, buf);
> + pr_err("%s: Invalid flags value: %s\n", __func__, __buf);
> return err;
> }
>
>
On 6/3/2015 3:58 AM, Borislav Petkov wrote:
> On Tue, Jun 02, 2015 at 03:36:00PM -0500, Aravind Gopalakrishnan wrote:
>
>
>> +static void amd_set_num_nodes(void)
>> +{
>> + if (cpu_has_topoext) {
>> + u32 ecx;
>> +
>> + ecx = cpuid_ecx(0x8000001e);
>> + nodes_per_processor = ((ecx >> 8) & 7) + 1;
>> + } else if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
>> + u64 value;
>> +
>> + rdmsrl(MSR_FAM10H_NODE_ID, value);
>> + nodes_per_processor = ((value >> 3) & 7) + 1;
>> + }
>> +}
>> +
>> /*
>> * Fixup core topology information for
>> * (1) AMD multi-node processors
>> * Assumption: Number of cores in each internal node is the same.
>> * (2) AMD processors supporting compute units
>> */
>> -#ifdef CONFIG_X86_HT
>> static void amd_get_topology(struct cpuinfo_x86 *c)
>> {
>> - u32 nodes, cores_per_cu = 1;
>> + u32 cores_per_cu = 1;
>> u8 node_id;
>> int cpu = smp_processor_id();
>>
>> + amd_set_num_nodes();
> When I said, you like to complicate stuff, I wasn't joking. :-)
>
> See below for what I actually meant. Diff is ontop of the X86_HT removal
> patch from today:
*facepalm*
Sorry about that.
Thanks for fixing it.
-Aravind.
> ---
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 5bd3a99dc20b..bdca795743e8 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -19,6 +19,8 @@
>
> #include "cpu.h"
>
> +static u32 nodes_per_processor = 1;
> +
> static inline int rdmsrl_amd_safe(unsigned msr, unsigned long long *p)
> {
> u32 gprs[8] = { 0 };
> @@ -291,7 +293,7 @@ static int nearby_node(int apicid)
> #ifdef CONFIG_SMP
> static void amd_get_topology(struct cpuinfo_x86 *c)
> {
> - u32 nodes, cores_per_cu = 1;
> + u32 cores_per_cu = 1;
> u8 node_id;
> int cpu = smp_processor_id();
>
> @@ -300,7 +302,7 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
> u32 eax, ebx, ecx, edx;
>
> cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
> - nodes = ((ecx >> 8) & 7) + 1;
> + nodes_per_processor = ((ecx >> 8) & 7) + 1;
> node_id = ecx & 7;
>
> /* get compute unit information */
> @@ -311,18 +313,18 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
> u64 value;
>
> rdmsrl(MSR_FAM10H_NODE_ID, value);
> - nodes = ((value >> 3) & 7) + 1;
> + nodes_per_processor = ((value >> 3) & 7) + 1;
> node_id = value & 7;
> } else
> return;
>
> /* fixup multi-node processor information */
> - if (nodes > 1) {
> + if (nodes_per_processor > 1) {
> u32 cores_per_node;
> u32 cus_per_node;
>
> set_cpu_cap(c, X86_FEATURE_AMD_DCM);
> - cores_per_node = c->x86_max_cores / nodes;
> + cores_per_node = c->x86_max_cores / nodes_per_processor;
> cus_per_node = cores_per_node / cores_per_cu;
>
> /* store NodeID, use llc_shared_map to store sibling info */
On Wed, Jun 03, 2015 at 10:34:50AM -0500, Aravind Gopalakrishnan wrote:
> Oddly, No. The only thing I got was:
> arch/x86/kernel/cpu/microcode/intel_early.c: In function
> âget_matching_model_microcode.isra.2.constprop.7â:
> arch/x86/kernel/cpu/microcode/intel_early.c:348:1: warning: the frame size
> of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^
32-bit?
Because on 64-bit, that frame size is relaxed to 2K.
config FRAME_WARN
int "Warn for stack frames larger than (needs gcc 4.4)"
range 0 8192
default 1024 if !64BIT
default 2048 if 64BIT
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On 6/3/2015 11:00 AM, Borislav Petkov wrote:
> On Wed, Jun 03, 2015 at 10:34:50AM -0500, Aravind Gopalakrishnan wrote:
>> Oddly, No. The only thing I got was:
>> arch/x86/kernel/cpu/microcode/intel_early.c: In function
>> âget_matching_model_microcode.isra.2.constprop.7â:
>> arch/x86/kernel/cpu/microcode/intel_early.c:348:1: warning: the frame size
>> of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>> }
>> ^
> 32-bit?
>
> Because on 64-bit, that frame size is relaxed to 2K.
>
> config FRAME_WARN
> int "Warn for stack frames larger than (needs gcc 4.4)"
> range 0 8192
> default 1024 if !64BIT
> default 2048 if 64BIT
>
Nope. But it's still set to 1K for me, and I think I know where the
problem might have originated from..
I used a generic distro (Ubuntu) config as base when I first built the
kernel,
and I see that on Ubuntu's 'generic' configs, the value for FRAME_WARN
is 1K.
(Btw, I did install a 64-bit version of the distro on the system to
begin with.. guess they just set it to 1K always)
When I do make defconfig, this is set to 2K properly.
Thanks,
-Aravind