All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chen, Zide" <zide.chen@intel.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v8 5/8] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU
Date: Mon, 17 Aug 2026 17:07:31 -0500	[thread overview]
Message-ID: <cf2fab68-a0be-403c-8ca8-80880fa3879d@intel.com> (raw)
In-Reply-To: <20260817190429.7CD481F000E9@smtp.kernel.org>



On 8/17/2026 2:04 PM, sashiko-bot@kernel.org wrote:
> 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()?

Yes, it's included in patch 7/8.

> 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.

False positive.

GLOBAL_STATUS is synced to pmu->global_status in
intel_mediated_pmu_load() in every VM exit. So it won't return stale
data to guest reads of the intercepted GLOBAL_STATUS.

> 
> [ ... ]
>> 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?

Yes, it's implemented in the following patches.>
> 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?

As the comments suggest, no #GP is caused.
> 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.

No, no #GP in this case.
>>  	rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, global_status);
>>  	toggle = pmu->global_status ^ global_status;
> 


  reply	other threads:[~2026-08-17 22:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 18:31 [PATCH v8 0/8] KVM: x86/pmu: Add hardware Topdown metrics support Zide Chen
2026-08-17 18:31 ` [PATCH v8 1/8] KVM: x86/pmu: Do not map fixed counters >= 3 to generic perf events Zide Chen
2026-08-17 18:31 ` [PATCH v8 2/8] KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU Zide Chen
2026-08-17 18:31 ` [PATCH v8 3/8] KVM: x86/pmu: Rename and move vcpu_get_perf_capabilities() to pmu.h Zide Chen
2026-08-17 18:52   ` sashiko-bot
2026-08-17 21:45     ` Chen, Zide
2026-08-17 18:31 ` [PATCH v8 4/8] KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host Zide Chen
2026-08-17 18:31 ` [PATCH v8 5/8] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU Zide Chen
2026-08-17 19:04   ` sashiko-bot
2026-08-17 22:07     ` Chen, Zide [this message]
2026-08-17 18:31 ` [PATCH v8 6/8] KVM: x86/pmu: Move RDPMC emulation into per-vendor callbacks Zide Chen
2026-08-17 18:31 ` [PATCH v8 7/8] KVM: x86/pmu: Emulate RDPMC on performance metrics Zide Chen
2026-08-17 19:01   ` sashiko-bot
2026-08-17 22:02     ` Chen, Zide
2026-08-17 18:31 ` [PATCH v8 8/8] KVM: selftests: Add PERF_METRICS and fixed counter 3 tests Zide Chen
2026-08-17 18:57   ` sashiko-bot
2026-08-17 21:44     ` Chen, Zide

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf2fab68-a0be-403c-8ca8-80880fa3879d@intel.com \
    --to=zide.chen@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.