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

"Chen, Zide" <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm
Message-ID <[email protected]>

On 7/27/2026 2:50 PM, [email protected] 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 <[email protected]>
> 
> 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);
>> +	}
>>  }
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.