All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 0/8] arm64: Support perf event modifiers :G and :H
@ 2019-03-08 12:07 ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

This patchset provides support for perf event modifiers :G and :H which
allows for filtering of PMU events between host and guests when used
with KVM.

As the underlying hardware cannot distinguish between guest and host
context we must switch the performance counters upon entry/exit to the
guest.

For non-VHE the counters must be stopped and started upon entry/exit to
the guest in __kvm_vcpu_run_nvhe.

For VHE we keep the counters enabled but instead change the event type
to include/exclude EL0 as appropriate. This allows us to perform this
switch outside the critical section of world-switch code in vcpu_load.

This has been tested with VHE and non-VHE kernels with a KVM guest.

Changes from v10:

 - Remove counter switch code from kvm_vcpu_run_vhe and replace with
   enable/disable EL0 event type in kvm_arch_vcpu_load instead

 - Reduce counter switch code in __kvm_vcpu_run_nvhe by moving some
   logic to kvm_set_pmu_events (kvm_pmu_switch_needed)

 - Simplify code by removing need for KVM_PMU_EVENTS_{HOST,GUEST}

 - Add kvm_host helper function to let PMU determine if event should
   start counting when it is enabled

 - Exclude EL2 on !VHE when exclude_host (to avoid counting host events
   as guest during entry/exit)

 - Moved PMU switching code to its own file

 - Rebased to v5.0

 - Added documentation

 - Removed Reviewed-By's for changed patches and updated commit messages

Changes from v9:

 - Rebased to v5.0-rc2
 - Ensure get_host_ctxt considers host_ctxt offset

Changes from v8:

 - Added additional comments
 - Fixed bisect build failure
 - Renamed cpu_ctxt variable to cpu_data

Changes from v7:

 - Added additional patch to encapsulate kvm_cpu_context in kvm_host_data

Changes from v6:

 - Move events_host/events_guest out of kvm_cpu_context

Changes from v5:

 - Tweak logic in use of kvm_set_pmu_events

Changes from v4:

 - Prevent unnecessary write_sysreg calls by improving
   __pmu_switch_to_xxx logic.

Changes from v3:

 - Remove confusing _only suffix from bitfields in kvm_cpu_context
 - Remove unnecessary condition when clearing event bits in disable
 - Simplify API of KVM accessors
 - Prevent unnecessary setting of pmcnten when guest/host events are
   the same.

Changes from v2:

 - Ensured that exclude_kernel works for guest
 - Removed unnecessary exclusion of EL2 with exclude_host on !VHE
 - Renamed kvm_clr_set_host_pmu_events to reflect args order
 - Added additional information to isb patch

Changes from v1:

 - Removed unnecessary exclusion of EL1 with exclude_guest on VHE
 - Removed unnecessary isb from existing perf_event.c driver
 - Folded perf_event.c patches together
 - Added additional information to last patch commit message


Andrew Murray (8):
  arm64: arm_pmu: remove unnecessary isb instruction
  arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  arm64: KVM: add accessors to track guest/host only counters
  arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest
    attributes
  arm64: KVM: Enable !VHE support for :G/:H perf event modifiers
  arm64: KVM: Enable VHE support for :G/:H perf event modifiers
  arm64: KVM: avoid isb's by using direct pmxevtyper sysreg
  arm64: docs: document perf event attributes

 Documentation/arm64/perf.txt      |  74 ++++++++++
 arch/arm/include/asm/kvm_host.h   |  11 +-
 arch/arm64/include/asm/kvm_asm.h  |   3 +-
 arch/arm64/include/asm/kvm_host.h |  37 ++++-
 arch/arm64/kernel/asm-offsets.c   |   1 +
 arch/arm64/kernel/perf_event.c    |  50 +++++--
 arch/arm64/kvm/Makefile           |   2 +-
 arch/arm64/kvm/hyp/switch.c       |   6 +
 arch/arm64/kvm/pmu.c              | 236 ++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c         |   3 +
 virt/kvm/arm/arm.c                |  14 +-
 11 files changed, 414 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/arm64/perf.txt
 create mode 100644 arch/arm64/kvm/pmu.c

-- 
2.21.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v11 0/8] arm64: Support perf event modifiers :G and :H
@ 2019-03-08 12:07 ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

This patchset provides support for perf event modifiers :G and :H which
allows for filtering of PMU events between host and guests when used
with KVM.

As the underlying hardware cannot distinguish between guest and host
context we must switch the performance counters upon entry/exit to the
guest.

For non-VHE the counters must be stopped and started upon entry/exit to
the guest in __kvm_vcpu_run_nvhe.

For VHE we keep the counters enabled but instead change the event type
to include/exclude EL0 as appropriate. This allows us to perform this
switch outside the critical section of world-switch code in vcpu_load.

This has been tested with VHE and non-VHE kernels with a KVM guest.

Changes from v10:

 - Remove counter switch code from kvm_vcpu_run_vhe and replace with
   enable/disable EL0 event type in kvm_arch_vcpu_load instead

 - Reduce counter switch code in __kvm_vcpu_run_nvhe by moving some
   logic to kvm_set_pmu_events (kvm_pmu_switch_needed)

 - Simplify code by removing need for KVM_PMU_EVENTS_{HOST,GUEST}

 - Add kvm_host helper function to let PMU determine if event should
   start counting when it is enabled

 - Exclude EL2 on !VHE when exclude_host (to avoid counting host events
   as guest during entry/exit)

 - Moved PMU switching code to its own file

 - Rebased to v5.0

 - Added documentation

 - Removed Reviewed-By's for changed patches and updated commit messages

Changes from v9:

 - Rebased to v5.0-rc2
 - Ensure get_host_ctxt considers host_ctxt offset

Changes from v8:

 - Added additional comments
 - Fixed bisect build failure
 - Renamed cpu_ctxt variable to cpu_data

Changes from v7:

 - Added additional patch to encapsulate kvm_cpu_context in kvm_host_data

Changes from v6:

 - Move events_host/events_guest out of kvm_cpu_context

Changes from v5:

 - Tweak logic in use of kvm_set_pmu_events

Changes from v4:

 - Prevent unnecessary write_sysreg calls by improving
   __pmu_switch_to_xxx logic.

Changes from v3:

 - Remove confusing _only suffix from bitfields in kvm_cpu_context
 - Remove unnecessary condition when clearing event bits in disable
 - Simplify API of KVM accessors
 - Prevent unnecessary setting of pmcnten when guest/host events are
   the same.

Changes from v2:

 - Ensured that exclude_kernel works for guest
 - Removed unnecessary exclusion of EL2 with exclude_host on !VHE
 - Renamed kvm_clr_set_host_pmu_events to reflect args order
 - Added additional information to isb patch

Changes from v1:

 - Removed unnecessary exclusion of EL1 with exclude_guest on VHE
 - Removed unnecessary isb from existing perf_event.c driver
 - Folded perf_event.c patches together
 - Added additional information to last patch commit message


Andrew Murray (8):
  arm64: arm_pmu: remove unnecessary isb instruction
  arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  arm64: KVM: add accessors to track guest/host only counters
  arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest
    attributes
  arm64: KVM: Enable !VHE support for :G/:H perf event modifiers
  arm64: KVM: Enable VHE support for :G/:H perf event modifiers
  arm64: KVM: avoid isb's by using direct pmxevtyper sysreg
  arm64: docs: document perf event attributes

 Documentation/arm64/perf.txt      |  74 ++++++++++
 arch/arm/include/asm/kvm_host.h   |  11 +-
 arch/arm64/include/asm/kvm_asm.h  |   3 +-
 arch/arm64/include/asm/kvm_host.h |  37 ++++-
 arch/arm64/kernel/asm-offsets.c   |   1 +
 arch/arm64/kernel/perf_event.c    |  50 +++++--
 arch/arm64/kvm/Makefile           |   2 +-
 arch/arm64/kvm/hyp/switch.c       |   6 +
 arch/arm64/kvm/pmu.c              | 236 ++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c         |   3 +
 virt/kvm/arm/arm.c                |  14 +-
 11 files changed, 414 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/arm64/perf.txt
 create mode 100644 arch/arm64/kvm/pmu.c

-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v11 1/8] arm64: arm_pmu: remove unnecessary isb instruction
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

The armv8pmu_enable_event_counter function issues an isb instruction
after enabling a pair of counters - this doesn't provide any value
and is inconsistent with the armv8pmu_disable_event_counter.

In any case armv8pmu_enable_event_counter is always called with the
PMU stopped. Starting the PMU with armv8pmu_start results in an isb
instruction being issued prior to writing to PMCR_EL0.

Let's remove the unnecessary isb instruction.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/perf_event.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 1620a371b1f5..1c717965b3ff 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -533,7 +533,6 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
 	armv8pmu_enable_counter(idx);
 	if (armv8pmu_event_is_chained(event))
 		armv8pmu_enable_counter(idx - 1);
-	isb();
 }
 
 static inline int armv8pmu_disable_counter(int idx)
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 1/8] arm64: arm_pmu: remove unnecessary isb instruction
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

The armv8pmu_enable_event_counter function issues an isb instruction
after enabling a pair of counters - this doesn't provide any value
and is inconsistent with the armv8pmu_disable_event_counter.

In any case armv8pmu_enable_event_counter is always called with the
PMU stopped. Starting the PMU with armv8pmu_start results in an isb
instruction being issued prior to writing to PMCR_EL0.

Let's remove the unnecessary isb instruction.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/perf_event.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 1620a371b1f5..1c717965b3ff 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -533,7 +533,6 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
 	armv8pmu_enable_counter(idx);
 	if (armv8pmu_event_is_chained(event))
 		armv8pmu_enable_counter(idx - 1);
-	isb();
 }
 
 static inline int armv8pmu_disable_counter(int idx)
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

The virt/arm core allocates a kvm_cpu_context_t percpu, at present this is
a typedef to kvm_cpu_context and is used to store host cpu context. The
kvm_cpu_context structure is also used elsewhere to hold vcpu context.
In order to use the percpu to hold additional future host information we
encapsulate kvm_cpu_context in a new structure and rename the typedef and
percpu to match.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  8 ++++++--
 arch/arm64/include/asm/kvm_asm.h  |  3 ++-
 arch/arm64/include/asm/kvm_host.h | 14 +++++++++-----
 arch/arm64/kernel/asm-offsets.c   |  1 +
 virt/kvm/arm/arm.c                | 12 +++++++-----
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 50e89869178a..a358cb15bb0d 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -146,7 +146,11 @@ struct kvm_cpu_context {
 	u32 cp15[NR_CP15_REGS];
 };
 
