From: Borislav Petkov <[email protected]>
Ok,
this is the first complete series implementing what we discussed
previously. It now uses simple bools and collects everything in a struct
mca_config which is nicely abstracted and collected in one place.
Please take a look and complain :)
Changelog:
==========
v0:
---
Right,
so I did give that a try and it turned out to be a bit more involved
than I thought. Basically, I'm relying on the assumption that if I use a
u64 bitfield and pass a pointer to it, accessing it through that pointer
as a u64 value works. Actually, I have the u64 bitfield as the first
member of a struct and if I cast a pointer to that struct to u64 *, I'm
expecting to have the 64 bits of the same bitfield.
Therefore, I can toggle the bits in the mce code with mca_cfg.<bitname>.
When defining accessing them through the device attributes in sysfs, I
use a new macro DEVICE_BIT_ATTR which gets the corresponding bit number
of that same bit in the bitfield. This gives only one function which
operates on a bitfield instead of a single function per bit in the
bitfield.
For example,
mca_cfg {
u64 dont_log_ce : 1,
is the first bit in the bitfield and I also have a MCA_CFG_DONT_LOG_CE
define which is 0, i.e. the first bit, which I use to toggle the
corresponding bit in the u64 in device_{show,store}_bit.
So I converted mce_dont_log_ce and mce_disabled (renaming it into the
more correct mca_disabled) and it seems to work.
The asm looks sane too. One other advantage is that it makes the code
much more cleaner and compact by collecting all those bool config values
in a single struct, which was my original incentive to do that.
So please take a look and let me know whether this is sane, especially
the above assumption that I can access a u64 bitfield through a u64 *
and the bits are where they're expected to be. gcc seems to do that...
and I don't see anything in the C99 standard that would object to it but
I could be overlooking something.
Thanks.
--
Borislav Petkov (5):
drivers/base: Add a DEVICE_BOOL_ATTR macro
x86, MCA: Convert dont_log_ce, banks and tolerant
x86, MCA: Convert rip_msr, mce_bootlog, monarch_timeout
x86, MCA: Convert the next three variables batch
x86, MCA: Finish mca_config conversion
arch/x86/include/asm/mce.h | 21 ++-
arch/x86/kernel/cpu/mcheck/mce-internal.h | 2 -
arch/x86/kernel/cpu/mcheck/mce-severity.c | 4 +-
arch/x86/kernel/cpu/mcheck/mce.c | 213 +++++++++++++++---------------
arch/x86/kernel/cpu/mcheck/mce_intel.c | 8 +-
arch/x86/lguest/boot.c | 2 +-
drivers/base/core.c | 24 ++++
include/linux/device.h | 7 +
8 files changed, 164 insertions(+), 117 deletions(-)
--
1.8.0.rc0.18.gf84667d
From: Borislav Petkov <[email protected]>
... which, analogous to DEVICE_INT_ATTR provides functionality to
set/clear bools. Its purpose is to be used where values need to be used
as booleans in configuration context.
Next patch uses this.
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/base/core.c | 24 ++++++++++++++++++++++++
include/linux/device.h | 7 +++++++
2 files changed, 31 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index abea76c36a4b..d298fbe7d1b4 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -171,6 +171,30 @@ ssize_t device_show_int(struct device *dev,
}
EXPORT_SYMBOL_GPL(device_show_int);
+ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct dev_ext_attribute *ea = to_ext_attr(attr);
+ u8 bval;
+
+ if (kstrtou8(buf, 10, &bval) < 0)
+ return -EINVAL;
+
+ *(bool *)ea->var = !!bval;
+
+ return size;
+}
+EXPORT_SYMBOL_GPL(device_store_bool);
+
+ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct dev_ext_attribute *ea = to_ext_attr(attr);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", *(bool *)(ea->var));
+}
+EXPORT_SYMBOL_GPL(device_show_bool);
+
/**
* device_release - free device structure.
* @kobj: device's kobject.
diff --git a/include/linux/device.h b/include/linux/device.h
index 86ef6ab553b1..50c85a26b003 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -496,6 +496,10 @@ ssize_t device_show_int(struct device *dev, struct device_attribute *attr,
char *buf);
ssize_t device_store_int(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count);
+ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
+ char *buf);
+ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count);
#define DEVICE_ATTR(_name, _mode, _show, _store) \
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)
@@ -505,6 +509,9 @@ ssize_t device_store_int(struct device *dev, struct device_attribute *attr,
#define DEVICE_INT_ATTR(_name, _mode, _var) \
struct dev_ext_attribute dev_attr_##_name = \
{ __ATTR(_name, _mode, device_show_int, device_store_int), &(_var) }
+#define DEVICE_BOOL_ATTR(_name, _mode, _var) \
+ struct dev_ext_attribute dev_attr_##_name = \
+ { __ATTR(_name, _mode, device_show_bool, device_store_bool), &(_var) }
#define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \
struct device_attribute dev_attr_##_name = \
__ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
--
1.8.0.rc2.4.g42e55a5
From: Borislav Petkov <[email protected]>
mce_ser, mce_bios_cmci_threshold and mce_disabled are the last three
bools which need conversion. Move them to the mca_config struct and
adjust usage sites accordingly.
Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/mce.h | 5 ++--
arch/x86/kernel/cpu/mcheck/mce-internal.h | 2 --
arch/x86/kernel/cpu/mcheck/mce-severity.c | 4 ++--
arch/x86/kernel/cpu/mcheck/mce.c | 40 ++++++++++++++-----------------
arch/x86/kernel/cpu/mcheck/mce_intel.c | 6 ++---
arch/x86/lguest/boot.c | 2 +-
6 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index a711e3f4fbc7..d90c2fccc30c 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -124,6 +124,9 @@ struct mca_config {
bool dont_log_ce;
bool cmci_disabled;
bool ignore_ce;
+ bool disabled;
+ bool ser;
+ bool bios_cmci_threshold;
u8 banks;
s8 bootlog;
int tolerant;
@@ -140,7 +143,6 @@ extern void mce_unregister_decode_chain(struct notifier_block *nb);
#include <linux/init.h>
#include <linux/atomic.h>
-extern int mce_disabled;
extern int mce_p5_enabled;
#ifdef CONFIG_X86_MCE
@@ -173,7 +175,6 @@ DECLARE_PER_CPU(struct device *, mce_device);
#define MAX_NR_BANKS 32
#ifdef CONFIG_X86_MCE_INTEL
-extern int mce_bios_cmci_threshold;
void mce_intel_feature_init(struct cpuinfo_x86 *c);
void cmci_clear(void);
void cmci_reenable(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mcheck/mce-internal.h
index 6a05c1d327a9..5b7d4fa5d3b7 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-internal.h
+++ b/arch/x86/kernel/cpu/mcheck/mce-internal.h
@@ -24,8 +24,6 @@ struct mce_bank {
int mce_severity(struct mce *a, int tolerant, char **msg);
struct dentry *mce_get_debugfs_dir(void);
-extern int mce_ser;
-
extern struct mce_bank *mce_banks;
#ifdef CONFIG_X86_MCE_INTEL
diff --git a/arch/x86/kernel/cpu/mcheck/mce-severity.c b/arch/x86/kernel/cpu/mcheck/mce-severity.c
index 13017626f9a8..beb1f1689e52 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-severity.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-severity.c
@@ -193,9 +193,9 @@ int mce_severity(struct mce *m, int tolerant, char **msg)
continue;
if ((m->mcgstatus & s->mcgmask) != s->mcgres)
continue;
- if (s->ser == SER_REQUIRED && !mce_ser)
+ if (s->ser == SER_REQUIRED && !mca_cfg.ser)
continue;
- if (s->ser == NO_SER && mce_ser)
+ if (s->ser == NO_SER && mca_cfg.ser)
continue;
if (s->context && ctx != s->context)
continue;
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index a66b1c2321d6..326f5b55c858 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -58,18 +58,13 @@ static DEFINE_MUTEX(mce_chrdev_read_mutex);
#define CREATE_TRACE_POINTS
#include <trace/events/mce.h>
-int mce_disabled __read_mostly;
-
#define SPINUNIT 100 /* 100ns */
atomic_t mce_entry;
DEFINE_PER_CPU(unsigned, mce_exception_count);
-int mce_ser __read_mostly;
-int mce_bios_cmci_threshold __read_mostly;
-
-struct mce_bank *mce_banks __read_mostly;
+struct mce_bank *mce_banks __read_mostly;
struct mca_config mca_cfg __read_mostly = {
.bootlog = -1,
@@ -510,7 +505,7 @@ static int mce_ring_add(unsigned long pfn)
int mce_available(struct cpuinfo_x86 *c)
{
- if (mce_disabled)
+ if (mca_cfg.disabled)
return 0;
return cpu_has(c, X86_FEATURE_MCE) && cpu_has(c, X86_FEATURE_MCA);
}
@@ -562,7 +557,7 @@ static void mce_read_aux(struct mce *m, int i)
/*
* Mask the reported address by the reported granularity.
*/
- if (mce_ser && (m->status & MCI_STATUS_MISCV)) {
+ if (mca_cfg.ser && (m->status & MCI_STATUS_MISCV)) {
u8 shift = MCI_MISC_ADDR_LSB(m->misc);
m->addr >>= shift;
m->addr <<= shift;
@@ -617,7 +612,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
* TBD do the same check for MCI_STATUS_EN here?
*/
if (!(flags & MCP_UC) &&
- (m.status & (mce_ser ? MCI_STATUS_S : MCI_STATUS_UC)))
+ (m.status & (mca_cfg.ser ? MCI_STATUS_S : MCI_STATUS_UC)))
continue;
mce_read_aux(&m, i);
@@ -1009,6 +1004,7 @@ static void mce_clear_info(struct mce_info *mi)
*/
void do_machine_check(struct pt_regs *regs, long error_code)
{
+ struct mca_config *cfg = &mca_cfg;
struct mce m, *final;
int i;
int worst = 0;
@@ -1036,7 +1032,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
this_cpu_inc(mce_exception_count);
- if (!mca_cfg.banks)
+ if (!cfg->banks)
goto out;
mce_gather_info(&m, regs);
@@ -1063,7 +1059,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
* because the first one to see it will clear it.
*/
order = mce_start(&no_way_out);
- for (i = 0; i < mca_cfg.banks; i++) {
+ for (i = 0; i < cfg->banks; i++) {
__clear_bit(i, toclear);
if (!test_bit(i, valid_banks))
continue;
@@ -1082,7 +1078,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
* Non uncorrected or non signaled errors are handled by
* machine_check_poll. Leave them alone, unless this panics.
*/
- if (!(m.status & (mce_ser ? MCI_STATUS_S : MCI_STATUS_UC)) &&
+ if (!(m.status & (cfg->ser ? MCI_STATUS_S : MCI_STATUS_UC)) &&
!no_way_out)
continue;
@@ -1091,7 +1087,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
*/
add_taint(TAINT_MACHINE_CHECK);
- severity = mce_severity(&m, mca_cfg.tolerant, NULL);
+ severity = mce_severity(&m, cfg->tolerant, NULL);
/*
* When machine check was for corrected handler don't touch,
@@ -1147,7 +1143,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
* issues we try to recover, or limit damage to the current
* process.
*/
- if (mca_cfg.tolerant < 3) {
+ if (cfg->tolerant < 3) {
if (no_way_out)
mce_panic("Fatal machine check on current CPU", &m, msg);
if (worst == MCE_AR_SEVERITY) {
@@ -1426,7 +1422,7 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
mca_cfg.rip_msr = MSR_IA32_MCG_EIP;
if (cap & MCG_SER_P)
- mce_ser = 1;
+ mca_cfg.ser = true;
return 0;
}
@@ -1675,7 +1671,7 @@ void (*machine_check_vector)(struct pt_regs *, long error_code) =
*/
void __cpuinit mcheck_cpu_init(struct cpuinfo_x86 *c)
{
- if (mce_disabled)
+ if (mca_cfg.disabled)
return;
if (__mcheck_cpu_ancient_init(c))
@@ -1685,7 +1681,7 @@ void __cpuinit mcheck_cpu_init(struct cpuinfo_x86 *c)
return;
if (__mcheck_cpu_cap_init() < 0 || __mcheck_cpu_apply_quirks(c) < 0) {
- mce_disabled = 1;
+ mca_cfg.disabled = true;
return;
}
@@ -1967,7 +1963,7 @@ static int __init mcheck_enable(char *str)
if (*str == '=')
str++;
if (!strcmp(str, "off"))
- mce_disabled = 1;
+ cfg->disabled = true;
else if (!strcmp(str, "no_cmci"))
cfg->cmci_disabled = true;
else if (!strcmp(str, "dont_log_ce"))
@@ -1977,7 +1973,7 @@ static int __init mcheck_enable(char *str)
else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
cfg->bootlog = (str[0] == 'b');
else if (!strcmp(str, "bios_cmci_threshold"))
- mce_bios_cmci_threshold = 1;
+ cfg->bios_cmci_threshold = true;
else if (isdigit(str[0])) {
get_option(&str, &(cfg->tolerant));
if (*str == ',') {
@@ -2219,8 +2215,8 @@ static struct dev_ext_attribute dev_attr_cmci_disabled = {
};
static struct dev_ext_attribute dev_attr_bios_cmci_threshold = {
- __ATTR(bios_cmci_threshold, 0444, device_show_int, NULL),
- &mce_bios_cmci_threshold
+ __ATTR(bios_cmci_threshold, 0444, device_show_bool, NULL),
+ &mca_cfg.bios_cmci_threshold
};
static struct device_attribute *mce_device_attrs[] = {
@@ -2441,7 +2437,7 @@ device_initcall_sync(mcheck_init_device);
*/
static int __init mcheck_disable(char *str)
{
- mce_disabled = 1;
+ mca_cfg.disabled = true;
return 1;
}
__setup("nomce", mcheck_disable);
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 1acd8ecba1c3..79b2b6b6e613 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -200,7 +200,7 @@ static void cmci_discover(int banks)
continue;
}
- if (!mce_bios_cmci_threshold) {
+ if (!mca_cfg.bios_cmci_threshold) {
val &= ~MCI_CTL2_CMCI_THRESHOLD_MASK;
val |= CMCI_THRESHOLD;
} else if (!(val & MCI_CTL2_CMCI_THRESHOLD_MASK)) {
@@ -227,7 +227,7 @@ static void cmci_discover(int banks)
* set the thresholds properly or does not work with
* this boot option. Note down now and report later.
*/
- if (mce_bios_cmci_threshold && bios_zero_thresh &&
+ if (mca_cfg.bios_cmci_threshold && bios_zero_thresh &&
(val & MCI_CTL2_CMCI_THRESHOLD_MASK))
bios_wrong_thresh = 1;
} else {
@@ -235,7 +235,7 @@ static void cmci_discover(int banks)
}
}
raw_spin_unlock_irqrestore(&cmci_discover_lock, flags);
- if (mce_bios_cmci_threshold && bios_wrong_thresh) {
+ if (mca_cfg.bios_cmci_threshold && bios_wrong_thresh) {
pr_info_once(
"bios_cmci_threshold: Some banks do not have valid thresholds set\n");
pr_info_once(
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 642d8805bc1b..df4176cdbb32 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -1412,7 +1412,7 @@ __init void lguest_init(void)
/* We don't have features. We have puppies! Puppies! */
#ifdef CONFIG_X86_MCE
- mce_disabled = 1;
+ mca_cfg.disabled = true;
#endif
#ifdef CONFIG_ACPI
acpi_disabled = 1;
--
1.8.0.rc2.4.g42e55a5
From: Borislav Petkov <[email protected]>
Move them into the mca_config struct and adjust code touching them
accordingly.
Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/mce.h | 6 ++++--
arch/x86/kernel/cpu/mcheck/mce.c | 35 ++++++++++++++++------------------
arch/x86/kernel/cpu/mcheck/mce_intel.c | 2 +-
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 73ba1853951a..a711e3f4fbc7 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -122,13 +122,17 @@ struct mce_log {
struct mca_config {
bool dont_log_ce;
+ bool cmci_disabled;
+ bool ignore_ce;
u8 banks;
s8 bootlog;
int tolerant;
int monarch_timeout;
+ int panic_timeout;
u32 rip_msr;
};
+extern struct mca_config mca_cfg;
extern void mce_register_decode_chain(struct notifier_block *nb);
extern void mce_unregister_decode_chain(struct notifier_block *nb);
@@ -169,8 +173,6 @@ DECLARE_PER_CPU(struct device *, mce_device);
#define MAX_NR_BANKS 32
#ifdef CONFIG_X86_MCE_INTEL
-extern int mce_cmci_disabled;
-extern int mce_ignore_ce;
extern int mce_bios_cmci_threshold;
void mce_intel_feature_init(struct cpuinfo_x86 *c);
void cmci_clear(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 37695ac8b894..a66b1c2321d6 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -66,9 +66,6 @@ atomic_t mce_entry;
DEFINE_PER_CPU(unsigned, mce_exception_count);
-static int mce_panic_timeout __read_mostly;
-int mce_cmci_disabled __read_mostly;
-int mce_ignore_ce __read_mostly;
int mce_ser __read_mostly;
int mce_bios_cmci_threshold __read_mostly;
@@ -302,7 +299,7 @@ static void wait_for_panic(void)
while (timeout-- > 0)
udelay(1);
if (panic_timeout == 0)
- panic_timeout = mce_panic_timeout;
+ panic_timeout = mca_cfg.panic_timeout;
panic("Panicing machine check CPU died");
}
@@ -360,7 +357,7 @@ static void mce_panic(char *msg, struct mce *final, char *exp)
pr_emerg(HW_ERR "Machine check: %s\n", exp);
if (!fake_panic) {
if (panic_timeout == 0)
- panic_timeout = mce_panic_timeout;
+ panic_timeout = mca_cfg.panic_timeout;
panic(msg);
} else
pr_emerg(HW_ERR "Fake kernel panic: %s\n", msg);
@@ -1600,7 +1597,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
if (cfg->monarch_timeout < 0)
cfg->monarch_timeout = 0;
if (cfg->bootlog != 0)
- mce_panic_timeout = 30;
+ cfg->panic_timeout = 30;
return 0;
}
@@ -1645,7 +1642,7 @@ static void mce_start_timer(unsigned int cpu, struct timer_list *t)
__this_cpu_write(mce_next_interval, iv);
- if (mce_ignore_ce || !iv)
+ if (mca_cfg.ignore_ce || !iv)
return;
t->expires = round_jiffies(jiffies + iv);
@@ -1972,11 +1969,11 @@ static int __init mcheck_enable(char *str)
if (!strcmp(str, "off"))
mce_disabled = 1;
else if (!strcmp(str, "no_cmci"))
- mce_cmci_disabled = 1;
+ cfg->cmci_disabled = true;
else if (!strcmp(str, "dont_log_ce"))
cfg->dont_log_ce = true;
else if (!strcmp(str, "ignore_ce"))
- mce_ignore_ce = 1;
+ cfg->ignore_ce = true;
else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
cfg->bootlog = (str[0] == 'b');
else if (!strcmp(str, "bios_cmci_threshold"))
@@ -2154,15 +2151,15 @@ static ssize_t set_ignore_ce(struct device *s,
if (strict_strtoull(buf, 0, &new) < 0)
return -EINVAL;
- if (mce_ignore_ce ^ !!new) {
+ if (mca_cfg.ignore_ce ^ !!new) {
if (new) {
/* disable ce features */
mce_timer_delete_all();
on_each_cpu(mce_disable_cmci, NULL, 1);
- mce_ignore_ce = 1;
+ mca_cfg.ignore_ce = true;
} else {
/* enable ce features */
- mce_ignore_ce = 0;
+ mca_cfg.ignore_ce = false;
on_each_cpu(mce_enable_ce, (void *)1, 1);
}
}
@@ -2178,14 +2175,14 @@ static ssize_t set_cmci_disabled(struct device *s,
if (strict_strtoull(buf, 0, &new) < 0)
return -EINVAL;
- if (mce_cmci_disabled ^ !!new) {
+ if (mca_cfg.cmci_disabled ^ !!new) {
if (new) {
/* disable cmci */
on_each_cpu(mce_disable_cmci, NULL, 1);
- mce_cmci_disabled = 1;
+ mca_cfg.cmci_disabled = true;
} else {
/* enable cmci */
- mce_cmci_disabled = 0;
+ mca_cfg.cmci_disabled = false;
on_each_cpu(mce_enable_ce, NULL, 1);
}
}
@@ -2212,13 +2209,13 @@ static struct dev_ext_attribute dev_attr_check_interval = {
};
static struct dev_ext_attribute dev_attr_ignore_ce = {
- __ATTR(ignore_ce, 0644, device_show_int, set_ignore_ce),
- &mce_ignore_ce
+ __ATTR(ignore_ce, 0644, device_show_bool, set_ignore_ce),
+ &mca_cfg.ignore_ce
};
static struct dev_ext_attribute dev_attr_cmci_disabled = {
- __ATTR(cmci_disabled, 0644, device_show_int, set_cmci_disabled),
- &mce_cmci_disabled
+ __ATTR(cmci_disabled, 0644, device_show_bool, set_cmci_disabled),
+ &mca_cfg.cmci_disabled
};
static struct dev_ext_attribute dev_attr_bios_cmci_threshold = {
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 5f88abf07e9c..1acd8ecba1c3 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -53,7 +53,7 @@ static int cmci_supported(int *banks)
{
u64 cap;
- if (mce_cmci_disabled || mce_ignore_ce)
+ if (mca_cfg.cmci_disabled || mca_cfg.ignore_ce)
return 0;
/*
--
1.8.0.rc2.4.g42e55a5
From: Borislav Petkov <[email protected]>
Move those MCA configuration variables into struct mca_config and adjust
the places they're used accordingly.
Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/mce.h | 7 +++
arch/x86/kernel/cpu/mcheck/mce.c | 97 ++++++++++++++++++++++------------------
2 files changed, 60 insertions(+), 44 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 54d73b1f00a0..e4060de88476 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -119,6 +119,13 @@ struct mce_log {
#define K8_MCE_THRESHOLD_BASE (MCE_EXTENDED_BANK + 1)
#ifdef __KERNEL__
+
+struct mca_config {
+ bool dont_log_ce;
+ u8 banks;
+ int tolerant;
+};
+
extern void mce_register_decode_chain(struct notifier_block *nb);
extern void mce_unregister_decode_chain(struct notifier_block *nb);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 29e87d3b2843..9673629d14d7 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -66,20 +66,10 @@ atomic_t mce_entry;
DEFINE_PER_CPU(unsigned, mce_exception_count);
-/*
- * Tolerant levels:
- * 0: always panic on uncorrected errors, log corrected errors
- * 1: panic or SIGBUS on uncorrected errors, log corrected errors
- * 2: SIGBUS or log uncorrected errors (if possible), log corrected errors
- * 3: never panic or SIGBUS, log all errors (for testing only)
- */
-static int tolerant __read_mostly = 1;
-static int banks __read_mostly;
static int rip_msr __read_mostly;
static int mce_bootlog __read_mostly = -1;
static int monarch_timeout __read_mostly = -1;
static int mce_panic_timeout __read_mostly;
-static int mce_dont_log_ce __read_mostly;
int mce_cmci_disabled __read_mostly;
int mce_ignore_ce __read_mostly;
int mce_ser __read_mostly;
@@ -87,6 +77,17 @@ int mce_bios_cmci_threshold __read_mostly;
struct mce_bank *mce_banks __read_mostly;
+struct mca_config mca_cfg __read_mostly = {
+ /*
+ * Tolerant levels:
+ * 0: always panic on uncorrected errors, log corrected errors
+ * 1: panic or SIGBUS on uncorrected errors, log corrected errors
+ * 2: SIGBUS or log uncorrected errors (if possible), log corr. errors
+ * 3: never panic or SIGBUS, log all errors (for testing only)
+ */
+ .tolerant = 1
+};
+
/* User mode helper program triggered by machine check event */
static unsigned long mce_need_notify;
static char mce_helper[128];
@@ -599,7 +600,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
mce_gather_info(&m, NULL);
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
if (!mce_banks[i].ctl || !test_bit(i, *b))
continue;
@@ -631,7 +632,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
* Don't get the IP here because it's unlikely to
* have anything to do with the actual error location.
*/
- if (!(flags & MCP_DONTLOG) && !mce_dont_log_ce)
+ if (!(flags & MCP_DONTLOG) && !mca_cfg.dont_log_ce)
mce_log(&m);
/*
@@ -658,14 +659,14 @@ static int mce_no_way_out(struct mce *m, char **msg, unsigned long *validp,
{
int i, ret = 0;
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
m->status = mce_rdmsrl(MSR_IA32_MCx_STATUS(i));
if (m->status & MCI_STATUS_VAL) {
__set_bit(i, validp);
if (quirk_no_way_out)
quirk_no_way_out(i, m, regs);
}
- if (mce_severity(m, tolerant, msg) >= MCE_PANIC_SEVERITY)
+ if (mce_severity(m, mca_cfg.tolerant, msg) >= MCE_PANIC_SEVERITY)
ret = 1;
}
return ret;
@@ -700,7 +701,7 @@ static int mce_timed_out(u64 *t)
goto out;
if ((s64)*t < SPINUNIT) {
/* CHECKME: Make panic default for 1 too? */
- if (tolerant < 1)
+ if (mca_cfg.tolerant < 1)
mce_panic("Timeout synchronizing machine check over CPUs",
NULL, NULL);
cpu_missing = 1;
@@ -750,7 +751,8 @@ static void mce_reign(void)
* Grade the severity of the errors of all the CPUs.
*/
for_each_possible_cpu(cpu) {
- int severity = mce_severity(&per_cpu(mces_seen, cpu), tolerant,
+ int severity = mce_severity(&per_cpu(mces_seen, cpu),
+ mca_cfg.tolerant,
&nmsg);
if (severity > global_worst) {
msg = nmsg;
@@ -764,7 +766,7 @@ static void mce_reign(void)
* This dumps all the mces in the log buffer and stops the
* other CPUs.
*/
- if (m && global_worst >= MCE_PANIC_SEVERITY && tolerant < 3)
+ if (m && global_worst >= MCE_PANIC_SEVERITY && mca_cfg.tolerant < 3)
mce_panic("Fatal Machine check", m, msg);
/*
@@ -777,7 +779,7 @@ static void mce_reign(void)
* No machine check event found. Must be some external
* source or one CPU is hung. Panic.
*/
- if (global_worst <= MCE_KEEP_SEVERITY && tolerant < 3)
+ if (global_worst <= MCE_KEEP_SEVERITY && mca_cfg.tolerant < 3)
mce_panic("Machine check from unknown source", NULL, NULL);
/*
@@ -946,7 +948,7 @@ static void mce_clear_state(unsigned long *toclear)
{
int i;
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
if (test_bit(i, toclear))
mce_wrmsrl(MSR_IA32_MCx_STATUS(i), 0);
}
@@ -1022,7 +1024,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
int order;
/*
* If no_way_out gets set, there is no safe way to recover from this
- * MCE. If tolerant is cranked up, we'll try anyway.
+ * MCE. If mca_cfg.tolerant is cranked up, we'll try anyway.
*/
int no_way_out = 0;
/*
@@ -1038,7 +1040,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
this_cpu_inc(mce_exception_count);
- if (!banks)
+ if (!mca_cfg.banks)
goto out;
mce_gather_info(&m, regs);
@@ -1065,7 +1067,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
* because the first one to see it will clear it.
*/
order = mce_start(&no_way_out);
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
__clear_bit(i, toclear);
if (!test_bit(i, valid_banks))
continue;
@@ -1093,7 +1095,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
*/
add_taint(TAINT_MACHINE_CHECK);
- severity = mce_severity(&m, tolerant, NULL);
+ severity = mce_severity(&m, mca_cfg.tolerant, NULL);
/*
* When machine check was for corrected handler don't touch,
@@ -1117,7 +1119,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
* When the ring overflows we just ignore the AO error.
* RED-PEN add some logging mechanism when
* usable_address or mce_add_ring fails.
- * RED-PEN don't ignore overflow for tolerant == 0
+ * RED-PEN don't ignore overflow for mca_cfg.tolerant == 0
*/
if (severity == MCE_AO_SEVERITY && mce_usable_address(&m))
mce_ring_add(m.addr >> PAGE_SHIFT);
@@ -1149,7 +1151,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
* issues we try to recover, or limit damage to the current
* process.
*/
- if (tolerant < 3) {
+ if (mca_cfg.tolerant < 3) {
if (no_way_out)
mce_panic("Fatal machine check on current CPU", &m, msg);
if (worst == MCE_AR_SEVERITY) {
@@ -1377,11 +1379,13 @@ EXPORT_SYMBOL_GPL(mce_notify_irq);
static int __cpuinit __mcheck_cpu_mce_banks_init(void)
{
int i;
+ u8 num_banks = mca_cfg.banks;
- mce_banks = kzalloc(banks * sizeof(struct mce_bank), GFP_KERNEL);
+ mce_banks = kzalloc(num_banks * sizeof(struct mce_bank), GFP_KERNEL);
if (!mce_banks)
return -ENOMEM;
- for (i = 0; i < banks; i++) {
+
+ for (i = 0; i < num_banks; i++) {
struct mce_bank *b = &mce_banks[i];
b->ctl = -1ULL;
@@ -1401,7 +1405,7 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
rdmsrl(MSR_IA32_MCG_CAP, cap);
b = cap & MCG_BANKCNT_MASK;
- if (!banks)
+ if (!mca_cfg.banks)
pr_info("CPU supports %d MCE banks\n", b);
if (b > MAX_NR_BANKS) {
@@ -1411,8 +1415,9 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
}
/* Don't support asymmetric configurations today */
- WARN_ON(banks != 0 && b != banks);
- banks = b;
+ WARN_ON(mca_cfg.banks != 0 && b != mca_cfg.banks);
+ mca_cfg.banks = b;
+
if (!mce_banks) {
int err = __mcheck_cpu_mce_banks_init();
@@ -1448,7 +1453,7 @@ static void __mcheck_cpu_init_generic(void)
if (cap & MCG_CTL_P)
wrmsr(MSR_IA32_MCG_CTL, 0xffffffff, 0xffffffff);
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
struct mce_bank *b = &mce_banks[i];
if (!b->init)
@@ -1489,6 +1494,8 @@ static void quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs)
/* Add per CPU specific workarounds here */
static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
{
+ struct mca_config *cfg = &mca_cfg;
+
if (c->x86_vendor == X86_VENDOR_UNKNOWN) {
pr_info("unknown CPU type - not enabling MCE support\n");
return -EOPNOTSUPP;
@@ -1496,7 +1503,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
/* This should be disabled by the BIOS, but isn't always */
if (c->x86_vendor == X86_VENDOR_AMD) {
- if (c->x86 == 15 && banks > 4) {
+ if (c->x86 == 15 && cfg->banks > 4) {
/*
* disable GART TBL walk error reporting, which
* trips off incorrectly with the IOMMU & 3ware
@@ -1515,7 +1522,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
* Various K7s with broken bank 0 around. Always disable
* by default.
*/
- if (c->x86 == 6 && banks > 0)
+ if (c->x86 == 6 && cfg->banks > 0)
mce_banks[0].ctl = 0;
/*
@@ -1566,7 +1573,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
* valid event later, merely don't write CTL0.
*/
- if (c->x86 == 6 && c->x86_model < 0x1A && banks > 0)
+ if (c->x86 == 6 && c->x86_model < 0x1A && cfg->banks > 0)
mce_banks[0].init = 0;
/*
@@ -1951,6 +1958,8 @@ static struct miscdevice mce_chrdev_device = {
*/
static int __init mcheck_enable(char *str)
{
+ struct mca_config *cfg = &mca_cfg;
+
if (*str == 0) {
enable_p5_mce();
return 1;
@@ -1962,7 +1971,7 @@ static int __init mcheck_enable(char *str)
else if (!strcmp(str, "no_cmci"))
mce_cmci_disabled = 1;
else if (!strcmp(str, "dont_log_ce"))
- mce_dont_log_ce = 1;
+ cfg->dont_log_ce = true;
else if (!strcmp(str, "ignore_ce"))
mce_ignore_ce = 1;
else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
@@ -1970,7 +1979,7 @@ static int __init mcheck_enable(char *str)
else if (!strcmp(str, "bios_cmci_threshold"))
mce_bios_cmci_threshold = 1;
else if (isdigit(str[0])) {
- get_option(&str, &tolerant);
+ get_option(&str, &(cfg->tolerant));
if (*str == ',') {
++str;
get_option(&str, &monarch_timeout);
@@ -2002,7 +2011,7 @@ static int mce_disable_error_reporting(void)
{
int i;
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
struct mce_bank *b = &mce_banks[i];
if (b->init)
@@ -2190,9 +2199,9 @@ static ssize_t store_int_with_restart(struct device *s,
}
static DEVICE_ATTR(trigger, 0644, show_trigger, set_trigger);
-static DEVICE_INT_ATTR(tolerant, 0644, tolerant);
+static DEVICE_INT_ATTR(tolerant, 0644, mca_cfg.tolerant);
static DEVICE_INT_ATTR(monarch_timeout, 0644, monarch_timeout);
-static DEVICE_INT_ATTR(dont_log_ce, 0644, mce_dont_log_ce);
+static DEVICE_BOOL_ATTR(dont_log_ce, 0644, mca_cfg.dont_log_ce);
static struct dev_ext_attribute dev_attr_check_interval = {
__ATTR(check_interval, 0644, device_show_int, store_int_with_restart),
@@ -2259,7 +2268,7 @@ static __cpuinit int mce_device_create(unsigned int cpu)
if (err)
goto error;
}
- for (j = 0; j < banks; j++) {
+ for (j = 0; j < mca_cfg.banks; j++) {
err = device_create_file(dev, &mce_banks[j].attr);
if (err)
goto error2;
@@ -2291,7 +2300,7 @@ static __cpuinit void mce_device_remove(unsigned int cpu)
for (i = 0; mce_device_attrs[i]; i++)
device_remove_file(dev, mce_device_attrs[i]);
- for (i = 0; i < banks; i++)
+ for (i = 0; i < mca_cfg.banks; i++)
device_remove_file(dev, &mce_banks[i].attr);
device_unregister(dev);
@@ -2310,7 +2319,7 @@ static void __cpuinit mce_disable_cpu(void *h)
if (!(action & CPU_TASKS_FROZEN))
cmci_clear();
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
struct mce_bank *b = &mce_banks[i];
if (b->init)
@@ -2328,7 +2337,7 @@ static void __cpuinit mce_reenable_cpu(void *h)
if (!(action & CPU_TASKS_FROZEN))
cmci_reenable();
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
struct mce_bank *b = &mce_banks[i];
if (b->init)
@@ -2381,7 +2390,7 @@ static __init void mce_init_banks(void)
{
int i;
- for (i = 0; i < banks; i++) {
+ for (i = 0; i < mca_cfg.banks; i++) {
struct mce_bank *b = &mce_banks[i];
struct device_attribute *a = &b->attr;
--
1.8.0.rc2.4.g42e55a5
From: Borislav Petkov <[email protected]>
Move above configuration variables into struct mca_config and adjust
usage places accordingly.
Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/mce.h | 3 +++
arch/x86/kernel/cpu/mcheck/mce.c | 51 +++++++++++++++++++++-------------------
2 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index e4060de88476..73ba1853951a 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -123,7 +123,10 @@ struct mce_log {
struct mca_config {
bool dont_log_ce;
u8 banks;
+ s8 bootlog;
int tolerant;
+ int monarch_timeout;
+ u32 rip_msr;
};
extern void mce_register_decode_chain(struct notifier_block *nb);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 9673629d14d7..37695ac8b894 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -66,9 +66,6 @@ atomic_t mce_entry;
DEFINE_PER_CPU(unsigned, mce_exception_count);
-static int rip_msr __read_mostly;
-static int mce_bootlog __read_mostly = -1;
-static int monarch_timeout __read_mostly = -1;
static int mce_panic_timeout __read_mostly;
int mce_cmci_disabled __read_mostly;
int mce_ignore_ce __read_mostly;
@@ -78,6 +75,7 @@ int mce_bios_cmci_threshold __read_mostly;
struct mce_bank *mce_banks __read_mostly;
struct mca_config mca_cfg __read_mostly = {
+ .bootlog = -1,
/*
* Tolerant levels:
* 0: always panic on uncorrected errors, log corrected errors
@@ -85,7 +83,8 @@ struct mca_config mca_cfg __read_mostly = {
* 2: SIGBUS or log uncorrected errors (if possible), log corr. errors
* 3: never panic or SIGBUS, log all errors (for testing only)
*/
- .tolerant = 1
+ .tolerant = 1,
+ .monarch_timeout = -1
};
/* User mode helper program triggered by machine check event */
@@ -373,7 +372,7 @@ static int msr_to_offset(u32 msr)
{
unsigned bank = __this_cpu_read(injectm.bank);
- if (msr == rip_msr)
+ if (msr == mca_cfg.rip_msr)
return offsetof(struct mce, ip);
if (msr == MSR_IA32_MCx_STATUS(bank))
return offsetof(struct mce, status);
@@ -452,8 +451,8 @@ static inline void mce_gather_info(struct mce *m, struct pt_regs *regs)
m->cs |= 3;
}
/* Use accurate RIP reporting if available. */
- if (rip_msr)
- m->ip = mce_rdmsrl(rip_msr);
+ if (mca_cfg.rip_msr)
+ m->ip = mce_rdmsrl(mca_cfg.rip_msr);
}
}
@@ -697,7 +696,7 @@ static int mce_timed_out(u64 *t)
rmb();
if (atomic_read(&mce_paniced))
wait_for_panic();
- if (!monarch_timeout)
+ if (!mca_cfg.monarch_timeout)
goto out;
if ((s64)*t < SPINUNIT) {
/* CHECKME: Make panic default for 1 too? */
@@ -803,7 +802,7 @@ static int mce_start(int *no_way_out)
{
int order;
int cpus = num_online_cpus();
- u64 timeout = (u64)monarch_timeout * NSEC_PER_USEC;
+ u64 timeout = (u64)mca_cfg.monarch_timeout * NSEC_PER_USEC;
if (!timeout)
return -1;
@@ -867,7 +866,7 @@ static int mce_start(int *no_way_out)
static int mce_end(int order)
{
int ret = -1;
- u64 timeout = (u64)monarch_timeout * NSEC_PER_USEC;
+ u64 timeout = (u64)mca_cfg.monarch_timeout * NSEC_PER_USEC;
if (!timeout)
goto reset;
@@ -1427,7 +1426,7 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
/* Use accurate RIP reporting if available. */
if ((cap & MCG_EXT_P) && MCG_EXT_CNT(cap) >= 9)
- rip_msr = MSR_IA32_MCG_EIP;
+ mca_cfg.rip_msr = MSR_IA32_MCG_EIP;
if (cap & MCG_SER_P)
mce_ser = 1;
@@ -1437,15 +1436,19 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
static void __mcheck_cpu_init_generic(void)
{
+ enum mcp_flags m_fl = 0;
mce_banks_t all_banks;
u64 cap;
int i;
+ if (!mca_cfg.bootlog)
+ m_fl = MCP_DONTLOG;
+
/*
* Log the machine checks left over from the previous reset.
*/
bitmap_fill(all_banks, MAX_NR_BANKS);
- machine_check_poll(MCP_UC|(!mce_bootlog ? MCP_DONTLOG : 0), &all_banks);
+ machine_check_poll(MCP_UC | m_fl, &all_banks);
set_in_cr4(X86_CR4_MCE);
@@ -1511,12 +1514,12 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
*/
clear_bit(10, (unsigned long *)&mce_banks[4].ctl);
}
- if (c->x86 <= 17 && mce_bootlog < 0) {
+ if (c->x86 <= 17 && cfg->bootlog < 0) {
/*
* Lots of broken BIOS around that don't clear them
* by default and leave crap in there. Don't log:
*/
- mce_bootlog = 0;
+ cfg->bootlog = 0;
}
/*
* Various K7s with broken bank 0 around. Always disable
@@ -1581,22 +1584,22 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
* synchronization with a one second timeout.
*/
if ((c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xe)) &&
- monarch_timeout < 0)
- monarch_timeout = USEC_PER_SEC;
+ cfg->monarch_timeout < 0)
+ cfg->monarch_timeout = USEC_PER_SEC;
/*
* There are also broken BIOSes on some Pentium M and
* earlier systems:
*/
- if (c->x86 == 6 && c->x86_model <= 13 && mce_bootlog < 0)
- mce_bootlog = 0;
+ if (c->x86 == 6 && c->x86_model <= 13 && cfg->bootlog < 0)
+ cfg->bootlog = 0;
if (c->x86 == 6 && c->x86_model == 45)
quirk_no_way_out = quirk_sandybridge_ifu;
}
- if (monarch_timeout < 0)
- monarch_timeout = 0;
- if (mce_bootlog != 0)
+ if (cfg->monarch_timeout < 0)
+ cfg->monarch_timeout = 0;
+ if (cfg->bootlog != 0)
mce_panic_timeout = 30;
return 0;
@@ -1975,14 +1978,14 @@ static int __init mcheck_enable(char *str)
else if (!strcmp(str, "ignore_ce"))
mce_ignore_ce = 1;
else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
- mce_bootlog = (str[0] == 'b');
+ cfg->bootlog = (str[0] == 'b');
else if (!strcmp(str, "bios_cmci_threshold"))
mce_bios_cmci_threshold = 1;
else if (isdigit(str[0])) {
get_option(&str, &(cfg->tolerant));
if (*str == ',') {
++str;
- get_option(&str, &monarch_timeout);
+ get_option(&str, &(cfg->monarch_timeout));
}
} else {
pr_info("mce argument %s ignored. Please use /sys\n", str);
@@ -2200,7 +2203,7 @@ static ssize_t store_int_with_restart(struct device *s,
static DEVICE_ATTR(trigger, 0644, show_trigger, set_trigger);
static DEVICE_INT_ATTR(tolerant, 0644, mca_cfg.tolerant);
-static DEVICE_INT_ATTR(monarch_timeout, 0644, monarch_timeout);
+static DEVICE_INT_ATTR(monarch_timeout, 0644, mca_cfg.monarch_timeout);
static DEVICE_BOOL_ATTR(dont_log_ce, 0644, mca_cfg.dont_log_ce);
static struct dev_ext_attribute dev_attr_check_interval = {
--
1.8.0.rc2.4.g42e55a5
Apart from a few nits below, patch series:
Acked-by: Naveen N. Rao <[email protected]>
Regards,
Naveen
On 10/17/2012 04:43 PM, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> Move those MCA configuration variables into struct mca_config and adjust
> the places they're used accordingly.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> ---
> arch/x86/include/asm/mce.h | 7 +++
> arch/x86/kernel/cpu/mcheck/mce.c | 97 ++++++++++++++++++++++------------------
> 2 files changed, 60 insertions(+), 44 deletions(-)
>
> diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> index 54d73b1f00a0..e4060de88476 100644
> --- a/arch/x86/include/asm/mce.h
> +++ b/arch/x86/include/asm/mce.h
> @@ -119,6 +119,13 @@ struct mce_log {
> #define K8_MCE_THRESHOLD_BASE (MCE_EXTENDED_BANK + 1)
>
> #ifdef __KERNEL__
> +
> +struct mca_config {
> + bool dont_log_ce;
> + u8 banks;
> + int tolerant;
> +};
> +
> extern void mce_register_decode_chain(struct notifier_block *nb);
> extern void mce_unregister_decode_chain(struct notifier_block *nb);
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index 29e87d3b2843..9673629d14d7 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -66,20 +66,10 @@ atomic_t mce_entry;
>
> DEFINE_PER_CPU(unsigned, mce_exception_count);
>
> -/*
> - * Tolerant levels:
> - * 0: always panic on uncorrected errors, log corrected errors
> - * 1: panic or SIGBUS on uncorrected errors, log corrected errors
> - * 2: SIGBUS or log uncorrected errors (if possible), log corrected errors
> - * 3: never panic or SIGBUS, log all errors (for testing only)
> - */
> -static int tolerant __read_mostly = 1;
> -static int banks __read_mostly;
> static int rip_msr __read_mostly;
> static int mce_bootlog __read_mostly = -1;
> static int monarch_timeout __read_mostly = -1;
> static int mce_panic_timeout __read_mostly;
> -static int mce_dont_log_ce __read_mostly;
> int mce_cmci_disabled __read_mostly;
> int mce_ignore_ce __read_mostly;
> int mce_ser __read_mostly;
> @@ -87,6 +77,17 @@ int mce_bios_cmci_threshold __read_mostly;
>
> struct mce_bank *mce_banks __read_mostly;
>
> +struct mca_config mca_cfg __read_mostly = {
> + /*
> + * Tolerant levels:
> + * 0: always panic on uncorrected errors, log corrected errors
> + * 1: panic or SIGBUS on uncorrected errors, log corrected errors
> + * 2: SIGBUS or log uncorrected errors (if possible), log corr. errors
> + * 3: never panic or SIGBUS, log all errors (for testing only)
> + */
> + .tolerant = 1
> +};
> +
> /* User mode helper program triggered by machine check event */
> static unsigned long mce_need_notify;
> static char mce_helper[128];
> @@ -599,7 +600,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
>
> mce_gather_info(&m, NULL);
>
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> if (!mce_banks[i].ctl || !test_bit(i, *b))
> continue;
>
> @@ -631,7 +632,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
> * Don't get the IP here because it's unlikely to
> * have anything to do with the actual error location.
> */
> - if (!(flags & MCP_DONTLOG) && !mce_dont_log_ce)
> + if (!(flags & MCP_DONTLOG) && !mca_cfg.dont_log_ce)
> mce_log(&m);
>
> /*
> @@ -658,14 +659,14 @@ static int mce_no_way_out(struct mce *m, char **msg, unsigned long *validp,
> {
> int i, ret = 0;
>
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> m->status = mce_rdmsrl(MSR_IA32_MCx_STATUS(i));
> if (m->status & MCI_STATUS_VAL) {
> __set_bit(i, validp);
> if (quirk_no_way_out)
> quirk_no_way_out(i, m, regs);
> }
> - if (mce_severity(m, tolerant, msg) >= MCE_PANIC_SEVERITY)
> + if (mce_severity(m, mca_cfg.tolerant, msg) >= MCE_PANIC_SEVERITY)
> ret = 1;
> }
> return ret;
> @@ -700,7 +701,7 @@ static int mce_timed_out(u64 *t)
> goto out;
> if ((s64)*t < SPINUNIT) {
> /* CHECKME: Make panic default for 1 too? */
> - if (tolerant < 1)
> + if (mca_cfg.tolerant < 1)
> mce_panic("Timeout synchronizing machine check over CPUs",
> NULL, NULL);
> cpu_missing = 1;
> @@ -750,7 +751,8 @@ static void mce_reign(void)
> * Grade the severity of the errors of all the CPUs.
> */
> for_each_possible_cpu(cpu) {
> - int severity = mce_severity(&per_cpu(mces_seen, cpu), tolerant,
> + int severity = mce_severity(&per_cpu(mces_seen, cpu),
> + mca_cfg.tolerant,
> &nmsg);
> if (severity > global_worst) {
> msg = nmsg;
> @@ -764,7 +766,7 @@ static void mce_reign(void)
> * This dumps all the mces in the log buffer and stops the
> * other CPUs.
> */
> - if (m && global_worst >= MCE_PANIC_SEVERITY && tolerant < 3)
> + if (m && global_worst >= MCE_PANIC_SEVERITY && mca_cfg.tolerant < 3)
> mce_panic("Fatal Machine check", m, msg);
>
> /*
> @@ -777,7 +779,7 @@ static void mce_reign(void)
> * No machine check event found. Must be some external
> * source or one CPU is hung. Panic.
> */
> - if (global_worst <= MCE_KEEP_SEVERITY && tolerant < 3)
> + if (global_worst <= MCE_KEEP_SEVERITY && mca_cfg.tolerant < 3)
> mce_panic("Machine check from unknown source", NULL, NULL);
>
> /*
> @@ -946,7 +948,7 @@ static void mce_clear_state(unsigned long *toclear)
> {
> int i;
>
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> if (test_bit(i, toclear))
> mce_wrmsrl(MSR_IA32_MCx_STATUS(i), 0);
> }
> @@ -1022,7 +1024,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
> int order;
> /*
> * If no_way_out gets set, there is no safe way to recover from this
> - * MCE. If tolerant is cranked up, we'll try anyway.
> + * MCE. If mca_cfg.tolerant is cranked up, we'll try anyway.
> */
> int no_way_out = 0;
> /*
> @@ -1038,7 +1040,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
>
> this_cpu_inc(mce_exception_count);
>
> - if (!banks)
> + if (!mca_cfg.banks)
> goto out;
>
> mce_gather_info(&m, regs);
> @@ -1065,7 +1067,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
> * because the first one to see it will clear it.
> */
> order = mce_start(&no_way_out);
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> __clear_bit(i, toclear);
> if (!test_bit(i, valid_banks))
> continue;
> @@ -1093,7 +1095,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
> */
> add_taint(TAINT_MACHINE_CHECK);
>
> - severity = mce_severity(&m, tolerant, NULL);
> + severity = mce_severity(&m, mca_cfg.tolerant, NULL);
>
> /*
> * When machine check was for corrected handler don't touch,
> @@ -1117,7 +1119,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
> * When the ring overflows we just ignore the AO error.
> * RED-PEN add some logging mechanism when
> * usable_address or mce_add_ring fails.
> - * RED-PEN don't ignore overflow for tolerant == 0
> + * RED-PEN don't ignore overflow for mca_cfg.tolerant == 0
> */
> if (severity == MCE_AO_SEVERITY && mce_usable_address(&m))
> mce_ring_add(m.addr >> PAGE_SHIFT);
> @@ -1149,7 +1151,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
> * issues we try to recover, or limit damage to the current
> * process.
> */
> - if (tolerant < 3) {
> + if (mca_cfg.tolerant < 3) {
> if (no_way_out)
> mce_panic("Fatal machine check on current CPU", &m, msg);
> if (worst == MCE_AR_SEVERITY) {
> @@ -1377,11 +1379,13 @@ EXPORT_SYMBOL_GPL(mce_notify_irq);
> static int __cpuinit __mcheck_cpu_mce_banks_init(void)
> {
> int i;
> + u8 num_banks = mca_cfg.banks;
Nit: no need for the above? Only 2 references below and I think we can
simply continue to use mca_cfg for uniformity. Ditto for the use of cfg
further below.
>
> - mce_banks = kzalloc(banks * sizeof(struct mce_bank), GFP_KERNEL);
> + mce_banks = kzalloc(num_banks * sizeof(struct mce_bank), GFP_KERNEL);
> if (!mce_banks)
> return -ENOMEM;
> - for (i = 0; i < banks; i++) {
> +
> + for (i = 0; i < num_banks; i++) {
> struct mce_bank *b = &mce_banks[i];
>
> b->ctl = -1ULL;
> @@ -1401,7 +1405,7 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
> rdmsrl(MSR_IA32_MCG_CAP, cap);
>
> b = cap & MCG_BANKCNT_MASK;
> - if (!banks)
> + if (!mca_cfg.banks)
> pr_info("CPU supports %d MCE banks\n", b);
>
> if (b > MAX_NR_BANKS) {
> @@ -1411,8 +1415,9 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
> }
>
> /* Don't support asymmetric configurations today */
> - WARN_ON(banks != 0 && b != banks);
> - banks = b;
> + WARN_ON(mca_cfg.banks != 0 && b != mca_cfg.banks);
> + mca_cfg.banks = b;
> +
> if (!mce_banks) {
> int err = __mcheck_cpu_mce_banks_init();
>
> @@ -1448,7 +1453,7 @@ static void __mcheck_cpu_init_generic(void)
> if (cap & MCG_CTL_P)
> wrmsr(MSR_IA32_MCG_CTL, 0xffffffff, 0xffffffff);
>
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> struct mce_bank *b = &mce_banks[i];
>
> if (!b->init)
> @@ -1489,6 +1494,8 @@ static void quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs)
> /* Add per CPU specific workarounds here */
> static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
> {
> + struct mca_config *cfg = &mca_cfg;
> +
Same as above. We could probably continue using mca_cfg for uniformity.
> if (c->x86_vendor == X86_VENDOR_UNKNOWN) {
> pr_info("unknown CPU type - not enabling MCE support\n");
> return -EOPNOTSUPP;
> @@ -1496,7 +1503,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
>
> /* This should be disabled by the BIOS, but isn't always */
> if (c->x86_vendor == X86_VENDOR_AMD) {
> - if (c->x86 == 15 && banks > 4) {
> + if (c->x86 == 15 && cfg->banks > 4) {
> /*
> * disable GART TBL walk error reporting, which
> * trips off incorrectly with the IOMMU & 3ware
> @@ -1515,7 +1522,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
> * Various K7s with broken bank 0 around. Always disable
> * by default.
> */
> - if (c->x86 == 6 && banks > 0)
> + if (c->x86 == 6 && cfg->banks > 0)
> mce_banks[0].ctl = 0;
>
> /*
> @@ -1566,7 +1573,7 @@ static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
> * valid event later, merely don't write CTL0.
> */
>
> - if (c->x86 == 6 && c->x86_model < 0x1A && banks > 0)
> + if (c->x86 == 6 && c->x86_model < 0x1A && cfg->banks > 0)
> mce_banks[0].init = 0;
>
> /*
> @@ -1951,6 +1958,8 @@ static struct miscdevice mce_chrdev_device = {
> */
> static int __init mcheck_enable(char *str)
> {
> + struct mca_config *cfg = &mca_cfg;
> +
Same as above.
> if (*str == 0) {
> enable_p5_mce();
> return 1;
> @@ -1962,7 +1971,7 @@ static int __init mcheck_enable(char *str)
> else if (!strcmp(str, "no_cmci"))
> mce_cmci_disabled = 1;
> else if (!strcmp(str, "dont_log_ce"))
> - mce_dont_log_ce = 1;
> + cfg->dont_log_ce = true;
> else if (!strcmp(str, "ignore_ce"))
> mce_ignore_ce = 1;
> else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
> @@ -1970,7 +1979,7 @@ static int __init mcheck_enable(char *str)
> else if (!strcmp(str, "bios_cmci_threshold"))
> mce_bios_cmci_threshold = 1;
> else if (isdigit(str[0])) {
> - get_option(&str, &tolerant);
> + get_option(&str, &(cfg->tolerant));
> if (*str == ',') {
> ++str;
> get_option(&str, &monarch_timeout);
> @@ -2002,7 +2011,7 @@ static int mce_disable_error_reporting(void)
> {
> int i;
>
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> struct mce_bank *b = &mce_banks[i];
>
> if (b->init)
> @@ -2190,9 +2199,9 @@ static ssize_t store_int_with_restart(struct device *s,
> }
>
> static DEVICE_ATTR(trigger, 0644, show_trigger, set_trigger);
> -static DEVICE_INT_ATTR(tolerant, 0644, tolerant);
> +static DEVICE_INT_ATTR(tolerant, 0644, mca_cfg.tolerant);
> static DEVICE_INT_ATTR(monarch_timeout, 0644, monarch_timeout);
> -static DEVICE_INT_ATTR(dont_log_ce, 0644, mce_dont_log_ce);
> +static DEVICE_BOOL_ATTR(dont_log_ce, 0644, mca_cfg.dont_log_ce);
>
> static struct dev_ext_attribute dev_attr_check_interval = {
> __ATTR(check_interval, 0644, device_show_int, store_int_with_restart),
> @@ -2259,7 +2268,7 @@ static __cpuinit int mce_device_create(unsigned int cpu)
> if (err)
> goto error;
> }
> - for (j = 0; j < banks; j++) {
> + for (j = 0; j < mca_cfg.banks; j++) {
> err = device_create_file(dev, &mce_banks[j].attr);
> if (err)
> goto error2;
> @@ -2291,7 +2300,7 @@ static __cpuinit void mce_device_remove(unsigned int cpu)
> for (i = 0; mce_device_attrs[i]; i++)
> device_remove_file(dev, mce_device_attrs[i]);
>
> - for (i = 0; i < banks; i++)
> + for (i = 0; i < mca_cfg.banks; i++)
> device_remove_file(dev, &mce_banks[i].attr);
>
> device_unregister(dev);
> @@ -2310,7 +2319,7 @@ static void __cpuinit mce_disable_cpu(void *h)
>
> if (!(action & CPU_TASKS_FROZEN))
> cmci_clear();
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> struct mce_bank *b = &mce_banks[i];
>
> if (b->init)
> @@ -2328,7 +2337,7 @@ static void __cpuinit mce_reenable_cpu(void *h)
>
> if (!(action & CPU_TASKS_FROZEN))
> cmci_reenable();
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> struct mce_bank *b = &mce_banks[i];
>
> if (b->init)
> @@ -2381,7 +2390,7 @@ static __init void mce_init_banks(void)
> {
> int i;
>
> - for (i = 0; i < banks; i++) {
> + for (i = 0; i < mca_cfg.banks; i++) {
> struct mce_bank *b = &mce_banks[i];
> struct device_attribute *a = &b->attr;
>
On Wed, Oct 17, 2012 at 05:37:54PM +0530, Naveen N. Rao wrote:
> Nit: no need for the above? Only 2 references below and I think we
> can simply continue to use mca_cfg for uniformity. Ditto for the use
> of cfg further below.
>
> >
> >- mce_banks = kzalloc(banks * sizeof(struct mce_bank), GFP_KERNEL);
> >+ mce_banks = kzalloc(num_banks * sizeof(struct mce_bank), GFP_KERNEL);
> > if (!mce_banks)
> > return -ENOMEM;
For the simple reason that this line violates the 80 columns and I
didn't want to bring any uglification to the code by breaking the line
in two.
For the generated asm it doesn't matter - it is simply done for keeping
the code as readable as possible.
> >- for (i = 0; i < banks; i++) {
> >+
> >+ for (i = 0; i < num_banks; i++) {
> > struct mce_bank *b = &mce_banks[i];
> >
> > b->ctl = -1ULL;
> >@@ -1401,7 +1405,7 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
> > rdmsrl(MSR_IA32_MCG_CAP, cap);
> >
> > b = cap & MCG_BANKCNT_MASK;
> >- if (!banks)
> >+ if (!mca_cfg.banks)
> > pr_info("CPU supports %d MCE banks\n", b);
> >
> > if (b > MAX_NR_BANKS) {
> >@@ -1411,8 +1415,9 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
> > }
> >
> > /* Don't support asymmetric configurations today */
> >- WARN_ON(banks != 0 && b != banks);
> >- banks = b;
> >+ WARN_ON(mca_cfg.banks != 0 && b != mca_cfg.banks);
> >+ mca_cfg.banks = b;
> >+
> > if (!mce_banks) {
> > int err = __mcheck_cpu_mce_banks_init();
> >
> >@@ -1448,7 +1453,7 @@ static void __mcheck_cpu_init_generic(void)
> > if (cap & MCG_CTL_P)
> > wrmsr(MSR_IA32_MCG_CTL, 0xffffffff, 0xffffffff);
> >
> >- for (i = 0; i < banks; i++) {
> >+ for (i = 0; i < mca_cfg.banks; i++) {
> > struct mce_bank *b = &mce_banks[i];
> >
> > if (!b->init)
> >@@ -1489,6 +1494,8 @@ static void quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs)
> > /* Add per CPU specific workarounds here */
> > static int __cpuinit __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
> > {
> >+ struct mca_config *cfg = &mca_cfg;
> >+
>
> Same as above. We could probably continue using mca_cfg for uniformity.
There are 13 mca_cfg references total in that function so having it
shorter with a declaration at the beginning of the function of what cfg
is, is better IMO.
[ … ]
> > __init mcheck_enable(char *str)
> > {
> >+ struct mca_config *cfg = &mca_cfg;
> >+
>
> Same as above.
Ditto, 8 refs here.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
On Wed, 2012-10-17 at 13:13 +0200, Borislav Petkov wrote:
> mce_ser, mce_bios_cmci_threshold and mce_disabled are the last three
> bools which need conversion. Move them to the mca_config struct and
> adjust usage sites accordingly.
[]
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
[]
> @@ -1009,6 +1004,7 @@ static void mce_clear_info(struct mce_info *mi)
> */
> void do_machine_check(struct pt_regs *regs, long error_code)
> {
> + struct mca_config *cfg = &mca_cfg;
trivia:
Why use cfg?
This indirection and the cfg-> uses just seem obfuscating.
Isn't mca_cfg. used everywhere else?
No line wrapping is saved by indirecting.
> @@ -1036,7 +1032,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
>
> this_cpu_inc(mce_exception_count);
>
> - if (!mca_cfg.banks)
> + if (!cfg->banks)
On Wed, Oct 17, 2012 at 01:13:51PM +0200, Borislav Petkov wrote:
> +ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct dev_ext_attribute *ea = to_ext_attr(attr);
> + u8 bval;
> +
> + if (kstrtou8(buf, 10, &bval) < 0)
> + return -EINVAL;
> +
> + *(bool *)ea->var = !!bval;
> +
> + return size;
> +}
> +EXPORT_SYMBOL_GPL(device_store_bool);
Wouldn't it be nice to be able to accept 'y' 'Y' 'n' 'N' '0' and '1'
like the module bool parameter code does? That would make things a bit
more consistent, and possibly allow you to use this function for the
module sysfs file handling as well.
greg k-h
On Wed, Oct 17, 2012 at 12:03:00PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Oct 17, 2012 at 01:13:51PM +0200, Borislav Petkov wrote:
> > +ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + struct dev_ext_attribute *ea = to_ext_attr(attr);
> > + u8 bval;
> > +
> > + if (kstrtou8(buf, 10, &bval) < 0)
> > + return -EINVAL;
> > +
> > + *(bool *)ea->var = !!bval;
> > +
> > + return size;
> > +}
> > +EXPORT_SYMBOL_GPL(device_store_bool);
>
> Wouldn't it be nice to be able to accept 'y' 'Y' 'n' 'N' '0' and '1'
> like the module bool parameter code does? That would make things a bit
> more consistent, and possibly allow you to use this function for the
> module sysfs file handling as well.
That could be easily done, let me try it tomorrow. Any other letters you
prefer? Maybe, 'G' 'r' 'e' 'g'... ?
:-)
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
On Wed, Oct 17, 2012 at 10:10:18PM +0200, Borislav Petkov wrote:
> On Wed, Oct 17, 2012 at 12:03:00PM -0700, Greg Kroah-Hartman wrote:
> > On Wed, Oct 17, 2012 at 01:13:51PM +0200, Borislav Petkov wrote:
> > > +ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
> > > + const char *buf, size_t size)
> > > +{
> > > + struct dev_ext_attribute *ea = to_ext_attr(attr);
> > > + u8 bval;
> > > +
> > > + if (kstrtou8(buf, 10, &bval) < 0)
> > > + return -EINVAL;
> > > +
> > > + *(bool *)ea->var = !!bval;
> > > +
> > > + return size;
> > > +}
> > > +EXPORT_SYMBOL_GPL(device_store_bool);
> >
> > Wouldn't it be nice to be able to accept 'y' 'Y' 'n' 'N' '0' and '1'
> > like the module bool parameter code does? That would make things a bit
> > more consistent, and possibly allow you to use this function for the
> > module sysfs file handling as well.
>
> That could be easily done, let me try it tomorrow. Any other letters you
> prefer? Maybe, 'G' 'r' 'e' 'g'... ?
Heh, no, but that would be funny :)
Take the function already from the module code, it should be reusable,
if not, it would be a good idea to make it so.
thanks,
greg k-h
On Wed, Oct 17, 2012 at 01:29:00PM -0700, Greg Kroah-Hartman wrote:
> Take the function already from the module code, it should be reusable,
> if not, it would be a good idea to make it so.
Function from module code? Do you mean param_get_bool/param_set_bool in
kernel/params.c?
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
On Wed, Oct 17, 2012 at 10:36:12PM +0200, Borislav Petkov wrote:
> On Wed, Oct 17, 2012 at 01:29:00PM -0700, Greg Kroah-Hartman wrote:
> > Take the function already from the module code, it should be reusable,
> > if not, it would be a good idea to make it so.
>
> Function from module code? Do you mean param_get_bool/param_set_bool in
> kernel/params.c?
Yes, that's what I was thinking of, but I guess it just comes down to a
call to strtobool(), which is what you need here.
greg k-h