Re: [PATCH v4 12/12] KVM: x86/cpuid: Add AMD CPUID ExtPerfMonAndDbg leaf 0x80000022

Sean Christopherson
2023-04-07T01:51:00Z

On Tue, Feb 14, 2023, Like Xu wrote:
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index f4a4691b4f4e..2472fa8746c2 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4916,6 +4916,12 @@ static __init void svm_set_cpu_caps(void)
> } else {
> /* AMD PMU PERFCTR_CORE CPUID */
> kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE);
> + /*
> + * KVM only supports AMD PerfMon V2, even if it supports V3+.

Ha! A perfect example of why I strongly prefer that changelogs and comments avoid
pronouns. The above "it" reads as:

* KVM only supports AMD PerfMon V2, even if KVM supports V3+.

which is clearly nonsensical.

> + * For PerfMon V3+, it's unsafe to expect V2 bit is set or cleared.

If it's unsafe to assume anything v3+ implying v2 support, then it's definitely
not safe to assume that KVM can blindly set v2 without future changes. I don't
see any reason not to do

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index bd324962bb7e..1192f605ad47 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -756,6 +756,10 @@ void kvm_set_cpu_caps(void)
F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */
);

+ kvm_cpu_cap_mask(CPUID_8000_0022_EAX,
+ F(PERFMON_V2)
+ );
+
/*
* Synthesize "LFENCE is serializing" into the AMD-defined entry in
* KVM's supported CPUID if the feature is reported as supported by the

and then this code can be:

if (kvm_pmu_cap.version != 2)
kvm_cpu_cap_clear(X86_FEATURE_PERFMON_V2);

Ah, but presumably the

if (kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS_CORE)

path also needs to clear PERFMON_V2. I think I'd still vote to grab host CPUID
and clear here (instead of setting).

What is the relationship between PERFCTR_CORE and PERFMON_V2? E.g. if v2 depends
on having PERFCTR_CORE, then we can do:

if (enable_pmu) {
if (kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS_CORE)
kvm_pmu_cap.num_counters_gp = AMD64_NUM_COUNTERS;
else
kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE);

if (kvm_pmu_cap.version != 2 ||
!kvm_cpu_cap_has(X86_FEATURE_PERFCTR_CORE))
kvm_cpu_cap_clear(X86_FEATURE_PERFMON_V2);