-typedef struct kvm_cpu_context kvm_cpu_context_t;
+struct kvm_host_data {
+	struct kvm_cpu_context host_ctxt;
+};
+
+typedef struct kvm_host_data kvm_host_data_t;
 
 struct vcpu_reset_state {
 	unsigned long	pc;
@@ -171,7 +175,7 @@ struct kvm_vcpu_arch {
 	struct kvm_vcpu_fault_info fault;
 
 	/* Host FP context */
-	kvm_cpu_context_t *host_cpu_context;
+	struct kvm_cpu_context *host_cpu_context;
 
 	/* VGIC state */
 	struct vgic_cpu vgic_cpu;
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index f5b79e995f40..ff73f5462aca 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -108,7 +108,8 @@ extern u32 __kvm_get_mdcr_el2(void);
 .endm
 
 .macro get_host_ctxt reg, tmp
-	hyp_adr_this_cpu \reg, kvm_host_cpu_state, \tmp
+	hyp_adr_this_cpu \reg, kvm_host_data, \tmp
+	add	\reg, \reg, #HOST_DATA_CONTEXT
 .endm
 
 .macro get_vcpu_ptr vcpu, ctxt
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index da3fc7324d68..1d36619d6650 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -207,7 +207,11 @@ struct kvm_cpu_context {
 	struct kvm_vcpu *__hyp_running_vcpu;
 };
 
-typedef struct kvm_cpu_context kvm_cpu_context_t;
+struct kvm_host_data {
+	struct kvm_cpu_context host_ctxt;
+};
+
+typedef struct kvm_host_data kvm_host_data_t;
 
 struct vcpu_reset_state {
 	unsigned long	pc;
@@ -250,7 +254,7 @@ struct kvm_vcpu_arch {
 	struct kvm_guest_debug_arch external_debug_state;
 
 	/* Pointer to host CPU context */
-	kvm_cpu_context_t *host_cpu_context;
+	struct kvm_cpu_context *host_cpu_context;
 
 	struct thread_info *host_thread_info;	/* hyp VA */
 	struct user_fpsimd_state *host_fpsimd_state;	/* hyp VA */
@@ -398,7 +402,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
 
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
-DECLARE_PER_CPU(kvm_cpu_context_t, kvm_host_cpu_state);
+DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
 
 void __kvm_enable_ssbs(void);
 
@@ -411,8 +415,8 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
 	 * kernel's mapping to the linear mapping, and store it in tpidr_el2
 	 * so that we can use adr_l to access per-cpu variables in EL2.
 	 */
-	u64 tpidr_el2 = ((u64)this_cpu_ptr(&kvm_host_cpu_state) -
-			 (u64)kvm_ksym_ref(kvm_host_cpu_state));
+	u64 tpidr_el2 = ((u64)this_cpu_ptr(&kvm_host_data) -
+			 (u64)kvm_ksym_ref(kvm_host_data));
 
 	/*
 	 * Call initialization code, and switch to the full blown HYP code.
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 65b8afc84466..9b03554ba5d2 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -146,6 +146,7 @@ int main(void)
   DEFINE(VCPU_FPEXC32_EL2,	offsetof(struct kvm_vcpu, arch.ctxt.sys_regs[FPEXC32_EL2]));
   DEFINE(VCPU_HOST_CONTEXT,	offsetof(struct kvm_vcpu, arch.host_cpu_context));
   DEFINE(HOST_CONTEXT_VCPU,	offsetof(struct kvm_cpu_context, __hyp_running_vcpu));
+  DEFINE(HOST_DATA_CONTEXT,	offsetof(struct kvm_host_data, host_ctxt));
 #endif
 #ifdef CONFIG_CPU_PM
   DEFINE(CPU_SUSPEND_SZ,	sizeof(struct cpu_suspend_ctx));
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 9c486fad3f9f..6958b98b8d52 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -56,7 +56,7 @@
 __asm__(".arch_extension	virt");
 #endif
 
-DEFINE_PER_CPU(kvm_cpu_context_t, kvm_host_cpu_state);
+DEFINE_PER_CPU(kvm_host_data_t, kvm_host_data);
 static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
 
 /* Per-CPU variable containing the currently running vcpu. */
@@ -363,8 +363,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 	int *last_ran;
+	kvm_host_data_t *cpu_data;
 
 	last_ran = this_cpu_ptr(vcpu->kvm->arch.last_vcpu_ran);
+	cpu_data = this_cpu_ptr(&kvm_host_data);
 
 	/*
 	 * We might get preempted before the vCPU actually runs, but
@@ -376,7 +378,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	}
 
 	vcpu->cpu = cpu;
-	vcpu->arch.host_cpu_context = this_cpu_ptr(&kvm_host_cpu_state);
+	vcpu->arch.host_cpu_context = &cpu_data->host_ctxt;
 
 	kvm_arm_set_running_vcpu(vcpu);
 	kvm_vgic_load(vcpu);
@@ -1568,10 +1570,10 @@ static int init_hyp_mode(void)
 	}
 
 	for_each_possible_cpu(cpu) {
-		kvm_cpu_context_t *cpu_ctxt;
+		kvm_host_data_t *cpu_data;
 
-		cpu_ctxt = per_cpu_ptr(&kvm_host_cpu_state, cpu);
-		err = create_hyp_mappings(cpu_ctxt, cpu_ctxt + 1, PAGE_HYP);
+		cpu_data = per_cpu_ptr(&kvm_host_data, cpu);
+		err = create_hyp_mappings(cpu_data, cpu_data + 1, PAGE_HYP);
 
 		if (err) {
 			kvm_err("Cannot map host CPU state: %d\n", err);
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

The virt/arm core allocates a kvm_cpu_context_t percpu, at present this is
a typedef to kvm_cpu_context and is used to store host cpu context. The
kvm_cpu_context structure is also used elsewhere to hold vcpu context.
In order to use the percpu to hold additional future host information we
encapsulate kvm_cpu_context in a new structure and rename the typedef and
percpu to match.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  8 ++++++--
 arch/arm64/include/asm/kvm_asm.h  |  3 ++-
 arch/arm64/include/asm/kvm_host.h | 14 +++++++++-----
 arch/arm64/kernel/asm-offsets.c   |  1 +
 virt/kvm/arm/arm.c                | 12 +++++++-----
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 50e89869178a..a358cb15bb0d 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -146,7 +146,11 @@ struct kvm_cpu_context {
 	u32 cp15[NR_CP15_REGS];
 };
 
-typedef struct kvm_cpu_context kvm_cpu_context_t;
+struct kvm_host_data {
+	struct kvm_cpu_context host_ctxt;
+};
+
+typedef struct kvm_host_data kvm_host_data_t;
 
 struct vcpu_reset_state {
 	unsigned long	pc;
@@ -171,7 +175,7 @@ struct kvm_vcpu_arch {
 	struct kvm_vcpu_fault_info fault;
 
 	/* Host FP context */
-	kvm_cpu_context_t *host_cpu_context;
+	struct kvm_cpu_context *host_cpu_context;
 
 	/* VGIC state */
 	struct vgic_cpu vgic_cpu;
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index f5b79e995f40..ff73f5462aca 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -108,7 +108,8 @@ extern u32 __kvm_get_mdcr_el2(void);
 .endm
 
 .macro get_host_ctxt reg, tmp
-	hyp_adr_this_cpu \reg, kvm_host_cpu_state, \tmp
+	hyp_adr_this_cpu \reg, kvm_host_data, \tmp
+	add	\reg, \reg, #HOST_DATA_CONTEXT
 .endm
 
 .macro get_vcpu_ptr vcpu, ctxt
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index da3fc7324d68..1d36619d6650 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -207,7 +207,11 @@ struct kvm_cpu_context {
 	struct kvm_vcpu *__hyp_running_vcpu;
 };
 
-typedef struct kvm_cpu_context kvm_cpu_context_t;
+struct kvm_host_data {
+	struct kvm_cpu_context host_ctxt;
+};
+
+typedef struct kvm_host_data kvm_host_data_t;
 
 struct vcpu_reset_state {
 	unsigned long	pc;
@@ -250,7 +254,7 @@ struct kvm_vcpu_arch {
 	struct kvm_guest_debug_arch external_debug_state;
 
 	/* Pointer to host CPU context */
-	kvm_cpu_context_t *host_cpu_context;
+	struct kvm_cpu_context *host_cpu_context;
 
 	struct thread_info *host_thread_info;	/* hyp VA */
 	struct user_fpsimd_state *host_fpsimd_state;	/* hyp VA */
@@ -398,7 +402,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
 
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
-DECLARE_PER_CPU(kvm_cpu_context_t, kvm_host_cpu_state);
+DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
 
 void __kvm_enable_ssbs(void);
 
@@ -411,8 +415,8 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
 	 * kernel's mapping to the linear mapping, and store it in tpidr_el2
 	 * so that we can use adr_l to access per-cpu variables in EL2.
 	 */
-	u64 tpidr_el2 = ((u64)this_cpu_ptr(&kvm_host_cpu_state) -
-			 (u64)kvm_ksym_ref(kvm_host_cpu_state));
+	u64 tpidr_el2 = ((u64)this_cpu_ptr(&kvm_host_data) -
+			 (u64)kvm_ksym_ref(kvm_host_data));
 
 	/*
 	 * Call initialization code, and switch to the full blown HYP code.
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 65b8afc84466..9b03554ba5d2 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -146,6 +146,7 @@ int main(void)
   DEFINE(VCPU_FPEXC32_EL2,	offsetof(struct kvm_vcpu, arch.ctxt.sys_regs[FPEXC32_EL2]));
   DEFINE(VCPU_HOST_CONTEXT,	offsetof(struct kvm_vcpu, arch.host_cpu_context));
   DEFINE(HOST_CONTEXT_VCPU,	offsetof(struct kvm_cpu_context, __hyp_running_vcpu));
+  DEFINE(HOST_DATA_CONTEXT,	offsetof(struct kvm_host_data, host_ctxt));
 #endif
 #ifdef CONFIG_CPU_PM
   DEFINE(CPU_SUSPEND_SZ,	sizeof(struct cpu_suspend_ctx));
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 9c486fad3f9f..6958b98b8d52 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -56,7 +56,7 @@
 __asm__(".arch_extension	virt");
 #endif
 
-DEFINE_PER_CPU(kvm_cpu_context_t, kvm_host_cpu_state);
+DEFINE_PER_CPU(kvm_host_data_t, kvm_host_data);
 static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
 
 /* Per-CPU variable containing the currently running vcpu. */
@@ -363,8 +363,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 	int *last_ran;
+	kvm_host_data_t *cpu_data;
 
 	last_ran = this_cpu_ptr(vcpu->kvm->arch.last_vcpu_ran);
+	cpu_data = this_cpu_ptr(&kvm_host_data);
 
 	/*
 	 * We might get preempted before the vCPU actually runs, but
@@ -376,7 +378,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	}
 
 	vcpu->cpu = cpu;
-	vcpu->arch.host_cpu_context = this_cpu_ptr(&kvm_host_cpu_state);
+	vcpu->arch.host_cpu_context = &cpu_data->host_ctxt;
 
 	kvm_arm_set_running_vcpu(vcpu);
 	kvm_vgic_load(vcpu);
@@ -1568,10 +1570,10 @@ static int init_hyp_mode(void)
 	}
 
 	for_each_possible_cpu(cpu) {
-		kvm_cpu_context_t *cpu_ctxt;
+		kvm_host_data_t *cpu_data;
 
-		cpu_ctxt = per_cpu_ptr(&kvm_host_cpu_state, cpu);
-		err = create_hyp_mappings(cpu_ctxt, cpu_ctxt + 1, PAGE_HYP);
+		cpu_data = per_cpu_ptr(&kvm_host_data, cpu);
+		err = create_hyp_mappings(cpu_data, cpu_data + 1, PAGE_HYP);
 
 		if (err) {
 			kvm_err("Cannot map host CPU state: %d\n", err);
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

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.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 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)
+{
+	return attr->exclude_host;
+}
+
 #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
+ *
+ * Copyright 2019 Arm Limited
+ * Author: Andrew Murray <Andrew.Murray@arm.com>
+ */
+#include <linux/kvm_host.h>
+#include <linux/perf_event.h>
+
+DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
+
+/*
+ * 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);
+}
+
+/*
+ * 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;
+}
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

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.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 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)
+{
+	return attr->exclude_host;
+}
+
 #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
+ *
+ * Copyright 2019 Arm Limited
+ * Author: Andrew Murray <Andrew.Murray@arm.com>
+ */
+#include <linux/kvm_host.h>
+#include <linux/perf_event.h>
+
+DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
+
+/*
+ * 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);
+}
+
+/*
+ * 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;
+}
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

Add support for the :G and :H attributes in perf by handling the
exclude_host/exclude_guest event attributes.

We notify KVM of counters that we wish to be enabled or disabled on
guest entry/exit and thus defer from starting or stopping events based
on their event attributes.

With !VHE we switch the counters between host/guest at EL2. We are able
to eliminate counters counting host events on the boundaries of guest
entry/exit when using :G by filtering out EL2 for exclude_host. When
using !exclude_hv there is a small blackout window at the guest
entry/exit where host events are not captured.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/kernel/perf_event.c | 43 ++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 1c717965b3ff..64f02a9fd7cd 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -26,6 +26,7 @@
 
 #include <linux/acpi.h>
 #include <linux/clocksource.h>
+#include <linux/kvm_host.h>
 #include <linux/of.h>
 #include <linux/perf/arm_pmu.h>
 #include <linux/platform_device.h>
@@ -528,11 +529,21 @@ static inline int armv8pmu_enable_counter(int idx)
 
 static inline void armv8pmu_enable_event_counter(struct perf_event *event)
 {
+	struct perf_event_attr *attr = &event->attr;
 	int idx = event->hw.idx;
+	u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
 
-	armv8pmu_enable_counter(idx);
 	if (armv8pmu_event_is_chained(event))
-		armv8pmu_enable_counter(idx - 1);
+		counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
+
+	kvm_set_pmu_events(counter_bits, attr);
+
+	/* We rely on the hypervisor switch code to enable guest counters */
+	if (!kvm_pmu_counter_defered(attr)) {
+		armv8pmu_enable_counter(idx);
+		if (armv8pmu_event_is_chained(event))
+			armv8pmu_enable_counter(idx - 1);
+	}
 }
 
 static inline int armv8pmu_disable_counter(int idx)
@@ -545,11 +556,21 @@ static inline int armv8pmu_disable_counter(int idx)
 static inline void armv8pmu_disable_event_counter(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
+	struct perf_event_attr *attr = &event->attr;
 	int idx = hwc->idx;
+	u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
 
 	if (armv8pmu_event_is_chained(event))
-		armv8pmu_disable_counter(idx - 1);
-	armv8pmu_disable_counter(idx);
+		counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
+
+	kvm_clr_pmu_events(counter_bits);
+
+	/* We rely on the hypervisor switch code to disable guest counters */
+	if (!kvm_pmu_counter_defered(attr)) {
+		if (armv8pmu_event_is_chained(event))
+			armv8pmu_disable_counter(idx - 1);
+		armv8pmu_disable_counter(idx);
+	}
 }
 
 static inline int armv8pmu_enable_intens(int idx)
@@ -829,11 +850,16 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
 		if (!attr->exclude_kernel)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
 	} else {
-		if (attr->exclude_kernel)
-			config_base |= ARMV8_PMU_EXCLUDE_EL1;
-		if (!attr->exclude_hv)
+		if (!attr->exclude_hv && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
 	}
+
+	/*
+	 * Filter out !VHE kernels and guest kernels
+	 */
+	if (attr->exclude_kernel)
+		config_base |= ARMV8_PMU_EXCLUDE_EL1;
+
 	if (attr->exclude_user)
 		config_base |= ARMV8_PMU_EXCLUDE_EL0;
 
@@ -863,6 +889,9 @@ static void armv8pmu_reset(void *info)
 		armv8pmu_disable_intens(idx);
 	}
 
+	/* Clear the counters we flip at guest entry/exit */
+	kvm_clr_pmu_events(U32_MAX);
+
 	/*
 	 * Initialize & Reset PMNC. Request overflow interrupt for
 	 * 64 bit cycle counter but cheat in armv8pmu_write_counter().
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

Add support for the :G and :H attributes in perf by handling the
exclude_host/exclude_guest event attributes.

We notify KVM of counters that we wish to be enabled or disabled on
guest entry/exit and thus defer from starting or stopping events based
on their event attributes.

With !VHE we switch the counters between host/guest at EL2. We are able
to eliminate counters counting host events on the boundaries of guest
entry/exit when using :G by filtering out EL2 for exclude_host. When
using !exclude_hv there is a small blackout window at the guest
entry/exit where host events are not captured.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/kernel/perf_event.c | 43 ++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 1c717965b3ff..64f02a9fd7cd 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -26,6 +26,7 @@
 
 #include <linux/acpi.h>
 #include <linux/clocksource.h>
+#include <linux/kvm_host.h>
 #include <linux/of.h>
 #include <linux/perf/arm_pmu.h>
 #include <linux/platform_device.h>
@@ -528,11 +529,21 @@ static inline int armv8pmu_enable_counter(int idx)
 
 static inline void armv8pmu_enable_event_counter(struct perf_event *event)
 {
+	struct perf_event_attr *attr = &event->attr;
 	int idx = event->hw.idx;
+	u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
 
-	armv8pmu_enable_counter(idx);
 	if (armv8pmu_event_is_chained(event))
-		armv8pmu_enable_counter(idx - 1);
+		counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
+
+	kvm_set_pmu_events(counter_bits, attr);
+
+	/* We rely on the hypervisor switch code to enable guest counters */
+	if (!kvm_pmu_counter_defered(attr)) {
+		armv8pmu_enable_counter(idx);
+		if (armv8pmu_event_is_chained(event))
+			armv8pmu_enable_counter(idx - 1);
+	}
 }
 
 static inline int armv8pmu_disable_counter(int idx)
@@ -545,11 +556,21 @@ static inline int armv8pmu_disable_counter(int idx)
 static inline void armv8pmu_disable_event_counter(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
+	struct perf_event_attr *attr = &event->attr;
 	int idx = hwc->idx;
+	u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
 
 	if (armv8pmu_event_is_chained(event))
-		armv8pmu_disable_counter(idx - 1);
-	armv8pmu_disable_counter(idx);
+		counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
+
+	kvm_clr_pmu_events(counter_bits);
+
+	/* We rely on the hypervisor switch code to disable guest counters */
+	if (!kvm_pmu_counter_defered(attr)) {
+		if (armv8pmu_event_is_chained(event))
+			armv8pmu_disable_counter(idx - 1);
+		armv8pmu_disable_counter(idx);
+	}
 }
 
 static inline int armv8pmu_enable_intens(int idx)
@@ -829,11 +850,16 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
 		if (!attr->exclude_kernel)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
 	} else {
-		if (attr->exclude_kernel)
-			config_base |= ARMV8_PMU_EXCLUDE_EL1;
-		if (!attr->exclude_hv)
+		if (!attr->exclude_hv && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
 	}
+
+	/*
+	 * Filter out !VHE kernels and guest kernels
+	 */
+	if (attr->exclude_kernel)
+		config_base |= ARMV8_PMU_EXCLUDE_EL1;
+
 	if (attr->exclude_user)
 		config_base |= ARMV8_PMU_EXCLUDE_EL0;
 
@@ -863,6 +889,9 @@ static void armv8pmu_reset(void *info)
 		armv8pmu_disable_intens(idx);
 	}
 
+	/* Clear the counters we flip at guest entry/exit */
+	kvm_clr_pmu_events(U32_MAX);
+
 	/*
 	 * Initialize & Reset PMNC. Request overflow interrupt for
 	 * 64 bit cycle counter but cheat in armv8pmu_write_counter().
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 5/8] arm64: KVM: Enable !VHE support for :G/:H perf event modifiers
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

Enable/disable event counters as appropriate when entering and exiting
the guest to enable support for guest or host only event counting.

For both VHE and non-VHE we switch the counters between host/guest at
EL2.

The PMU may be on when we change which counters are enabled however
we avoid adding an isb as we instead rely on existing context
synchronisation events: the eret to enter the guest (__guest_enter)
and eret in kvm_call_hyp for __kvm_vcpu_run_nvhe on returning.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 arch/arm64/kvm/hyp/switch.c       |  6 +++++
 arch/arm64/kvm/pmu.c              | 40 +++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 4b7219128f2d..7ca4e094626d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -498,6 +498,9 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 
 void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
 void kvm_clr_pmu_events(u32 clr);
+
+void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
+bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
 #else
 static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
 static inline void kvm_clr_pmu_events(u32 clr) {}
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 421ebf6f7086..7c543ff7e458 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -523,6 +523,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
 	struct kvm_cpu_context *guest_ctxt;
+	bool pmu_switch_needed;
 	u64 exit_code;
 
 	vcpu = kern_hyp_va(vcpu);
@@ -531,6 +532,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	host_ctxt->__hyp_running_vcpu = vcpu;
 	guest_ctxt = &vcpu->arch.ctxt;
 
+	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
+
 	__sysreg_save_state_nvhe(host_ctxt);
 
 	__activate_vm(kern_hyp_va(vcpu->kvm));
@@ -577,6 +580,9 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	 */
 	__debug_switch_to_host(vcpu);
 
+	if (pmu_switch_needed)
+		__pmu_switch_to_host(host_ctxt);
+
 	return exit_code;
 }
 
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 43965a3cc0f4..a1cee7919953 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -7,6 +7,7 @@
  */
 #include <linux/kvm_host.h>
 #include <linux/perf_event.h>
+#include <asm/kvm_hyp.h>
 
 DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
 
@@ -47,3 +48,42 @@ void kvm_clr_pmu_events(u32 clr)
 	ctx->pmu_events.events_host &= ~clr;
 	ctx->pmu_events.events_guest &= ~clr;
 }
