Re: [PATCH v7 4/9] KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host

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

On 7/27/2026 2:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete replacement of raw MSR_IA32_PERF_CAPABILITIES read leaves KVM vulnerable to kernel panics during module initialization.
> --
> 
> --- Patch [4]: [PATCH v7 4/9] KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host ---
> commit 6a6d946c1225761f83ef8a8498aa4ccab5425b72
> Author: Mingwei Zhang <mizhang@google.com>
> 
> KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host
>     
> Cache the unadulterated snapshot of perf_capabilities so that KVM can
> compare guest vPMU capabilities against raw hardware capabilities.
> 
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 0626e835e9eb7..e0114f248a9ef 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
> [ ... ]
>> @@ -7033,6 +7033,10 @@ int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
>>  	if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
>>  		rdmsrq(MSR_IA32_ARCH_CAPABILITIES, kvm_host.arch_capabilities);
>>  
>> +	if (boot_cpu_has(X86_FEATURE_PDCM))
>> +		rdmsrq_safe(MSR_IA32_PERF_CAPABILITIES,
>> +			    &kvm_host.perf_capabilities);
>> +
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but there is an incomplete
> replacement of a raw MSR_IA32_PERF_CAPABILITIES read elsewhere that leaves
> KVM vulnerable to kernel panics during module initialization.
> 
> If a nested hypervisor exposes CPUID.PDCM but faults on the MSR access, the
> raw rdmsrq() in intel_pmu_is_mediated_pmu_supported() can trigger a #GP and
> panic the kernel:

I don't think KVM needs to handle that case. In
intel_pmu_is_mediated_pmu_supported(), If CPUID.1:ECX.PDCM is advertised
in an Intel platform but reading IA32_PERF_CAPABILITIES causes a #GP,
that's a bug in the underlying hypervisor.

The reason I used rdmsrq_safe() here is that this code lives in common
x86 code. In theory, a non-Intel architecture could potentially
implement CPUID.1:ECX[15] without supporting IA32_PERF_CAPABILITIES.

rdmsrq_safe() is a defensive safeguard.

> arch/x86/kvm/vmx/pmu_intel.c:intel_pmu_is_mediated_pmu_supported() {
>     ...
>     rdmsrq(MSR_IA32_PERF_CAPABILITIES, host_perf_cap);
>     ...
> }
> 
> While this patch safely caches the MSR using rdmsrq_safe() here in
> kvm_x86_vendor_init(), this snapshot is populated after
> kvm_init_pmu_capability() has already run.
> 
> Because of this ordering, intel_pmu_is_mediated_pmu_supported() cannot
> currently use this safely cached value, as it would incorrectly read 0.
> 
> Would it make sense to reorder the initialization so the snapshot is
> populated before kvm_init_pmu_capability() runs, and then update
> intel_pmu_is_mediated_pmu_supported() to use kvm_host.perf_capabilities
> to avoid the #GP risk?
> 
>>  	WARN_ON_ONCE(kvm_nr_uret_msrs);
>>  
>>  	r = ops->hardware_setup();
>