public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86, perf, nmi:  Disable perf if counters are not accessable
@ 2010-11-22 21:55 Don Zickus
  2010-11-23 10:46 ` Peter Zijlstra
  2010-11-26 15:01 ` [tip:perf/core] x86, perf, nmi: Disable perf if counters are not accessible tip-bot for Don Zickus
  0 siblings, 2 replies; 14+ messages in thread
From: Don Zickus @ 2010-11-22 21:55 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Peter Zijlstra, jason.wessel, gorcunov, LKML, Don Zickus

In a kvm virt guests, the perf counters are not emulated.  Instead they
return zero on a rdmsrl.  The perf nmi handler uses the fact that crossing
a zero means the counter overflowed (for those counters that do not have
specific interrupt bits).  Therefore on kvm guests, perf will swallow all
NMIs thinking the counters overflowed.

This causes problems for subsystems like kgdb which needs NMIs to do its
magic.  This problem was discovered by running kgdb tests.

The solution is to write garbage into a perf counter during the
initialization and hopefully reading back the same number.  On kvm
guests, the value will be read back as zero and we disable perf as
a result.

Reported-by: Jason Wessel <jason.wessel@windriver.com>
Patch-inspired-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/kernel/cpu/perf_event.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index bbe3c4a..e4c4899 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -381,6 +381,19 @@ static void release_pmc_hardware(void) {}
 
 #endif
 
+static bool check_hw_exists(void)
+{
+	u64 val, val_new;
+
+	val = 0xabcdUL;
+	(void) checking_wrmsrl(x86_pmu.perfctr, val);
+	rdmsrl_safe(x86_pmu.perfctr, &val_new);
+	if (val != val_new)
+		return false;
+
+	return true;
+}
+
 static void reserve_ds_buffers(void);
 static void release_ds_buffers(void);
 
@@ -1371,6 +1384,12 @@ void __init init_hw_perf_events(void)
 
 	pmu_check_apic();
 
+	/* sanity check that the hardware exists or is emulated */
+	if (!check_hw_exists()) {
+		pr_cont("Broken PMU hardware detected, software events only.\n");
+		return;
+	}
+
 	pr_cont("%s PMU driver.\n", x86_pmu.name);
 
 	if (x86_pmu.quirks)
-- 
1.7.3.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH] x86, perf, nmi: Disable perf if counters are not accessable
@ 2010-11-23 15:15 Sedat Dilek
  2010-11-23 15:19 ` Peter Zijlstra
  2010-11-23 16:56 ` Don Zickus
  0 siblings, 2 replies; 14+ messages in thread
From: Sedat Dilek @ 2010-11-23 15:15 UTC (permalink / raw)
  To: LKML; +Cc: Peter Zijlstra, dzickus

Hi,

I am seeing for a while this warning in my system-wide logs:

Nov 23 11:54:14 tbox kernel: [    0.040335] NMI watchdog failed to
create perf event on cpu0: ffffffa1

As I saw this patch from [1], I was hoping it's also fixing my problem
on an Intel Pentium-M (Banias) Single-Core CPU:

"So I changed it to:

static bool check_hw_exists(void)
{
        u64 val, val_new = 0;
        int ret = 0;
        val = 0xabcdUL;
        ret |= checking_wrmsrl(x86_pmu.perfctr, val);
        ret |= rdmsrl_safe(x86_pmu.perfctr, &val_new);
        if (ret || val != val_new)
                return false;
        return true;
}
And have applied the patch,

Thanks Don!"

Where did you apply it? Shouldn't that be in linux-2.6-tip perf/* GIT tree?

Regards,
- Sedat -

[1] http://lkml.org/lkml/2010/11/23/121
[2] http://git.kernel.org/?p=linux/kernel/git/tip/linux-2.6-tip.git;a=summary

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] x86, perf, nmi: Disable perf if counters are not accessable
@ 2010-11-23 19:18 Cyrill Gorcunov
  0 siblings, 0 replies; 14+ messages in thread
From: Cyrill Gorcunov @ 2010-11-23 19:18 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Don Zickus, sedat.dilek, LKML, Ingo Molnar

On Tue, Nov 23, 2010 at 08:07:47PM +0100, Peter Zijlstra wrote:
> On Tue, 2010-11-23 at 22:04 +0300, Cyrill Gorcunov wrote:
> > On Tue, Nov 23, 2010 at 07:27:07PM +0100, Peter Zijlstra wrote:
> > > On Tue, 2010-11-23 at 19:21 +0100, Sedat Dilek wrote:
> > > > Due to BIOS l(ocal)apic is not possible:
> > > > 
> > > > # dmesg | grep -i apic
> > > > [    0.000000] Using APIC driver default
> > > > [    0.000000] Local APIC disabled by BIOS -- you can enable it with "lapic"
> > > > [    0.000000] APIC: disable apic facility
> > > > [    0.000000] APIC: switched to apic NOOP
> > > > [    0.008891] no APIC, boot with the "lapic" boot parameter to force-enable it.
> > > > [    0.036141] Local APIC not detected. Using dummy APIC emulation. 
> > > 
> > > Have you tried booting with "lapic" as the second last msg suggests you
> > > do?
> > 
> >  Peter, Don, might not we need something like the patch below -- ie to check for
> > apic earlier and do not acquire cpu for PERF cpu bit, and its cpu model, etc
> > if there is no active apic? And perhaps for nmi-watchdog, we should not try
> > to creat perf event for same reason and simply report that nmi-watchdog is
> > disabled (though of course hpet based one should try to continue).
> > 
> >  No?
> > 
> 
> Ah, no.. now I get what you mean.
> 
> We can use the pmu without interrupt with we miss the lapic, that is
> perf-stat will still work.
> 

Yeah, I forgot about perf-stat

 Cyrill

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-11-26 15:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-22 21:55 [PATCH] x86, perf, nmi: Disable perf if counters are not accessable Don Zickus
2010-11-23 10:46 ` Peter Zijlstra
2010-11-23 14:13   ` Don Zickus
2010-11-26 15:01 ` [tip:perf/core] x86, perf, nmi: Disable perf if counters are not accessible tip-bot for Don Zickus
  -- strict thread matches above, loose matches on Subject: below --
2010-11-23 15:15 [PATCH] x86, perf, nmi: Disable perf if counters are not accessable Sedat Dilek
2010-11-23 15:19 ` Peter Zijlstra
2010-11-23 16:56 ` Don Zickus
2010-11-23 18:21   ` Sedat Dilek
2010-11-23 18:27     ` Peter Zijlstra
2010-11-23 18:29       ` Sedat Dilek
2010-11-23 18:37         ` Sedat Dilek
2010-11-23 19:04       ` Cyrill Gorcunov
2010-11-23 19:07         ` Peter Zijlstra
2010-11-23 19:18 Cyrill Gorcunov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox