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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] Unvalidated guest/userspace writes to MSR_PERF_METRICS cause host kernel WARN/Oops during PMU context load.
- [High] Missing RDPMC emulation for PERF_METRICS causes guest #GP when RDPMC is intercepted.
- [High] Missing RDPMC intercept allows information leak of host MSR_PERF_METRICS.

Pre-existing issues:
- [High] Intercepting MSR_CORE_PERF_GLOBAL_STATUS during mediated PMU operation breaks PMIs by returning stale overflow data to the guest.
--

--- Patch [5]: [PATCH v8 5/8] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU ---
commit 3c012e6d223702ac2057eaf2c6a15810c5334d8c
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU

This patch exposes the IA32_PERF_METRICS MSR to the guest when mediated
vPMU is enabled. It allows host/guest PMU context switching to save and
restore this MSR, and intercepts MSR_CORE_PERF_GLOBAL_CTRL if the host
supports PERF_METRICS but it is not exposed to the guest.

> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 913a0ecbf2609..2fddcbc8924d4 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -806,9 +806,16 @@ static bool kvm_need_any_pmc_intercept(struct kvm_vcpu *vcpu)
>  	       pmu->nr_arch_fixed_counters != kvm_host_pmu.num_counters_fixed;
>  }
>  
> +static bool kvm_need_perf_metrics_intercept(struct kvm_vcpu *vcpu)
> +{
> +	return (kvm_host.perf_capabilities & PERF_CAP_PERF_METRICS) &&
> +		!kvm_vcpu_has_perf_metrics(vcpu);
> +}
> +

[Severity: High]
Does this code miss checking for kvm_need_perf_metrics_intercept() in
kvm_need_rdpmc_intercept()?

If mediated PMU is active and the PMU counter topology matches the host,
RDPMC exiting is disabled. The guest can execute RDPMC with ECX=(1<<29)
directly on the hardware. Since intel_mediated_pmu_load() skips clearing
MSR_PERF_METRICS when the guest lacks the feature, the guest might observe
the host's raw active MSR_PERF_METRICS value.

>  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));
>  }

[Severity: High]
This is a pre-existing issue, but does forcing intercepts here break PMIs
for guests lacking PERF_METRICS?

By returning true here, MSR_CORE_PERF_GLOBAL_STATUS and
MSR_CORE_PERF_GLOBAL_OVF_CTRL are intercepted. During mediated PMU
operation, hardware updates the physical GLOBAL_STATUS directly on
overflow. Because KVM does not sync the physical MSR to
pmu->global_status on every VM-exit (only on vcpu_put()), guest reads
of the intercepted GLOBAL_STATUS could return stale cached data (0),
potentially hiding hardware PMIs from the guest.

[ ... ]
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 100c072b7fb38..1fd0480baf680 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
[ ... ]
> @@ -346,6 +348,9 @@ 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:
> +		msr_info->data = pmu->perf_metrics;
> +		break;

[Severity: High]
Does KVM also need to emulate RDPMC for PERF_METRICS?

The SDM specifies that IA32_PERF_METRICS can be read using the RDPMC
instruction with ECX = (1<<29) | 0. However, intel_rdpmc_ecx_to_pmc() only
recognizes GP counters (type 0) and fixed counters (type 1<<30).

If KVM intercepts RDPMC, could a guest attempt to read PERF_METRICS via
RDPMC fall through to the default case and return NULL, causing KVM to
inject a #GP?

>  	case MSR_IA32_PEBS_ENABLE:
>  		msr_info->data = pmu->pebs_enable;
>  		break;
[ ... ]
> @@ -395,6 +400,14 @@ 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:
> +		/*
> +		 * Bits [63:32] are ignored on hardware that supports only
> +		 * level-1 metrics, but may be valid on hardware that supports
> +		 * level-2 metrics. Preserve guest writes verbatim.
> +		 */
> +		pmu->perf_metrics = data;
> +		break;

[Severity: Critical]
Is it safe to accept untrusted guest or userspace data here without
validation?

If a malicious guest or host userspace (via KVM_SET_MSRS) injects a value
with reserved bits set into pmu->perf_metrics, it could cause a host #GP
during vcpu_load() when intel_mediated_pmu_load() blindly writes this
value to the physical MSR using the unsafe wrmsrq() macro.

>  	case MSR_IA32_PEBS_ENABLE:
>  		if (data & pmu->pebs_enable_rsvd)
>  			return 1;
[ ... ]
> @@ -803,6 +821,9 @@ static void intel_mediated_pmu_load(struct kvm_vcpu *vcpu)
>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>  	u64 global_status, toggle;
>  
> +	if (kvm_vcpu_has_perf_metrics(vcpu))
> +		wrmsrq(MSR_PERF_METRICS, pmu->perf_metrics);
> +

[Severity: Critical]
Could this wrmsrq() cause a host oops or panic?

As mentioned above, if pmu->perf_metrics contains unvalidated data with
reserved bits set, wrmsrq() (which lacks an exception table entry) will
trigger a host #GP.

>  	rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, global_status);
>  	toggle = pmu->global_status ^ global_status;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817183143.226156-1-zide.chen@intel.com?part=5