All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Thomas Gleixner <tglx@kernel.org>,
	 Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  Paolo Bonzini <pbonzini@redhat.com>,
	linux-perf-users@vger.kernel.org,  linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org,  Mingwei Zhang <mizhang@google.com>,
	Stephane Eranian <eranian@google.com>,
	 Dapeng Mi <dapeng1.mi@linux.intel.com>
Subject: Re: [PATCH v2 4/4] perf/x86: KVM: Have perf define a dedicated struct for getting guest PEBS data
Date: Mon, 4 May 2026 10:19:35 -0700	[thread overview]
Message-ID: <afjU2ZbwRq7Jig0w@google.com> (raw)
In-Reply-To: <CALMp9eSt-ZDCxhyVjEiov2475ToZ7M_94HSpKg6hqrh=m3iZAg@mail.gmail.com>

On Fri, May 01, 2026, Jim Mattson wrote:
> On Thu, Apr 23, 2026 at 8:03 AM Sean Christopherson <seanjc@google.com> wrote:
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index 7403ca721b6a..04d9c51335d7 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -14,7 +14,6 @@
> >  #include <linux/slab.h>
> >  #include <linux/export.h>
> >  #include <linux/nmi.h>
> > -#include <linux/kvm_host.h>
> >
> >  #include <asm/cpufeature.h>
> >  #include <asm/debugreg.h>
> > @@ -4992,11 +4991,11 @@ static int intel_pmu_hw_config(struct perf_event *event)
> >   * when it uses {RD,WR}MSR, which should be handled by the KVM context,
> >   * specifically in the intel_pmu_{get,set}_msr().
> >   */
> > -static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr, void *data)
> > +static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr,
> > +                                                         struct x86_guest_pebs *guest_pebs)
> >  {
> >         struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> >         struct perf_guest_switch_msr *arr = cpuc->guest_switch_msrs;
> > -       struct kvm_pmu *kvm_pmu = (struct kvm_pmu *)data;
> >         u64 intel_ctrl = hybrid(cpuc->pmu, intel_ctrl);
> >         u64 pebs_mask = cpuc->pebs_enabled & x86_pmu.pebs_capable;
> >         u64 guest_pebs_mask = pebs_mask & ~cpuc->intel_ctrl_host_mask;
> > @@ -5052,7 +5051,7 @@ static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr, void *data)
> >          * wrong counter(s).  Similarly, disallow PEBS in the guest if the host
> >          * is using PEBS, to avoid bleeding host state into PEBS records.
> >          */
> > -       guest_pebs_mask &= kvm_pmu->pebs_enable & ~kvm_pmu->host_cross_mapped_mask;
> > +       guest_pebs_mask &= guest_pebs->enable & ~guest_pebs->cross_mapped_mask;
> 
> It would be helpful to save this mask somewhere, so that it can be
> used when calculating guest_pebs_idxs in x86_pmu_handle_guest_pebs().
> I think that code needs a fix similar to the one in commit
> 58f6217e5d01 ("perf/x86/intel: KVM: Mask PEBS_ENABLE loaded for guest
> with vCPU's value.").

Blech.  This all feels like a losing game of whack-a-mole.  Proxying the PMU
through perf is a mediocre approximation for non-PEBS events, and it seems like
it's downright awful for PEBS.  Ideally, we'd just rip out all of the perf-based
PEBS virtualization support, and only support PEBS through the mediated PMU.  :-/

Absent drastic measures though, saving the effective guest_pebs_enable in the
per-CPU tracking does seem like the least awful approach.  Though I don't quite
understand why we can't use GLOBAL_STATUS for x86_pmu_handle_guest_pebs().  I.e.
what happens if x86_pmu_handle_guest_pebs() only processes counters that actually
got marked as overflowing?

Regardless, I'm not going to try and address that mess in this series.  AFAICT,
it's not urgent, and I don't want to snowball into a broader cleanup.

  reply	other threads:[~2026-05-04 17:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-23 15:03 [PATCH v2 0/4] perf/x86: Don't write PEBS_ENABLED on KVM transitions Sean Christopherson
2026-04-23 15:03 ` [PATCH v2 1/4] perf/x86/intel: Don't write PEBS_ENABLED on host<=>guest xfers if CPU has isolation Sean Christopherson
2026-04-23 16:22   ` Peter Zijlstra
2026-04-23 17:59   ` Jim Mattson
2026-04-27  2:10     ` Mi, Dapeng
2026-04-27 18:07       ` Sean Christopherson
2026-04-28  2:32         ` Mi, Dapeng
2026-04-23 15:03 ` [PATCH v2 2/4] perf/x86/intel: Don't context switch DS_AREA (and PEBS config) if PEBS is unused Sean Christopherson
2026-04-27  2:24   ` Mi, Dapeng
2026-04-27 17:34     ` Sean Christopherson
2026-04-23 15:03 ` [PATCH v2 3/4] perf/x86/intel: Make @data a mandatory param for intel_guest_get_msrs() Sean Christopherson
2026-04-27  2:28   ` Mi, Dapeng
2026-04-23 15:03 ` [PATCH v2 4/4] perf/x86: KVM: Have perf define a dedicated struct for getting guest PEBS data Sean Christopherson
2026-04-23 18:14   ` Jim Mattson
2026-04-23 23:31   ` sashiko-bot
2026-04-27 17:37     ` Sean Christopherson
2026-05-02  0:04   ` Jim Mattson
2026-05-04 17:19     ` Sean Christopherson [this message]
2026-05-04 19:42       ` Jim Mattson
2026-05-06 14:02         ` Sean Christopherson
2026-04-23 15:33 ` [PATCH v2 0/4] perf/x86: Don't write PEBS_ENABLED on KVM transitions Jim Mattson
2026-04-23 16:16 ` Peter Zijlstra
2026-04-24 12:17   ` Mi, Dapeng
2026-04-24 12:23     ` Peter Zijlstra

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=afjU2ZbwRq7Jig0w@google.com \
    --to=seanjc@google.com \
    --cc=acme@kernel.org \
    --cc=bp@alien8.de \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mizhang@google.com \
    --cc=namhyung@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@kernel.org \
    --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.