Re: [PATCH v9 05/12] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU

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

On 10/23/2026 1:27 PM, Jim Mattson wrote:
> On Fri, Sep 18, 2026 at 12:39 PM Zide Chen <zide.chen@intel.com> wrote:
>>
>> +static inline bool kvm_vcpu_has_perf_metrics(struct kvm_vcpu *vcpu)
>> +{
>> +	return kvm_vcpu_get_perf_caps(vcpu) & PERF_CAP_PERF_METRICS;
>> +}
> 
> The SDM, volume 3, section 22.3.9.3, says:
> 
>> IA32_PERF_GLOBAL_CTRL. EN_PERF_METRICS[48]: If this bit is set *and*
>> fixed-function performance-monitoring counter 3 is enabled, built-in
>> performance metrics are enabled. [emphasis mine]
> 
> and
> 
>> The values in MSR_PERF_METRICS are derived from fixed-function
>> performance-monitoring counter 3.
> 
> Hence, a dependency on fixed counter 3 belongs in this predicate.
> Perhaps something like:
> 
> 	static inline bool kvm_pmu_has_fixed_counter(struct kvm_pmu *pmu,
> 						     unsigned int idx)
> 	{
> 		return idx < pmu->nr_arch_fixed_counters;
> 	}
> 
> 	static inline bool kvm_vcpu_has_perf_metrics(struct kvm_vcpu *vcpu)
> 	{
> 		return (kvm_vcpu_get_perf_caps(vcpu) & PERF_CAP_PERF_METRICS) &&
> 		       kvm_pmu_has_fixed_counter(vcpu_to_pmu(vcpu),
> 						 INTEL_PMC_IDX_FIXED_SLOTS -
> 						 INTEL_PMC_IDX_FIXED);
> 	}
> 
> with intel_pmu_refresh() calling kvm_vcpu_has_perf_metrics() instead of
> testing perf_capabilities directly. (nr_arch_fixed_counters has already
> been computed at that point; all_valid_pmc_idx has not.)

Thanks for the detailed analysis. That makes total sense.

Initially I didn't gate PERF_METRICS on fixed counter 3 out of concern
about policing userspace configurations. Additionally, as in the quoted
SDM text above, it means PERF_METRICS can't function without fixed
counter 3, but strictly speaking, the SDM doesn't prevent other
operations, for example, writing the METRICS_CLEAR_EN bit.

But having IA32_PERF_CAPABILITIES[15] set in the absence of FC3 is not
going to happen natively, so I think it doesn't break any rules if KVM
chooses to gate PERF_METRICS on FC3. And as you pointed out, it's a
simple and effective way to protect KVM invariants and migration integrity.

I will add the FC3 check to kvm_vcpu_has_perf_metrics().

> 
> KVM only supports contiguous fixed counters today (see patch 2), so a
> simple comparison against nr_arch_fixed_counters suffices. However, if KVM
> ever honors the CPUID.0AH:ECX fixed counter bitmap, a separate helper
> keeps that assumption in one place.

Before the pmu v5 series is merged:
https://lore.kernel.org/kvm/20260827223755.143247-1-zide.chen@intel.com/

I have to continue use nr_arch_fixed_counters in this series.

> 
> As it stands, if userspace sets PERF_CAP_PERF_METRICS for a version 1
> vPMU, MSR_PERF_METRICS is passed through (kvm_vcpu_has_mediated_pmu()
> only requires a non-zero version), but intel_pmu_refresh() bails out
> before it gets to the PERF_METRICS code. With patch 11, that leaves
> perf_metrics_rsvd at ~0ull, so a value the guest writes directly to
> hardware can be saved with KVM_GET_MSRS, but not restored with
> KVM_SET_MSRS.
> 
> Since nr_arch_fixed_counters is 0 for a v1 vPMU, the above handles that
> case too. It also takes care of the METRICS_CLEAR_EN unmasking without
> fixed counter 3 that Sashiko flagged on patch 12, and it obviates the
> NULL pmc check in intel_emulate_rdpmc().

Yes, with FC3 gating, these issues can be easily resolved.
> [...]
> 
>> +	case MSR_PERF_METRICS:
>> +		pmu->perf_metrics = data;
>> +		break;
> 
> On a part with only four metrics (e.g. Ice Lake), userspace can stuff
> bits 63:32 into pmu->perf_metrics here, and the unchecked wrmsrq() in
> intel_mediated_pmu_load() will then #GP on every VM-entry. Patch 11
> isn't an optional follow-on; it needs to be folded into this patch.

Yes, agree.