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

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

On 9/18/2026 3:22 PM, sashiko-bot@kernel.org wrote:
> 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?

This is a defensive check against invalid guest CPUID configuration. If
the SLOTS counter is not available, clearing PERF_METRICS is meaningless
and messy.

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

KVM is not intended to check against all possible invalid
configurations, as long as it protects host integrity, I think this is fine.

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

I don't think this needs a change.

The clear/re-apply here happen while the vCPU isn't running, so there's
no window for a native RDPMC to see it cleared: if allowed, the guest's
METRICS_CLEAR_EN bit is copied through; if not allowed, counter 3 isn't
counting anyway.

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