Re: [PATCH v2 4/4] perf/x86: KVM: Have perf define a dedicated struct for getting guest PEBS data

[Jim Mattson]

.

On Thu, Apr 23, 2026 at 8:03 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> Have perf define a struct for getting guest PEBS data from KVM instead of
> poking into the kvm_pmu structure.  Passing in an entire "struct kvm_pmu"
> _as an opaque pointer_ to get at four fields is silly, especially since
> one of the fields exists purely to convey information to perf, i.e. isn't
> used by KVM.
>
> Perf should also own its APIs, i.e. define what fields/data it needs, not
> rely on KVM to throw fields into data structures that effectively hold
> KVM-internal state.
>
> Opportunistically rephrase the comment about cross-mapped counters to
> explain *why* PEBS needs to be disabled.
>
> Reviewed-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> ...
> 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.").