All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 0/8] arm64: Support perf event modifiers :G and :H
@ 2019-03-28 10:37 ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 v11:

 - Rebased to v5.1-rc2

 - Minor changes to commit messages, fixing typos, removing superfluous
   comments, code.

 - Change ^ to != in kvm_pmu_switch_needed

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 |  39 ++++-
 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              | 232 ++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c         |   3 +
 virt/kvm/arm/arm.c                |  16 ++-
 11 files changed, 412 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/arm64/perf.txt
 create mode 100644 arch/arm64/kvm/pmu.c

-- 
2.21.0

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

* [PATCH v12 0/8] arm64: Support perf event modifiers :G and :H
@ 2019-03-28 10:37 ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 v11:

 - Rebased to v5.1-rc2

 - Minor changes to commit messages, fixing typos, removing superfluous
   comments, code.

 - Change ^ to != in kvm_pmu_switch_needed

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 |  39 ++++-
 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              | 232 ++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c         |   3 +
 virt/kvm/arm/arm.c                |  16 ++-
 11 files changed, 412 insertions(+), 25 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] 48+ messages in thread

* [PATCH v12 1/8] arm64: arm_pmu: remove unnecessary isb instruction
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 4addb38bc250..cccf4fc86d67 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] 48+ messages in thread

* [PATCH v12 1/8] arm64: arm_pmu: remove unnecessary isb instruction
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 4addb38bc250..cccf4fc86d67 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] 48+ messages in thread

* [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 | 16 ++++++++++------
 arch/arm64/kernel/asm-offsets.c   |  1 +
 virt/kvm/arm/arm.c                | 14 ++++++++------
 5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 770d73257ad9..427c28be6452 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -150,7 +150,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;
 
 static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
 					     int cpu)
@@ -182,7 +186,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 a01fe087e022..6530ad522a16 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -212,7 +212,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;
@@ -255,7 +259,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 */
@@ -432,9 +436,9 @@ 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);
 
-static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
+static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt,
 					     int cpu)
 {
 	/* The host's MPIDR is immutable, so let's set it up at boot time */
@@ -452,8 +456,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 7f40dcbdd51d..fbf0409cc2be 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -128,6 +128,7 @@ int main(void)
   DEFINE(CPU_GP_REGS,		offsetof(struct kvm_cpu_context, gp_regs));
   DEFINE(CPU_USER_PT_REGS,	offsetof(struct kvm_regs, regs));
   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_CTX_SP,		offsetof(struct cpu_suspend_ctx, sp));
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 99c37384ba7b..6bcda52c0ea6 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. */
@@ -360,8 +360,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
@@ -373,7 +375,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);
@@ -1549,11 +1551,11 @@ 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);
-		kvm_init_host_cpu_context(cpu_ctxt, cpu);
-		err = create_hyp_mappings(cpu_ctxt, cpu_ctxt + 1, PAGE_HYP);
+		cpu_data = per_cpu_ptr(&kvm_host_data, cpu);
+		kvm_init_host_cpu_context(&cpu_data->host_ctxt, 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] 48+ messages in thread

* [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 | 16 ++++++++++------
 arch/arm64/kernel/asm-offsets.c   |  1 +
 virt/kvm/arm/arm.c                | 14 ++++++++------
 5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 770d73257ad9..427c28be6452 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -150,7 +150,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;
 
 static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
 					     int cpu)
@@ -182,7 +186,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 a01fe087e022..6530ad522a16 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -212,7 +212,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;
@@ -255,7 +259,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 */
@@ -432,9 +436,9 @@ 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);
 
-static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
+static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt,
 					     int cpu)
 {
 	/* The host's MPIDR is immutable, so let's set it up at boot time */
@@ -452,8 +456,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 7f40dcbdd51d..fbf0409cc2be 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -128,6 +128,7 @@ int main(void)
   DEFINE(CPU_GP_REGS,		offsetof(struct kvm_cpu_context, gp_regs));
   DEFINE(CPU_USER_PT_REGS,	offsetof(struct kvm_regs, regs));
   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_CTX_SP,		offsetof(struct cpu_suspend_ctx, sp));
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 99c37384ba7b..6bcda52c0ea6 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. */
@@ -360,8 +360,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
@@ -373,7 +375,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);
@@ -1549,11 +1551,11 @@ 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);
-		kvm_init_host_cpu_context(cpu_ctxt, cpu);
-		err = create_hyp_mappings(cpu_ctxt, cpu_ctxt + 1, PAGE_HYP);
+		cpu_data = per_cpu_ptr(&kvm_host_data, cpu);
+		kvm_init_host_cpu_context(&cpu_data->host_ctxt, 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] 48+ messages in thread

* [PATCH v12 3/8] arm64: KVM: add accessors to track guest/host only counters
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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,
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              | 45 +++++++++++++++++++++++++++++++
 3 files changed, 63 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 6530ad522a16..213465196e6b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -212,8 +212,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;
