| Message ID | 20230214050757.9623-10-likexu@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Add AMD Guest PerfMonV2 PMU support | expand |
On Tue, Feb 14, 2023, Like Xu wrote: > From: Like Xu <likexu@tencent.com> > > KVM should sanity check the number of counters enumerated by perf and > explicitly drop PERFCTR_CORE support if the min isn't met. E.g. if KVM > needs 6 counters and perf says there are 4, then something is wrong and > enumerating 6 to the guest is only going to cause more issues. > > Opportunistically, the existing kvm_cpu_cap_check_and_set() makes it > easier to simplify the host check before setting the PERFCTR_CORE flag. Once again, state what the patch does. > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Like Xu <likexu@tencent.com> > --- > arch/x86/kvm/svm/svm.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index dd21e8b1a259..f4a4691b4f4e 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -4910,9 +4910,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); > + if (enable_pmu) { A comment would be very helpful here. Even better would be if we can convince perf to rename AMD64_NUM_COUNTERS to AMD64_LEGACY_NUM_COUNTERS. > + if (kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS_CORE) { > + kvm_pmu_cap.num_counters_gp = AMD64_NUM_COUNTERS; Hmm, this subtly relies on kvm_init_pmu_capability() running first. That _probably_ won't change, but like the arch LBRs side of things I would prefer to avoid taking unnecessarily. E.g. something like this? /* * Enumerate support for PERFCTR_CORE if and only if KVM has * access to enough counters to virtualize "core" support, * otherwise limit vPMU support to the legacy number of counters. */ if (kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS_CORE) kvm_pmu_cap.num_counters_gp = min(AMD64_NUM_COUNTERS, kvm_pmu_cap.num_counters_gp); It's a bit paranoid, but practically speaking it doesn't cost us anything. > + } else { > + /* AMD PMU PERFCTR_CORE CPUID */ Meh, this is not a very helpful comment, no need to carry it forward. And if you drop it, then the need for curly braces goes away. > + kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE); > + } > + } > > /* CPUID 0x8000001F (SME/SEV features) */ > sev_set_cpu_caps(); > -- > 2.39.1 >
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index dd21e8b1a259..f4a4691b4f4e 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4910,9 +4910,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); + if (enable_pmu) { + if (kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS_CORE) { + kvm_pmu_cap.num_counters_gp = AMD64_NUM_COUNTERS; + } else { + /* AMD PMU PERFCTR_CORE CPUID */ + kvm_cpu_cap_check_and_set(X86_FEATURE_PERFCTR_CORE); + } + } /* CPUID 0x8000001F (SME/SEV features) */ sev_set_cpu_caps();