Re: [PATCH 08/15] KVM: x86/pmu: Switch to bitmask-based KVM PMU capabilities

[Jim Mattson]

On Tue, Jul 7, 2026 at 11:44 AM Zide Chen <zide.chen@xxxxxxxxx> wrote:
>
> From: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>
> Intel platforms support non-contiguous fixed counters via CPUID.0AH:ECX
> starting with PerfMon v5, and support non-contiguous GP counters
> through the Architectural PerfMon Extension (CPUID leaf 23H).
>
> struct x86_pmu_capability now exposes {,fixed_}cntr_mask64 bitmaps,
> which may contain sparse bits representing non-contiguous counters.
> Switch KVM's kvm_host_pmu and kvm_pmu_cap consumers over to the new
> bitmask fields.
>
> CPUID.0AH:EAX[15:8] and CPUID.0AH:EDX[4:0] enumerate only contiguous
> counters. Derive these values from kvm_pmu_cap.{,fixed_}cntr_mask64 as
> the number of consecutive counters starting at index 0.
>
> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> Co-developed-by: Zide Chen <zide.chen@xxxxxxxxx>
> Signed-off-by: Zide Chen <zide.chen@xxxxxxxxx>
> ---
>  arch/x86/kvm/cpuid.c         | 14 +++++++++++---
>  arch/x86/kvm/msrs.c          | 12 ++++++------
>  arch/x86/kvm/pmu.c           | 34 +++++++++++-----------------------
>  arch/x86/kvm/svm/pmu.c       |  2 +-
>  arch/x86/kvm/svm/svm.c       |  9 +++++----
>  arch/x86/kvm/vmx/pmu_intel.c |  7 ++++---
>  arch/x86/kvm/vmx/vmx.c       |  7 +++++--
>  7 files changed, 43 insertions(+), 42 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 2698fa42cd97..151a4794f834 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1514,10 +1514,18 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>                 }
>
>                 eax.split.version_id = kvm_pmu_cap.version;
> -               eax.split.num_counters = kvm_pmu_cap.num_counters_gp;
> +
> +               /* Contiguous GP counters only. */
> +               eax.split.num_counters =
> +                       find_first_zero_bit(kvm_pmu_cap.cntr_mask,
> +                                           KVM_MAX_NR_GP_COUNTERS);
>                 eax.split.bit_width = kvm_pmu_cap.bit_width_gp;
>                 eax.split.mask_length = kvm_pmu_cap.events_mask_len;
> -               edx.split.num_counters_fixed = kvm_pmu_cap.num_counters_fixed;
> +
> +               /* Contiguous fixed counters only. */
> +               edx.split.num_counters_fixed =
> +                       find_first_zero_bit(kvm_pmu_cap.fixed_cntr_mask,
> +                                           KVM_MAX_NR_FIXED_COUNTERS);
>                 edx.split.bit_width_fixed = kvm_pmu_cap.bit_width_fixed;
>
>                 if (kvm_pmu_cap.version)
> @@ -1882,7 +1890,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>
>                 cpuid_entry_override(entry, CPUID_8000_0022_EAX);
>
> -               ebx.split.num_core_pmc = kvm_pmu_cap.num_counters_gp;
> +               ebx.split.num_core_pmc = hweight64(kvm_pmu_cap.cntr_mask64);
>                 entry->ebx = ebx.full;
>                 break;
>         }
> diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
> index c751a8dbd45d..7524d019f1be 100644
> --- a/arch/x86/kvm/msrs.c
> +++ b/arch/x86/kvm/msrs.c
> @@ -2631,20 +2631,20 @@ static void kvm_probe_msr_to_save(u32 msr_index)
>                 break;
>         case MSR_ARCH_PERFMON_PERFCTR0 ...
>              MSR_ARCH_PERFMON_PERFCTR0 + KVM_MAX_NR_GP_COUNTERS - 1:
> -               if (msr_index - MSR_ARCH_PERFMON_PERFCTR0 >=
> -                   kvm_pmu_cap.num_counters_gp)
> +               if (!(BIT_ULL(msr_index - MSR_ARCH_PERFMON_PERFCTR0) &
> +                     kvm_pmu_cap.cntr_mask64))
>                         return;
>                 break;
>         case MSR_ARCH_PERFMON_EVENTSEL0 ...
>              MSR_ARCH_PERFMON_EVENTSEL0 + KVM_MAX_NR_GP_COUNTERS - 1:
> -               if (msr_index - MSR_ARCH_PERFMON_EVENTSEL0 >=
> -                   kvm_pmu_cap.num_counters_gp)
> +               if (!(BIT_ULL(msr_index - MSR_ARCH_PERFMON_EVENTSEL0) &
> +                     kvm_pmu_cap.cntr_mask64))
>                         return;
>                 break;
>         case MSR_ARCH_PERFMON_FIXED_CTR0 ...
>              MSR_ARCH_PERFMON_FIXED_CTR0 + KVM_MAX_NR_FIXED_COUNTERS - 1:
> -               if (msr_index - MSR_ARCH_PERFMON_FIXED_CTR0 >=
> -                   kvm_pmu_cap.num_counters_fixed)
> +               if (!(BIT_ULL(msr_index - MSR_ARCH_PERFMON_FIXED_CTR0) &
> +                     kvm_pmu_cap.fixed_cntr_mask64))
>                         return;
>                 break;
>         case MSR_AMD64_PERF_CNTR_GLOBAL_CTL:
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 3647ce3f0e3f..bc2ca60114e9 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -20,7 +20,6 @@
>  #include <asm/perf_event.h>
>  #include <asm/cpu_device_id.h>
>  #include "x86.h"
> -#include "cpuid.h"
>  #include "lapic.h"
>  #include "pmu.h"
>
> @@ -136,8 +135,6 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>  {
>         bool is_intel = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
>         int min_nr_gp_ctrs = pmu_ops->MIN_NR_GP_COUNTERS;
> -       union cpuid10_edx edx;
> -       u32 eax, ebx, ecx;
>
>         /*
>          * Hybrid PMUs don't play nice with virtualization without careful
> @@ -159,8 +156,8 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>                  * there are a non-zero number of counters, but fewer than what
>                  * is architecturally required.
>                  */
> -               if (!kvm_host_pmu.num_counters_gp ||
> -                   WARN_ON_ONCE(kvm_host_pmu.num_counters_gp < min_nr_gp_ctrs))
> +               if (!kvm_host_pmu.cntr_mask64 ||
> +                   WARN_ON_ONCE(hweight64(kvm_host_pmu.cntr_mask64) < min_nr_gp_ctrs))
>                         enable_pmu = false;
>                 else if (is_intel && !kvm_host_pmu.version)
>                         enable_pmu = false;
> @@ -180,23 +177,14 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>
>         memcpy(&kvm_pmu_cap, &kvm_host_pmu, sizeof(kvm_host_pmu));
>         kvm_pmu_cap.version = min(kvm_pmu_cap.version, 2);
> -       kvm_pmu_cap.num_counters_gp = min(kvm_pmu_cap.num_counters_gp,
> -                                         pmu_ops->MAX_NR_GP_COUNTERS);
> -       kvm_pmu_cap.num_counters_fixed = min(kvm_pmu_cap.num_counters_fixed,
> -                                            KVM_MAX_NR_FIXED_COUNTERS);
> +       kvm_pmu_cap.cntr_mask64 &=
> +               GENMASK_ULL(pmu_ops->MAX_NR_GP_COUNTERS - 1, 0);
> +       kvm_pmu_cap.fixed_cntr_mask64 &=
> +               GENMASK_ULL(KVM_MAX_NR_FIXED_COUNTERS - 1, 0);
>
> -       /*
> -        * Currently, KVM doesn't support non-contiguous fixed counters; make
> -        * sure only contiguous ones are retained in kvm_pmu_cap.
> -        */
> -       if (kvm_host_pmu.version >= 5) {
> -               cpuid(0xa, &eax, &ebx, &ecx, &edx.full);
> -               if (kvm_pmu_cap.num_counters_fixed > edx.split.num_counters_fixed)
> -                       kvm_pmu_cap.num_counters_fixed = edx.split.num_counters_fixed;
> -       }
> -
> -       if (!enable_mediated_pmu && kvm_pmu_cap.num_counters_fixed > 3)
> -               kvm_pmu_cap.num_counters_fixed = 3;
> +       /* Legacy vPMU exposes at most 3 fixed counters. */
> +       if (!enable_mediated_pmu)
> +               kvm_pmu_cap.fixed_cntr_mask64 &= GENMASK_ULL(2, 0);
>
>         kvm_pmu_eventsel.INSTRUCTIONS_RETIRED =
>                 perf_get_hw_event_config(PERF_COUNT_HW_INSTRUCTIONS);
> @@ -796,8 +784,8 @@ static bool kvm_need_any_pmc_intercept(struct kvm_vcpu *vcpu)
>          * KVM's capabilities are constrained based on KVM support, i.e. KVM's
>          * capabilities themselves may be a subset of hardware capabilities.
>          */
> -       return kvm_gp_pmc_mask(pmu) != BIT_ULL(kvm_host_pmu.num_counters_gp) - 1 ||
> -              kvm_fixed_pmc_mask(pmu) != BIT_ULL(kvm_host_pmu.num_counters_fixed) - 1;
> +       return kvm_gp_pmc_mask(pmu) != kvm_host_pmu.cntr_mask64 ||
> +              kvm_fixed_pmc_mask(pmu) != kvm_host_pmu.fixed_cntr_mask64;
>  }
>
>  bool kvm_need_perf_global_ctrl_intercept(struct kvm_vcpu *vcpu)
> diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
> index 02cf960a215a..d519eba518bf 100644
> --- a/arch/x86/kvm/svm/pmu.c
> +++ b/arch/x86/kvm/svm/pmu.c
> @@ -208,7 +208,7 @@ static void amd_pmu_refresh(struct kvm_vcpu *vcpu)
>         }
>
>         pmu->all_valid_pmc_mask64 = (BIT_ULL(nr_gp_counters) - 1) &
> -                                   (BIT_ULL(kvm_pmu_cap.num_counters_gp) - 1);
> +                                   kvm_pmu_cap.cntr_mask64;
>
>         if (pmu->version > 1) {
>                 pmu->global_ctrl_rsvd = ~pmu->all_valid_pmc_mask64;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3b3e98b6abb6..002cdd074fd7 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -754,6 +754,7 @@ static void svm_recalc_pmu_msr_intercepts(struct kvm_vcpu *vcpu)
>         bool intercept = !kvm_vcpu_has_mediated_pmu(vcpu);
>         struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>         unsigned long gp_mask = kvm_gp_pmc_mask(pmu);
> +       unsigned long host_only_gp_mask;
>         int i;
>
>         if (!enable_mediated_pmu)
> @@ -769,7 +770,8 @@ static void svm_recalc_pmu_msr_intercepts(struct kvm_vcpu *vcpu)
>                 svm_set_intercept_for_msr(vcpu, MSR_F15H_PERF_CTR + 2 * i,
>                                           MSR_TYPE_RW, intercept);

Our internal Sashiko says:

This is a pre-existing issue, but does the legacy MSR intercept need to be
reverted here as well when gp_mask shrinks?
When guest PMU capabilities are updated and gp_mask shrinks, the new
host_only_gp_mask loop correctly reinstates the intercepts for
MSR_F15H_PERF_CTR. However, looking at the code just above this hunk in
svm_recalc_pmu_msr_intercepts():
    /* Legacy counters are always available for AMD CPUs with a PMU. */
    for_each_set_bit(i, &gp_mask, AMD64_NUM_COUNTERS)
        svm_set_intercept_for_msr(vcpu, MSR_K7_PERFCTR0 + i,
                                  MSR_TYPE_RW, intercept);
There does not seem to be a corresponding loop to reinstate the intercept
for MSR_K7_PERFCTR0 + i.
If userspace initially exposes 4 counters, disabling intercepts for
MSR_K7_PERFCTR0 through MSR_K7_PERFCTR3, and later updates CPUID to expose
only 2 counters, could the removed legacy counters remain unintercepted?
This sequence might allow an unprivileged userspace to grant the guest
direct read/write access to host MSR_K7_PERFCTR MSRs without KVM
intercepting them.

> -       for ( ; i < kvm_pmu_cap.num_counters_gp; i++)
> +       host_only_gp_mask = kvm_pmu_cap.cntr_mask64 & ~gp_mask;
> +       kvm_for_each_set_pmc_idx(i, host_only_gp_mask, AMD_GP)
>                 svm_enable_intercept_for_msr(vcpu, MSR_F15H_PERF_CTR + 2 * i,
>                                              MSR_TYPE_RW);
>
> @@ -5574,9 +5576,8 @@ static __init void svm_set_cpu_caps(void)
>                  * access to enough counters to virtualize "core" support,
>                  * otherwise limit vPMU support to the legacy number of counters.
>                  */
> -               if (kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS_CORE)
> -                       kvm_pmu_cap.num_counters_gp = min(AMD64_NUM_COUNTERS,
> -                                                         kvm_pmu_cap.num_counters_gp);
> +               if (hweight64(kvm_pmu_cap.cntr_mask64) < AMD64_NUM_COUNTERS_CORE)
> +                       kvm_pmu_cap.cntr_mask64 &= GENMASK_ULL(AMD64_NUM_COUNTERS - 1, 0);
>                 else
>                         kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE);
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 31422bd20d96..425f17aa9be2 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -518,7 +518,8 @@ static u64 intel_get_fixed_pmc_eventsel(unsigned int index)
>          * have a known encoding for the associated general purpose event.
>          */
>         eventsel = perf_get_hw_event_config(fixed_pmc_perf_ids[index]);
> -       WARN_ON_ONCE(!eventsel && index < kvm_pmu_cap.num_counters_fixed);
> +       WARN_ON_ONCE(!eventsel &&
> +                    (kvm_pmu_cap.fixed_cntr_mask64 & BIT_ULL(index)));
>         return eventsel;
>  }
>
> @@ -575,7 +576,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
>         pmu->available_event_types = ~entry->ebx & (BIT_ULL(eax.split.mask_length) - 1);
>
>         fixed_cntr_mask = BIT_ULL(edx.split.num_counters_fixed) - 1;
> -       fixed_cntr_mask &= BIT_ULL(kvm_pmu_cap.num_counters_fixed) - 1;
> +       fixed_cntr_mask &= kvm_pmu_cap.fixed_cntr_mask64;
>
>         /*
>          * The number of counters comes from guest CPUID data. Clamp the value
> @@ -583,7 +584,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
>          */
>         nr_gp_counters = min_t(int, eax.split.num_counters, X86_PMC_IDX_MAX - 1);
>         pmu->all_valid_pmc_mask64 = (BIT_ULL(nr_gp_counters) - 1) &
> -                                   (BIT_ULL(kvm_pmu_cap.num_counters_gp) - 1);
> +                                   kvm_pmu_cap.cntr_mask64;
>
>         entry = kvm_find_cpuid_entry_index(vcpu, 7, 0);
>         if (entry &&
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 0a6880bb7ebe..2a59bbe52bd8 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4226,6 +4226,7 @@ static void vmx_recalc_pmu_msr_intercepts(struct kvm_vcpu *vcpu)
>         bool has_mediated_pmu = kvm_vcpu_has_mediated_pmu(vcpu);
>         struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>         struct vcpu_vmx *vmx = to_vmx(vcpu);
> +       unsigned long host_only_gp_mask, host_only_fixed_mask;
>         unsigned long fixed_mask = kvm_fixed_pmc_mask(pmu);
>         unsigned long gp_mask = kvm_gp_pmc_mask(pmu);
>         bool intercept = !has_mediated_pmu;
> @@ -4248,23 +4249,25 @@ static void vmx_recalc_pmu_msr_intercepts(struct kvm_vcpu *vcpu)
>
>         vm_exit_controls_changebit(vmx, vm_exit_controls_bits, has_mediated_pmu);
>
> +       host_only_gp_mask = kvm_host_pmu.cntr_mask64 & ~gp_mask;
>         kvm_for_each_set_pmc_idx(i, gp_mask, INTEL_GP) {
>                 vmx_set_intercept_for_msr(vcpu, MSR_IA32_PERFCTR0 + i,
>                                           MSR_TYPE_RW, intercept);
>                 vmx_set_intercept_for_msr(vcpu, MSR_IA32_PMC0 + i, MSR_TYPE_RW,
>                                           intercept || !fw_writes_is_enabled(vcpu));
>         }
> -       for ( ; i < kvm_pmu_cap.num_counters_gp; i++) {
> +       for_each_set_bit(i, &host_only_gp_mask, INTEL_PMC_MAX_GENERIC) {
>                 vmx_set_intercept_for_msr(vcpu, MSR_IA32_PERFCTR0 + i,
>                                           MSR_TYPE_RW, true);
>                 vmx_set_intercept_for_msr(vcpu, MSR_IA32_PMC0 + i,
>                                           MSR_TYPE_RW, true);
>         }
>
> +       host_only_fixed_mask = kvm_host_pmu.fixed_cntr_mask64 & ~fixed_mask;
>         kvm_for_each_set_pmc_idx(i, fixed_mask, INTEL_FIXED)
>                 vmx_set_intercept_for_msr(vcpu, MSR_CORE_PERF_FIXED_CTR0 + i,
>                                           MSR_TYPE_RW, intercept);
> -       for ( ; i < kvm_pmu_cap.num_counters_fixed; i++)
> +       for_each_set_bit(i, &host_only_fixed_mask, INTEL_PMC_MAX_FIXED)
>                 vmx_set_intercept_for_msr(vcpu, MSR_CORE_PERF_FIXED_CTR0 + i,
>                                           MSR_TYPE_RW, true);
>
> --
> 2.54.0
>