@@ -520,11 +526,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_deferred(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 690e033a91c0..3ac1a64d2fb9 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -17,7 +17,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..5414b134f99a
--- /dev/null
+++ b/arch/arm64/kvm/pmu.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Arm Limited
+ * Author: Andrew Murray <Andrew.Murray@arm.com>
+ */
+#include <linux/kvm_host.h>
+#include <linux/perf_event.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);
+}
+
+/*
+ * 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] 48+ messages in thread

* [PATCH v12 3/8] arm64: KVM: add accessors to track guest/host only counters
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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,
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              | 45 +++++++++++++++++++++++++++++++
 3 files changed, 63 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 6530ad522a16..213465196e6b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -212,8 +212,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;
@@ -520,11 +526,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_deferred(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 690e033a91c0..3ac1a64d2fb9 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -17,7 +17,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..5414b134f99a
--- /dev/null
+++ b/arch/arm64/kvm/pmu.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Arm Limited
+ * Author: Andrew Murray <Andrew.Murray@arm.com>
+ */
+#include <linux/kvm_host.h>
+#include <linux/perf_event.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);
+}
+
+/*
+ * 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] 48+ messages in thread

* [PATCH v12 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 cccf4fc86d67..6bb28aaf5aea 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_deferred(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_deferred(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] 48+ messages in thread

* [PATCH v12 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 cccf4fc86d67..6bb28aaf5aea 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_deferred(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_deferred(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] 48+ messages in thread

* [PATCH v12 5/8] arm64: KVM: Enable !VHE support for :G/:H perf event modifiers
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 213465196e6b..a3bfb75f0be9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -539,6 +539,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 3563fe655cd5..d1dc40e53f22 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -524,6 +524,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;
 
 	/*
@@ -543,6 +544,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));
@@ -589,6 +592,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);
+
 	/* Returning to host will clear PSR.I, remask PMR if needed */
 	if (system_uses_irq_prio_masking())
 		gic_write_pmr(GIC_PRIO_IRQOFF);
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 5414b134f99a..48949f0cdba0 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -5,6 +5,7 @@
  */
 #include <linux/kvm_host.h>
 #include <linux/perf_event.h>
+#include <asm/kvm_hyp.h>
 
 /*
  * Given the exclude_{host,guest} attributes, determine if we are going
@@ -43,3 +44,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] 48+ messages in thread

* [PATCH v12 5/8] arm64: KVM: Enable !VHE support for :G/:H perf event modifiers
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 213465196e6b..a3bfb75f0be9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -539,6 +539,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 3563fe655cd5..d1dc40e53f22 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -524,6 +524,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;
 
 	/*
@@ -543,6 +544,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));
@@ -589,6 +592,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);
+
 	/* Returning to host will clear PSR.I, remask PMR if needed */
 	if (system_uses_irq_prio_masking())
 		gic_write_pmr(GIC_PRIO_IRQOFF);
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 5414b134f99a..48949f0cdba0 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -5,6 +5,7 @@
  */
 #include <linux/kvm_host.h>
 #include <linux/perf_event.h>
+#include <asm/kvm_hyp.h>
 
 /*
  * Given the exclude_{host,guest} attributes, determine if we are going
@@ -43,3 +44,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] 48+ messages in thread

* [PATCH v12 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 427c28be6452..481411295b3b 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
 
 static inline bool kvm_pmu_counter_deferred(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 */
@@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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 48949f0cdba0..3407a529e116 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -8,11 +8,19 @@
 #include <asm/kvm_hyp.h>
 
 /*
- * 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);
 }
@@ -83,3 +91,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 539feecda5b8..c7fa47ad2387 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 6bcda52c0ea6..d805bb470dda 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -382,6 +382,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);
@@ -395,6 +396,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] 48+ messages in thread

* [PATCH v12 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 427c28be6452..481411295b3b 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
 
 static inline bool kvm_pmu_counter_deferred(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 */
@@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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 48949f0cdba0..3407a529e116 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -8,11 +8,19 @@
 #include <asm/kvm_hyp.h>
 
 /*
- * 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);
 }
@@ -83,3 +91,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 539feecda5b8..c7fa47ad2387 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 6bcda52c0ea6..d805bb470dda 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -382,6 +382,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);
@@ -395,6 +396,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] 48+ messages in thread

* [PATCH v12 7/8] arm64: KVM: avoid isb's by using direct pmxevtyper sysreg
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 3407a529e116..4d55193ccc71 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -91,6 +91,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
  */
@@ -100,11 +168,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);
 	}
 }
 
@@ -117,11 +183,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] 48+ messages in thread

* [PATCH v12 7/8] arm64: KVM: avoid isb's by using direct pmxevtyper sysreg
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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 3407a529e116..4d55193ccc71 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -91,6 +91,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
  */
@@ -100,11 +168,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);
 	}
 }
 
@@ -117,11 +183,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] 48+ messages in thread

* [PATCH v12 8/8] arm64: docs: document perf event attributes
  2019-03-28 10:37 ` Andrew Murray
@ 2019-03-28 10:37   ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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] 48+ messages in thread

* [PATCH v12 8/8] arm64: docs: document perf event attributes
@ 2019-03-28 10:37   ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-03-28 10:37 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] 48+ messages in thread

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  2019-03-28 10:37   ` Andrew Murray
@ 2019-03-28 15:28     ` Suzuki K Poulose
  -1 siblings, 0 replies; 48+ messages in thread
From: Suzuki K Poulose @ 2019-03-28 15:28 UTC (permalink / raw)
  To: andrew.murray, christoffer.dall, marc.zyngier, catalin.marinas,
	will.deacon, mark.rutland
  Cc: kvmarm, linux-arm-kernel

On 03/28/2019 10:37 AM, Andrew Murray wrote:
> 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 | 16 ++++++++++------
>   arch/arm64/kernel/asm-offsets.c   |  1 +
>   virt/kvm/arm/arm.c                | 14 ++++++++------
>   5 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 770d73257ad9..427c28be6452 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -150,7 +150,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;
>   
>   static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
>   					     int cpu)

We need to fix this function prototype to accept struct kvm_cpu_context,
instead of the now removed kvm_cpu_context_t, to prevent a build break
on arm32 ?

With that :

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
@ 2019-03-28 15:28     ` Suzuki K Poulose
  0 siblings, 0 replies; 48+ messages in thread