+
+/**
+ * Disable host events, enable guest events
+ */
+bool __hyp_text __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
+{
+	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
+
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	pmu = &host->pmu_events;
+
+	if (pmu->events_host)
+		write_sysreg(pmu->events_host, pmcntenclr_el0);
+
+	if (pmu->events_guest)
+		write_sysreg(pmu->events_guest, pmcntenset_el0);
+
+	return (pmu->events_host || pmu->events_guest);
+}
+
+/**
+ * Disable guest events, enable host events
+ */
+void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
+{
+	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
+
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	pmu = &host->pmu_events;
+
+	if (pmu->events_guest)
+		write_sysreg(pmu->events_guest, pmcntenclr_el0);
+
+	if (pmu->events_host)
+		write_sysreg(pmu->events_host, pmcntenset_el0);
+}
+
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 5/8] arm64: KVM: Enable !VHE support for :G/:H perf event modifiers
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

Enable/disable event counters as appropriate when entering and exiting
the guest to enable support for guest or host only event counting.

For both VHE and non-VHE we switch the counters between host/guest at
EL2.

The PMU may be on when we change which counters are enabled however
we avoid adding an isb as we instead rely on existing context
synchronisation events: the eret to enter the guest (__guest_enter)
and eret in kvm_call_hyp for __kvm_vcpu_run_nvhe on returning.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 arch/arm64/kvm/hyp/switch.c       |  6 +++++
 arch/arm64/kvm/pmu.c              | 40 +++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 4b7219128f2d..7ca4e094626d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -498,6 +498,9 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 
 void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
 void kvm_clr_pmu_events(u32 clr);
+
+void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
+bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
 #else
 static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
 static inline void kvm_clr_pmu_events(u32 clr) {}
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 421ebf6f7086..7c543ff7e458 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -523,6 +523,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
 	struct kvm_cpu_context *guest_ctxt;
+	bool pmu_switch_needed;
 	u64 exit_code;
 
 	vcpu = kern_hyp_va(vcpu);
@@ -531,6 +532,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	host_ctxt->__hyp_running_vcpu = vcpu;
 	guest_ctxt = &vcpu->arch.ctxt;
 
+	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
+
 	__sysreg_save_state_nvhe(host_ctxt);
 
 	__activate_vm(kern_hyp_va(vcpu->kvm));
@@ -577,6 +580,9 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	 */
 	__debug_switch_to_host(vcpu);
 
+	if (pmu_switch_needed)
+		__pmu_switch_to_host(host_ctxt);
+
 	return exit_code;
 }
 
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 43965a3cc0f4..a1cee7919953 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -7,6 +7,7 @@
  */
 #include <linux/kvm_host.h>
 #include <linux/perf_event.h>
+#include <asm/kvm_hyp.h>
 
 DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
 
@@ -47,3 +48,42 @@ void kvm_clr_pmu_events(u32 clr)
 	ctx->pmu_events.events_host &= ~clr;
 	ctx->pmu_events.events_guest &= ~clr;
 }
+
+/**
+ * Disable host events, enable guest events
+ */
+bool __hyp_text __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
+{
+	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
+
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	pmu = &host->pmu_events;
+
+	if (pmu->events_host)
+		write_sysreg(pmu->events_host, pmcntenclr_el0);
+
+	if (pmu->events_guest)
+		write_sysreg(pmu->events_guest, pmcntenset_el0);
+
+	return (pmu->events_host || pmu->events_guest);
+}
+
+/**
+ * Disable guest events, enable host events
+ */
+void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
+{
+	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
+
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	pmu = &host->pmu_events;
+
+	if (pmu->events_guest)
+		write_sysreg(pmu->events_guest, pmcntenclr_el0);
+
+	if (pmu->events_host)
+		write_sysreg(pmu->events_host, pmcntenset_el0);
+}
+
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

With VHE different exception levels are used between the host (EL2) and
guest (EL1) with a shared exception level for userpace (EL0). We can take
advantage of this and use the PMU's exception level filtering to avoid
enabling/disabling counters in the world-switch code. Instead we just
modify the counter type to include or exclude EL0 at vcpu_{load,put} time.

We also ensure that trapped PMU system register writes do not re-enable
EL0 when reconfiguring the backing perf events.

This approach completely avoids blackout windows seen with !VHE.

Suggested-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  3 ++
 arch/arm64/include/asm/kvm_host.h |  5 +-
 arch/arm64/kernel/perf_event.c    |  6 ++-
 arch/arm64/kvm/pmu.c              | 87 ++++++++++++++++++++++++++++++-
 arch/arm64/kvm/sys_regs.c         |  3 ++
 virt/kvm/arm/arm.c                |  2 +
 6 files changed, 102 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index a358cb15bb0d..3ce429954306 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -326,6 +326,9 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
 
+static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
+static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
+
 static inline void kvm_arm_vhe_guest_enter(void) {}
 static inline void kvm_arm_vhe_guest_exit(void) {}
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7ca4e094626d..d631528898b5 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -487,7 +487,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
 
 static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
 {
-	return attr->exclude_host;
+	return (!has_vhe() && attr->exclude_host);
 }
 
 #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
@@ -501,6 +501,9 @@ void kvm_clr_pmu_events(u32 clr);
 
 void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
 bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
