* [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU
@ 2010-09-03 9:13 Peter Zijlstra
2010-09-03 13:53 ` Arjan van de Ven
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2010-09-03 9:13 UTC (permalink / raw)
To: Ingo Molnar, Stephane Eranian, robert.richter
Cc: Linus Torvalds, LKML, Arjan Van De Ven, H. Peter Anvin
Disable all of perf if we find any active PMCs on boot.
It has been reported that some BIOSes access the PMU, for obscure things
better done by the OS. Come down hard on this practice and fully disable
the PMU.
[ Boot tested on a westmere system with a sane BIOS -- I don't actually
have an affected system to test this on. ]
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
Robert, do you know if any AMD system BIOSes carry similar
Feat^H^H^H^HFailureAdd?
arch/x86/kernel/cpu/perf_event_intel.c | 35 ++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index ee05c90..6849653 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -923,6 +923,19 @@ static void intel_clovertown_quirks(void)
x86_pmu.pebs_constraints = NULL;
}
+static void print_BIOS_fail(void)
+{
+ printk(KERN_ERR "\n");
+ printk(KERN_ERR "=============================================\n");
+ printk(KERN_ERR "It appears the BIOS is actively using the PMU\n");
+ printk(KERN_ERR "this avoids Linux from using it, please de- \n");
+ printk(KERN_ERR "activate this BIOS feature or request a BIOS \n");
+ printk(KERN_ERR "update from your vendor. \n");
+ printk(KERN_ERR "=============================================\n");
+
+ memset(&x86_pmu, 0, sizeof(x86_pmu));
+}
+
static __init int intel_pmu_init(void)
{
union cpuid10_edx edx;
@@ -930,6 +943,8 @@ static __init int intel_pmu_init(void)
unsigned int unused;
unsigned int ebx;
int version;
+ u64 val;
+ int i;
if (!cpu_has(&boot_cpu_data, X86_FEATURE_ARCH_PERFMON)) {
switch (boot_cpu_data.x86) {
@@ -968,6 +983,26 @@ static __init int intel_pmu_init(void)
x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
/*
+ * Check to see if the BIOS enabled any of the counters, if so
+ * complain and bail.
+ */
+ for (i = 0; i < x86_pmu.num_counters; i++) {
+ rdmsrl(x86_pmu.eventsel + i, val);
+ if (val & ARCH_PERFMON_EVENTSEL_ENABLE) {
+ print_BIOS_fail();
+ return -EBUSY;
+ }
+ }
+
+ for (i = 0; i < x86_pmu.num_counters_fixed; i++) {
+ rdmsrl(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, val);
+ if (val & (0x03 << i*4)) {
+ print_BIOS_fail();
+ return -EBUSY;
+ }
+ }
+
+ /*
* v2 and above have a perf capabilities MSR
*/
if (version > 1) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU
2010-09-03 9:13 [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU Peter Zijlstra
@ 2010-09-03 13:53 ` Arjan van de Ven
2010-09-03 14:32 ` Matt Domsch
0 siblings, 1 reply; 6+ messages in thread
From: Arjan van de Ven @ 2010-09-03 13:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Stephane Eranian, robert.richter, Linus Torvalds,
LKML, H. Peter Anvin
On 9/3/2010 2:13 AM, Peter Zijlstra wrote:
>
> +static void print_BIOS_fail(void)
> +{
> + printk(KERN_ERR "\n");
> + printk(KERN_ERR "=============================================\n");
> + printk(KERN_ERR "It appears the BIOS is actively using the PMU\n");
> + printk(KERN_ERR "this avoids Linux from using it, please de- \n");
> + printk(KERN_ERR "activate this BIOS feature or request a BIOS \n");
> + printk(KERN_ERR "update from your vendor. \n");
> + printk(KERN_ERR "=============================================\n");
> +
> + memset(&x86_pmu, 0, sizeof(x86_pmu));
> +}
> +
>
tell us how you really feel :-)
do you want to add a phone number of the support line of the vendor
(based on DMI data of course) ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU
2010-09-03 13:53 ` Arjan van de Ven
@ 2010-09-03 14:32 ` Matt Domsch
2010-09-03 14:46 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Matt Domsch @ 2010-09-03 14:32 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Peter Zijlstra, Ingo Molnar, Stephane Eranian, robert.richter,
Linus Torvalds, LKML, H. Peter Anvin
On Fri, Sep 03, 2010 at 06:53:59AM -0700, Arjan van de Ven wrote:
> On 9/3/2010 2:13 AM, Peter Zijlstra wrote:
> >
> >+static void print_BIOS_fail(void)
> >+{
> >+ printk(KERN_ERR "\n");
> >+ printk(KERN_ERR "=============================================\n");
> >+ printk(KERN_ERR "It appears the BIOS is actively using the PMU\n");
> >+ printk(KERN_ERR "this avoids Linux from using it, please de- \n");
> >+ printk(KERN_ERR "activate this BIOS feature or request a BIOS \n");
> >+ printk(KERN_ERR "update from your vendor. \n");
> >+ printk(KERN_ERR "=============================================\n");
> >+
> >+ memset(&x86_pmu, 0, sizeof(x86_pmu));
> >+}
> >+
> >
>
>
> tell us how you really feel :-)
>
> do you want to add a phone number of the support line of the vendor
> (based on DMI data of course) ?
I must object to messages that repeatedly (at least on every boot)
tell system administrators to contact their hardware vendor's support
lines, when it's not clear what the BIOS is doing is incorrect. There
are plenty of valid reasons why BIOS itself would use PMU counters.
Dell PowerEdge server power management, handled by the BIOS, certainly
does use one.
My understanding is that there is a mechanism for the OS to request
BIOS to release use of PMU counters. Are we doing that? If BIOS does
not release the counters when asked, ok, that's something to
(potentially) warn about. But blanket "BIOS is using a CPU feature!
Bad BIOS! No treat for you!" - that's not helpful to anyone.
Thanks,
Matt
--
Matt Domsch
Technology Strategist
Dell | Office of the CTO
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU
2010-09-03 14:32 ` Matt Domsch
@ 2010-09-03 14:46 ` Peter Zijlstra
2010-09-07 15:15 ` Robert Richter
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2010-09-03 14:46 UTC (permalink / raw)
To: Matt Domsch
Cc: Arjan van de Ven, Ingo Molnar, Stephane Eranian, robert.richter,
Linus Torvalds, LKML, H. Peter Anvin
On Fri, 2010-09-03 at 09:32 -0500, Matt Domsch wrote:
> On Fri, Sep 03, 2010 at 06:53:59AM -0700, Arjan van de Ven wrote:
> > On 9/3/2010 2:13 AM, Peter Zijlstra wrote:
> > >
> > >+static void print_BIOS_fail(void)
> > >+{
> > >+ printk(KERN_ERR "\n");
> > >+ printk(KERN_ERR "=============================================\n");
> > >+ printk(KERN_ERR "It appears the BIOS is actively using the PMU\n");
> > >+ printk(KERN_ERR "this avoids Linux from using it, please de- \n");
> > >+ printk(KERN_ERR "activate this BIOS feature or request a BIOS \n");
> > >+ printk(KERN_ERR "update from your vendor. \n");
> > >+ printk(KERN_ERR "=============================================\n");
> > >+
> > >+ memset(&x86_pmu, 0, sizeof(x86_pmu));
> > >+}
> I must object to messages that repeatedly (at least on every boot)
> tell system administrators to contact their hardware vendor's support
> lines, when it's not clear what the BIOS is doing is incorrect.
It is using the PMU, it should not _ever_ do that.
> There
> are plenty of valid reasons why BIOS itself would use PMU counters.
> Dell PowerEdge server power management, handled by the BIOS, certainly
> does use one.
Them make them stop doing that, or at least provide a BIOS option to
disable this Feat^WFailure-add.
An no, doing power-management from the BIOS is most certainly not a
valid reason. That's not what BIOSes are for, a BIOS should bring up the
system and then sod off.
> My understanding is that there is a mechanism for the OS to request
> BIOS to release use of PMU counters. Are we doing that?
I'm not aware of any such thing. The Intel Arch docs most certainly
don't specify anything about that.
> If BIOS does
> not release the counters when asked, ok, that's something to
> (potentially) warn about. But blanket "BIOS is using a CPU feature!
> Bad BIOS! No treat for you!" - that's not helpful to anyone.
Again, I'm not aware there is a spec on how to ask anything of the BIOS,
let alone a part pertaining to PMU functionality.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU
2010-09-03 14:46 ` Peter Zijlstra
@ 2010-09-07 15:15 ` Robert Richter
2010-09-07 16:51 ` Robert Richter
0 siblings, 1 reply; 6+ messages in thread
From: Robert Richter @ 2010-09-07 15:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Matt Domsch, Arjan van de Ven, Ingo Molnar, Stephane Eranian,
Linus Torvalds, LKML, H. Peter Anvin
> > > On 9/3/2010 2:13 AM, Peter Zijlstra wrote:
> > > >+static void print_BIOS_fail(void)
> > > >+{
> > > >+ printk(KERN_ERR "\n");
> > > >+ printk(KERN_ERR "=============================================\n");
> > > >+ printk(KERN_ERR "It appears the BIOS is actively using the PMU\n");
> > > >+ printk(KERN_ERR "this avoids Linux from using it, please de- \n");
> > > >+ printk(KERN_ERR "activate this BIOS feature or request a BIOS \n");
> > > >+ printk(KERN_ERR "update from your vendor. \n");
> > > >+ printk(KERN_ERR "=============================================\n");
I would rather prefer this:
BIOS bug, cpu 1, invalid <register=value> ...
... which is a much better information on one line, explains the bug
and is also better parsable. I intend to implement messages like
this. So maybe we could find consensus with this or something similar.
A simple grep of dmesg will then give a list of BIOS bugs.
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU
2010-09-07 15:15 ` Robert Richter
@ 2010-09-07 16:51 ` Robert Richter
0 siblings, 0 replies; 6+ messages in thread
From: Robert Richter @ 2010-09-07 16:51 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Matt Domsch, Arjan van de Ven, Ingo Molnar, Stephane Eranian,
Linus Torvalds, LKML, H. Peter Anvin
On 07.09.10 17:15:41, Robert Richter wrote:
> > > > On 9/3/2010 2:13 AM, Peter Zijlstra wrote:
> > > > >+static void print_BIOS_fail(void)
> > > > >+{
> > > > >+ printk(KERN_ERR "\n");
> > > > >+ printk(KERN_ERR "=============================================\n");
> > > > >+ printk(KERN_ERR "It appears the BIOS is actively using the PMU\n");
> > > > >+ printk(KERN_ERR "this avoids Linux from using it, please de- \n");
> > > > >+ printk(KERN_ERR "activate this BIOS feature or request a BIOS \n");
> > > > >+ printk(KERN_ERR "update from your vendor. \n");
> > > > >+ printk(KERN_ERR "=============================================\n");
>
> I would rather prefer this:
>
> BIOS bug, cpu 1, invalid <register=value> ...
I got the note there is already a macro for this in kernel.h:
#define FW_BUG "[Firmware Bug]: "
So, we would have something like:
[Firmware Bug]: cpu 1, invalid <register=value> ...
in dmesg.
-Robert
>
> ... which is a much better information on one line, explains the bug
> and is also better parsable. I intend to implement messages like
> this. So maybe we could find consensus with this or something similar.
>
> A simple grep of dmesg will then give a list of BIOS bugs.
>
> -Robert
>
> --
> Advanced Micro Devices, Inc.
> Operating System Research Center
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-07 16:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-03 9:13 [PATCH] perf, x86: Disable perf if the BIOS got its grubby paws on the PMU Peter Zijlstra
2010-09-03 13:53 ` Arjan van de Ven
2010-09-03 14:32 ` Matt Domsch
2010-09-03 14:46 ` Peter Zijlstra
2010-09-07 15:15 ` Robert Richter
2010-09-07 16:51 ` Robert Richter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox