| Message ID | 20230530060423.32361-6-likexu@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Add AMD Guest PerfMonV2 PMU support | expand |
On Tue, May 30, 2023, Like Xu wrote: > According to Intel SDM, "Intel Core Solo and Intel Core Duo processors > support base level functionality identified by version ID of 1. Processors > based on Intel Core microarchitecture support, at a minimum, the base > level functionality of architectural performance monitoring." Those > antique processors mentioned above all had at least two GP counters, > subsequent processors had more and more GP counters, and given KVM's > quirky handling of MSR_P6_PERFCTR0/1, the value of MIN_NR_GP_COUNTERS > for the Intel Arch PMU can safely be 2. Not sure what you mean by "safely", but I don't think this is correct. KVM can, and more importantly has up until this point, supported a vPMU so long as the CPU has at least one counter. Perf's support for P6/Core CPUs does appear to expect and require 2 counters, but unless I'm misreading arch/x86/events/intel/core.c, perf will happily chug along with a single counter when running on a modern CPU. I doubt such a CPU exists in real silicon, but I can certainly imagine a virtual CPU being configured with a single counter, and this change would break such a setup. And *if* we really want to raise the minimum to '2', that should be done in a separate commit. But I don't see any reason to force the issue. No need to send v7 just for this, I can fixup when applying (planning on reviewing the series tomorrow).
diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h index dd7c7d4ffe3b..019a8ed51b12 100644 --- a/arch/x86/kvm/pmu.h +++ b/arch/x86/kvm/pmu.h @@ -36,6 +36,7 @@ struct kvm_pmu_ops { const u64 EVENTSEL_EVENT; const int MAX_NR_GP_COUNTERS; + const int MIN_NR_GP_COUNTERS; }; void kvm_pmu_ops_update(const struct kvm_pmu_ops *pmu_ops); @@ -160,6 +161,7 @@ extern struct x86_pmu_capability kvm_pmu_cap; static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops) { bool is_intel = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL; + int min_nr_gp_ctrs = pmu_ops->MIN_NR_GP_COUNTERS; /* * Hybrid PMUs don't play nice with virtualization without careful @@ -174,11 +176,15 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops) perf_get_x86_pmu_capability(&kvm_pmu_cap); /* - * For Intel, only support guest architectural pmu - * on a host with architectural pmu. + * WARN if perf did NOT disable hardware PMU if the number of + * architecturally required GP counters aren't present, i.e. if + * there are a non-zero number of counters, but fewer than what + * is architecturally required. */ - if ((is_intel && !kvm_pmu_cap.version) || - !kvm_pmu_cap.num_counters_gp) + if (!kvm_pmu_cap.num_counters_gp || + WARN_ON_ONCE(kvm_pmu_cap.num_counters_gp < min_nr_gp_ctrs)) + enable_pmu = false; + else if (is_intel && !kvm_pmu_cap.version) enable_pmu = false; } diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c index 70143275e0a7..e5c69062a909 100644 --- a/arch/x86/kvm/svm/pmu.c +++ b/arch/x86/kvm/svm/pmu.c @@ -224,4 +224,5 @@ struct kvm_pmu_ops amd_pmu_ops __initdata = { .reset = amd_pmu_reset, .EVENTSEL_EVENT = AMD64_EVENTSEL_EVENT, .MAX_NR_GP_COUNTERS = KVM_AMD_PMC_MAX_GENERIC, + .MIN_NR_GP_COUNTERS = AMD64_NUM_COUNTERS, }; diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index 99d07ccb1869..08851b49e1d4 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -811,4 +811,5 @@ struct kvm_pmu_ops intel_pmu_ops __initdata = { .cleanup = intel_pmu_cleanup, .EVENTSEL_EVENT = ARCH_PERFMON_EVENTSEL_EVENT, .MAX_NR_GP_COUNTERS = KVM_INTEL_PMC_MAX_GENERIC, + .MIN_NR_GP_COUNTERS = 2, };