Re: [PATCH v3 7/8] KVM: x86/cpuid: Add AMD CPUID ExtPerfMonAndDbg leaf 0x80000022

Sean Christopherson
2023-01-25T00:16:24Z

On Fri, Nov 11, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
>
> CPUID leaf 0x80000022 i.e. ExtPerfMonAndDbg advertises some new
> performance monitoring features for AMD processors.
>
> Bit 0 of EAX indicates support for Performance Monitoring Version 2
> (PerfMonV2) features. If found to be set during PMU initialization,
> the EBX bits of the same CPUID function can be used to determine
> the number of available PMCs for different PMU types.
>
> Expose the relevant bits via KVM_GET_SUPPORTED_CPUID so that
> guests can make use of the PerfMonV2 features.
>
> Co-developed-by: Sandipan Das <sandipan.das@amd.com>
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> arch/x86/kvm/cpuid.c | 30 +++++++++++++++++++++++++++++-
> arch/x86/kvm/svm/svm.c | 11 ++++++++---
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 6b5912578edd..df551fa66ccc 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1113,7 +1113,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> entry->edx = 0;
> break;
> case 0x80000000:
> - entry->eax = min(entry->eax, 0x80000021);
> + entry->eax = min(entry->eax, 0x80000022);
> /*
> * Serializing LFENCE is reported in a multitude of ways, and
> * NullSegClearsBase is not reported in CPUID on Zen2; help
> @@ -1229,6 +1229,34 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> if (!static_cpu_has_bug(X86_BUG_NULL_SEG))
> entry->eax |= BIT(6);
> break;
> + /* AMD Extended Performance Monitoring and Debug */
> + case 0x80000022: {
> + union cpuid_0x80000022_ebx ebx;
> +
> + entry->ecx = entry->edx = 0;
> + if (!enable_pmu || !kvm_cpu_cap_has(X86_FEATURE_AMD_PMU_V2)) {
> + entry->eax = entry->ebx;
> + break;
> + }
> +
> + cpuid_entry_override(entry, CPUID_8000_0022_EAX);
> +
> + if (kvm_cpu_cap_has(X86_FEATURE_AMD_PMU_V2))
> + ebx.split.num_core_pmc = min(kvm_pmu_cap.num_counters_gp,
> + KVM_AMD_PMC_MAX_GENERIC);

Similar to the previous patch, sanitizing kvm_pmu_cap.num_counters_gp should be
handled during PMU initialization.

> +
> + if (kvm_cpu_cap_has(X86_FEATURE_PERFCTR_CORE))
> + ebx.split.num_core_pmc = max_t(unsigned int,
> + ebx.split.num_core_pmc,
> + AMD64_NUM_COUNTERS_CORE);
> + else
> + ebx.split.num_core_pmc = max_t(unsigned int,
> + ebx.split.num_core_pmc,
> + AMD64_NUM_COUNTERS);

Again like the previous patch, I would expect this to be an if-elif-else sequence.

> +
> + entry->ebx = ebx.full;
> + break;
> + }
> /*Add support for Centaur's CPUID instruction*/
> case 0xC0000000:
> /*Just support up to 0xC0000004 now*/
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 527f18d8cc44..127983ab8307 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4914,9 +4914,14 @@ static __init void svm_set_cpu_caps(void)
> boot_cpu_has(X86_FEATURE_AMD_SSBD))
> kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
>
> - /* AMD PMU PERFCTR_CORE CPUID */
> - if (enable_pmu && boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
> - kvm_cpu_cap_set(X86_FEATURE_PERFCTR_CORE);

Another existing wart. This existing code can be:

if (enable_pmu)
kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE)

Can you add a patch to do that minor cleanup? Then this patch can yield:

if (enable_pmu) {
kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE);
kvm_cpu_cap_check_and_set(X86_FEATURE_PERFMON_V2);
}

KVM's reverse-CPUID magic should Just Work for PERFMON_V2 even though it's scattered.

> + if (enable_pmu) {
> + /* AMD PMU PERFCTR_CORE CPUID */
> + if (boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
> + kvm_cpu_cap_set(X86_FEATURE_PERFCTR_CORE);
> + /* KVM only support AMD PerfMon V2 */
> + if (kvm_pmu_cap.version > 1)
> + kvm_cpu_cap_set(X86_FEATURE_AMD_PMU_V2);
> + }
>
> /* CPUID 0x8000001F (SME/SEV features) */
> sev_set_cpu_caps();
> --
> 2.38.1
>