From: Peter Zijlstra <peterz@infradead.org>
To: Fernand Sieber <sieberf@amazon.com>
Cc: seanjc@google.com, pbonzini@redhat.com,
"Jan H. Schönherr" <jschoenh@amazon.de>,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
dwmw@amazon.co.uk, hborghor@amazon.de, nh-open-source@amazon.com,
abusse@amazon.de, nsaenz@amazon.com, stable@vger.kernel.org
Subject: Re: [PATCH] KVM: x86/pmu: Do not accidentally create BTS events
Date: Tue, 2 Dec 2025 13:44:23 +0100 [thread overview]
Message-ID: <20251202124423.GC2458571@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20251202100311.GB2458571@noisy.programming.kicks-ass.net>
On Tue, Dec 02, 2025 at 11:03:11AM +0100, Peter Zijlstra wrote:
> On Mon, Dec 01, 2025 at 04:23:57PM +0200, Fernand Sieber wrote:
> > arch/x86/kvm/pmu.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> > index 487ad19a236e..547512028e24 100644
> > --- a/arch/x86/kvm/pmu.c
> > +++ b/arch/x86/kvm/pmu.c
> > @@ -225,6 +225,19 @@ static u64 get_sample_period(struct kvm_pmc *pmc, u64 counter_value)
> > {
> > u64 sample_period = (-counter_value) & pmc_bitmask(pmc);
> >
> > + /*
> > + * A sample_period of 1 might get mistaken by perf for a BTS event, see
> > + * intel_pmu_has_bts_period(). This would prevent re-arming the counter
> > + * via pmc_resume_counter(), followed by the accidental creation of an
> > + * actual BTS event, which we do not want.
> > + *
> > + * Avoid this by bumping the sampling period. Note, that we do not lose
> > + * any precision, because the same quirk happens later anyway (for
> > + * different reasons) in x86_perf_event_set_period().
> > + */
> > + if (sample_period == 1)
> > + sample_period = 2;
> > +
> > if (!sample_period)
> > sample_period = pmc_bitmask(pmc) + 1;
> > return sample_period;
>
> Oh gawd, I so hate this kvm code. It is so ludicrously bad. The way it
> keeps recreating counters is just stupid. And then they complain it
> sucks, it does :-(
>
> Anyway, yes this is terrible. Let me try and untangle all this, see if
> there's a saner solution.
Does something like so work? It is still terrible, but perhaps slightly
less so.
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 2b969386dcdd..493e6ba51e06 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1558,13 +1558,22 @@ static inline bool intel_pmu_has_bts_period(struct perf_event *event, u64 period
struct hw_perf_event *hwc = &event->hw;
unsigned int hw_event, bts_event;
- if (event->attr.freq)
+ /*
+ * Only use BTS for fixed rate period==1 events.
+ */
+ if (event->attr.freq || period != 1)
+ return false;
+
+ /*
+ * BTS doesn't virtualize.
+ */
+ if (event->attr.exclude_host)
return false;
hw_event = hwc->config & INTEL_ARCH_EVENT_MASK;
bts_event = x86_pmu.event_map(PERF_COUNT_HW_BRANCH_INSTRUCTIONS);
- return hw_event == bts_event && period == 1;
+ return hw_event == bts_event;
}
static inline bool intel_pmu_has_bts(struct perf_event *event)
next prev parent reply other threads:[~2025-12-02 12:44 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-01 14:23 [PATCH] KVM: x86/pmu: Do not accidentally create BTS events Fernand Sieber
2025-12-01 14:26 ` kernel test robot
2025-12-01 14:45 ` Woodhouse, David
2025-12-02 9:35 ` Peter Zijlstra
2025-12-02 9:59 ` Fernand Sieber
2025-12-02 2:19 ` Like Xu
2025-12-02 10:03 ` Peter Zijlstra
2025-12-02 12:44 ` Peter Zijlstra [this message]
2025-12-02 16:03 ` Sean Christopherson
2025-12-10 10:11 ` Fernand Sieber
2025-12-10 11:16 ` Peter Zijlstra
2025-12-11 18:36 ` [PATCH v2] perf/x86/intel: Do not enable BTS for guests Fernand Sieber
2025-12-11 18:38 ` kernel test robot
2026-01-14 17:25 ` David Woodhouse
2026-01-21 13:57 ` Fernand Sieber
2026-01-21 15:31 ` Peter Zijlstra
2026-01-21 15:50 ` [tip: perf/urgent] " tip-bot2 for Fernand Sieber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251202124423.GC2458571@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=abusse@amazon.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=dwmw@amazon.co.uk \
--cc=hborghor@amazon.de \
--cc=hpa@zytor.com \
--cc=jschoenh@amazon.de \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=nh-open-source@amazon.com \
--cc=nsaenz@amazon.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=sieberf@amazon.com \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.