+
+void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
+void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
 #else
 static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
 static inline void kvm_clr_pmu_events(u32 clr) {}
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 64f02a9fd7cd..a121a82fc54c 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -847,8 +847,12 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
 	 * with other architectures (x86 and Power).
 	 */
 	if (is_kernel_in_hyp_mode()) {
-		if (!attr->exclude_kernel)
+		if (!attr->exclude_kernel && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
+		if (attr->exclude_guest)
+			config_base |= ARMV8_PMU_EXCLUDE_EL1;
+		if (attr->exclude_host)
+			config_base |= ARMV8_PMU_EXCLUDE_EL0;
 	} else {
 		if (!attr->exclude_hv && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index a1cee7919953..a0830c70ece5 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -12,11 +12,19 @@
 DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
 
 /*
- * Given the exclude_{host,guest} attributes, determine if we are going
- * to need to switch counters at guest entry/exit.
+ * Given the perf event attributes and system type, 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)
 {
+	/**
+	 * With VHE the guest kernel runs at EL1 and the host at EL2,
+	 * where user (EL0) is excluded then we have no reason to switch
+	 * counters.
+	 */
+	if (has_vhe() && attr->exclude_user)
+		return false;
+
 	/* Only switch if attributes are different */
 	return (attr->exclude_host ^ attr->exclude_guest);
 }
@@ -87,3 +95,78 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
 		write_sysreg(pmu->events_host, pmcntenset_el0);
 }
 
+/*
+ * Modify ARMv8 PMU events to include EL0 counting
+ */
+static void kvm_vcpu_pmu_enable_el0(unsigned long events)
+{
+	u64 typer;
+	u32 counter;
+
+	for_each_set_bit(counter, &events, 32) {
+		write_sysreg(counter, pmselr_el0);
+		isb();
+		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
+		write_sysreg(typer, pmxevtyper_el0);
+		isb();
+	}
+}
+
+/*
+ * Modify ARMv8 PMU events to exclude EL0 counting
+ */
+static void kvm_vcpu_pmu_disable_el0(unsigned long events)
+{
+	u64 typer;
+	u32 counter;
+
+	for_each_set_bit(counter, &events, 32) {
+		write_sysreg(counter, pmselr_el0);
+		isb();
+		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
+		write_sysreg(typer, pmxevtyper_el0);
+		isb();
+	}
+}
+
+/*
+ * On VHE ensure that only guest events have EL0 counting enabled
+ */
+void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpu_context *host_ctxt;
+	struct kvm_host_data *host;
+	u32 events_guest, events_host;
+
+	if (!has_vhe())
+		return;
+
+	host_ctxt = vcpu->arch.host_cpu_context;
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	events_guest = host->pmu_events.events_guest;
+	events_host = host->pmu_events.events_host;
+
+	kvm_vcpu_pmu_enable_el0(events_guest);
+	kvm_vcpu_pmu_disable_el0(events_host);
+}
+
+/*
+ * On VHE ensure that only guest host have EL0 counting enabled
+ */
+void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpu_context *host_ctxt;
+	struct kvm_host_data *host;
+	u32 events_guest, events_host;
+
+	if (!has_vhe())
+		return;
+
+	host_ctxt = vcpu->arch.host_cpu_context;
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	events_guest = host->pmu_events.events_guest;
+	events_host = host->pmu_events.events_host;
+
+	kvm_vcpu_pmu_enable_el0(events_host);
+	kvm_vcpu_pmu_disable_el0(events_guest);
+}
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c936aa40c3f4..209f9dd97bcb 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -695,6 +695,7 @@ static bool access_pmcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 		val |= p->regval & ARMV8_PMU_PMCR_MASK;
 		__vcpu_sys_reg(vcpu, PMCR_EL0) = val;
 		kvm_pmu_handle_pmcr(vcpu, val);
+		kvm_vcpu_pmu_restore_guest(vcpu);
 	} else {
 		/* PMCR.P & PMCR.C are RAZ */
 		val = __vcpu_sys_reg(vcpu, PMCR_EL0)
@@ -850,6 +851,7 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	if (p->is_write) {
 		kvm_pmu_set_counter_event_type(vcpu, p->regval, idx);
 		__vcpu_sys_reg(vcpu, reg) = p->regval & ARMV8_PMU_EVTYPE_MASK;
+		kvm_vcpu_pmu_restore_guest(vcpu);
 	} else {
 		p->regval = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_MASK;
 	}
@@ -875,6 +877,7 @@ static bool access_pmcnten(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 			/* accessing PMCNTENSET_EL0 */
 			__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) |= val;
 			kvm_pmu_enable_counter(vcpu, val);
+			kvm_vcpu_pmu_restore_guest(vcpu);
 		} else {
 			/* accessing PMCNTENCLR_EL0 */
 			__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) &= ~val;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 6958b98b8d52..1306bf0ad025 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -385,6 +385,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	kvm_timer_vcpu_load(vcpu);
 	kvm_vcpu_load_sysregs(vcpu);
 	kvm_arch_vcpu_load_fp(vcpu);
+	kvm_vcpu_pmu_restore_guest(vcpu);
 
 	if (single_task_running())
 		vcpu_clear_wfe_traps(vcpu);
@@ -398,6 +399,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 	kvm_vcpu_put_sysregs(vcpu);
 	kvm_timer_vcpu_put(vcpu);
 	kvm_vgic_put(vcpu);
+	kvm_vcpu_pmu_restore_host(vcpu);
 
 	vcpu->cpu = -1;
 
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

With VHE different exception levels are used between the host (EL2) and
guest (EL1) with a shared exception level for userpace (EL0). We can take
advantage of this and use the PMU's exception level filtering to avoid
enabling/disabling counters in the world-switch code. Instead we just
modify the counter type to include or exclude EL0 at vcpu_{load,put} time.

We also ensure that trapped PMU system register writes do not re-enable
EL0 when reconfiguring the backing perf events.

This approach completely avoids blackout windows seen with !VHE.

Suggested-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  3 ++
 arch/arm64/include/asm/kvm_host.h |  5 +-
 arch/arm64/kernel/perf_event.c    |  6 ++-
 arch/arm64/kvm/pmu.c              | 87 ++++++++++++++++++++++++++++++-
 arch/arm64/kvm/sys_regs.c         |  3 ++
 virt/kvm/arm/arm.c                |  2 +
 6 files changed, 102 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index a358cb15bb0d..3ce429954306 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -326,6 +326,9 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
 
+static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
+static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
+
 static inline void kvm_arm_vhe_guest_enter(void) {}
 static inline void kvm_arm_vhe_guest_exit(void) {}
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7ca4e094626d..d631528898b5 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -487,7 +487,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
 
 static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
 {
-	return attr->exclude_host;
+	return (!has_vhe() && attr->exclude_host);
 }
 
 #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
@@ -501,6 +501,9 @@ void kvm_clr_pmu_events(u32 clr);
 
 void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
 bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
+
+void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
+void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
 #else
 static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
 static inline void kvm_clr_pmu_events(u32 clr) {}
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 64f02a9fd7cd..a121a82fc54c 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -847,8 +847,12 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
 	 * with other architectures (x86 and Power).
 	 */
 	if (is_kernel_in_hyp_mode()) {
-		if (!attr->exclude_kernel)
+		if (!attr->exclude_kernel && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
+		if (attr->exclude_guest)
+			config_base |= ARMV8_PMU_EXCLUDE_EL1;
+		if (attr->exclude_host)
+			config_base |= ARMV8_PMU_EXCLUDE_EL0;
 	} else {
 		if (!attr->exclude_hv && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index a1cee7919953..a0830c70ece5 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -12,11 +12,19 @@
 DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
 
 /*
- * Given the exclude_{host,guest} attributes, determine if we are going
- * to need to switch counters at guest entry/exit.
+ * Given the perf event attributes and system type, 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)
 {
+	/**
+	 * With VHE the guest kernel runs at EL1 and the host at EL2,
+	 * where user (EL0) is excluded then we have no reason to switch
+	 * counters.
+	 */
+	if (has_vhe() && attr->exclude_user)
+		return false;
+
 	/* Only switch if attributes are different */
 	return (attr->exclude_host ^ attr->exclude_guest);
 }
@@ -87,3 +95,78 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
 		write_sysreg(pmu->events_host, pmcntenset_el0);
 }
 
+/*
+ * Modify ARMv8 PMU events to include EL0 counting
+ */
+static void kvm_vcpu_pmu_enable_el0(unsigned long events)
+{
+	u64 typer;
+	u32 counter;
+
+	for_each_set_bit(counter, &events, 32) {
+		write_sysreg(counter, pmselr_el0);
+		isb();
+		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
+		write_sysreg(typer, pmxevtyper_el0);
+		isb();
+	}
+}
+
+/*
+ * Modify ARMv8 PMU events to exclude EL0 counting
+ */
+static void kvm_vcpu_pmu_disable_el0(unsigned long events)
+{
+	u64 typer;
+	u32 counter;
+
+	for_each_set_bit(counter, &events, 32) {
+		write_sysreg(counter, pmselr_el0);
+		isb();
+		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
+		write_sysreg(typer, pmxevtyper_el0);
+		isb();
+	}
+}
+
+/*
+ * On VHE ensure that only guest events have EL0 counting enabled
+ */
+void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpu_context *host_ctxt;
+	struct kvm_host_data *host;
+	u32 events_guest, events_host;
+
+	if (!has_vhe())
+		return;
+
+	host_ctxt = vcpu->arch.host_cpu_context;
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	events_guest = host->pmu_events.events_guest;
+	events_host = host->pmu_events.events_host;
+
+	kvm_vcpu_pmu_enable_el0(events_guest);
+	kvm_vcpu_pmu_disable_el0(events_host);
+}
+
+/*
+ * On VHE ensure that only guest host have EL0 counting enabled
+ */
+void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpu_context *host_ctxt;
+	struct kvm_host_data *host;
+	u32 events_guest, events_host;
+
+	if (!has_vhe())
+		return;
+
+	host_ctxt = vcpu->arch.host_cpu_context;
+	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+	events_guest = host->pmu_events.events_guest;
+	events_host = host->pmu_events.events_host;
+
+	kvm_vcpu_pmu_enable_el0(events_host);
+	kvm_vcpu_pmu_disable_el0(events_guest);
+}
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c936aa40c3f4..209f9dd97bcb 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -695,6 +695,7 @@ static bool access_pmcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 		val |= p->regval & ARMV8_PMU_PMCR_MASK;
 		__vcpu_sys_reg(vcpu, PMCR_EL0) = val;
 		kvm_pmu_handle_pmcr(vcpu, val);
+		kvm_vcpu_pmu_restore_guest(vcpu);
 	} else {
 		/* PMCR.P & PMCR.C are RAZ */
 		val = __vcpu_sys_reg(vcpu, PMCR_EL0)
@@ -850,6 +851,7 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	if (p->is_write) {
 		kvm_pmu_set_counter_event_type(vcpu, p->regval, idx);
 		__vcpu_sys_reg(vcpu, reg) = p->regval & ARMV8_PMU_EVTYPE_MASK;
+		kvm_vcpu_pmu_restore_guest(vcpu);
 	} else {
 		p->regval = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_MASK;
 	}
@@ -875,6 +877,7 @@ static bool access_pmcnten(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 			/* accessing PMCNTENSET_EL0 */
 			__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) |= val;
 			kvm_pmu_enable_counter(vcpu, val);
+			kvm_vcpu_pmu_restore_guest(vcpu);
 		} else {
 			/* accessing PMCNTENCLR_EL0 */
 			__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) &= ~val;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 6958b98b8d52..1306bf0ad025 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -385,6 +385,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	kvm_timer_vcpu_load(vcpu);
 	kvm_vcpu_load_sysregs(vcpu);
 	kvm_arch_vcpu_load_fp(vcpu);
+	kvm_vcpu_pmu_restore_guest(vcpu);
 
 	if (single_task_running())
 		vcpu_clear_wfe_traps(vcpu);
@@ -398,6 +399,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 	kvm_vcpu_put_sysregs(vcpu);
 	kvm_timer_vcpu_put(vcpu);
 	kvm_vgic_put(vcpu);
+	kvm_vcpu_pmu_restore_host(vcpu);
 
 	vcpu->cpu = -1;
 
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 7/8] arm64: KVM: avoid isb's by using direct pmxevtyper sysreg
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

Upon entering or exiting a guest we may modify multiple PMU counters to
enable of disable EL0 filtering. We presently do this via the indirect
PMXEVTYPER_EL0 system register (where the counter we modify is selected
by PMSELR). With this approach it is necessary to order the writes via
isb instructions such that we select the correct counter before modifying
it.

Let's avoid potentially expensive instruction barriers by using the
direct PMEVTYPER<n>_EL0 registers instead.

As the change to counter type relates only to EL0 filtering we can rely
on the implicit instruction barrier which occurs when we transition from
EL2 to EL1 on entering the guest. On returning to userspace we can, at the
latest, rely on the implicit barrier between EL2 and EL0. We can also
depend on the explicit isb in armv8pmu_select_counter to order our write
against any other kernel changes by the PMU driver to the type register as
a result of preemption.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/kvm/pmu.c | 84 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index a0830c70ece5..1cfc298d9a3b 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -95,6 +95,74 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
 		write_sysreg(pmu->events_host, pmcntenset_el0);
 }
 
