Re: [PATCH 1/3] KVM: x86/pmu: Do not map fixed counters >= 3 to generic perf events

["Chen, Zide" <zide.chen@intel.com>]

On 3/23/2026 10:48 PM, Mi, Dapeng wrote:
> 
> On 2/27/2026 7:06 AM, Zide Chen wrote:
>> Only fixed counters 0..2 have matching generic cross-platform
>> hardware perf events (INSTRUCTIONS, CPU_CYCLES, REF_CPU_CYCLES).
>> Therefore, perf_get_hw_event_config() is only applicable to these
>> counters.
>>
>> KVM does not intend to emulate fixed counters >= 3 on legacy
>> (non-mediated) vPMU, while for mediated vPMU, KVM does not care what
>> the fixed counter event mappings are.  Therefore, return 0 for their
>> eventsel.
>>
>> Also remove __always_inline as BUILD_BUG_ON() is no longer needed.
>>
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>> ---
>>  arch/x86/kvm/vmx/pmu_intel.c | 26 ++++++++++++++------------
>>  1 file changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
>> index 27eb76e6b6a0..4bfd16a9e6c7 100644
>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
>> @@ -454,28 +454,30 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   * different perf_event is already utilizing the requested counter, but the end
>>   * result is the same (ignoring the fact that using a general purpose counter
>>   * will likely exacerbate counter contention).
>> - *
>> - * Forcibly inlined to allow asserting on @index at build time, and there should
>> - * never be more than one user.
>>   */
>> -static __always_inline u64 intel_get_fixed_pmc_eventsel(unsigned int index)
>> +static u64 intel_get_fixed_pmc_eventsel(unsigned int index)
>>  {
>>  	const enum perf_hw_id fixed_pmc_perf_ids[] = {
>>  		[0] = PERF_COUNT_HW_INSTRUCTIONS,
>>  		[1] = PERF_COUNT_HW_CPU_CYCLES,
>>  		[2] = PERF_COUNT_HW_REF_CPU_CYCLES,
>>  	};
>> -	u64 eventsel;
>> -
>> -	BUILD_BUG_ON(ARRAY_SIZE(fixed_pmc_perf_ids) != KVM_MAX_NR_INTEL_FIXED_COUNTERS);
>> -	BUILD_BUG_ON(index >= KVM_MAX_NR_INTEL_FIXED_COUNTERS);
>> +	u64 eventsel = 0;
> 
> I'm not sure if we can directly add the "slots" event support in the
> perf_hw_id list. It looks more straightforward, but it need to involve perf
> changes and impact all architectures. Not sure if it's worthy to do it.
> Current way is fine for me if we decided not to support the fixed counter 3
> for  the legacy emulated vPMU.

If the emulated vPMU does not intend to support fixed counter 3 and
above, which is currently the case, I do not see any benefit in
extending enum perf_hw_id, which is generic across platforms.

> Maybe one minor optimization, we can directly return here and then don't
> need to add so much indents, e.g.,
> 
> if (index >=3)
> 
>     return eventsel;

Yes, Agreed. The code is cleaner and it's more logical.

> 
> 
>>  
>>  	/*
>> -	 * Yell if perf reports support for a fixed counter but perf doesn't
>> -	 * have a known encoding for the associated general purpose event.
>> +	 * Fixed counters 3 and above don't have corresponding generic hardware
>> +	 * perf event, and KVM does not intend to emulate them on non-mediated
>> +	 * vPMU.
>>  	 */
>> -	eventsel = perf_get_hw_event_config(fixed_pmc_perf_ids[index]);
>> -	WARN_ON_ONCE(!eventsel && index < kvm_pmu_cap.num_counters_fixed);
>> +	if (index < 3) {
>> +		/*
>> +		 * Yell if perf reports support for a fixed counter but perf
>> +		 * doesn't 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);
>> +	}
>>  	return eventsel;
>>  }
>>