Re: [PATCH v3 13/18] KVM: x86/pmu: Populate CPUID.0AH:ECX fixed-counter bitmap

[Mi, Dapeng]

On 11/10/2026 4:41 AM, Chen, Zide wrote:
>
> On 11/6/2026 3:00 PM, Yosry Ahmed wrote:
>> On Thu, Nov 05, 2026 at 06:39:22PM -0700, Zide Chen wrote:
>>> PerfMon v5 adds a fixed-counter support bitmap in CPUID.0AH:ECX.
>>> This is the superset of EDX[4:0] which indicates the contiguous
>>> counters starting from 0. For backward compatibility, it's recommended
>>> to use the following to determine if a fixed counter is supported:
>>>
>>> FxCtr[i]_is_supported := ECX[i] || (EDX[4:0] > i);
>>>
>>> Originally-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
>>> Signed-off-by: Zide Chen <zide.chen@xxxxxxxxx>
>>> ---
>>> v3:
>>> - Remove '&' when populating entry->ecx. (Jim)
>>> - Gate entry->ecx on kvm_pmu_cap.version >= 5 in intel_pmu_refresh() to
>>>   avoid enabling v5 fixed counters on hosts where KVM caps PMU at v2.
>>> ---
>>>  arch/x86/kvm/cpuid.c         | 5 ++++-
>>>  arch/x86/kvm/vmx/pmu_intel.c | 8 ++++++++
>>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>>> index 3310c370c5fa..00a2ca6d3049 100644
>>> --- a/arch/x86/kvm/cpuid.c
>>> +++ b/arch/x86/kvm/cpuid.c
>>> @@ -1554,7 +1554,10 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>>>  
>>>  		entry->eax = eax.full;
>>>  		entry->ebx = kvm_pmu_cap.events_mask;
>>> -		entry->ecx = 0;
>>> +		if (kvm_pmu_cap.version >= 5)
>>> +			entry->ecx = (u32)kvm_pmu_cap.fixed_cntr_mask64;
>>> +		else
>>> +			entry->ecx = 0;
>>>  		entry->edx = edx.full;
>>>  		break;
>>>  	}
>>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
>>> index 1372091dc1f8..37b34b588601 100644
>>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
>>> @@ -597,7 +597,15 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
>>>  				  find_first_zero_bit(kvm_pmu_cap.fixed_cntr_mask,
>>>  						      KVM_MAX_NR_FIXED_COUNTERS));
>>>  
>>> +	/* FxCtr[i]_is_supported := ECX[i] || (EDX[4:0] > i) */
>>>  	fixed_mask = BIT_ULL(nr_fixed_counters) - 1;
>>> +	/*
>>> +	 * Do not enable fixed counters via the v5 ECX bitmap if KVM caps
>>> +	 * PMU version below 5 (e.g. when mediated PMU is disabled or on
>>> +	 * older hosts), matching KVM's advertised CPUID.0AH:ECX capability.
>>> +	 */
>>> +	if (kvm_pmu_cap.version >= 5 && pmu->version >= 5)
>> Having to check both versions is confusing. Can they go out of sync?
>> Should we clamp pmu->version to kvm_pmu_cap.version and only check the
>> former?
> Yes, actually, other fields (bit_width, mask_length, and
> nr_{gp,fixed}_counters) in intel_pmu_refresh() are clamped to
> kvm_pmu_cap as well.
>
> pmu->version = min_t(int, eax.split.version_id, kvm_pmu_cap.version);

I'm not quite sure if we could do such clamp. I remember Sean ever said KVM
should allow user space to set the PMU capabilities which KVM doesn't
support, like what currently kselftest pmu_counters_test does, e.g., KVM
only supports up to PMU version 2 in non-mediated vPMU mode, but
pmu_counters_test can set PMU versions larger than 2.


>
> Without this, as more advanced PMU features are supported in KVM, we
> will have to manually validate guest features, and it seems this doesn't
> violate the general principles of "not policing CPUID configurations"
> and using the "effective vPMU version".
>
>>> +		fixed_mask |= entry->ecx;
>>>  	fixed_mask &= kvm_pmu_cap.fixed_cntr_mask64;
>> I see why this is needed now, to re-apply KVM's fixed counter mask after
>> reading it, but if this is only needed there, it's probably clearer to
>> do:
>>
>> 	if (version >= 5)
>> 		fixed_mask |= (entry->ecx & kvm_pmu_cap.fixed_cntr_mask64);
> Agreed, this is cleaner. Will update accordingly. Thanks!