+#define PMEVTYPER_READ_CASE(idx)				\
+	case idx:						\
+		return read_sysreg(pmevtyper##idx##_el0)
+
+#define PMEVTYPER_WRITE_CASE(idx)				\
+	case idx:						\
+		write_sysreg(val, pmevtyper##idx##_el0);	\
+		break
+
+#define PMEVTYPER_CASES(readwrite)				\
+	PMEVTYPER_##readwrite##_CASE(0);			\
+	PMEVTYPER_##readwrite##_CASE(1);			\
+	PMEVTYPER_##readwrite##_CASE(2);			\
+	PMEVTYPER_##readwrite##_CASE(3);			\
+	PMEVTYPER_##readwrite##_CASE(4);			\
+	PMEVTYPER_##readwrite##_CASE(5);			\
+	PMEVTYPER_##readwrite##_CASE(6);			\
+	PMEVTYPER_##readwrite##_CASE(7);			\
+	PMEVTYPER_##readwrite##_CASE(8);			\
+	PMEVTYPER_##readwrite##_CASE(9);			\
+	PMEVTYPER_##readwrite##_CASE(10);			\
+	PMEVTYPER_##readwrite##_CASE(11);			\
+	PMEVTYPER_##readwrite##_CASE(12);			\
+	PMEVTYPER_##readwrite##_CASE(13);			\
+	PMEVTYPER_##readwrite##_CASE(14);			\
+	PMEVTYPER_##readwrite##_CASE(15);			\
+	PMEVTYPER_##readwrite##_CASE(16);			\
+	PMEVTYPER_##readwrite##_CASE(17);			\
+	PMEVTYPER_##readwrite##_CASE(18);			\
+	PMEVTYPER_##readwrite##_CASE(19);			\
+	PMEVTYPER_##readwrite##_CASE(20);			\
+	PMEVTYPER_##readwrite##_CASE(21);			\
+	PMEVTYPER_##readwrite##_CASE(22);			\
+	PMEVTYPER_##readwrite##_CASE(23);			\
+	PMEVTYPER_##readwrite##_CASE(24);			\
+	PMEVTYPER_##readwrite##_CASE(25);			\
+	PMEVTYPER_##readwrite##_CASE(26);			\
+	PMEVTYPER_##readwrite##_CASE(27);			\
+	PMEVTYPER_##readwrite##_CASE(28);			\
+	PMEVTYPER_##readwrite##_CASE(29);			\
+	PMEVTYPER_##readwrite##_CASE(30)
+
+/*
+ * Read a value direct from PMEVTYPER<idx>
+ */
+static u64 kvm_vcpu_pmu_read_evtype_direct(int idx)
+{
+	switch (idx) {
+	PMEVTYPER_CASES(READ);
+	default:
+		WARN_ON(1);
+	}
+
+	return 0;
+}
+
+/*
+ * Write a value direct to PMEVTYPER<idx>
+ */
+static void kvm_vcpu_pmu_write_evtype_direct(int idx, u32 val)
+{
+	switch (idx) {
+	PMEVTYPER_CASES(WRITE);
+	default:
+		WARN_ON(1);
+	}
+}
+
 /*
  * Modify ARMv8 PMU events to include EL0 counting
  */
@@ -104,11 +172,9 @@ static void kvm_vcpu_pmu_enable_el0(unsigned long events)
 	u32 counter;
 
 	for_each_set_bit(counter, &events, 32) {
-		write_sysreg(counter, pmselr_el0);
-		isb();
-		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
-		write_sysreg(typer, pmxevtyper_el0);
-		isb();
+		typer = kvm_vcpu_pmu_read_evtype_direct(counter);
+		typer &= ~ARMV8_PMU_EXCLUDE_EL0;
+		kvm_vcpu_pmu_write_evtype_direct(counter, typer);
 	}
 }
 
@@ -121,11 +187,9 @@ static void kvm_vcpu_pmu_disable_el0(unsigned long events)
 	u32 counter;
 
 	for_each_set_bit(counter, &events, 32) {
-		write_sysreg(counter, pmselr_el0);
-		isb();
-		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
-		write_sysreg(typer, pmxevtyper_el0);
-		isb();
+		typer = kvm_vcpu_pmu_read_evtype_direct(counter);
+		typer |= ARMV8_PMU_EXCLUDE_EL0;
+		kvm_vcpu_pmu_write_evtype_direct(counter, typer);
 	}
 }
 
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 7/8] arm64: KVM: avoid isb's by using direct pmxevtyper sysreg
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

Upon entering or exiting a guest we may modify multiple PMU counters to
enable of disable EL0 filtering. We presently do this via the indirect
PMXEVTYPER_EL0 system register (where the counter we modify is selected
by PMSELR). With this approach it is necessary to order the writes via
isb instructions such that we select the correct counter before modifying
it.

Let's avoid potentially expensive instruction barriers by using the
direct PMEVTYPER<n>_EL0 registers instead.

As the change to counter type relates only to EL0 filtering we can rely
on the implicit instruction barrier which occurs when we transition from
EL2 to EL1 on entering the guest. On returning to userspace we can, at the
latest, rely on the implicit barrier between EL2 and EL0. We can also
depend on the explicit isb in armv8pmu_select_counter to order our write
against any other kernel changes by the PMU driver to the type register as
a result of preemption.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/kvm/pmu.c | 84 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index a0830c70ece5..1cfc298d9a3b 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -95,6 +95,74 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
 		write_sysreg(pmu->events_host, pmcntenset_el0);
 }
 
+#define PMEVTYPER_READ_CASE(idx)				\
+	case idx:						\
+		return read_sysreg(pmevtyper##idx##_el0)
+
+#define PMEVTYPER_WRITE_CASE(idx)				\
+	case idx:						\
+		write_sysreg(val, pmevtyper##idx##_el0);	\
+		break
+
+#define PMEVTYPER_CASES(readwrite)				\
+	PMEVTYPER_##readwrite##_CASE(0);			\
+	PMEVTYPER_##readwrite##_CASE(1);			\
+	PMEVTYPER_##readwrite##_CASE(2);			\
+	PMEVTYPER_##readwrite##_CASE(3);			\
+	PMEVTYPER_##readwrite##_CASE(4);			\
+	PMEVTYPER_##readwrite##_CASE(5);			\
+	PMEVTYPER_##readwrite##_CASE(6);			\
+	PMEVTYPER_##readwrite##_CASE(7);			\
+	PMEVTYPER_##readwrite##_CASE(8);			\
+	PMEVTYPER_##readwrite##_CASE(9);			\
+	PMEVTYPER_##readwrite##_CASE(10);			\
+	PMEVTYPER_##readwrite##_CASE(11);			\
+	PMEVTYPER_##readwrite##_CASE(12);			\
+	PMEVTYPER_##readwrite##_CASE(13);			\
+	PMEVTYPER_##readwrite##_CASE(14);			\
+	PMEVTYPER_##readwrite##_CASE(15);			\
+	PMEVTYPER_##readwrite##_CASE(16);			\
+	PMEVTYPER_##readwrite##_CASE(17);			\
+	PMEVTYPER_##readwrite##_CASE(18);			\
+	PMEVTYPER_##readwrite##_CASE(19);			\
+	PMEVTYPER_##readwrite##_CASE(20);			\
+	PMEVTYPER_##readwrite##_CASE(21);			\
+	PMEVTYPER_##readwrite##_CASE(22);			\
+	PMEVTYPER_##readwrite##_CASE(23);			\
+	PMEVTYPER_##readwrite##_CASE(24);			\
+	PMEVTYPER_##readwrite##_CASE(25);			\
+	PMEVTYPER_##readwrite##_CASE(26);			\
+	PMEVTYPER_##readwrite##_CASE(27);			\
+	PMEVTYPER_##readwrite##_CASE(28);			\
+	PMEVTYPER_##readwrite##_CASE(29);			\
+	PMEVTYPER_##readwrite##_CASE(30)
+
+/*
+ * Read a value direct from PMEVTYPER<idx>
+ */
+static u64 kvm_vcpu_pmu_read_evtype_direct(int idx)
+{
+	switch (idx) {
+	PMEVTYPER_CASES(READ);
+	default:
+		WARN_ON(1);
+	}
+
+	return 0;
+}
+
+/*
+ * Write a value direct to PMEVTYPER<idx>
+ */
+static void kvm_vcpu_pmu_write_evtype_direct(int idx, u32 val)
+{
+	switch (idx) {
+	PMEVTYPER_CASES(WRITE);
+	default:
+		WARN_ON(1);
+	}
+}
+
 /*
  * Modify ARMv8 PMU events to include EL0 counting
  */
@@ -104,11 +172,9 @@ static void kvm_vcpu_pmu_enable_el0(unsigned long events)
 	u32 counter;
 
 	for_each_set_bit(counter, &events, 32) {
-		write_sysreg(counter, pmselr_el0);
-		isb();
-		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
-		write_sysreg(typer, pmxevtyper_el0);
-		isb();
+		typer = kvm_vcpu_pmu_read_evtype_direct(counter);
+		typer &= ~ARMV8_PMU_EXCLUDE_EL0;
+		kvm_vcpu_pmu_write_evtype_direct(counter, typer);
 	}
 }
 
@@ -121,11 +187,9 @@ static void kvm_vcpu_pmu_disable_el0(unsigned long events)
 	u32 counter;
 
 	for_each_set_bit(counter, &events, 32) {
-		write_sysreg(counter, pmselr_el0);
-		isb();
-		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
-		write_sysreg(typer, pmxevtyper_el0);
-		isb();
+		typer = kvm_vcpu_pmu_read_evtype_direct(counter);
+		typer |= ARMV8_PMU_EXCLUDE_EL0;
+		kvm_vcpu_pmu_write_evtype_direct(counter, typer);
 	}
 }
 
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 8/8] arm64: docs: document perf event attributes
  2019-03-08 12:07 ` Andrew Murray
@ 2019-03-08 12:07   ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, linux-arm-kernel

The interaction between the exclude_{host,guest} flags,
exclude_{user,kernel,hv} flags and presence of VHE can result in
different exception levels being filtered by the ARMv8 PMU. As this
can be confusing let's document how they work on arm64.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 Documentation/arm64/perf.txt | 74 ++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/arm64/perf.txt

diff --git a/Documentation/arm64/perf.txt b/Documentation/arm64/perf.txt
new file mode 100644
index 000000000000..604446c1f720
--- /dev/null
+++ b/Documentation/arm64/perf.txt
@@ -0,0 +1,74 @@
+Perf Event Attributes
+=====================
+
+Author: Andrew Murray <andrew.murray@arm.com>
+Date: 2019-03-06
+
+exclude_user
+------------
+
+This attribute excludes userspace.
+
+Userspace always runs at EL0 and thus this attribute will exclude EL0.
+
+
+exclude_kernel
+--------------
+
+This attribute excludes the kernel.
+
+The kernel runs at EL2 with VHE and EL1 without. Guest kernels always run
+at EL1.
+
+This attribute will exclude EL1 and additionally EL2 on a VHE system.
+
+
+exclude_hv
+----------
+
+This attribute excludes the hypervisor, we ignore this flag on a VHE system
+as we consider the host kernel to be the hypervisor.
+
+On a non-VHE system we consider the hypervisor to be any code that runs at
+EL2 which is predominantly used for guest/host transitions.
+
+This attribute will exclude EL2 on a non-VHE system.
+
+
+exclude_host / exclude_guest
+----------------------------
+
+This attribute excludes the KVM host.
+
+The KVM host may run at EL0 (userspace), EL1 (non-VHE kernel) and EL2 (VHE
+kernel or non-VHE hypervisor).
+
+The KVM guest may run at EL0 (userspace) and EL1 (kernel).
+
+Due to the overlapping exception levels between host and guests we cannot
+exclusively rely on the PMU's hardware exception filtering - therefore we
+must enable/disable counting on the entry and exit to the guest. This is
+performed differently on VHE and non-VHE systems.
+
+For non-VHE systems we exclude EL2 for exclude_host - upon entering and
+exiting the guest we disable/enable the event as appropriate based on the
+exclude_host and exclude_guest attributes.
+
+For VHE systems we exclude EL1 for exclude_guest and exclude both EL0,EL2
+for exclude_host. Upon entering and exiting the guest we modify the event
+to include/exclude EL0 as appropriate based on the exclude_host and
+exclude_guest attributes.
+
+
+Accuracy
+--------
+
+On non-VHE systems we enable/disable counters on the entry/exit of
+host/guest transition at EL2 - however there is a period of time between
+enabling/disabling the counters and entering/exiting the guest. We are
+able to eliminate counters counting host events on the boundaries of guest
+entry/exit when counting guest events by filtering out EL2 for
+exclude_host. However when using !exclude_hv there is a small blackout
+window at the guest entry/exit where host events are not captured.
+
+On VHE systems there are no blackout windows.
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v11 8/8] arm64: docs: document perf event attributes
@ 2019-03-08 12:07   ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-08 12:07 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Julien Thierry, kvmarm, linux-arm-kernel, Suzuki K Poulose

The interaction between the exclude_{host,guest} flags,
exclude_{user,kernel,hv} flags and presence of VHE can result in
different exception levels being filtered by the ARMv8 PMU. As this
can be confusing let's document how they work on arm64.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 Documentation/arm64/perf.txt | 74 ++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/arm64/perf.txt

diff --git a/Documentation/arm64/perf.txt b/Documentation/arm64/perf.txt
new file mode 100644
index 000000000000..604446c1f720
--- /dev/null
+++ b/Documentation/arm64/perf.txt
@@ -0,0 +1,74 @@
+Perf Event Attributes
+=====================
+
+Author: Andrew Murray <andrew.murray@arm.com>
+Date: 2019-03-06
+
+exclude_user
+------------
+
+This attribute excludes userspace.
+
+Userspace always runs at EL0 and thus this attribute will exclude EL0.
+
+
+exclude_kernel
+--------------
+
+This attribute excludes the kernel.
+
+The kernel runs at EL2 with VHE and EL1 without. Guest kernels always run
+at EL1.
+
+This attribute will exclude EL1 and additionally EL2 on a VHE system.
+
+
+exclude_hv
+----------
+
+This attribute excludes the hypervisor, we ignore this flag on a VHE system
+as we consider the host kernel to be the hypervisor.
+
+On a non-VHE system we consider the hypervisor to be any code that runs at
+EL2 which is predominantly used for guest/host transitions.
+
+This attribute will exclude EL2 on a non-VHE system.
+
+
+exclude_host / exclude_guest
+----------------------------
+
+This attribute excludes the KVM host.
+
+The KVM host may run at EL0 (userspace), EL1 (non-VHE kernel) and EL2 (VHE
+kernel or non-VHE hypervisor).
+
+The KVM guest may run at EL0 (userspace) and EL1 (kernel).
+
+Due to the overlapping exception levels between host and guests we cannot
+exclusively rely on the PMU's hardware exception filtering - therefore we
+must enable/disable counting on the entry and exit to the guest. This is
+performed differently on VHE and non-VHE systems.
+
+For non-VHE systems we exclude EL2 for exclude_host - upon entering and
+exiting the guest we disable/enable the event as appropriate based on the
+exclude_host and exclude_guest attributes.
+
+For VHE systems we exclude EL1 for exclude_guest and exclude both EL0,EL2
+for exclude_host. Upon entering and exiting the guest we modify the event
+to include/exclude EL0 as appropriate based on the exclude_host and
+exclude_guest attributes.
+
+
+Accuracy
+--------
+
+On non-VHE systems we enable/disable counters on the entry/exit of
+host/guest transition at EL2 - however there is a period of time between
+enabling/disabling the counters and entering/exiting the guest. We are
+able to eliminate counters counting host events on the boundaries of guest
+entry/exit when counting guest events by filtering out EL2 for
+exclude_host. However when using !exclude_hv there is a small blackout
+window at the guest entry/exit where host events are not captured.
+
+On VHE systems there are no blackout windows.
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
  2019-03-08 12:07   ` Andrew Murray
