perf, x86: Complain louder about BIOSen corrupting CPU/PMU state and continue

Commit-ID:  45daae575e08bbf7405c5a3633e956fa364d1b4f
Gitweb:     http://git.kernel.org/tip/45daae575e08bbf7405c5a3633e956fa364d1b4f
Author:     Ingo Molnar <[email protected]>
AuthorDate: Fri, 25 Mar 2011 10:24:23 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 25 Mar 2011 11:23:41 +0100

perf, x86: Complain louder about BIOSen corrupting CPU/PMU state and continue

Eric Dumazet reported that hardware PMU events do not work on his system, due to the BIOS corrupting PMU state:

Performance Events: PEBS fmt0+, Core2 events, Broken BIOS detected, using software events only. [Firmware Bug]: the BIOS has corrupted hw-PMU resources (MSR 186 is 43003c)

Linus suggested that we continue in the face of such BIOS-induced CPU state corruption:

http://lkml.org/lkml/2011/3/24/608

Such BIOSes will have to be fixed - Linux developers rely on a working and fully capable PMU and the BIOS interfering with the CPU's PMU state is simply not acceptable.

So this patch changes perf to continue when it detects such BIOS interaction, some hardware events may be unreliable due to the BIOS writing and re-writing them - there's not much the kernel can do about that but to detect the corruption and report it.

LKML-Reference: <new-submission>

---
 arch/x86/kernel/cpu/perf_event.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

index ec46eea..eb00677 100644
@@ -500,12 +500,17 @@ static bool check_hw_exists(void)
 	return true;
 
 bios_fail:
-	printk(KERN_CONT "Broken BIOS detected, using software events only.\n");
+	/*
+	 * We still allow the PMU driver to operate:
+	 */
+	printk(KERN_CONT "Broken BIOS detected, complain to your hardware vendor.\n");
 	printk(KERN_ERR FW_BUG "the BIOS has corrupted hw-PMU resources (MSR %x is %Lx)\n", reg, val);
-	return false;
+
+	return true;
 
 msr_fail:
 	printk(KERN_CONT "Broken PMU hardware detected, using software events only.\n");
+
 	return false;
 }
 

I don't think this should go into "urgent". It's more of an annoyance than anything we need to fix in 39.

Also, don't we need to make sure that the event isn't tied to the SCI line? I thought the affected BIOSen did something odd with the local apic setup too? But that might be some fevered nightmare dream of mine, and/or just a misunderstanding from the hallway discussion I've had with people about this.

This might also be something we do want to give a kernel command line option for. As in "bios=allow-annoying-pmu-thing" or whatever.

Linus

I don't think they actually can do this, there is only a single PMU interrupt line and we program that to the NMI vector on startup. Now the SMM crap could of course read out the APIC programming and put an SMI trap on whatever interrupt it points to, but I'm not sure its doing that. Afaik its simply polling state or somesuch.

I had such a patch, Ingo argued against it and we decided not to do it, but we could still.

---
Subject: perf, x86: Allow forcing the PMU
From: Peter Zijlstra <[email protected]>
Date: Thu Dec 09 15:03:11 CET 2010

Allow an admin to force the PMU, even though it appears to be in use.

Use with caustion, abusers get to keep all pieces.

---
 Documentation/kernel-parameters.txt |    6 ++++++
 arch/x86/kernel/cpu/perf_event.c    |   20 ++++++++++++++++++++
 2 files changed, 26 insertions(+)

Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -2020,6 +2020,12 @@ and is between 256 and 4096 characters.
 			allocator.  This parameter is primarily	for debugging
 			and performance comparison.
 
+	perf_event.force.	[x86] Force the usage of the PMU, even if there were
+			active counters. This usually indicates a broken BIOS
+			and forcing it will result in an ill-functioning system.
+			It is useful however with kexec/kdump when you know
+			the BIOS is good and the previous OS didn't clean up.
+
 	pf.		[PARIDE]
 			See Documentation/blockdev/paride.txt.
 

Index: linux-2.6/arch/x86/kernel/cpu/perf_event.c
===================================================================
@@ -43,6 +43,18 @@ do {								\
 #endif
 
 /*
+ * Kernel parameter to force the PMU despite it appearing in-use.
+ *
+ * If you use this when the BIOS actually has stolen a resource you will end up
+ * with an ill-functioning system.
+ *
+ * This flag is only useful for kexec/kdump like things where the previous
+ * kernel did set up PMU resources and forgot to shut them down.
+ */
+static int force;
+module_param(force, bool, 0444);
+
+/*
  * best effort, GUP based copy_from_user() that assumes IRQ or NMI context
  */
 static unsigned long

@@ -195,6 +207,7 @@ struct x86_pmu {
 	 */
 	const char	*name;
 	int		version;
+	int		forced;
 	int		(*handle_irq)(struct pt_regs *);
 	void		(*disable_all)(void);
 	void		(*enable_all)(int added);

@@ -415,6 +428,10 @@ static bool check_hw_exists(void)
 	return true;
 
 bios_fail:
+	if (force) {
+		x86_pmu.forced = 1;
+		return true;
+	}
 	printk(KERN_CONT "Broken BIOS detected, software events only.\n");
 	printk(KERN_ERR FW_BUG "the BIOS has corrupted hw-PMU resources (MSR: %x=%Lx)\n", reg, val);
 	return false;

@@ -1421,6 +1438,9 @@ int __init init_hw_perf_events(void)
 
 	pr_cont("%s PMU driver.\n", x86_pmu.name);
 
+	if (x86_pmu.forced)
+		printk(KERN_WARNING "WARNING: Forced the PMU, might not work correctly due to BIOS wreckage.\n");
+
 	if (x86_pmu.quirks)
 		x86_pmu.quirks();
 

Basically for many kernel releases we ignored the BIOS corrupting the PMU - so one could fairly argue that this patch fixes a perf functionality regression for Eric. We'll still warn in the syslog about the BIOS so there's an indication that something is going on.

Linus, which variant is the most efficient one in your opinion? I'm leaning slightly in favor of this patch in perf/urgent: it restores Eric's system to the before-BIOS-corruption-detection state, but also warns about the BIOS craziness.

Ingo