Re: [PATCH] x86: mce: Honour bios-set CMCI threshold

From: Naveen N. Rao
Date: Mon Aug 27 2012 - 00:58:47 EST


On 08/22/2012 06:16 PM, Borislav Petkov wrote:
On Wed, Aug 22, 2012 at 06:00:54PM +0530, Naveen N. Rao wrote:
The ACPI spec doesn't provide for a way for the bios to pass down
recommended thresholds to the OS on a _per-bank_ basis. This patch adds
a new boot option, which if passed, allows bios to initialize the CMCI
threshold. In such a case, we simply skip programming any threshold
value.

As fail-safe, we initialize threshold to 1 if some banks have not been
initialized by the bios and warn the user.

Signed-off-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
---
Documentation/x86/x86_64/boot-options.txt | 5 ++++
arch/x86/include/asm/mce.h | 1 +
arch/x86/kernel/cpu/mcheck/mce.c | 4 +++
arch/x86/kernel/cpu/mcheck/mce_intel.c | 39 +++++++++++++++++++++++++++--
4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
index c54b4f5..ec92540 100644
--- a/Documentation/x86/x86_64/boot-options.txt
+++ b/Documentation/x86/x86_64/boot-options.txt
@@ -50,6 +50,11 @@ Machine check
monarchtimeout:
Sets the time in us to wait for other CPUs on machine checks. 0
to disable.
+ mce=bios_cmci_threshold
+ Don't overwrite the bios-set CMCI threshold. This boot option
+ prevents Linux from overwriting the CMCI threshold set by the
+ bios. Without this option, Linux always sets the CMCI
+ threshold to 1.

nomce (for compatibility with i386): same as mce=off

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index a3ac52b..8ad5078 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -171,6 +171,7 @@ DECLARE_PER_CPU(struct device *, mce_device);
#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);
void cmci_reenable(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 292d025..401359d 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -82,6 +82,7 @@ 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_bios_cmci_threshold __read_mostly;
int mce_ser __read_mostly;

AFAICT, this is actually a single-bit flag but we're using a whole
integer for it and from looking at the other boot options a couple of
them are used as flags too.

Care to define a

struct boot_flags {
__u64 mce_bios_cmci_threshold : 1,
__reserved : 63;
};

and use

boot_flags.mce_bios_cmci_threshold

in the conditionals below instead?

Sure - sounds like a good idea. Further, a #define could eliminate the need to change other references, but I'm not sure that's GENERALLacceptable

#define mce_bios_cmci_threshold boot_flags.mce_bios_cmci_threshold

could eliminate the need to change other references, but I'm not sure that's acceptable

But, I just had a quick look and it seems to me that these were defined as integers since they are exposed via sysfs. For instance:

static struct dev_ext_attribute dev_attr_cmci_disabled = {
__ATTR(cmci_disabled, 0644, device_show_int, set_cmci_disabled),
&mce_cmci_disabled
};

Converting mce_cmci_disabled to a bit-field doesn't work since we take its address above. We could ignore and not set the second field at all (dev_ext_attribute->var) and define our own callbacks, but that'll be more work and I'm not sure if we work fine without



I'll try to convert the rest of them to that struct and thus save some
more space...

Thanks.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/