Hi Andrew,
On 08/03/2019 12:07, Andrew Murray wrote:
In order to effeciently switch events_{guest,host} perf counters at
guest entry/exit we add bitfields to kvm_cpu_context for guest and host
events as well as accessors for updating them.
A function is also provided which allows the PMU driver to determine
if a counter should start counting when it is enabled. With exclude_host,
events on !VHE we may only start counting when entering the guest.
Some minor comments below.
Signed-off-by: Andrew Murray <andrew.murray@xxxxxxx>
---
arch/arm64/include/asm/kvm_host.h | 17 +++++++++++
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/pmu.c | 49 +++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/pmu.c
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1d36619d6650..4b7219128f2d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -207,8 +207,14 @@ struct kvm_cpu_context {
struct kvm_vcpu *__hyp_running_vcpu;
};
+struct kvm_pmu_events {
+ u32 events_host;
+ u32 events_guest;
+};
+
struct kvm_host_data {
struct kvm_cpu_context host_ctxt;
+ struct kvm_pmu_events pmu_events;
};
typedef struct kvm_host_data kvm_host_data_t;
@@ -479,11 +485,22 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
+static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
nit: s/defered/deferred/
+{
+ return attr->exclude_host;
+}
+
Does it need a definition for !CONFIG_KVM case, to make sure that
the events are always enabled for that case ? i.e, does this introduce
a change in behavior for !CONFIG_KVM case ?
#ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
{
return kvm_arch_vcpu_run_map_fp(vcpu);
}
+
+void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
+void kvm_clr_pmu_events(u32 clr);
+#else
+static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
+static inline void kvm_clr_pmu_events(u32 clr) {}
#endif
static inline void kvm_arm_vhe_guest_enter(void)
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 0f2a135ba15b..f34cb49b66ae 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
-kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
+kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o pmu.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
new file mode 100644
index 000000000000..43965a3cc0f4
--- /dev/null
+++ b/arch/arm64/kvm/pmu.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * arch/arm64/kvm/pmu.c: Switching between guest and host counters
minor nit: You don't need the file name, it is superfluous.
+ *
+ * Copyright 2019 Arm Limited
+ * Author: Andrew Murray <Andrew.Murray@xxxxxxx>
+ */
+#include <linux/kvm_host.h>
+#include <linux/perf_event.h>
+
+DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
nit: Do we really need this ? This is already in asm/kvm_host.h.
+
+/*
+ * Given the exclude_{host,guest} attributes, determine if we are going
+ * to need to switch counters at guest entry/exit.
+ */
+static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
+{
+ /* Only switch if attributes are different */
+ return (attr->exclude_host ^ attr->exclude_guest);
I back Julien's suggestion to keep this simple.
+}
+
+/*
+ * Add events to track that we may want to switch at guest entry/exit
+ * time.
+ */
+void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
+{
+ struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
+
+ if (!kvm_pmu_switch_needed(attr))
+ return;
+
+ if (!attr->exclude_host)
+ ctx->pmu_events.events_host |= set;
+ if (!attr->exclude_guest)
+ ctx->pmu_events.events_guest |= set;
+}
+
+/*
+ * Stop tracking events
+ */
+void kvm_clr_pmu_events(u32 clr)
+{
+ struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
+
+ ctx->pmu_events.events_host &= ~clr;
+ ctx->pmu_events.events_guest &= ~clr;
+}
Rest looks fine.
Suzuki
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
