Re: [PATCH v2 09/16] KVM: x86/pmu: Switch to bitmask-based KVM PMU capabilities

[Chen, Zide]

On 10/28/2026 12:57 PM, Jim Mattson wrote:
> On Thu, Aug 27, 2026 at 3:48 PM 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>
>> ---
> [ ... ]
>> @@ -772,7 +773,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);
>>
>> -       for ( ; i < kvm_pmu_cap.num_counters_gp; i++)
>> +       host_only_gp_mask = kvm_pmu_cap.cntr_mask64 & ~gp_mask;
> 
> Should kvm_pmu_cap.cntr_mask64 be kvm_host_pmu.cntr_mask64, as it is
> on the VMX side?

Yes, thank you for pointing it out.
> [ ... ]
> 
>> @@ -556,7 +557,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->pmc_exists64 = (BIT_ULL(nr_gp_counters) - 1) &
>> -                           (BIT_ULL(kvm_pmu_cap.num_counters_gp) - 1);
>> +                           kvm_pmu_cap.cntr_mask64;
> 
> CPUID.0AH:EAX[15:8] (eax.split.num_counters) can only enumerate contiguous
> GP counters 0 .. nr_gp_counters - 1. If kvm_pmu_cap.cntr_mask64 ever had a
> hole below nr_gp_counters (when leaf 0x23 isn't exposed to the guest to
> advertise a sparse mask), masking (BIT_ULL(nr_gp_counters) - 1) with
> kvm_pmu_cap.cntr_mask64 would silently punch a hole in the guest's GP
> counters even though guest CPUID.0AH:EAX[15:8] told the guest that all
> counters 0 .. nr_gp_counters - 1 exist. Should nr_gp_counters be clamped to
> find_first_zero_bit(kvm_pmu_cap.cntr_mask, KVM_MAX_NR_GP_COUNTERS) until
> leaf 0x23 is supported?

Regardless of whether or not CPUID 0x23 is supported in KVM, I think "if
kvm_pmu_cap.cntr_mask64 ever had a hole below nr_gp_counters" would only
happen in the case of an invalid guest configuration.

Anyway, it is good to clamp nr_gp_counters to remove any theoretical
hole, keeping it semantically correct.  For example:

/*
 * CPUID.0AH:EAX[15:8] can only enumerate contiguous counters. Clamp the
 * value to the number of contiguous GP counters supported by KVM.
 */
nr_gp_counters = min_t(int, eax.split.num_counters,
                       find_first_zero_bit(kvm_pmu_cap.cntr_mask,
                                           KVM_MAX_NR_GP_COUNTERS));
pmu->pmc_exists64 = BIT_ULL(nr_gp_counters) - 1;

Thanks for the review!