Re: [PATCH V4 3/4] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU

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

On 6/22/2026 11:48 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Emulation of guest RDMSR and WRMSR for MSR_PERF_METRICS unconditionally injects a #GP fault, breaking emulation and nested virtualization.
> - [High] KVM_SET_MSRS for MSR_PERF_METRICS is silently discarded, breaking live migration of TMA metrics.
> - [High] Host Topdown microarchitectural metrics are leaked to the guest via the RDPMC instruction when mediated PMU is enabled but the guest lacks the PERF_METRICS capability.
> --
> 
> commit 3d014d661f0ac59ad10f0335ced10697e72d7bf6
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Subject: KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU
> 
> This commit exposes the IA32_PERF_METRICS MSR to the guest when mediated vPMU
> is enabled. It handles the MSR in kvm_pmu_{get,set}_msr() for host accesses
> and saves/restores the value on PMU context switches to avoid clobbering host
> data or leaking guest state.
> 
>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
>> index 59b7a90c79e12..2c3367f5e2dfb 100644
>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> [ ... ]
>> @@ -345,6 +347,11 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>  	case MSR_CORE_PERF_FIXED_CTR_CTRL:
>>  		msr_info->data = pmu->fixed_ctr_ctrl;
>>  		break;
>> +	case MSR_PERF_METRICS:
>> +		if (!msr_info->host_initiated)
>> +			return 1;
> 
> [Severity: High]
> If KVM needs to emulate a guest RDMSR instruction targeting MSR_PERF_METRICS,
> won't this unconditional return inject a #GP fault into the guest?

I can't think of a case that KVM needs to emulate RDMSR/WRMSR on
MSR_PERF_METRICS.
>> +		msr_info->data = pmu->perf_metrics;
>> +		break;
>>  	case MSR_IA32_PEBS_ENABLE:
>>  		msr_info->data = pmu->pebs_enable;
>>  		break;
>> @@ -394,6 +401,16 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>  		if (pmu->fixed_ctr_ctrl != data)
>>  			reprogram_fixed_counters(pmu, data);
>>  		break;
>> +	case MSR_PERF_METRICS:
>> +		if (!msr_info->host_initiated)
>> +			return 1;
> 
> [Severity: High]
> Similarly, if KVM emulates a guest WRMSR instruction targeting
> MSR_PERF_METRICS, does returning 1 here cause the emulator to incorrectly
> inject a #GP fault?

Ditto.
>> +
>> +		/*
>> +		 * If TMA level 2 is not supported, bits [63:32] are reserved
>> +		 * and ignored on write, so no validation is needed here.
>> +		 */
>> +		pmu->perf_metrics = data;
> 
> [Severity: High]
> When restoring guest state during live migration, userspace calls KVM_SET_MSRS
> which is wrapped in vcpu_load() and vcpu_put(). Since intel_pmu_set_msr()
> updates pmu->perf_metrics without updating the hardware MSR, won't the
> subsequent intel_mediated_pmu_put() during vcpu_put() read the stale hardware
> MSR and overwrite this newly restored value?

No, this won't happen.
>> +		break;
>>  	case MSR_IA32_PEBS_ENABLE:
>>  		if (data & pmu->pebs_enable_rsvd)
>>  			return 1;
> [ ... ]
>> @@ -803,6 +828,13 @@ static void intel_mediated_pmu_load(struct kvm_vcpu *vcpu)
>>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>  	u64 global_status, toggle;
>>  
>> +	/*
>> +	 * PERF_METRICS MSR must be restored closely after fixed counter 3,
>> +	 * which was just restored by kvm_pmu_load_guest_pmcs().
>> +	 */
>> +	if (vcpu_has_perf_metrics(vcpu))
>> +		wrmsrq(MSR_PERF_METRICS, pmu->perf_metrics);
>> +
> 
> [Severity: High]
> If the guest is configured without PERF_CAP_PERF_METRICS but with mediated
> PMU enabled, RDPMC exiting might be disabled. Since
> intel_mediated_pmu_load() skips writing to MSR_PERF_METRICS in this case,
> could the guest execute RDPMC and read the un-scrubbed host metrics, leaking
> host microarchitectural state?

You are right. RDPMC exiting needs to be disabled in this case.
>>  	rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, global_status);
>>  	toggle = pmu->global_status ^ global_status;
>>  	if (global_status & toggle)
>> @@ -831,6 +863,12 @@ 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 (vcpu_has_perf_metrics(vcpu)) {
>> +		pmu->perf_metrics = rdpmc(INTEL_PMC_FIXED_RDPMC_METRICS);
>> +		if (pmu->perf_metrics)
>> +			wrmsrq(MSR_PERF_METRICS, 0);
>> +	}
>>  }
>