Re: [PATCH v9 12/12] KVM: x86/pmu: Support RDPMC Metrics Clear Mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] pmu: event filter mistakenly strips METRICS_CLEAR_EN from fixed_ctr_ctrl_hw
- [Medium] pmu: METRICS_CLEAR_EN unmasked without validating fixed counter bounds
- [Low] pmu: emulation incorrectly ties PERF_METRICS clearing to SLOTS PMC existence
--

commit 8ba3725c32dbde4a1c3935cb433e898e06425b83
Author: Zide Chen <zide.chen@intel.com>

KVM: x86/pmu: Support RDPMC Metrics Clear Mode

This commit adds support for PERF_CAP_RDPMC_METRICS_CLEAR (bit 19 of
IA32_PERF_CAPABILITIES). It advertises this capability to guests when the
mediated PMU is enabled and the host reports it, unmasking the relevant
control bit. It also updates intel_emulate_rdpmc() to replicate the
clear-on-read behavior for consistency.

> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 98e2fb80347a1..3e5e8bae67ab0 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
[ ... ]
> @@ -138,6 +149,18 @@ static int intel_emulate_rdpmc(struct kvm_vcpu *vcpu, unsigned int idx,
>  			return 1;
>  
>  		*data = pmu->perf_metrics;
> +
> +		/*
> +		 * Per the SDM, RDPMC of PERF_METRICS clears both PERF_METRICS
> +		 * and the SLOTS fixed counter when Metrics Clear Mode is
> +		 * enabled via INTEL_FIXED_3_METRICS_CLEAR in FIXED_CTR_CTRL.
> +		 */
> +		pmc = kvm_pmc_idx_to_pmc(pmu, INTEL_PMC_IDX_FIXED_SLOTS);
> +		if (pmc && intel_pmu_metrics_clear_enabled(pmu)) {
> +			pmu->perf_metrics = 0;
> +			pmc_write_counter(pmc, 0);
> +		}
> +

[Severity: Low]
Does tying the clearing of PERF_METRICS to the existence of the SLOTS PMC in
intel_emulate_rdpmc() violate the hardware specification?

If the guest executes RDPMC natively and triggers an interception, and the
SLOTS counter is not exposed to the guest (making kvm_pmc_idx_to_pmc return
NULL), but METRICS_CLEAR_EN is enabled, this code silently skips clearing
PERF_METRICS.

Should the clearing of pmu->perf_metrics be performed independently of whether
the software PMC object for the SLOTS counter exists?

>  		return 0;
>  	default:
>  		return 1;
[ ... ]
> @@ -601,6 +624,11 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
>  						 INTEL_FIXED_0_USER |
>  						 INTEL_FIXED_0_ENABLE_PMI);
>  
> +	if (perf_capabilities & PERF_CAP_RDPMC_METRICS_CLEAR)
> +		pmu->fixed_ctr_ctrl_rsvd &=
> +			~intel_fixed_bits_by_idx(INTEL_FIXED_SLOTS_IDX,
> +						 INTEL_FIXED_3_METRICS_CLEAR);
> +

[Severity: Medium]
Does intel_pmu_refresh() unconditionally unmask the METRICS_CLEAR_EN bit
without verifying if the guest topology actually exposes the SLOTS counter?

If a VM is configured with 3 or fewer fixed counters, this unmasking would
allow the unprivileged guest OS to write to bit 14 in
MSR_CORE_PERF_FIXED_CTR_CTRL without triggering the expected #GP.

Should this be gated by checking if pmu->nr_arch_fixed_counters >
INTEL_FIXED_SLOTS_IDX?

[Severity: High]
Does this patch inadvertently cause event filters to strip the METRICS_CLEAR_EN
bit in kvm_mediated_pmu_refresh_event_filter()?

When a host-level event filter denies the SLOTS counter (counter 3) from
counting, KVM applies a 0xf hardware mask that zeroes all 4 configuration bits
for the counter:

arch/x86/kvm/pmu.c:kvm_mediated_pmu_refresh_event_filter() {
...
	} else {
		u64 mask = intel_fixed_bits_by_idx(pmc->idx - KVM_FIXED_PMC_BASE_IDX, 0xf);

		pmu->fixed_ctr_ctrl_hw &= ~mask;
		if (allowed)
			pmu->fixed_ctr_ctrl_hw |= pmu->fixed_ctr_ctrl & mask;
	}
}

Since this mask covers the entire 4-bit configuration block, it will also clear
bit 14 (METRICS_CLEAR_EN) from hardware. If the guest sets METRICS_CLEAR_EN and
then executes RDPMC natively, it will lead to un-cleared stale metrics.

Should the mask be updated to avoid interfering with this newly introduced
feature bit?

>  	counter_rsvd = ~((BIT_ULL(pmu->nr_arch_gp_counters) - 1) |
>  			 ((BIT_ULL(pmu->nr_arch_fixed_counters) - 1) << KVM_FIXED_PMC_BASE_IDX));
>  	pmu->global_ctrl_rsvd = counter_rsvd;

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