From: Like Xu <[email protected]>
Disable PMU support when running on AMD and perf reports fewer than four
general purpose counters. All AMD PMUs must define at least four counters
due to AMD's legacy architecture hardcoding the number of counters
without providing a way to enumerate the number of counters to software,
e.g. from AMD's APM:
The legacy architecture defines four performance counters (PerfCtrn)
and corresponding event-select registers (PerfEvtSeln).
Virtualizing fewer than four counters can lead to guest instability as
software expects four counters to be available. Rather than bleed AMD
details into the common code, just define a const unsigned int and
provide a convenient location to document why Intel and AMD have different
mins (in particular, AMD's lack of any way to enumerate less than four
counters to the guest).
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.
Cc: Jim Mattson <[email protected]>
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Like Xu <[email protected]>
---
arch/x86/kvm/pmu.h | 14 ++++++++++----
arch/x86/kvm/svm/pmu.c | 1 +
arch/x86/kvm/vmx/pmu_intel.c | 1 +
3 files changed, 12 insertions(+), 4 deletions(-)
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,
};
--
2.40.1
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).