[v6,02/20] KVM: x86/pmu: Don't enumerate support for fixed counters KVM can't virtualize
Commit Message
Hide fixed counters for which perf is incapable of creating the associated
architectural event. Except for the so called pseudo-architectural event
for counting TSC reference cycle, KVM virtualizes fixed counters by
creating a perf event for the associated general purpose architectural
event. If the associated event isn't supported in hardware, KVM can't
actually virtualize the fixed counter because perf will likely not program
up the correct event.
Note, this issue is almost certainly limited to running KVM on a funky
virtual CPU model, no known real hardware has an asymmetric PMU where a
fixed counter is supported but the associated architectural event is not.
Fixes: f5132b01386b ("KVM: Expose a version 2 architectural PMU to a guests")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/pmu.h | 4 ++++
arch/x86/kvm/vmx/pmu_intel.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
Comments
On Fri, Nov 3, 2023 at 5:02 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Hide fixed counters for which perf is incapable of creating the associated
> architectural event. Except for the so called pseudo-architectural event
> for counting TSC reference cycle, KVM virtualizes fixed counters by
> creating a perf event for the associated general purpose architectural
> event. If the associated event isn't supported in hardware, KVM can't
> actually virtualize the fixed counter because perf will likely not program
> up the correct event.
Won't it? My understanding was that perf preferred to use a fixed
counter when there was a choice of fixed or general purpose counter.
Unless the fixed counter is already assigned to a perf_event, KVM's
request should be satisfied by assigning the fixed counter.
> Note, this issue is almost certainly limited to running KVM on a funky
> virtual CPU model, no known real hardware has an asymmetric PMU where a
> fixed counter is supported but the associated architectural event is not.
This seems like a fix looking for a problem. Has the "problem"
actually been encountered?
> Fixes: f5132b01386b ("KVM: Expose a version 2 architectural PMU to a guests")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/pmu.h | 4 ++++
> arch/x86/kvm/vmx/pmu_intel.c | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index 1d64113de488..5341e8f69a22 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
> @@ -19,6 +19,7 @@
> #define VMWARE_BACKDOOR_PMC_APPARENT_TIME 0x10002
>
> struct kvm_pmu_ops {
> + void (*init_pmu_capability)(void);
> bool (*hw_event_available)(struct kvm_pmc *pmc);
> struct kvm_pmc *(*pmc_idx_to_pmc)(struct kvm_pmu *pmu, int pmc_idx);
> struct kvm_pmc *(*rdpmc_ecx_to_pmc)(struct kvm_vcpu *vcpu,
> @@ -218,6 +219,9 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops)
> pmu_ops->MAX_NR_GP_COUNTERS);
> kvm_pmu_cap.num_counters_fixed = min(kvm_pmu_cap.num_counters_fixed,
> KVM_PMC_MAX_FIXED);
> +
> + if (pmu_ops->init_pmu_capability)
> + pmu_ops->init_pmu_capability();
> }
>
> static inline void kvm_pmu_request_counter_reprogram(struct kvm_pmc *pmc)
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 1b13a472e3f2..3316fdea212a 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -68,6 +68,36 @@ static int fixed_pmc_events[] = {
> [2] = PSEUDO_ARCH_REFERENCE_CYCLES,
> };
>
> +static void intel_init_pmu_capability(void)
> +{
> + int i;
> +
> + /*
> + * Perf may (sadly) back a guest fixed counter with a general purpose
> + * counter, and so KVM must hide fixed counters whose associated
> + * architectural event are unsupported. On real hardware, this should
> + * never happen, but if KVM is running on a funky virtual CPU model...
> + *
> + * TODO: Drop this horror if/when KVM stops using perf events for
> + * guest fixed counters, or can explicitly request fixed counters.
> + */
> + for (i = 0; i < kvm_pmu_cap.num_counters_fixed; i++) {
> + int event = fixed_pmc_events[i];
> +
> + /*
> + * Ignore pseudo-architectural events, they're a bizarre way of
> + * requesting events from perf that _can't_ be backed with a
> + * general purpose architectural event, i.e. they're guaranteed
> + * to be backed by the real fixed counter.
> + */
> + if (event < NR_REAL_INTEL_ARCH_EVENTS &&
> + (kvm_pmu_cap.events_mask & BIT(event)))
> + break;
> + }
> +
> + kvm_pmu_cap.num_counters_fixed = i;
> +}
> +
> static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data)
> {
> struct kvm_pmc *pmc;
> @@ -789,6 +819,7 @@ void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu)
> }
>
> struct kvm_pmu_ops intel_pmu_ops __initdata = {
> + .init_pmu_capability = intel_init_pmu_capability,
> .hw_event_available = intel_hw_event_available,
> .pmc_idx_to_pmc = intel_pmc_idx_to_pmc,
> .rdpmc_ecx_to_pmc = intel_rdpmc_ecx_to_pmc,
> --
> 2.42.0.869.gea05f2083d-goog
>
On Sat, Nov 04, 2023, Jim Mattson wrote:
> On Fri, Nov 3, 2023 at 5:02 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Hide fixed counters for which perf is incapable of creating the associated
> > architectural event. Except for the so called pseudo-architectural event
> > for counting TSC reference cycle, KVM virtualizes fixed counters by
> > creating a perf event for the associated general purpose architectural
> > event. If the associated event isn't supported in hardware, KVM can't
> > actually virtualize the fixed counter because perf will likely not program
> > up the correct event.
>
> Won't it? My understanding was that perf preferred to use a fixed
> counter when there was a choice of fixed or general purpose counter.
> Unless the fixed counter is already assigned to a perf_event, KVM's
> request should be satisfied by assigning the fixed counter.
>
> > Note, this issue is almost certainly limited to running KVM on a funky
> > virtual CPU model, no known real hardware has an asymmetric PMU where a
> > fixed counter is supported but the associated architectural event is not.
>
> This seems like a fix looking for a problem. Has the "problem"
> actually been encountered?
Heh, yes, I "encountered" the problem in a curated VM I created. But I completely
agree that this is unnecessary, especially since odds are very, very good that
requesting the architectural general purpose encoding will still work. E.g. in
my goofy setup, the underlying hardware does support the architectural event and
so even if perf doesn't use the fixed counter for whatever reason, the GP counter
will still count the right event.
@@ -19,6 +19,7 @@
#define VMWARE_BACKDOOR_PMC_APPARENT_TIME 0x10002
struct kvm_pmu_ops {
+ void (*init_pmu_capability)(void);
bool (*hw_event_available)(struct kvm_pmc *pmc);
struct kvm_pmc *(*pmc_idx_to_pmc)(struct kvm_pmu *pmu, int pmc_idx);
struct kvm_pmc *(*rdpmc_ecx_to_pmc)(struct kvm_vcpu *vcpu,
@@ -218,6 +219,9 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops)
pmu_ops->MAX_NR_GP_COUNTERS);
kvm_pmu_cap.num_counters_fixed = min(kvm_pmu_cap.num_counters_fixed,
KVM_PMC_MAX_FIXED);
+
+ if (pmu_ops->init_pmu_capability)
+ pmu_ops->init_pmu_capability();
}
static inline void kvm_pmu_request_counter_reprogram(struct kvm_pmc *pmc)
@@ -68,6 +68,36 @@ static int fixed_pmc_events[] = {
[2] = PSEUDO_ARCH_REFERENCE_CYCLES,
};
+static void intel_init_pmu_capability(void)
+{
+ int i;
+
+ /*
+ * Perf may (sadly) back a guest fixed counter with a general purpose
+ * counter, and so KVM must hide fixed counters whose associated
+ * architectural event are unsupported. On real hardware, this should
+ * never happen, but if KVM is running on a funky virtual CPU model...
+ *
+ * TODO: Drop this horror if/when KVM stops using perf events for
+ * guest fixed counters, or can explicitly request fixed counters.
+ */
+ for (i = 0; i < kvm_pmu_cap.num_counters_fixed; i++) {
+ int event = fixed_pmc_events[i];
+
+ /*
+ * Ignore pseudo-architectural events, they're a bizarre way of
+ * requesting events from perf that _can't_ be backed with a
+ * general purpose architectural event, i.e. they're guaranteed
+ * to be backed by the real fixed counter.
+ */
+ if (event < NR_REAL_INTEL_ARCH_EVENTS &&
+ (kvm_pmu_cap.events_mask & BIT(event)))
+ break;
+ }
+
+ kvm_pmu_cap.num_counters_fixed = i;
+}
+
static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data)
{
struct kvm_pmc *pmc;
@@ -789,6 +819,7 @@ void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu)
}
struct kvm_pmu_ops intel_pmu_ops __initdata = {
+ .init_pmu_capability = intel_init_pmu_capability,
.hw_event_available = intel_hw_event_available,
.pmc_idx_to_pmc = intel_pmc_idx_to_pmc,
.rdpmc_ecx_to_pmc = intel_rdpmc_ecx_to_pmc,