@ 2019-03-08 16:40     ` Julien Thierry
  -1 siblings, 0 replies; 30+ messages in thread
From: Julien Thierry @ 2019-03-08 16:40 UTC (permalink / raw)
  To: Andrew Murray, Christoffer Dall, Marc Zyngier, Catalin Marinas,
	Will Deacon, Mark Rutland
  Cc: kvmarm, linux-arm-kernel

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.
> 

I might have missed something here. Why is that true only for !VHE? Is
it because with VHE we can just exclude EL1?
(It's also a bit confusing since the patch does not seem to contain any
VHE/nVHE distinction)

> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  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)
> +{
> +	return attr->exclude_host;
> +}
> +
>  #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
> + *
> + * Copyright 2019 Arm Limited
> + * Author: Andrew Murray <Andrew.Murray@arm.com>
> + */
> +#include <linux/kvm_host.h>
> +#include <linux/perf_event.h>
> +
> +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> +
> +/*
> + * 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);

Nit:

Is there any benefit to this rather than doing "attr->exclude_host !=
attr->exclude_guest" ? The code generated is most likely the same, I
just find the latter slightly more straightforward.

Cheers,

-- 
Julien Thierry

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
@ 2019-03-08 16:40     ` Julien Thierry
  0 siblings, 0 replies; 30+ messages in thread
From: Julien Thierry @ 2019-03-08 16:40 UTC (permalink / raw)
  To: Andrew Murray, Christoffer Dall, Marc Zyngier, Catalin Marinas,
	Will Deacon, Mark Rutland
  Cc: kvmarm, linux-arm-kernel, Suzuki K Poulose

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.
> 

I might have missed something here. Why is that true only for !VHE? Is
it because with VHE we can just exclude EL1?
(It's also a bit confusing since the patch does not seem to contain any
VHE/nVHE distinction)

> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  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)
> +{
> +	return attr->exclude_host;
> +}
> +
>  #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
> + *
> + * Copyright 2019 Arm Limited
> + * Author: Andrew Murray <Andrew.Murray@arm.com>
> + */
> +#include <linux/kvm_host.h>
> +#include <linux/perf_event.h>
> +
> +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> +
> +/*
> + * 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);

Nit:

Is there any benefit to this rather than doing "attr->exclude_host !=
attr->exclude_guest" ? The code generated is most likely the same, I
just find the latter slightly more straightforward.

Cheers,

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
  2019-03-08 12:07   ` Andrew Murray
@ 2019-03-11  9:39     ` Julien Thierry
  -1 siblings, 0 replies; 30+ messages in thread
From: Julien Thierry @ 2019-03-11  9:39 UTC (permalink / raw)
  To: Andrew Murray, Christoffer Dall, Marc Zyngier, Catalin Marinas,
	Will Deacon, Mark Rutland
  Cc: kvmarm, linux-arm-kernel, Suzuki K Poulose

Hi Andrew,

On 08/03/2019 12:07, Andrew Murray wrote:
> With VHE different exception levels are used between the host (EL2) and
> guest (EL1) with a shared exception level for userpace (EL0). We can take
> advantage of this and use the PMU's exception level filtering to avoid
> enabling/disabling counters in the world-switch code. Instead we just
> modify the counter type to include or exclude EL0 at vcpu_{load,put} time.
> 
> We also ensure that trapped PMU system register writes do not re-enable
> EL0 when reconfiguring the backing perf events.
> 
> This approach completely avoids blackout windows seen with !VHE.
> 
> Suggested-by: Christoffer Dall <christoffer.dall@arm.com>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   |  3 ++
>  arch/arm64/include/asm/kvm_host.h |  5 +-
>  arch/arm64/kernel/perf_event.c    |  6 ++-
>  arch/arm64/kvm/pmu.c              | 87 ++++++++++++++++++++++++++++++-
>  arch/arm64/kvm/sys_regs.c         |  3 ++
>  virt/kvm/arm/arm.c                |  2 +
>  6 files changed, 102 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index a358cb15bb0d..3ce429954306 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -326,6 +326,9 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
>  
> +static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
> +static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
> +
>  static inline void kvm_arm_vhe_guest_enter(void) {}
>  static inline void kvm_arm_vhe_guest_exit(void) {}
>  
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7ca4e094626d..d631528898b5 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -487,7 +487,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>  
>  static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
>  {
> -	return attr->exclude_host;
> +	return (!has_vhe() && attr->exclude_host);
>  }
>  
>  #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> @@ -501,6 +501,9 @@ void kvm_clr_pmu_events(u32 clr);
>  
>  void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
>  bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
> +
> +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
> +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>  #else
>  static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
>  static inline void kvm_clr_pmu_events(u32 clr) {}
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 64f02a9fd7cd..a121a82fc54c 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -847,8 +847,12 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
>  	 * with other architectures (x86 and Power).
>  	 */
>  	if (is_kernel_in_hyp_mode()) {
> -		if (!attr->exclude_kernel)
> +		if (!attr->exclude_kernel && !attr->exclude_host)
>  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> +		if (attr->exclude_guest)
> +			config_base |= ARMV8_PMU_EXCLUDE_EL1;
> +		if (attr->exclude_host)
> +			config_base |= ARMV8_PMU_EXCLUDE_EL0;
>  	} else {
>  		if (!attr->exclude_hv && !attr->exclude_host)
>  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index a1cee7919953..a0830c70ece5 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
> @@ -12,11 +12,19 @@
>  DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
>  
>  /*
> - * Given the exclude_{host,guest} attributes, determine if we are going
> - * to need to switch counters at guest entry/exit.
> + * Given the perf event attributes and system type, 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)
>  {
> +	/**
> +	 * With VHE the guest kernel runs at EL1 and the host at EL2,
> +	 * where user (EL0) is excluded then we have no reason to switch
> +	 * counters.
> +	 */
> +	if (has_vhe() && attr->exclude_user)
> +		return false;
> +
>  	/* Only switch if attributes are different */
>  	return (attr->exclude_host ^ attr->exclude_guest);
>  }
> @@ -87,3 +95,78 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
>  		write_sysreg(pmu->events_host, pmcntenset_el0);
>  }
>  
> +/*
> + * Modify ARMv8 PMU events to include EL0 counting
> + */
> +static void kvm_vcpu_pmu_enable_el0(unsigned long events)
> +{
> +	u64 typer;
> +	u32 counter;
> +
> +	for_each_set_bit(counter, &events, 32) {
> +		write_sysreg(counter, pmselr_el0);
> +		isb();
> +		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
> +		write_sysreg(typer, pmxevtyper_el0);
> +		isb();
> +	}
> +}
> +
> +/*
> + * Modify ARMv8 PMU events to exclude EL0 counting
> + */
> +static void kvm_vcpu_pmu_disable_el0(unsigned long events)
> +{
> +	u64 typer;
> +	u32 counter;
> +
> +	for_each_set_bit(counter, &events, 32) {
> +		write_sysreg(counter, pmselr_el0);
> +		isb();
> +		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
> +		write_sysreg(typer, pmxevtyper_el0);
> +		isb();
> +	}
> +}
> +
> +/*
> + * On VHE ensure that only guest events have EL0 counting enabled
> + */
> +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpu_context *host_ctxt;
> +	struct kvm_host_data *host;
> +	u32 events_guest, events_host;
> +
> +	if (!has_vhe())
> +		return;
> +
> +	host_ctxt = vcpu->arch.host_cpu_context;
> +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> +	events_guest = host->pmu_events.events_guest;
> +	events_host = host->pmu_events.events_host;
> +
> +	kvm_vcpu_pmu_enable_el0(events_guest);
> +	kvm_vcpu_pmu_disable_el0(events_host);

So, we load a vcpu, and all events common to the guest and the host
(events_guest & events_host) get the EXCLUDE_EL0 flag set.

I don't see anything that will remove that flag before running the
guest. Am I missing something? Should these lines be as follows?

	kvm_vcpu_pmu_enable_el0(events_guest & events_host);
	kvm_vcpu_pmu_enable_el0(events_guest ^ events_host);

> +}
> +
> +/*
> + * On VHE ensure that only guest host have EL0 counting enabled
> + */
> +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpu_context *host_ctxt;
> +	struct kvm_host_data *host;
> +	u32 events_guest, events_host;
> +
> +	if (!has_vhe())
> +		return;
> +
> +	host_ctxt = vcpu->arch.host_cpu_context;
> +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> +	events_guest = host->pmu_events.events_guest;
> +	events_host = host->pmu_events.events_host;
> +
> +	kvm_vcpu_pmu_enable_el0(events_host);
> +	kvm_vcpu_pmu_disable_el0(events_guest);

Same question as above, after vcpu_put, it seems we've disabled at EL0
host events that are common to the guest and the host.

Thanks,

-- 
Julien Thierry

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-03-11  9:39     ` Julien Thierry
  0 siblings, 0 replies; 30+ messages in thread
From: Julien Thierry @ 2019-03-11  9:39 UTC (permalink / raw)
  To: Andrew Murray, Christoffer Dall, Marc Zyngier, Catalin Marinas,
	Will Deacon, Mark Rutland
  Cc: kvmarm, linux-arm-kernel, Suzuki K Poulose

Hi Andrew,

On 08/03/2019 12:07, Andrew Murray wrote:
> With VHE different exception levels are used between the host (EL2) and
> guest (EL1) with a shared exception level for userpace (EL0). We can take
> advantage of this and use the PMU's exception level filtering to avoid
> enabling/disabling counters in the world-switch code. Instead we just
> modify the counter type to include or exclude EL0 at vcpu_{load,put} time.
> 
> We also ensure that trapped PMU system register writes do not re-enable
> EL0 when reconfiguring the backing perf events.
> 
> This approach completely avoids blackout windows seen with !VHE.
> 
> Suggested-by: Christoffer Dall <christoffer.dall@arm.com>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   |  3 ++
>  arch/arm64/include/asm/kvm_host.h |  5 +-
>  arch/arm64/kernel/perf_event.c    |  6 ++-
>  arch/arm64/kvm/pmu.c              | 87 ++++++++++++++++++++++++++++++-
>  arch/arm64/kvm/sys_regs.c         |  3 ++
>  virt/kvm/arm/arm.c                |  2 +
>  6 files changed, 102 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index a358cb15bb0d..3ce429954306 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -326,6 +326,9 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
>  
> +static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
> +static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
> +
>  static inline void kvm_arm_vhe_guest_enter(void) {}
>  static inline void kvm_arm_vhe_guest_exit(void) {}
>  
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7ca4e094626d..d631528898b5 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -487,7 +487,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>  
>  static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
>  {
> -	return attr->exclude_host;
> +	return (!has_vhe() && attr->exclude_host);
>  }
>  
>  #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> @@ -501,6 +501,9 @@ void kvm_clr_pmu_events(u32 clr);
>  
>  void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
>  bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
> +
> +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
> +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>  #else
>  static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
>  static inline void kvm_clr_pmu_events(u32 clr) {}
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 64f02a9fd7cd..a121a82fc54c 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -847,8 +847,12 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
>  	 * with other architectures (x86 and Power).
>  	 */
>  	if (is_kernel_in_hyp_mode()) {
> -		if (!attr->exclude_kernel)
> +		if (!attr->exclude_kernel && !attr->exclude_host)
>  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> +		if (attr->exclude_guest)
> +			config_base |= ARMV8_PMU_EXCLUDE_EL1;
> +		if (attr->exclude_host)
> +			config_base |= ARMV8_PMU_EXCLUDE_EL0;
>  	} else {
>  		if (!attr->exclude_hv && !attr->exclude_host)
>  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index a1cee7919953..a0830c70ece5 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
> @@ -12,11 +12,19 @@
>  DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
>  
>  /*
> - * Given the exclude_{host,guest} attributes, determine if we are going
> - * to need to switch counters at guest entry/exit.
> + * Given the perf event attributes and system type, 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)
>  {
> +	/**
> +	 * With VHE the guest kernel runs at EL1 and the host at EL2,
> +	 * where user (EL0) is excluded then we have no reason to switch
> +	 * counters.
> +	 */
> +	if (has_vhe() && attr->exclude_user)
> +		return false;
> +
>  	/* Only switch if attributes are different */
>  	return (attr->exclude_host ^ attr->exclude_guest);
>  }
> @@ -87,3 +95,78 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
>  		write_sysreg(pmu->events_host, pmcntenset_el0);
>  }
>  
> +/*
> + * Modify ARMv8 PMU events to include EL0 counting
> + */
> +static void kvm_vcpu_pmu_enable_el0(unsigned long events)
> +{
> +	u64 typer;
> +	u32 counter;
> +
> +	for_each_set_bit(counter, &events, 32) {
> +		write_sysreg(counter, pmselr_el0);
> +		isb();
> +		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
> +		write_sysreg(typer, pmxevtyper_el0);
> +		isb();
> +	}
> +}
> +
> +/*
> + * Modify ARMv8 PMU events to exclude EL0 counting
> + */
> +static void kvm_vcpu_pmu_disable_el0(unsigned long events)
> +{
> +	u64 typer;
> +	u32 counter;
> +
> +	for_each_set_bit(counter, &events, 32) {
> +		write_sysreg(counter, pmselr_el0);
> +		isb();
> +		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
> +		write_sysreg(typer, pmxevtyper_el0);
> +		isb();
> +	}
> +}
> +
> +/*
> + * On VHE ensure that only guest events have EL0 counting enabled
> + */
> +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpu_context *host_ctxt;
> +	struct kvm_host_data *host;
> +	u32 events_guest, events_host;
> +
> +	if (!has_vhe())
> +		return;
> +
> +	host_ctxt = vcpu->arch.host_cpu_context;
> +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> +	events_guest = host->pmu_events.events_guest;
> +	events_host = host->pmu_events.events_host;
> +
> +	kvm_vcpu_pmu_enable_el0(events_guest);
> +	kvm_vcpu_pmu_disable_el0(events_host);

So, we load a vcpu, and all events common to the guest and the host
(events_guest & events_host) get the EXCLUDE_EL0 flag set.

I don't see anything that will remove that flag before running the
guest. Am I missing something? Should these lines be as follows?

	kvm_vcpu_pmu_enable_el0(events_guest & events_host);
	kvm_vcpu_pmu_enable_el0(events_guest ^ events_host);

> +}
> +
> +/*
> + * On VHE ensure that only guest host have EL0 counting enabled
> + */
> +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpu_context *host_ctxt;
> +	struct kvm_host_data *host;
> +	u32 events_guest, events_host;
> +
> +	if (!has_vhe())
> +		return;
> +
> +	host_ctxt = vcpu->arch.host_cpu_context;
> +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> +	events_guest = host->pmu_events.events_guest;
> +	events_host = host->pmu_events.events_host;
> +
> +	kvm_vcpu_pmu_enable_el0(events_host);
> +	kvm_vcpu_pmu_disable_el0(events_guest);

Same question as above, after vcpu_put, it seems we've disabled at EL0
host events that are common to the guest and the host.

Thanks,

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
  2019-03-08 12:07   ` Andrew Murray
