Re: [PATCH v9 08/12] KVM: selftests: Add PERF_METRICS and fixed counter 3 tests

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

On 9/18/2026 3:08 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] selftests: unconditional MSR read causes assertion failure on older kernels
> --
> 
> commit c5ae582e90bae306393c4bf2f37f04a11e37ba5f
> Author: Zide Chen <zide.chen@intel.com>
> 
> KVM: selftests: Add PERF_METRICS and fixed counter 3 tests
> 
> This commit adds a test case to exercise IA32_PERF_METRICS for Topdown Level
> 1 metrics. It also extends the PMU selftest to cover Intel fixed counter 3 by
> bumping the maximum number of fixed counters and validating functionality.
> 
>> diff --git a/tools/testing/selftests/kvm/x86/pmu_counters_test.c b/tools/testing/selftests/kvm/x86/pmu_counters_test.c
>> --- a/tools/testing/selftests/kvm/x86/pmu_counters_test.c
>> +++ b/tools/testing/selftests/kvm/x86/pmu_counters_test.c
> [ ... ]
>> @@ -653,8 +657,85 @@ static void test_fixed_counters(u8 pmu_version, u64 perf_capabilities)
> [ ... ]
>>  static void test_intel_counters(void)
>>  {
>> +	u64 advertised_perf_caps = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
> 
> [Severity: Medium]
> Will this unconditional MSR read cause an assertion failure on older kernels?
> 
> When running the selftest on a host kernel prior to 5.9, KVM does not
> expose MSR_IA32_PERF_CAPABILITIES as a feature MSR. Calling
> kvm_get_feature_msr() unconditionally in test_intel_counters() causes the
> underlying ioctl to fail and return 0, which triggers an assertion failure
> rather than skipping the feature gracefully.
> 
> Other MSR accesses in this file appear to be guarded by checking
> kvm_has_perf_caps first. Could we apply a similar check before reading
> advertised_perf_caps so the test remains compatible with older environments?

Yes, you are right. It should be guarded with kvm_has_perf_caps.

>>  	u8 pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
>>  	unsigned int i;
>>  	u8 v;
> [ ... ]
>