Re: [PATCH v7 5/9] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU

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

On 7/27/2026 2:50 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Guest PMU state in MSR_PERF_METRICS is leaked to the host and other VMs because KVM fails to clear it on PMU context switch when the guest is configured without the PERF_CAP_PERF_METRICS feature.
> --
> 
> --- Patch [5]: [PATCH v7 5/9] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU ---
> commit 2933f2f58fa948287c388e166de23d159332fdf8
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
> 
> KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU
> 
> This commit exposes the IA32_PERF_METRICS MSR to guests when mediated
> vPMU is enabled. It handles passthrough of the MSR and saves or restores
> its state during PMU context switches to prevent clobbering or leaking
> state.
> 
>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> 
> [ ... ]
> 
>> @@ -831,6 +852,19 @@ static void intel_mediated_pmu_put(struct kvm_vcpu *vcpu)
>>  	 */
>>  	if (pmu->fixed_ctr_ctrl_hw)
>>  		wrmsrq(MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
>> +
>> +	if (kvm_vcpu_has_perf_metrics(vcpu)) {
>         ^^^^
> 
> [Severity: High]
> Could this leak guest PMU state to the host or other VMs?
> 
> In intel_mediated_pmu_put(), this checks if the guest has the
> PERF_CAP_PERF_METRICS capability before clearing MSR_PERF_METRICS.
> However, if the physical CPU supports this feature but the guest is
> configured without it, could the guest still activate it?
> 
> If KVM configures MSR_CORE_PERF_GLOBAL_CTRL for passthrough (which happens
> when counter counts match, as kvm_need_perf_global_ctrl_intercept() only
> checks counter counts), the guest can write to bit 48 of
> MSR_CORE_PERF_GLOBAL_CTRL. The hardware PMU would then start updating
> MSR_PERF_METRICS.

Good catch. It's a basic yet important issue and something I should have
thought of earlier.

If the host supports PerfMetrics but it is not exposed to the guest,
MSR_CORE_PERF_GLOBAL_CTRL must be intercepted so the guest cannot enable
bit 48.

With the following code, the guest cannot activate MSR_PERF_METRICS, and
the leak concern goes away.

diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 9292407f0353..474207c6413d 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -819,6 +819,7 @@ static bool kvm_need_any_pmc_intercept(struct
kvm_vcpu *vcpu)
 bool kvm_need_perf_global_ctrl_intercept(struct kvm_vcpu *vcpu)
 {
        return kvm_need_any_pmc_intercept(vcpu) ||
+              kvm_need_perf_metrics_intercept(vcpu) ||
               !kvm_pmu_has_perf_global_ctrl(vcpu_to_pmu(vcpu));
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_need_perf_global_ctrl_intercept);

> Since kvm_vcpu_has_perf_metrics() returns false in this scenario, this path
> would skip clearing the physical MSR_PERF_METRICS on VM-exit, leaving guest
> state in the hardware register for the next VM or host to read.
> 
>> +		pmu->perf_metrics = rdpmc(INTEL_PMC_FIXED_RDPMC_METRICS);
>> +		/*
>> +		 * The SDM requires restoring fixed counter 3 before
>> +		 * PERF_METRICS. However, this path writes 0 to PERF_METRICS
>> +		 * before fixed counter 3. For this all-zero case, the
>> +		 * resulting hardware state is therefore the same regardless
>> +		 * of write order.
>> +		 */
>> +		if (pmu->perf_metrics)
>> +			wrmsrq(MSR_PERF_METRICS, 0);
>> +	}
>>  }
>