@ 2019-03-11 11:00     ` Suzuki K Poulose
  -1 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2019-03-11 11:00 UTC (permalink / raw)
  To: andrew.murray, christoffer.dall, marc.zyngier, catalin.marinas,
	will.deacon, mark.rutland
  Cc: kvmarm, linux-arm-kernel

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@arm.com>
> ---
>   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@arm.com>
> + */
> +#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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
@ 2019-03-11 11:00     ` Suzuki K Poulose
  0 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2019-03-11 11:00 UTC (permalink / raw)
  To: andrew.murray, christoffer.dall, marc.zyngier, catalin.marinas,
	will.deacon, mark.rutland
  Cc: kvmarm, linux-arm-kernel, julien.thierry

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@arm.com>
> ---
>   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@arm.com>
> + */
> +#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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
  2019-03-08 16:40     ` Julien Thierry
@ 2019-03-11 11:45       ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-11 11:45 UTC (permalink / raw)
  To: Julien Thierry
  Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel

On Fri, Mar 08, 2019 at 04:40:51PM +0000, Julien Thierry wrote:
> 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.
> > 
> 
> I might have missed something here. Why is that true only for !VHE? Is
> it because with VHE we can just exclude EL1?

That's correct. For VHE we never defer counting and can rely on the PMU
filtering capabilities (even though for EL0 we have to reconfigure the
filtering upon entering/exiting the guest).

> (It's also a bit confusing since the patch does not seem to contain any
> VHE/nVHE distinction)

This is updated in the later patches of this series. I felt the series
would be easier to understand if I add the special VHE case last.

I will update the commit message such that it reads "With exclude_host, we
may only start counting when entering the guest.". I.e. the changes here
are valid for both VHE and !VHE until the later patches change it for VHE.

> 
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >  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)
> > +{
> > +	return attr->exclude_host;
> > +}
> > +
> >  #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
> > + *
> > + * Copyright 2019 Arm Limited
> > + * Author: Andrew Murray <Andrew.Murray@arm.com>
> > + */
> > +#include <linux/kvm_host.h>
> > +#include <linux/perf_event.h>
> > +
> > +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> > +
> > +/*
> > + * 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);
> 
> Nit:
> 
> Is there any benefit to this rather than doing "attr->exclude_host !=
> attr->exclude_guest" ? The code generated is most likely the same, I
> just find the latter slightly more straightforward.

Nope, I'll change it. Not sure why I ended up with this.

Thanks,

Andrew Murray

> 
> Cheers,
> 
> -- 
> Julien Thierry

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
@ 2019-03-11 11:45       ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-11 11:45 UTC (permalink / raw)
  To: Julien Thierry
  Cc: Mark Rutland, Suzuki K Poulose, Marc Zyngier, Catalin Marinas,
	Will Deacon, Christoffer Dall, kvmarm, linux-arm-kernel

On Fri, Mar 08, 2019 at 04:40:51PM +0000, Julien Thierry wrote:
> 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.
> > 
> 
> I might have missed something here. Why is that true only for !VHE? Is
> it because with VHE we can just exclude EL1?

That's correct. For VHE we never defer counting and can rely on the PMU
filtering capabilities (even though for EL0 we have to reconfigure the
filtering upon entering/exiting the guest).

> (It's also a bit confusing since the patch does not seem to contain any
> VHE/nVHE distinction)

This is updated in the later patches of this series. I felt the series
would be easier to understand if I add the special VHE case last.

I will update the commit message such that it reads "With exclude_host, we
may only start counting when entering the guest.". I.e. the changes here
are valid for both VHE and !VHE until the later patches change it for VHE.

> 
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >  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)
> > +{
> > +	return attr->exclude_host;
> > +}
> > +
> >  #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
> > + *
> > + * Copyright 2019 Arm Limited
> > + * Author: Andrew Murray <Andrew.Murray@arm.com>
> > + */
> > +#include <linux/kvm_host.h>
> > +#include <linux/perf_event.h>
> > +
> > +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> > +
> > +/*
> > + * 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);
> 
> Nit:
> 
> Is there any benefit to this rather than doing "attr->exclude_host !=
> attr->exclude_guest" ? The code generated is most likely the same, I
> just find the latter slightly more straightforward.

Nope, I'll change it. Not sure why I ended up with this.

Thanks,

Andrew Murray

> 
> Cheers,
> 
> -- 
> Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
  2019-03-11 11:00     ` Suzuki K Poulose
@ 2019-03-11 11:59       ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-11 11:59 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: marc.zyngier, catalin.marinas, will.deacon, kvmarm, linux-arm-kernel

On Mon, Mar 11, 2019 at 11:00:04AM +0000, Suzuki K Poulose wrote:
> 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@arm.com>
> > ---
> >   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/

Thanks.