From: Suzuki K Poulose @ 2019-03-28 15:28 UTC (permalink / raw)
  To: andrew.murray, christoffer.dall, marc.zyngier, catalin.marinas,
	will.deacon, mark.rutland
  Cc: kvmarm, linux-arm-kernel, julien.thierry

On 03/28/2019 10:37 AM, Andrew Murray wrote:
> 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 | 16 ++++++++++------
>   arch/arm64/kernel/asm-offsets.c   |  1 +
>   virt/kvm/arm/arm.c                | 14 ++++++++------
>   5 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 770d73257ad9..427c28be6452 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -150,7 +150,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;
>   
>   static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
>   					     int cpu)

We need to fix this function prototype to accept struct kvm_cpu_context,
instead of the now removed kvm_cpu_context_t, to prevent a build break
on arm32 ?

With that :

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  2019-03-28 10:37   ` Andrew Murray
@ 2019-04-04 16:09     ` Will Deacon
  -1 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-04 16:09 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 10:37:25AM +0000, Andrew Murray wrote:
> 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 | 16 ++++++++++------
>  arch/arm64/kernel/asm-offsets.c   |  1 +
>  virt/kvm/arm/arm.c                | 14 ++++++++------
>  5 files changed, 27 insertions(+), 15 deletions(-)

As per usual, I'll need an Ack from Christoffer and/or Marc on this one :)

Will

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

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

On Thu, Mar 28, 2019 at 10:37:25AM +0000, Andrew Murray wrote:
> 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 | 16 ++++++++++------
>  arch/arm64/kernel/asm-offsets.c   |  1 +
>  virt/kvm/arm/arm.c                | 14 ++++++++------
>  5 files changed, 27 insertions(+), 15 deletions(-)

As per usual, I'll need an Ack from Christoffer and/or Marc on this one :)

Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  2019-04-04 16:09     ` Will Deacon
@ 2019-04-04 16:14       ` Will Deacon
  -1 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-04 16:14 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Apr 04, 2019 at 05:09:58PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:25AM +0000, Andrew Murray wrote:
> > 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 | 16 ++++++++++------
> >  arch/arm64/kernel/asm-offsets.c   |  1 +
> >  virt/kvm/arm/arm.c                | 14 ++++++++------
> >  5 files changed, 27 insertions(+), 15 deletions(-)
> 
> As per usual, I'll need an Ack from Christoffer and/or Marc on this one :)

Actually... given that most of this series is now firmly in KVM territory,
it probably makes more sense for the kvm/arm tree to take this on a separate
branch, which I can also pull into the arm64 perf tree if I run into
conflicts on the non-kvm parts.

Will

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

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

On Thu, Apr 04, 2019 at 05:09:58PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:25AM +0000, Andrew Murray wrote:
> > 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 | 16 ++++++++++------
> >  arch/arm64/kernel/asm-offsets.c   |  1 +
> >  virt/kvm/arm/arm.c                | 14 ++++++++------
> >  5 files changed, 27 insertions(+), 15 deletions(-)
> 
> As per usual, I'll need an Ack from Christoffer and/or Marc on this one :)

Actually... given that most of this series is now firmly in KVM territory,
it probably makes more sense for the kvm/arm tree to take this on a separate
branch, which I can also pull into the arm64 perf tree if I run into
conflicts on the non-kvm parts.

Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 8/8] arm64: docs: document perf event attributes
  2019-03-28 10:37   ` Andrew Murray
@ 2019-04-04 16:21     ` Will Deacon
  -1 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-04 16:21 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> 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.

I find this last sentence a bit confusing, because it can be read to imply
that if you don't set exclude_kernel and you're in a guest on a VHE system,
then you can profile EL2.

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

Similar comment as the above: I don't think this makes sense when you look
at things from the guest perspective.

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

But there are two attributes...

Will

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

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

On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> 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.

I find this last sentence a bit confusing, because it can be read to imply
that if you don't set exclude_kernel and you're in a guest on a VHE system,
then you can profile EL2.

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

Similar comment as the above: I don't think this makes sense when you look
at things from the guest perspective.

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

But there are two attributes...

Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes
  2019-03-28 10:37   ` Andrew Murray
@ 2019-04-04 16:34     ` Will Deacon
  -1 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-04 16:34 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 10:37:27AM +0000, Andrew Murray wrote:
