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

[Yosry Ahmed <yosry@kernel.org>]

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@intel.com>
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---
> 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?

> +		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);