> 
> > +{
> > +	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 ?

I think this hunk is correct. It makes sense to not count with exclude_host
regardless to if there are guests or not. (By the way v10 has this test,
we've just moved it to this file.)

Later in this series we update the hunk to condition it on !has_vhe(), this
is still OK - it means for VHE we always enable to counter (despite
exclude_host) but that's OK because on VHE with exclude_host we exclude EL2,
EL0 (armv8pmu_set_event_filter). This does mean that we're enabling a counter
that doesn't do anything, but then from the user perspective it is a bit
pointless to use exclude_host on a system without KVM or guests.

> 
> >   #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@arm.com>
> > + */
> > +#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.

No we don't - I'll remove it.

> 
> > +
> > +/*
> > + * 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.

Thanks for the review and nits.

Andrew Murray

> 
> Suzuki

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters
@ 2019-03-11 11:59       ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-11 11:59 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mark.rutland, julien.thierry, marc.zyngier, catalin.marinas,
	will.deacon, christoffer.dall, kvmarm, linux-arm-kernel

On Mon, Mar 11, 2019 at 11:00:04AM +0000, Suzuki K Poulose wrote:
> 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@arm.com>
> > ---
> >   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/

Thanks.

> 
> > +{
> > +	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 ?

I think this hunk is correct. It makes sense to not count with exclude_host
regardless to if there are guests or not. (By the way v10 has this test,
we've just moved it to this file.)

Later in this series we update the hunk to condition it on !has_vhe(), this
is still OK - it means for VHE we always enable to counter (despite
exclude_host) but that's OK because on VHE with exclude_host we exclude EL2,
EL0 (armv8pmu_set_event_filter). This does mean that we're enabling a counter
that doesn't do anything, but then from the user perspective it is a bit
pointless to use exclude_host on a system without KVM or guests.

> 
> >   #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@arm.com>
> > + */
> > +#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.

No we don't - I'll remove it.

> 
> > +
> > +/*
> > + * 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.

Thanks for the review and nits.

Andrew Murray

> 
> Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
  2019-03-11  9:39     ` Julien Thierry
@ 2019-03-11 12:16       ` Andrew Murray
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-11 12:16 UTC (permalink / raw)
  To: Julien Thierry
  Cc: Mark Rutland, Suzuki K Poulose, Marc Zyngier, Catalin Marinas,
	Will Deacon, Christoffer Dall, kvmarm, linux-arm-kernel

On Mon, Mar 11, 2019 at 09:39:19AM +0000, Julien Thierry wrote:
> Hi Andrew,
> 
> On 08/03/2019 12:07, Andrew Murray wrote:
> > With VHE different exception levels are used between the host (EL2) and
> > guest (EL1) with a shared exception level for userpace (EL0). We can take
> > advantage of this and use the PMU's exception level filtering to avoid
> > enabling/disabling counters in the world-switch code. Instead we just
> > modify the counter type to include or exclude EL0 at vcpu_{load,put} time.
> > 
> > We also ensure that trapped PMU system register writes do not re-enable
> > EL0 when reconfiguring the backing perf events.
> > 
> > This approach completely avoids blackout windows seen with !VHE.
> > 
> > Suggested-by: Christoffer Dall <christoffer.dall@arm.com>
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >  arch/arm/include/asm/kvm_host.h   |  3 ++
> >  arch/arm64/include/asm/kvm_host.h |  5 +-
> >  arch/arm64/kernel/perf_event.c    |  6 ++-
> >  arch/arm64/kvm/pmu.c              | 87 ++++++++++++++++++++++++++++++-
> >  arch/arm64/kvm/sys_regs.c         |  3 ++
> >  virt/kvm/arm/arm.c                |  2 +
> >  6 files changed, 102 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> > index a358cb15bb0d..3ce429954306 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -326,6 +326,9 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
> >  static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
> >  static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
> >  
> > +static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
> > +static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
> > +
> >  static inline void kvm_arm_vhe_guest_enter(void) {}
> >  static inline void kvm_arm_vhe_guest_exit(void) {}
> >  
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 7ca4e094626d..d631528898b5 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -487,7 +487,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> >  
> >  static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
> >  {
> > -	return attr->exclude_host;
> > +	return (!has_vhe() && attr->exclude_host);
> >  }
> >  
> >  #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> > @@ -501,6 +501,9 @@ void kvm_clr_pmu_events(u32 clr);
> >  
> >  void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
> >  bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
> > +
> > +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
> > +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
> >  #else
> >  static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
> >  static inline void kvm_clr_pmu_events(u32 clr) {}
> > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > index 64f02a9fd7cd..a121a82fc54c 100644
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -847,8 +847,12 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
> >  	 * with other architectures (x86 and Power).
> >  	 */
> >  	if (is_kernel_in_hyp_mode()) {
> > -		if (!attr->exclude_kernel)
> > +		if (!attr->exclude_kernel && !attr->exclude_host)
> >  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> > +		if (attr->exclude_guest)
> > +			config_base |= ARMV8_PMU_EXCLUDE_EL1;
> > +		if (attr->exclude_host)
> > +			config_base |= ARMV8_PMU_EXCLUDE_EL0;
> >  	} else {
> >  		if (!attr->exclude_hv && !attr->exclude_host)
> >  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > index a1cee7919953..a0830c70ece5 100644
> > --- a/arch/arm64/kvm/pmu.c
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -12,11 +12,19 @@
> >  DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> >  
> >  /*
> > - * Given the exclude_{host,guest} attributes, determine if we are going
> > - * to need to switch counters at guest entry/exit.
> > + * Given the perf event attributes and system type, 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)
> >  {
> > +	/**
> > +	 * With VHE the guest kernel runs at EL1 and the host at EL2,
> > +	 * where user (EL0) is excluded then we have no reason to switch
> > +	 * counters.
> > +	 */
> > +	if (has_vhe() && attr->exclude_user)
> > +		return false;
> > +
> >  	/* Only switch if attributes are different */
> >  	return (attr->exclude_host ^ attr->exclude_guest);
> >  }
> > @@ -87,3 +95,78 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
> >  		write_sysreg(pmu->events_host, pmcntenset_el0);
> >  }
> >  
> > +/*
> > + * Modify ARMv8 PMU events to include EL0 counting
> > + */
> > +static void kvm_vcpu_pmu_enable_el0(unsigned long events)
> > +{
> > +	u64 typer;
> > +	u32 counter;
> > +
> > +	for_each_set_bit(counter, &events, 32) {
> > +		write_sysreg(counter, pmselr_el0);
> > +		isb();
> > +		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
> > +		write_sysreg(typer, pmxevtyper_el0);
> > +		isb();
> > +	}
> > +}
> > +
> > +/*
> > + * Modify ARMv8 PMU events to exclude EL0 counting
> > + */
> > +static void kvm_vcpu_pmu_disable_el0(unsigned long events)
> > +{
> > +	u64 typer;
> > +	u32 counter;
> > +
> > +	for_each_set_bit(counter, &events, 32) {
> > +		write_sysreg(counter, pmselr_el0);
> > +		isb();
> > +		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
> > +		write_sysreg(typer, pmxevtyper_el0);
> > +		isb();
> > +	}
> > +}
> > +
> > +/*
> > + * On VHE ensure that only guest events have EL0 counting enabled
> > + */
> > +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_cpu_context *host_ctxt;
> > +	struct kvm_host_data *host;
> > +	u32 events_guest, events_host;
> > +
> > +	if (!has_vhe())
> > +		return;
> > +
> > +	host_ctxt = vcpu->arch.host_cpu_context;
> > +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> > +	events_guest = host->pmu_events.events_guest;
> > +	events_host = host->pmu_events.events_host;
> > +
> > +	kvm_vcpu_pmu_enable_el0(events_guest);
> > +	kvm_vcpu_pmu_disable_el0(events_host);
> 
> So, we load a vcpu, and all events common to the guest and the host
> (events_guest & events_host) get the EXCLUDE_EL0 flag set.
> 
> I don't see anything that will remove that flag before running the
> guest. Am I missing something? Should these lines be as follows?
> 
> 	kvm_vcpu_pmu_enable_el0(events_guest & events_host);
> 	kvm_vcpu_pmu_enable_el0(events_guest ^ events_host);
> 

For VHE and !exclude_user, where an event is common to both guest and
host (i.e.  exclude_kernel = exclude_host = 0) then:

 - The event is never deferred (so we always start counting)
 - When we set the event filter at start of day (armv8pmu_set_event_filter)
   we don't exclude EL0.
 - Also we don't add this event to pmu_events due to kvm_pmu_switch_needed
   in kvm_set_pmu_events - so we don't actually do anything here.

I think the logic you were expecting is moved to start of day when the
counter is first created, thus reducing the effort here.

Thanks,

Andrew Murray 


> > +}
> > +
> > +/*
> > + * On VHE ensure that only guest host have EL0 counting enabled
> > + */
> > +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_cpu_context *host_ctxt;
> > +	struct kvm_host_data *host;
> > +	u32 events_guest, events_host;
> > +
> > +	if (!has_vhe())
> > +		return;
> > +
> > +	host_ctxt = vcpu->arch.host_cpu_context;
> > +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> > +	events_guest = host->pmu_events.events_guest;
> > +	events_host = host->pmu_events.events_host;
> > +
> > +	kvm_vcpu_pmu_enable_el0(events_host);
> > +	kvm_vcpu_pmu_disable_el0(events_guest);
> 
> Same question as above, after vcpu_put, it seems we've disabled at EL0
> host events that are common to the guest and the host.
> 
> Thanks,
> 
> -- 
> Julien Thierry

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v11 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-03-11 12:16       ` Andrew Murray
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Murray @ 2019-03-11 12:16 UTC (permalink / raw)
  To: Julien Thierry
  Cc: Mark Rutland, Suzuki K Poulose, Marc Zyngier, Catalin Marinas,
	Will Deacon, Christoffer Dall, kvmarm, linux-arm-kernel

On Mon, Mar 11, 2019 at 09:39:19AM +0000, Julien Thierry wrote:
> Hi Andrew,
> 
> On 08/03/2019 12:07, Andrew Murray wrote:
> > With VHE different exception levels are used between the host (EL2) and
> > guest (EL1) with a shared exception level for userpace (EL0). We can take
> > advantage of this and use the PMU's exception level filtering to avoid
> > enabling/disabling counters in the world-switch code. Instead we just
> > modify the counter type to include or exclude EL0 at vcpu_{load,put} time.
> > 
> > We also ensure that trapped PMU system register writes do not re-enable
> > EL0 when reconfiguring the backing perf events.
> > 
> > This approach completely avoids blackout windows seen with !VHE.
> > 
> > Suggested-by: Christoffer Dall <christoffer.dall@arm.com>
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >  arch/arm/include/asm/kvm_host.h   |  3 ++
> >  arch/arm64/include/asm/kvm_host.h |  5 +-
> >  arch/arm64/kernel/perf_event.c    |  6 ++-
> >  arch/arm64/kvm/pmu.c              | 87 ++++++++++++++++++++++++++++++-
> >  arch/arm64/kvm/sys_regs.c         |  3 ++
> >  virt/kvm/arm/arm.c                |  2 +
> >  6 files changed, 102 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> > index a358cb15bb0d..3ce429954306 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -326,6 +326,9 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
> >  static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
> >  static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
> >  
> > +static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
> > +static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
> > +
> >  static inline void kvm_arm_vhe_guest_enter(void) {}
> >  static inline void kvm_arm_vhe_guest_exit(void) {}
> >  
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 7ca4e094626d..d631528898b5 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -487,7 +487,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> >  
> >  static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
> >  {
> > -	return attr->exclude_host;
> > +	return (!has_vhe() && attr->exclude_host);
> >  }
> >  
> >  #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> > @@ -501,6 +501,9 @@ void kvm_clr_pmu_events(u32 clr);
> >  
> >  void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt);
> >  bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt);
> > +
> > +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
> > +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
> >  #else
> >  static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
> >  static inline void kvm_clr_pmu_events(u32 clr) {}
> > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > index 64f02a9fd7cd..a121a82fc54c 100644
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -847,8 +847,12 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
> >  	 * with other architectures (x86 and Power).
> >  	 */
> >  	if (is_kernel_in_hyp_mode()) {
> > -		if (!attr->exclude_kernel)
> > +		if (!attr->exclude_kernel && !attr->exclude_host)
> >  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> > +		if (attr->exclude_guest)
> > +			config_base |= ARMV8_PMU_EXCLUDE_EL1;
> > +		if (attr->exclude_host)
> > +			config_base |= ARMV8_PMU_EXCLUDE_EL0;
> >  	} else {
> >  		if (!attr->exclude_hv && !attr->exclude_host)
> >  			config_base |= ARMV8_PMU_INCLUDE_EL2;
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > index a1cee7919953..a0830c70ece5 100644
> > --- a/arch/arm64/kvm/pmu.c
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -12,11 +12,19 @@
> >  DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> >  
> >  /*
> > - * Given the exclude_{host,guest} attributes, determine if we are going
> > - * to need to switch counters at guest entry/exit.
> > + * Given the perf event attributes and system type, 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)
> >  {
> > +	/**
> > +	 * With VHE the guest kernel runs at EL1 and the host at EL2,
> > +	 * where user (EL0) is excluded then we have no reason to switch
> > +	 * counters.
> > +	 */
> > +	if (has_vhe() && attr->exclude_user)
> > +		return false;
> > +
> >  	/* Only switch if attributes are different */
> >  	return (attr->exclude_host ^ attr->exclude_guest);
> >  }
> > @@ -87,3 +95,78 @@ void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
> >  		write_sysreg(pmu->events_host, pmcntenset_el0);
> >  }
> >  
> > +/*
> > + * Modify ARMv8 PMU events to include EL0 counting
> > + */
> > +static void kvm_vcpu_pmu_enable_el0(unsigned long events)
> > +{
> > +	u64 typer;
> > +	u32 counter;
> > +
> > +	for_each_set_bit(counter, &events, 32) {
> > +		write_sysreg(counter, pmselr_el0);
> > +		isb();
> > +		typer = read_sysreg(pmxevtyper_el0) & ~ARMV8_PMU_EXCLUDE_EL0;
> > +		write_sysreg(typer, pmxevtyper_el0);
> > +		isb();
> > +	}
> > +}
> > +
> > +/*
> > + * Modify ARMv8 PMU events to exclude EL0 counting
> > + */
> > +static void kvm_vcpu_pmu_disable_el0(unsigned long events)
> > +{
> > +	u64 typer;
> > +	u32 counter;
> > +
> > +	for_each_set_bit(counter, &events, 32) {
> > +		write_sysreg(counter, pmselr_el0);
> > +		isb();
> > +		typer = read_sysreg(pmxevtyper_el0) | ARMV8_PMU_EXCLUDE_EL0;
> > +		write_sysreg(typer, pmxevtyper_el0);
> > +		isb();
> > +	}
> > +}
> > +
> > +/*
> > + * On VHE ensure that only guest events have EL0 counting enabled
> > + */
> > +void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_cpu_context *host_ctxt;
> > +	struct kvm_host_data *host;
> > +	u32 events_guest, events_host;
> > +
> > +	if (!has_vhe())
> > +		return;
> > +
> > +	host_ctxt = vcpu->arch.host_cpu_context;
> > +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> > +	events_guest = host->pmu_events.events_guest;
> > +	events_host = host->pmu_events.events_host;
> > +
> > +	kvm_vcpu_pmu_enable_el0(events_guest);
> > +	kvm_vcpu_pmu_disable_el0(events_host);
> 
> So, we load a vcpu, and all events common to the guest and the host
> (events_guest & events_host) get the EXCLUDE_EL0 flag set.
> 
> I don't see anything that will remove that flag before running the
> guest. Am I missing something? Should these lines be as follows?
> 
> 	kvm_vcpu_pmu_enable_el0(events_guest & events_host);
> 	kvm_vcpu_pmu_enable_el0(events_guest ^ events_host);
> 

For VHE and !exclude_user, where an event is common to both guest and
host (i.e.  exclude_kernel = exclude_host = 0) then:

 - The event is never deferred (so we always start counting)
 - When we set the event filter at start of day (armv8pmu_set_event_filter)
   we don't exclude EL0.
 - Also we don't add this event to pmu_events due to kvm_pmu_switch_needed
   in kvm_set_pmu_events - so we don't actually do anything here.

I think the logic you were expecting is moved to start of day when the
counter is first created, thus reducing the effort here.

Thanks,

Andrew Murray 


> > +}
> > +
> > +/*
> > + * On VHE ensure that only guest host have EL0 counting enabled
> > + */
> > +void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
> > +{
> > +	struct kvm_cpu_context *host_ctxt;
> > +	struct kvm_host_data *host;
> > +	u32 events_guest, events_host;
> > +
> > +	if (!has_vhe())
> > +		return;
> > +
> > +	host_ctxt = vcpu->arch.host_cpu_context;
> > +	host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
> > +	events_guest = host->pmu_events.events_guest;
> > +	events_host = host->pmu_events.events_host;
> > +
> > +	kvm_vcpu_pmu_enable_el0(events_host);
> > +	kvm_vcpu_pmu_disable_el0(events_guest);
> 
> Same question as above, after vcpu_put, it seems we've disabled at EL0
> host events that are common to the guest and the host.
> 
> Thanks,
> 
> -- 
> Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2019-03-11 12:16 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 12:07 [PATCH v11 0/8] arm64: Support perf event modifiers :G and :H Andrew Murray
2019-03-08 12:07 ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 1/8] arm64: arm_pmu: remove unnecessary isb instruction Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 3/8] arm64: KVM: add accessors to track guest/host only counters Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-08 16:40   ` Julien Thierry
2019-03-08 16:40     ` Julien Thierry
2019-03-11 11:45     ` Andrew Murray
2019-03-11 11:45       ` Andrew Murray
2019-03-11 11:00   ` Suzuki K Poulose
2019-03-11 11:00     ` Suzuki K Poulose
2019-03-11 11:59     ` Andrew Murray
2019-03-11 11:59       ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 5/8] arm64: KVM: Enable !VHE support for :G/:H perf event modifiers Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 6/8] arm64: KVM: Enable VHE " Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-11  9:39   ` Julien Thierry
2019-03-11  9:39     ` Julien Thierry
2019-03-11 12:16     ` Andrew Murray
2019-03-11 12:16       ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 7/8] arm64: KVM: avoid isb's by using direct pmxevtyper sysreg Andrew Murray
2019-03-08 12:07   ` Andrew Murray
2019-03-08 12:07 ` [PATCH v11 8/8] arm64: docs: document perf event attributes Andrew Murray
2019-03-08 12:07   ` Andrew Murray

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.