> 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 cccf4fc86d67..6bb28aaf5aea 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_deferred(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_deferred(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;

FWIW, that doesn't align with my personal view of what the host is, but
I'm not going to argue if Marc and Christoffer are happy with it.

Anyway, thanks for persevering with this:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

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

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

On Thu, Mar 28, 2019 at 10:37:27AM +0000, Andrew Murray wrote:
> 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 cccf4fc86d67..6bb28aaf5aea 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_deferred(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_deferred(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;

FWIW, that doesn't align with my personal view of what the host is, but
I'm not going to argue if Marc and Christoffer are happy with it.

Anyway, thanks for persevering with this:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 8/8] arm64: docs: document perf event attributes
  2019-04-04 16:21     ` Will Deacon
@ 2019-04-04 19:33       ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-04 19:33 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > 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.
> 
> I find this last sentence a bit confusing, because it can be read to imply
> that if you don't set exclude_kernel and you're in a guest on a VHE system,
> then you can profile EL2.

Yes this could be misleading.

However from the perspective of the guest, when exclude_kernel is not set we
do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
thus the statement above is correct in terms of what the kernel believes it is
doing.

I think these statements are less confusing if we treat the exception levels
as those 'detected' by the running context (e.g. consider the impact of nested
virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
stops counting upon switching between guest/host, translating PMU filters in
kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
for those wishing to change this logic (which is the intent) rather than those
trying to understand how we filter for EL levels as seen bare-metal.

With regards to the example you gave (exclude_kernel, EL2) - yes we want the
kernel to believe it can count EL2 - because one day we may want to update
KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
time associated with the guest).

I could write some preface that describes this outlook. Alternatively I could
just spell out what happens on a guest, e.g.

"For the host this attribute will exclude EL1 and additionally EL2 on a VHE
system.

For the guest this attribute will exclude EL1."

Though I'm less comfortable with this, as the last statement "For the guest this
attribute will exclude EL1." describes the product of both
kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
to work out and also makes an assumption that we don't have nested virt (true
for now at least) and also reasons about bare-metal EL levels which probably
aren't that useful for someone changing this logic or understanding what the
flags do for there performance analysis.

Do you have a preference for how this is improved?

> 
> > +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.
> 
> Similar comment as the above: I don't think this makes sense when you look
> at things from the guest perspective.
> 
> > +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.
> 
> But there are two attributes...

Oh, I'll have to update this.

Thanks for the review,

Andrew Murray

> 
> Will

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

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

On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > 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.
> 
> I find this last sentence a bit confusing, because it can be read to imply
> that if you don't set exclude_kernel and you're in a guest on a VHE system,
> then you can profile EL2.

Yes this could be misleading.

However from the perspective of the guest, when exclude_kernel is not set we
do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
thus the statement above is correct in terms of what the kernel believes it is
doing.

I think these statements are less confusing if we treat the exception levels
as those 'detected' by the running context (e.g. consider the impact of nested
virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
stops counting upon switching between guest/host, translating PMU filters in
kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
for those wishing to change this logic (which is the intent) rather than those
trying to understand how we filter for EL levels as seen bare-metal.

With regards to the example you gave (exclude_kernel, EL2) - yes we want the
kernel to believe it can count EL2 - because one day we may want to update
KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
time associated with the guest).

I could write some preface that describes this outlook. Alternatively I could
just spell out what happens on a guest, e.g.

"For the host this attribute will exclude EL1 and additionally EL2 on a VHE
system.

For the guest this attribute will exclude EL1."

Though I'm less comfortable with this, as the last statement "For the guest this
attribute will exclude EL1." describes the product of both
kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
to work out and also makes an assumption that we don't have nested virt (true
for now at least) and also reasons about bare-metal EL levels which probably
aren't that useful for someone changing this logic or understanding what the
flags do for there performance analysis.

Do you have a preference for how this is improved?

> 
> > +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.
> 
> Similar comment as the above: I don't think this makes sense when you look
> at things from the guest perspective.
> 
> > +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.
> 
> But there are two attributes...

Oh, I'll have to update this.

Thanks for the review,

Andrew Murray

> 
> Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
  2019-04-04 16:14       ` Will Deacon
@ 2019-04-04 19:34         ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-04 19:34 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Apr 04, 2019 at 05:14:16PM +0100, Will Deacon wrote:
> On Thu, Apr 04, 2019 at 05:09:58PM +0100, Will Deacon wrote:
> > On Thu, Mar 28, 2019 at 10:37:25AM +0000, Andrew Murray wrote:
> > > 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 | 16 ++++++++++------
> > >  arch/arm64/kernel/asm-offsets.c   |  1 +
> > >  virt/kvm/arm/arm.c                | 14 ++++++++------
> > >  5 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > As per usual, I'll need an Ack from Christoffer and/or Marc on this one :)
> 
> Actually... given that most of this series is now firmly in KVM territory,
> it probably makes more sense for the kvm/arm tree to take this on a separate
> branch, which I can also pull into the arm64 perf tree if I run into
> conflicts on the non-kvm parts.

No problem.

Thanks,

Andrew Murray

> 
> Will

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

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

On Thu, Apr 04, 2019 at 05:14:16PM +0100, Will Deacon wrote:
> On Thu, Apr 04, 2019 at 05:09:58PM +0100, Will Deacon wrote:
> > On Thu, Mar 28, 2019 at 10:37:25AM +0000, Andrew Murray wrote:
> > > 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 | 16 ++++++++++------
> > >  arch/arm64/kernel/asm-offsets.c   |  1 +
> > >  virt/kvm/arm/arm.c                | 14 ++++++++------
> > >  5 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > As per usual, I'll need an Ack from Christoffer and/or Marc on this one :)
> 
> Actually... given that most of this series is now firmly in KVM territory,
> it probably makes more sense for the kvm/arm tree to take this on a separate
> branch, which I can also pull into the arm64 perf tree if I run into
> conflicts on the non-kvm parts.

No problem.

Thanks,

Andrew Murray

> 
> Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes
  2019-04-04 16:34     ` Will Deacon
@ 2019-04-04 19:48       ` Andrew Murray
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-04 19:48 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Apr 04, 2019 at 05:34:00PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:27AM +0000, Andrew Murray wrote:
> > 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 cccf4fc86d67..6bb28aaf5aea 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_deferred(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_deferred(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;
> 
> FWIW, that doesn't align with my personal view of what the host is, but
> I'm not going to argue if Marc and Christoffer are happy with it.

Ideally armv8pmu_set_event_filter needs no knowledge of exclude_guest
and exclude_host as we leave the config_base alone and instead rely on
turning the counters on/off at guest entry/exit...

The addition of "&& !attr->exclude_host" here (!VHE) and likewise for VHE in
"arm64: KVM: Enable VHE support for :G/:H perf event modifiers" (in case
you missed it) is to eliminate counters counting host events on the boundaries
of guest entry/exit when counting for guest_only (:G). Thus this is really
an optimisation for more accurate counting.

Consider the case where a user wants to record guest events only - we switch
over to the guest counters at EL2 on world-switch (both VHE and !VHE host) -
now there is a small period of time between when we start the guest counters
and when we actually jump into the guest. This period of time in the host
shouldn't be counted - therefore so long as exclude_host is set we can filter
EL2 to remove these unwanted counts.

Therefore the above hunks likely won't describe what our view of the host is.

> 
> Anyway, thanks for persevering with this:

It's fun even though it hurts my head every time each time I come back to this.

> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Thanks,

Andrew Murray

> 
> Will

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

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

On Thu, Apr 04, 2019 at 05:34:00PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:27AM +0000, Andrew Murray wrote:
> > 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 cccf4fc86d67..6bb28aaf5aea 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_deferred(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_deferred(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;
> 
> FWIW, that doesn't align with my personal view of what the host is, but
> I'm not going to argue if Marc and Christoffer are happy with it.

Ideally armv8pmu_set_event_filter needs no knowledge of exclude_guest
and exclude_host as we leave the config_base alone and instead rely on
turning the counters on/off at guest entry/exit...

The addition of "&& !attr->exclude_host" here (!VHE) and likewise for VHE in
"arm64: KVM: Enable VHE support for :G/:H perf event modifiers" (in case
you missed it) is to eliminate counters counting host events on the boundaries
of guest entry/exit when counting for guest_only (:G). Thus this is really
an optimisation for more accurate counting.

Consider the case where a user wants to record guest events only - we switch
over to the guest counters at EL2 on world-switch (both VHE and !VHE host) -
now there is a small period of time between when we start the guest counters
and when we actually jump into the guest. This period of time in the host
shouldn't be counted - therefore so long as exclude_host is set we can filter
EL2 to remove these unwanted counts.

Therefore the above hunks likely won't describe what our view of the host is.

> 
> Anyway, thanks for persevering with this:

It's fun even though it hurts my head every time each time I come back to this.

> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Thanks,

Andrew Murray

> 
> Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 8/8] arm64: docs: document perf event attributes
  2019-04-04 19:33       ` Andrew Murray
@ 2019-04-05 12:43         ` Will Deacon
  -1 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-05 12:43 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Apr 04, 2019 at 08:33:51PM +0100, Andrew Murray wrote:
> On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> > On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > > +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.
> > 
> > I find this last sentence a bit confusing, because it can be read to imply
> > that if you don't set exclude_kernel and you're in a guest on a VHE system,
> > then you can profile EL2.
> 
> Yes this could be misleading.
> 
> However from the perspective of the guest, when exclude_kernel is not set we
> do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
> thus the statement above is correct in terms of what the kernel believes it is
> doing.
> 
> I think these statements are less confusing if we treat the exception levels
> as those 'detected' by the running context (e.g. consider the impact of nested
> virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
> stops counting upon switching between guest/host, translating PMU filters in
> kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
> for those wishing to change this logic (which is the intent) rather than those
> trying to understand how we filter for EL levels as seen bare-metal.
> 
> With regards to the example you gave (exclude_kernel, EL2) - yes we want the
> kernel to believe it can count EL2 - because one day we may want to update
> KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
> time associated with the guest).

If we were to support this in the future, then exclude_hv will suddenly
start meaning something in a guest, so this could be considered to be an ABI
break.

> I could write some preface that describes this outlook. Alternatively I could
> just spell out what happens on a guest, e.g.
> 
> "For the host this attribute will exclude EL1 and additionally EL2 on a VHE
> system.
> 
> For the guest this attribute will exclude EL1."
> 
> Though I'm less comfortable with this, as the last statement "For the guest this
> attribute will exclude EL1." describes the product of both
> kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
> to work out and also makes an assumption that we don't have nested virt (true
> for now at least) and also reasons about bare-metal EL levels which probably
> aren't that useful for someone changing this logic or understanding what the
> flags do for there performance analysis.
> 
> Do you have a preference for how this is improved?

I think you should be explicit about what is counted. If we don't count EL2
when profiling in a guest (regardless of the exclude_*) flags, then we
should say that. By not documenting this we don't actually buy ourselves
room to change things in future, we should have an emergent behaviour which
isn't covered by our docs.

Will

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

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

On Thu, Apr 04, 2019 at 08:33:51PM +0100, Andrew Murray wrote:
> On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> > On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > > +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.
> > 
> > I find this last sentence a bit confusing, because it can be read to imply
> > that if you don't set exclude_kernel and you're in a guest on a VHE system,
> > then you can profile EL2.
> 
> Yes this could be misleading.
> 
> However from the perspective of the guest, when exclude_kernel is not set we
> do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
> thus the statement above is correct in terms of what the kernel believes it is
> doing.
> 
> I think these statements are less confusing if we treat the exception levels
> as those 'detected' by the running context (e.g. consider the impact of nested
> virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
> stops counting upon switching between guest/host, translating PMU filters in
> kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
> for those wishing to change this logic (which is the intent) rather than those
> trying to understand how we filter for EL levels as seen bare-metal.
> 
> With regards to the example you gave (exclude_kernel, EL2) - yes we want the
> kernel to believe it can count EL2 - because one day we may want to update
> KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
> time associated with the guest).

If we were to support this in the future, then exclude_hv will suddenly
start meaning something in a guest, so this could be considered to be an ABI
break.

> I could write some preface that describes this outlook. Alternatively I could
> just spell out what happens on a guest, e.g.
> 
> "For the host this attribute will exclude EL1 and additionally EL2 on a VHE
> system.
> 
> For the guest this attribute will exclude EL1."
> 
> Though I'm less comfortable with this, as the last statement "For the guest this
> attribute will exclude EL1." describes the product of both
> kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
> to work out and also makes an assumption that we don't have nested virt (true
> for now at least) and also reasons about bare-metal EL levels which probably
> aren't that useful for someone changing this logic or understanding what the
> flags do for there performance analysis.
> 
> Do you have a preference for how this is improved?

I think you should be explicit about what is counted. If we don't count EL2
when profiling in a guest (regardless of the exclude_*) flags, then we
should say that. By not documenting this we don't actually buy ourselves
room to change things in future, we should have an emergent behaviour which
isn't covered by our docs.

Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
@ 2019-04-09 10:52       ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 10:52 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: marc.zyngier, catalin.marinas, will.deacon, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 03:28:37PM +0000, Suzuki K Poulose wrote:
> On 03/28/2019 10:37 AM, Andrew Murray wrote:
> > 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 | 16 ++++++++++------
> >   arch/arm64/kernel/asm-offsets.c   |  1 +
> >   virt/kvm/arm/arm.c                | 14 ++++++++------
> >   5 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> > index 770d73257ad9..427c28be6452 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -150,7 +150,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;
> >   static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
> >   					     int cpu)
> 
> We need to fix this function prototype to accept struct kvm_cpu_context,
> instead of the now removed kvm_cpu_context_t, to prevent a build break
> on arm32 ?

Yes this was a breakage, thanks for pointing this out.

Thanks,

Andrew Murray

> 
> With that :
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
@ 2019-04-09 10:52       ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 10:52 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: marc.zyngier, catalin.marinas, will.deacon, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 03:28:37PM +0000, Suzuki K Poulose wrote:
> On 03/28/2019 10:37 AM, Andrew Murray wrote:
> > 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 | 16 ++++++++++------
> >   arch/arm64/kernel/asm-offsets.c   |  1 +
> >   virt/kvm/arm/arm.c                | 14 ++++++++------
> >   5 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> > index 770d73257ad9..427c28be6452 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -150,7 +150,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;
> >   static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
> >   					     int cpu)
> 
> We need to fix this function prototype to accept struct kvm_cpu_context,
> instead of the now removed kvm_cpu_context_t, to prevent a build break
> on arm32 ?

Yes this was a breakage, thanks for pointing this out.

Thanks,

Andrew Murray

> 
> With that :
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data
@ 2019-04-09 10:52       ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 10:52 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 Thu, Mar 28, 2019 at 03:28:37PM +0000, Suzuki K Poulose wrote:
> On 03/28/2019 10:37 AM, Andrew Murray wrote:
> > 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 | 16 ++++++++++------
> >   arch/arm64/kernel/asm-offsets.c   |  1 +
> >   virt/kvm/arm/arm.c                | 14 ++++++++------
> >   5 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> > index 770d73257ad9..427c28be6452 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -150,7 +150,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;
> >   static inline void kvm_init_host_cpu_context(kvm_cpu_context_t *cpu_ctxt,
> >   					     int cpu)
> 
> We need to fix this function prototype to accept struct kvm_cpu_context,
> instead of the now removed kvm_cpu_context_t, to prevent a build break
> on arm32 ?

Yes this was a breakage, thanks for pointing this out.

Thanks,

Andrew Murray

> 
> With that :
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 8/8] arm64: docs: document perf event attributes
@ 2019-04-09 11:00           ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 11:00 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Fri, Apr 05, 2019 at 01:43:08PM +0100, Will Deacon wrote:
> On Thu, Apr 04, 2019 at 08:33:51PM +0100, Andrew Murray wrote:
> > On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> > > On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > > > +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.
> > > 
> > > I find this last sentence a bit confusing, because it can be read to imply
> > > that if you don't set exclude_kernel and you're in a guest on a VHE system,
> > > then you can profile EL2.
> > 
> > Yes this could be misleading.
> > 
> > However from the perspective of the guest, when exclude_kernel is not set we
> > do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
> > thus the statement above is correct in terms of what the kernel believes it is
> > doing.
> > 
> > I think these statements are less confusing if we treat the exception levels
> > as those 'detected' by the running context (e.g. consider the impact of nested
> > virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
> > stops counting upon switching between guest/host, translating PMU filters in
> > kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
> > for those wishing to change this logic (which is the intent) rather than those
> > trying to understand how we filter for EL levels as seen bare-metal.
> > 
> > With regards to the example you gave (exclude_kernel, EL2) - yes we want the
> > kernel to believe it can count EL2 - because one day we may want to update
> > KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
> > time associated with the guest).
> 
> If we were to support this in the future, then exclude_hv will suddenly
> start meaning something in a guest, so this could be considered to be an ABI
> break.
> 
> > I could write some preface that describes this outlook. Alternatively I could
> > just spell out what happens on a guest, e.g.
> > 
> > "For the host this attribute will exclude EL1 and additionally EL2 on a VHE
> > system.
> > 
> > For the guest this attribute will exclude EL1."
> > 
> > Though I'm less comfortable with this, as the last statement "For the guest this
> > attribute will exclude EL1." describes the product of both
> > kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
> > to work out and also makes an assumption that we don't have nested virt (true
> > for now at least) and also reasons about bare-metal EL levels which probably
> > aren't that useful for someone changing this logic or understanding what the
> > flags do for there performance analysis.
> > 
> > Do you have a preference for how this is improved?
> 
> I think you should be explicit about what is counted. If we don't count EL2
> when profiling in a guest (regardless of the exclude_*) flags, then we
> should say that. By not documenting this we don't actually buy ourselves
> room to change things in future, we should have an emergent behaviour which
> isn't covered by our docs.

OK no problem, I'll update this.

Andrew Murray

> 
> Will

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

* Re: [PATCH v12 8/8] arm64: docs: document perf event attributes
@ 2019-04-09 11:00           ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 11:00 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Fri, Apr 05, 2019 at 01:43:08PM +0100, Will Deacon wrote:
> On Thu, Apr 04, 2019 at 08:33:51PM +0100, Andrew Murray wrote:
> > On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> > > On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > > > +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.
> > > 
> > > I find this last sentence a bit confusing, because it can be read to imply
> > > that if you don't set exclude_kernel and you're in a guest on a VHE system,
> > > then you can profile EL2.
> > 
> > Yes this could be misleading.
> > 
> > However from the perspective of the guest, when exclude_kernel is not set we
> > do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
> > thus the statement above is correct in terms of what the kernel believes it is
> > doing.
> > 
> > I think these statements are less confusing if we treat the exception levels
> > as those 'detected' by the running context (e.g. consider the impact of nested
> > virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
> > stops counting upon switching between guest/host, translating PMU filters in
> > kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
> > for those wishing to change this logic (which is the intent) rather than those
> > trying to understand how we filter for EL levels as seen bare-metal.
> > 
> > With regards to the example you gave (exclude_kernel, EL2) - yes we want the
> > kernel to believe it can count EL2 - because one day we may want to update
> > KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
> > time associated with the guest).
> 
> If we were to support this in the future, then exclude_hv will suddenly
> start meaning something in a guest, so this could be considered to be an ABI
> break.
> 
> > I could write some preface that describes this outlook. Alternatively I could
> > just spell out what happens on a guest, e.g.
> > 
> > "For the host this attribute will exclude EL1 and additionally EL2 on a VHE
> > system.
> > 
> > For the guest this attribute will exclude EL1."
> > 
> > Though I'm less comfortable with this, as the last statement "For the guest this
> > attribute will exclude EL1." describes the product of both
> > kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
> > to work out and also makes an assumption that we don't have nested virt (true
> > for now at least) and also reasons about bare-metal EL levels which probably
> > aren't that useful for someone changing this logic or understanding what the
> > flags do for there performance analysis.
> > 
> > Do you have a preference for how this is improved?
> 
> I think you should be explicit about what is counted. If we don't count EL2
> when profiling in a guest (regardless of the exclude_*) flags, then we
> should say that. By not documenting this we don't actually buy ourselves
> room to change things in future, we should have an emergent behaviour which
> isn't covered by our docs.

OK no problem, I'll update this.

Andrew Murray

> 
> Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

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

On Fri, Apr 05, 2019 at 01:43:08PM +0100, Will Deacon wrote:
> On Thu, Apr 04, 2019 at 08:33:51PM +0100, Andrew Murray wrote:
> > On Thu, Apr 04, 2019 at 05:21:28PM +0100, Will Deacon wrote:
> > > On Thu, Mar 28, 2019 at 10:37:31AM +0000, Andrew Murray wrote:
> > > > +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.
> > > 
> > > I find this last sentence a bit confusing, because it can be read to imply
> > > that if you don't set exclude_kernel and you're in a guest on a VHE system,
> > > then you can profile EL2.
> > 
> > Yes this could be misleading.
> > 
> > However from the perspective of the guest, when exclude_kernel is not set we
> > do indeed allow the guest to program it's PMU with ARMV8_PMU_INCLUDE_EL2 - and
> > thus the statement above is correct in terms of what the kernel believes it is
> > doing.
> > 
> > I think these statements are less confusing if we treat the exception levels
> > as those 'detected' by the running context (e.g. consider the impact of nested
> > virt here) - and we if ignore what the hypervisor (KVM) does outside (e.g.
> > stops counting upon switching between guest/host, translating PMU filters in
> > kvm_pmu_set_counter_event_type etc, etc). This then makes this document useful
> > for those wishing to change this logic (which is the intent) rather than those
> > trying to understand how we filter for EL levels as seen bare-metal.
> > 
> > With regards to the example you gave (exclude_kernel, EL2) - yes we want the
> > kernel to believe it can count EL2 - because one day we may want to update
> > KVM to allow the guest to count it's hypervisor overhead (e.g. host kernel
> > time associated with the guest).
> 
> If we were to support this in the future, then exclude_hv will suddenly
> start meaning something in a guest, so this could be considered to be an ABI
> break.
> 
> > I could write some preface that describes this outlook. Alternatively I could
> > just spell out what happens on a guest, e.g.
> > 
> > "For the host this attribute will exclude EL1 and additionally EL2 on a VHE
> > system.
> > 
> > For the guest this attribute will exclude EL1."
> > 
> > Though I'm less comfortable with this, as the last statement "For the guest this
> > attribute will exclude EL1." describes the product of both
> > kvm_pmu_set_counter_event_type and armv8pmu_set_event_filter which is confusing
> > to work out and also makes an assumption that we don't have nested virt (true
> > for now at least) and also reasons about bare-metal EL levels which probably
> > aren't that useful for someone changing this logic or understanding what the
> > flags do for there performance analysis.
> > 
> > Do you have a preference for how this is improved?
> 
> I think you should be explicit about what is counted. If we don't count EL2
> when profiling in a guest (regardless of the exclude_*) flags, then we
> should say that. By not documenting this we don't actually buy ourselves
> room to change things in future, we should have an emergent behaviour which
> isn't covered by our docs.

OK no problem, I'll update this.

Andrew Murray

> 
> Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-04-09 17:52     ` Will Deacon
  0 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-09 17:52 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 10:37:29AM +0000, 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 427c28be6452..481411295b3b 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>  
>  static inline bool kvm_pmu_counter_deferred(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 */
> @@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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;

I still don't really like these semantics, but it's consistent and
you're documenting it so:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

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

* Re: [PATCH v12 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-04-09 17:52     ` Will Deacon
  0 siblings, 0 replies; 48+ messages in thread
From: Will Deacon @ 2019-04-09 17:52 UTC (permalink / raw)
  To: Andrew Murray; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Thu, Mar 28, 2019 at 10:37:29AM +0000, 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 427c28be6452..481411295b3b 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>  
>  static inline bool kvm_pmu_counter_deferred(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 */
> @@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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;

I still don't really like these semantics, but it's consistent and
you're documenting it so:

Acked-by: Will Deacon <will.deacon@arm.com>

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

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

On Thu, Mar 28, 2019 at 10:37:29AM +0000, 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 427c28be6452..481411295b3b 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>  
>  static inline bool kvm_pmu_counter_deferred(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 */
> @@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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;

I still don't really like these semantics, but it's consistent and
you're documenting it so:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

_______________________________________________
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] 48+ messages in thread

* Re: [PATCH v12 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-04-09 19:13       ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 19:13 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Tue, Apr 09, 2019 at 06:52:27PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:29AM +0000, 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 427c28be6452..481411295b3b 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> >  
> >  static inline bool kvm_pmu_counter_deferred(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 */
> > @@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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;
> 
> I still don't really like these semantics, but it's consistent and
> you're documenting it so:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Much appreciated.

Thanks,

Andrew Murray

> 
> Will

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

* Re: [PATCH v12 6/8] arm64: KVM: Enable VHE support for :G/:H perf event modifiers
@ 2019-04-09 19:13       ` Andrew Murray
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Murray @ 2019-04-09 19:13 UTC (permalink / raw)
  To: Will Deacon; +Cc: Marc Zyngier, Catalin Marinas, kvmarm, linux-arm-kernel

On Tue, Apr 09, 2019 at 06:52:27PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:29AM +0000, 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 427c28be6452..481411295b3b 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> >  
> >  static inline bool kvm_pmu_counter_deferred(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 */
> > @@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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;
> 
> I still don't really like these semantics, but it's consistent and
> you're documenting it so:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Much appreciated.

Thanks,

Andrew Murray

> 
> Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

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

On Tue, Apr 09, 2019 at 06:52:27PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:29AM +0000, 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 427c28be6452..481411295b3b 100644
> > --- a/arch/arm/include/asm/kvm_host.h
> > +++ b/arch/arm/include/asm/kvm_host.h
> > @@ -365,6 +365,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 a3bfb75f0be9..4f290dad3a48 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -528,7 +528,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> >  
> >  static inline bool kvm_pmu_counter_deferred(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 */
> > @@ -542,6 +542,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 6bb28aaf5aea..314b1adedf06 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;
> 
> I still don't really like these semantics, but it's consistent and
> you're documenting it so:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Much appreciated.

Thanks,

Andrew Murray

> 
> Will

_______________________________________________
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] 48+ messages in thread

end of thread, other threads:[~2019-04-09 19:14 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-28 10:37 [PATCH v12 0/8] arm64: Support perf event modifiers :G and :H Andrew Murray
2019-03-28 10:37 ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 1/8] arm64: arm_pmu: remove unnecessary isb instruction Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 2/8] arm64: KVM: encapsulate kvm_cpu_context in kvm_host_data Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-03-28 15:28   ` Suzuki K Poulose
2019-03-28 15:28     ` Suzuki K Poulose
2019-04-09 10:52     ` Andrew Murray
2019-04-09 10:52       ` Andrew Murray
2019-04-09 10:52       ` Andrew Murray
2019-04-04 16:09   ` Will Deacon
2019-04-04 16:09     ` Will Deacon
2019-04-04 16:14     ` Will Deacon
2019-04-04 16:14       ` Will Deacon
2019-04-04 19:34       ` Andrew Murray
2019-04-04 19:34         ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 3/8] arm64: KVM: add accessors to track guest/host only counters Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 4/8] arm64: arm_pmu: Add !VHE support for exclude_host/exclude_guest attributes Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-04-04 16:34   ` Will Deacon
2019-04-04 16:34     ` Will Deacon
2019-04-04 19:48     ` Andrew Murray
2019-04-04 19:48       ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 5/8] arm64: KVM: Enable !VHE support for :G/:H perf event modifiers Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 6/8] arm64: KVM: Enable VHE " Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-04-09 17:52   ` Will Deacon
2019-04-09 17:52     ` Will Deacon
2019-04-09 17:52     ` Will Deacon
2019-04-09 19:13     ` Andrew Murray
2019-04-09 19:13       ` Andrew Murray
2019-04-09 19:13       ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 7/8] arm64: KVM: avoid isb's by using direct pmxevtyper sysreg Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-03-28 10:37 ` [PATCH v12 8/8] arm64: docs: document perf event attributes Andrew Murray
2019-03-28 10:37   ` Andrew Murray
2019-04-04 16:21   ` Will Deacon
2019-04-04 16:21     ` Will Deacon
2019-04-04 19:33     ` Andrew Murray
2019-04-04 19:33       ` Andrew Murray
2019-04-05 12:43       ` Will Deacon
2019-04-05 12:43         ` Will Deacon
2019-04-09 11:00         ` Andrew Murray
2019-04-09 11:00           ` Andrew Murray
2019-04-09 11:00           ` 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.