[PATCH v3 0/4] KVM: arm64: Do not communicate host pmu event changes by accessing hyp data

[PATCH v3 0/4] KVM: arm64: Do not communicate host pmu event changes by accessing hyp data

[Fuad Tabba]

Hi,

This series changes the way KVM communicates host pmu event changes to the
hypervisor in nvhe and protected mode. Instead of accessing hyp data directly
from the host, the data is passed to hyp via the loaded vcpu. This provides
more isolation between the host and the hypervisor, and allows us to use pmu in
protected mode.

This series is based on kvmarm/next (8c22fd2d4cfa).

Changes since V2 [1]:
- Update the guest view of the pmu events after interrupts have been disabled

Changes since V1 [2]:
- Repack struct kvm_pmu to remove all (two) holes
- Updating the guest's view of the pmu events before every vcpu run rather than
vcpu load to avoid missing updates

Thanks,
/fuad

[1] https://lore.kernel.org/all/20220509095500.2408785-1-tabba@google.com/
[2] https://lore.kernel.org/linux-arm-kernel/20220408084052.3310931-1-tabba@google.com/

Fuad Tabba (4):
  KVM: arm64: Wrapper for getting pmu_events
  KVM: arm64: Repack struct kvm_pmu to reduce size
  KVM: arm64: Pass pmu events to hyp via vcpu
  KVM: arm64: Reenable pmu in Protected Mode

 arch/arm64/include/asm/kvm_host.h |  8 ++-----
 arch/arm64/kvm/arm.c              | 15 ++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c  | 20 +++++-----------
 arch/arm64/kvm/pmu-emul.c         |  3 +--
 arch/arm64/kvm/pmu.c              | 40 ++++++++++++++++++-------------
 include/kvm/arm_pmu.h             | 10 ++++++--
 6 files changed, 55 insertions(+), 41 deletions(-)


base-commit: 8c22fd2d4cfaab2ae3c5859496c894df58ab014b
-- 
2.36.0.512.ge40c2bad7a-goog

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

[PATCH v3 0/4] KVM: arm64: Do not communicate host pmu event changes by accessing hyp data

[Fuad Tabba]

Hi,

This series changes the way KVM communicates host pmu event changes to the
hypervisor in nvhe and protected mode. Instead of accessing hyp data directly
from the host, the data is passed to hyp via the loaded vcpu. This provides
more isolation between the host and the hypervisor, and allows us to use pmu in
protected mode.

This series is based on kvmarm/next (8c22fd2d4cfa).

Changes since V2 [1]:
- Update the guest view of the pmu events after interrupts have been disabled

Changes since V1 [2]:
- Repack struct kvm_pmu to remove all (two) holes
- Updating the guest's view of the pmu events before every vcpu run rather than
vcpu load to avoid missing updates

Thanks,
/fuad

[1] https://lore.kernel.org/all/20220509095500.2408785-1-tabba@google.com/
[2] https://lore.kernel.org/linux-arm-kernel/20220408084052.3310931-1-tabba@google.com/

Fuad Tabba (4):
  KVM: arm64: Wrapper for getting pmu_events
  KVM: arm64: Repack struct kvm_pmu to reduce size
  KVM: arm64: Pass pmu events to hyp via vcpu
  KVM: arm64: Reenable pmu in Protected Mode

 arch/arm64/include/asm/kvm_host.h |  8 ++-----
 arch/arm64/kvm/arm.c              | 15 ++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c  | 20 +++++-----------
 arch/arm64/kvm/pmu-emul.c         |  3 +--
 arch/arm64/kvm/pmu.c              | 40 ++++++++++++++++++-------------
 include/kvm/arm_pmu.h             | 10 ++++++--
 6 files changed, 55 insertions(+), 41 deletions(-)


base-commit: 8c22fd2d4cfaab2ae3c5859496c894df58ab014b
-- 
2.36.0.512.ge40c2bad7a-goog


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

[PATCH v3 1/4] KVM: arm64: Wrapper for getting pmu_events

[Fuad Tabba]

Eases migrating away from using hyp data and simplifies the code.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/pmu.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 03a6c1f4a09a..4bd38ff34221 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -25,21 +25,31 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
 	return (attr->exclude_host != attr->exclude_guest);
 }
 
+static struct kvm_pmu_events *kvm_get_pmu_events(void)
+{
+	struct kvm_host_data *ctx = this_cpu_ptr_hyp_sym(kvm_host_data);
+
+	if (!ctx)
+		return NULL;
+
+	return &ctx->pmu_events;
+}
+
 /*
  * 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_hyp_sym(kvm_host_data);
+	struct kvm_pmu_events *pmu = kvm_get_pmu_events();
 
-	if (!kvm_arm_support_pmu_v3() || !ctx || !kvm_pmu_switch_needed(attr))
+	if (!kvm_arm_support_pmu_v3() || !pmu || !kvm_pmu_switch_needed(attr))
 		return;
 
 	if (!attr->exclude_host)
-		ctx->pmu_events.events_host |= set;
+		pmu->events_host |= set;
 	if (!attr->exclude_guest)
-		ctx->pmu_events.events_guest |= set;
+		pmu->events_guest |= set;
 }
 
 /*
@@ -47,13 +57,13 @@ void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
  */
 void kvm_clr_pmu_events(u32 clr)
 {
-	struct kvm_host_data *ctx = this_cpu_ptr_hyp_sym(kvm_host_data);
+	struct kvm_pmu_events *pmu = kvm_get_pmu_events();
 
-	if (!kvm_arm_support_pmu_v3() || !ctx)
+	if (!kvm_arm_support_pmu_v3() || !pmu)
 		return;
 
-	ctx->pmu_events.events_host &= ~clr;
-	ctx->pmu_events.events_guest &= ~clr;
+	pmu->events_host &= ~clr;
+	pmu->events_guest &= ~clr;
 }
 
 #define PMEVTYPER_READ_CASE(idx)				\
@@ -169,16 +179,16 @@ static void kvm_vcpu_pmu_disable_el0(unsigned long events)
  */
 void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
 {
-	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
 	u32 events_guest, events_host;
 
 	if (!kvm_arm_support_pmu_v3() || !has_vhe())
 		return;
 
 	preempt_disable();
-	host = this_cpu_ptr_hyp_sym(kvm_host_data);
-	events_guest = host->pmu_events.events_guest;
-	events_host = host->pmu_events.events_host;
+	pmu = kvm_get_pmu_events();
+	events_guest = pmu->events_guest;
+	events_host = pmu->events_host;
 
 	kvm_vcpu_pmu_enable_el0(events_guest);
 	kvm_vcpu_pmu_disable_el0(events_host);
@@ -190,15 +200,15 @@ void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
  */
 void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
 {
-	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
 	u32 events_guest, events_host;
 
 	if (!kvm_arm_support_pmu_v3() || !has_vhe())
 		return;
 
-	host = this_cpu_ptr_hyp_sym(kvm_host_data);
-	events_guest = host->pmu_events.events_guest;
-	events_host = host->pmu_events.events_host;
+	pmu = kvm_get_pmu_events();
+	events_guest = pmu->events_guest;
+	events_host = pmu->events_host;
 
 	kvm_vcpu_pmu_enable_el0(events_host);
 	kvm_vcpu_pmu_disable_el0(events_guest);
-- 
2.36.0.512.ge40c2bad7a-goog

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

[PATCH v3 1/4] KVM: arm64: Wrapper for getting pmu_events

[Fuad Tabba]

Eases migrating away from using hyp data and simplifies the code.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/pmu.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 03a6c1f4a09a..4bd38ff34221 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -25,21 +25,31 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
 	return (attr->exclude_host != attr->exclude_guest);
 }
 
+static struct kvm_pmu_events *kvm_get_pmu_events(void)
+{
+	struct kvm_host_data *ctx = this_cpu_ptr_hyp_sym(kvm_host_data);
+
+	if (!ctx)
+		return NULL;
+
+	return &ctx->pmu_events;
+}
+
 /*
  * 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_hyp_sym(kvm_host_data);
+	struct kvm_pmu_events *pmu = kvm_get_pmu_events();
 
-	if (!kvm_arm_support_pmu_v3() || !ctx || !kvm_pmu_switch_needed(attr))
+	if (!kvm_arm_support_pmu_v3() || !pmu || !kvm_pmu_switch_needed(attr))
 		return;
 
 	if (!attr->exclude_host)
-		ctx->pmu_events.events_host |= set;
+		pmu->events_host |= set;
 	if (!attr->exclude_guest)
-		ctx->pmu_events.events_guest |= set;
+		pmu->events_guest |= set;
 }
 
 /*
@@ -47,13 +57,13 @@ void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
  */
 void kvm_clr_pmu_events(u32 clr)
 {
-	struct kvm_host_data *ctx = this_cpu_ptr_hyp_sym(kvm_host_data);
+	struct kvm_pmu_events *pmu = kvm_get_pmu_events();
 
-	if (!kvm_arm_support_pmu_v3() || !ctx)
+	if (!kvm_arm_support_pmu_v3() || !pmu)
 		return;
 
-	ctx->pmu_events.events_host &= ~clr;
-	ctx->pmu_events.events_guest &= ~clr;
+	pmu->events_host &= ~clr;
+	pmu->events_guest &= ~clr;
 }
 
 #define PMEVTYPER_READ_CASE(idx)				\
@@ -169,16 +179,16 @@ static void kvm_vcpu_pmu_disable_el0(unsigned long events)
  */
 void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
 {
-	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
 	u32 events_guest, events_host;
 
 	if (!kvm_arm_support_pmu_v3() || !has_vhe())
 		return;
 
 	preempt_disable();
-	host = this_cpu_ptr_hyp_sym(kvm_host_data);
-	events_guest = host->pmu_events.events_guest;
-	events_host = host->pmu_events.events_host;
+	pmu = kvm_get_pmu_events();
+	events_guest = pmu->events_guest;
+	events_host = pmu->events_host;
 
 	kvm_vcpu_pmu_enable_el0(events_guest);
 	kvm_vcpu_pmu_disable_el0(events_host);
@@ -190,15 +200,15 @@ void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu)
  */
 void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
 {
-	struct kvm_host_data *host;
+	struct kvm_pmu_events *pmu;
 	u32 events_guest, events_host;
 
 	if (!kvm_arm_support_pmu_v3() || !has_vhe())
 		return;
 
-	host = this_cpu_ptr_hyp_sym(kvm_host_data);
-	events_guest = host->pmu_events.events_guest;
-	events_host = host->pmu_events.events_host;
+	pmu = kvm_get_pmu_events();
+	events_guest = pmu->events_guest;
+	events_host = pmu->events_host;
 
 	kvm_vcpu_pmu_enable_el0(events_host);
 	kvm_vcpu_pmu_disable_el0(events_guest);
-- 
2.36.0.512.ge40c2bad7a-goog


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

[PATCH v3 2/4] KVM: arm64: Repack struct kvm_pmu to reduce size

[Fuad Tabba]

struct kvm_pmu has 2 holes using 10 bytes. This is instantiated
in all vcpus, so it adds up. This repacking removes all holes.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 include/kvm/arm_pmu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 20193416d214..eaa8290b116f 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -21,12 +21,12 @@ struct kvm_pmc {
 };
 
 struct kvm_pmu {
-	int irq_num;
+	struct irq_work overflow_work;
 	struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
 	DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
+	int irq_num;
 	bool created;
 	bool irq_level;
-	struct irq_work overflow_work;
 };
 
 struct arm_pmu_entry {
-- 
2.36.0.512.ge40c2bad7a-goog

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

[PATCH v3 2/4] KVM: arm64: Repack struct kvm_pmu to reduce size

[Fuad Tabba]

struct kvm_pmu has 2 holes using 10 bytes. This is instantiated
in all vcpus, so it adds up. This repacking removes all holes.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 include/kvm/arm_pmu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 20193416d214..eaa8290b116f 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -21,12 +21,12 @@ struct kvm_pmc {
 };
 
 struct kvm_pmu {
-	int irq_num;
+	struct irq_work overflow_work;
 	struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
 	DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
+	int irq_num;
 	bool created;
 	bool irq_level;
-	struct irq_work overflow_work;
 };
 
 struct arm_pmu_entry {
-- 
2.36.0.512.ge40c2bad7a-goog


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

[PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Fuad Tabba]

Instead of the host accessing hyp data directly, pass the pmu
events of the current cpu to hyp via the vcpu.

This adds 64 bits (in two fields) to the vcpu that need to be
synced before every vcpu run in nvhe and protected modes.
However, it isolates the hypervisor from the host, which allows
us to use pmu in protected mode in a subsequent patch.

No visible side effects in behavior intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_host.h |  8 ++------
 arch/arm64/kvm/arm.c              | 15 +++++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
 arch/arm64/kvm/pmu.c              | 12 ++++--------
 include/kvm/arm_pmu.h             |  6 ++++++
 5 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index dfd360404dd8..90476e713643 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -273,14 +273,8 @@ 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;
 };
 
 struct kvm_host_psci_config {
@@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
 DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
+DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
 
 static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
 {
@@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
 void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
 void kvm_clr_pmu_events(u32 clr);
 
+struct kvm_pmu_events *kvm_get_pmu_events(void);
 void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
 void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
 #else
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 2adb5832a756..86bcdb2a23a8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu)
 	return ret;
 }
 
+/*
+ * Updates the vcpu's view of the pmu events for this cpu.
+ * Must be called before every vcpu run after disabling interrupts, to ensure
+ * that an interrupt cannot fire and update the structure.
+ */
+static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu)
+{
+	if (has_vhe() || !kvm_vcpu_has_pmu(vcpu))
+		return;
+
+	vcpu->arch.pmu.events = *kvm_get_pmu_events();
+}
+
 /**
  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
  * @vcpu:	The VCPU pointer
@@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		kvm_vgic_flush_hwstate(vcpu);
 
+		kvm_pmu_update_vcpu_events(vcpu);
+
 		/*
 		 * Ensure we set mode to IN_GUEST_MODE after we disable
 		 * interrupts and before the final VCPU requests check.
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 0716163313d6..c61120ec8d1a 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
 /*
  * Disable host events, enable guest events
  */
-static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
+static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu)
 {
-	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;
+	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
 
 	if (pmu->events_host)
 		write_sysreg(pmu->events_host, pmcntenclr_el0);
@@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
 /*
  * Disable guest events, enable host events
  */
-static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
+static void __pmu_switch_to_host(struct kvm_vcpu *vcpu)
 {
-	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;
+	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
 
 	if (pmu->events_guest)
 		write_sysreg(pmu->events_guest, pmcntenclr_el0);
@@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	host_ctxt->__hyp_running_vcpu = vcpu;
 	guest_ctxt = &vcpu->arch.ctxt;
 
-	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
+	pmu_switch_needed = __pmu_switch_to_guest(vcpu);
 
 	__sysreg_save_state_nvhe(host_ctxt);
 	/*
@@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	__debug_restore_host_buffers_nvhe(vcpu);
 
 	if (pmu_switch_needed)
-		__pmu_switch_to_host(host_ctxt);
+		__pmu_switch_to_host(vcpu);
 
 	/* Returning to host will clear PSR.I, remask PMR if needed */
 	if (system_uses_irq_prio_masking())
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 4bd38ff34221..c19bf6e4969e 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -5,7 +5,8 @@
  */
 #include <linux/kvm_host.h>
 #include <linux/perf_event.h>
-#include <asm/kvm_hyp.h>
+
+DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
 
 /*
  * Given the perf event attributes and system type, determine
@@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
 	return (attr->exclude_host != attr->exclude_guest);
 }
 
-static struct kvm_pmu_events *kvm_get_pmu_events(void)
+struct kvm_pmu_events *kvm_get_pmu_events(void)
 {
-	struct kvm_host_data *ctx = this_cpu_ptr_hyp_sym(kvm_host_data);
-
-	if (!ctx)
-		return NULL;
-
-	return &ctx->pmu_events;
+	return this_cpu_ptr(&kvm_pmu_events);
 }
 
 /*
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index eaa8290b116f..35a0903cae32 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -20,8 +20,14 @@ struct kvm_pmc {
 	struct perf_event *perf_event;
 };
 
+struct kvm_pmu_events {
+	u32 events_host;
+	u32 events_guest;
+};
+
 struct kvm_pmu {
 	struct irq_work overflow_work;
+	struct kvm_pmu_events events;
 	struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
 	DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
 	int irq_num;
-- 
2.36.0.512.ge40c2bad7a-goog

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

[PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Fuad Tabba]

Instead of the host accessing hyp data directly, pass the pmu
events of the current cpu to hyp via the vcpu.

This adds 64 bits (in two fields) to the vcpu that need to be
synced before every vcpu run in nvhe and protected modes.
However, it isolates the hypervisor from the host, which allows
us to use pmu in protected mode in a subsequent patch.

No visible side effects in behavior intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_host.h |  8 ++------
 arch/arm64/kvm/arm.c              | 15 +++++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
 arch/arm64/kvm/pmu.c              | 12 ++++--------
 include/kvm/arm_pmu.h             |  6 ++++++
 5 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index dfd360404dd8..90476e713643 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -273,14 +273,8 @@ 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;
 };
 
 struct kvm_host_psci_config {
@@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
 DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
+DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
 
 static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
 {
@@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
 void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
 void kvm_clr_pmu_events(u32 clr);
 
+struct kvm_pmu_events *kvm_get_pmu_events(void);
 void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
 void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
 #else
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 2adb5832a756..86bcdb2a23a8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu)
 	return ret;
 }
 
+/*
+ * Updates the vcpu's view of the pmu events for this cpu.
+ * Must be called before every vcpu run after disabling interrupts, to ensure
+ * that an interrupt cannot fire and update the structure.
+ */
+static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu)
+{
+	if (has_vhe() || !kvm_vcpu_has_pmu(vcpu))
+		return;
+
+	vcpu->arch.pmu.events = *kvm_get_pmu_events();
+}
+
 /**
  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
  * @vcpu:	The VCPU pointer
@@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		kvm_vgic_flush_hwstate(vcpu);
 
+		kvm_pmu_update_vcpu_events(vcpu);
+
 		/*
 		 * Ensure we set mode to IN_GUEST_MODE after we disable
 		 * interrupts and before the final VCPU requests check.
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 0716163313d6..c61120ec8d1a 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
 /*
  * Disable host events, enable guest events
  */
-static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
+static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu)
 {
-	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;
+	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
 
 	if (pmu->events_host)
 		write_sysreg(pmu->events_host, pmcntenclr_el0);
@@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
 /*
  * Disable guest events, enable host events
  */
-static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
+static void __pmu_switch_to_host(struct kvm_vcpu *vcpu)
 {
-	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;
+	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
 
 	if (pmu->events_guest)
 		write_sysreg(pmu->events_guest, pmcntenclr_el0);
@@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	host_ctxt->__hyp_running_vcpu = vcpu;
 	guest_ctxt = &vcpu->arch.ctxt;
 
-	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
+	pmu_switch_needed = __pmu_switch_to_guest(vcpu);
 
 	__sysreg_save_state_nvhe(host_ctxt);
 	/*
@@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	__debug_restore_host_buffers_nvhe(vcpu);
 
 	if (pmu_switch_needed)
-		__pmu_switch_to_host(host_ctxt);
+		__pmu_switch_to_host(vcpu);
 
 	/* Returning to host will clear PSR.I, remask PMR if needed */
 	if (system_uses_irq_prio_masking())
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 4bd38ff34221..c19bf6e4969e 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -5,7 +5,8 @@
  */
 #include <linux/kvm_host.h>
 #include <linux/perf_event.h>
-#include <asm/kvm_hyp.h>
+
+DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
 
 /*
  * Given the perf event attributes and system type, determine
@@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
 	return (attr->exclude_host != attr->exclude_guest);
 }
 
-static struct kvm_pmu_events *kvm_get_pmu_events(void)
+struct kvm_pmu_events *kvm_get_pmu_events(void)
 {
-	struct kvm_host_data *ctx = this_cpu_ptr_hyp_sym(kvm_host_data);
-
-	if (!ctx)
-		return NULL;
-
-	return &ctx->pmu_events;
+	return this_cpu_ptr(&kvm_pmu_events);
 }
 
 /*
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index eaa8290b116f..35a0903cae32 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -20,8 +20,14 @@ struct kvm_pmc {
 	struct perf_event *perf_event;
 };
 
+struct kvm_pmu_events {
+	u32 events_host;
+	u32 events_guest;
+};
+
 struct kvm_pmu {
 	struct irq_work overflow_work;
+	struct kvm_pmu_events events;
 	struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
 	DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
 	int irq_num;
-- 
2.36.0.512.ge40c2bad7a-goog


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

[PATCH v3 4/4] KVM: arm64: Reenable pmu in Protected Mode

[Fuad Tabba]

Now that the pmu code does not access hyp data, reenable it in
protected mode.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/pmu-emul.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 3dc990ac4f44..11c43bed5f97 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -774,8 +774,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
 {
 	struct arm_pmu_entry *entry;
 
-	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF ||
-	    is_protected_kvm_enabled())
+	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF)
 		return;
 
 	mutex_lock(&arm_pmus_lock);
-- 
2.36.0.512.ge40c2bad7a-goog

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

[PATCH v3 4/4] KVM: arm64: Reenable pmu in Protected Mode

[Fuad Tabba]

Now that the pmu code does not access hyp data, reenable it in
protected mode.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/pmu-emul.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 3dc990ac4f44..11c43bed5f97 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -774,8 +774,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
 {
 	struct arm_pmu_entry *entry;
 
-	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF ||
-	    is_protected_kvm_enabled())
+	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF)
 		return;
 
 	mutex_lock(&arm_pmus_lock);
-- 
2.36.0.512.ge40c2bad7a-goog


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

Re: [PATCH v3 1/4] KVM: arm64: Wrapper for getting pmu_events

[Oliver Upton]

On Mon, May 09, 2022 at 12:03:27PM +0000, Fuad Tabba wrote:
> Eases migrating away from using hyp data and simplifies the code.
> 
> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>

Reviewed-by: Oliver Upton <oupton@google.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 1/4] KVM: arm64: Wrapper for getting pmu_events

[Oliver Upton]

On Mon, May 09, 2022 at 12:03:27PM +0000, Fuad Tabba wrote:
> Eases migrating away from using hyp data and simplifies the code.
> 
> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>

Reviewed-by: Oliver Upton <oupton@google.com>

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

Re: [PATCH v3 2/4] KVM: arm64: Repack struct kvm_pmu to reduce size

[Oliver Upton]

On Mon, May 09, 2022 at 12:03:28PM +0000, Fuad Tabba wrote:
> struct kvm_pmu has 2 holes using 10 bytes. This is instantiated
> in all vcpus, so it adds up. This repacking removes all holes.

nit: commit messages should be written in the imperative. So maybe
'Repack the structure to remove all holes'.

> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>

Otherwise:

Reviewed-by: Oliver Upton <oupton@google.com>

> ---
>  include/kvm/arm_pmu.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 20193416d214..eaa8290b116f 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -21,12 +21,12 @@ struct kvm_pmc {
>  };
>  
>  struct kvm_pmu {
> -	int irq_num;
> +	struct irq_work overflow_work;
>  	struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
>  	DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> +	int irq_num;
>  	bool created;
>  	bool irq_level;
> -	struct irq_work overflow_work;
>  };
>  
>  struct arm_pmu_entry {
> -- 
> 2.36.0.512.ge40c2bad7a-goog
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 2/4] KVM: arm64: Repack struct kvm_pmu to reduce size

[Oliver Upton]

On Mon, May 09, 2022 at 12:03:28PM +0000, Fuad Tabba wrote:
> struct kvm_pmu has 2 holes using 10 bytes. This is instantiated
> in all vcpus, so it adds up. This repacking removes all holes.

nit: commit messages should be written in the imperative. So maybe
'Repack the structure to remove all holes'.

> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>

Otherwise:

Reviewed-by: Oliver Upton <oupton@google.com>

> ---
>  include/kvm/arm_pmu.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 20193416d214..eaa8290b116f 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -21,12 +21,12 @@ struct kvm_pmc {
>  };
>  
>  struct kvm_pmu {
> -	int irq_num;
> +	struct irq_work overflow_work;
>  	struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
>  	DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> +	int irq_num;
>  	bool created;
>  	bool irq_level;
> -	struct irq_work overflow_work;
>  };
>  
>  struct arm_pmu_entry {
> -- 
> 2.36.0.512.ge40c2bad7a-goog
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Oliver Upton]

On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote:
> Instead of the host accessing hyp data directly, pass the pmu
> events of the current cpu to hyp via the vcpu.
> 
> This adds 64 bits (in two fields) to the vcpu that need to be
> synced before every vcpu run in nvhe and protected modes.
> However, it isolates the hypervisor from the host, which allows
> us to use pmu in protected mode in a subsequent patch.
> 
> No visible side effects in behavior intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 ++------
>  arch/arm64/kvm/arm.c              | 15 +++++++++++++++
>  arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
>  arch/arm64/kvm/pmu.c              | 12 ++++--------
>  include/kvm/arm_pmu.h             |  6 ++++++
>  5 files changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index dfd360404dd8..90476e713643 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -273,14 +273,8 @@ struct kvm_cpu_context {
>  	struct kvm_vcpu *__hyp_running_vcpu;
>  };
>  
> -struct kvm_pmu_events {
> -	u32 events_host;
> -	u32 events_guest;
> -};
> -

Looks like you're moving this to arm_pmu.h as well. Probably a better
home for it, but unclear why it is done in this patch.

>  struct kvm_host_data {
>  	struct kvm_cpu_context host_ctxt;
> -	struct kvm_pmu_events pmu_events;
>  };
>  

Are we going to need this struct any more since it now has a single
member?

>  struct kvm_host_psci_config {
> @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
> +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);

Why do you need this declaration? I don't see the percpu data being
accessed outside of pmu.c.

>  static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
>  {
> @@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
>  void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
>  void kvm_clr_pmu_events(u32 clr);
>  
> +struct kvm_pmu_events *kvm_get_pmu_events(void);
>  void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>  #else
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 2adb5832a756..86bcdb2a23a8 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu)
>  	return ret;
>  }
>  
> +/*
> + * Updates the vcpu's view of the pmu events for this cpu.
> + * Must be called before every vcpu run after disabling interrupts, to ensure
> + * that an interrupt cannot fire and update the structure.
> + */
> +static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu)
> +{
> +	if (has_vhe() || !kvm_vcpu_has_pmu(vcpu))
> +		return;
> +
> +	vcpu->arch.pmu.events = *kvm_get_pmu_events();
> +}
> +
>  /**
>   * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
>   * @vcpu:	The VCPU pointer
> @@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  
>  		kvm_vgic_flush_hwstate(vcpu);
>  
> +		kvm_pmu_update_vcpu_events(vcpu);
> +
>  		/*
>  		 * Ensure we set mode to IN_GUEST_MODE after we disable
>  		 * interrupts and before the final VCPU requests check.
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 0716163313d6..c61120ec8d1a 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
>  /*
>   * Disable host events, enable guest events
>   */
> -static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
> +static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu)
>  {
> -	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;
> +	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
>  
>  	if (pmu->events_host)
>  		write_sysreg(pmu->events_host, pmcntenclr_el0);
> @@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
>  /*
>   * Disable guest events, enable host events
>   */
> -static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
> +static void __pmu_switch_to_host(struct kvm_vcpu *vcpu)
>  {
> -	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;
> +	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
>  
>  	if (pmu->events_guest)
>  		write_sysreg(pmu->events_guest, pmcntenclr_el0);
> @@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  	host_ctxt->__hyp_running_vcpu = vcpu;
>  	guest_ctxt = &vcpu->arch.ctxt;
>  
> -	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
> +	pmu_switch_needed = __pmu_switch_to_guest(vcpu);
>  
>  	__sysreg_save_state_nvhe(host_ctxt);
>  	/*
> @@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  	__debug_restore_host_buffers_nvhe(vcpu);
>  
>  	if (pmu_switch_needed)
> -		__pmu_switch_to_host(host_ctxt);
> +		__pmu_switch_to_host(vcpu);
>  
>  	/* Returning to host will clear PSR.I, remask PMR if needed */
>  	if (system_uses_irq_prio_masking())
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index 4bd38ff34221..c19bf6e4969e 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
> @@ -5,7 +5,8 @@
>   */
>  #include <linux/kvm_host.h>
>  #include <linux/perf_event.h>
> -#include <asm/kvm_hyp.h>
> +
> +DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
>  
>  /*
>   * Given the perf event attributes and system type, determine
> @@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
>  	return (attr->exclude_host != attr->exclude_guest);
>  }
>  
> -static struct kvm_pmu_events *kvm_get_pmu_events(void)
> +struct kvm_pmu_events *kvm_get_pmu_events(void)

Why not make this function visible in patch 1? It seems benign even
though there are no other users at that moment outside of the
compilation unit.

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Oliver Upton]

On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote:
> Instead of the host accessing hyp data directly, pass the pmu
> events of the current cpu to hyp via the vcpu.
> 
> This adds 64 bits (in two fields) to the vcpu that need to be
> synced before every vcpu run in nvhe and protected modes.
> However, it isolates the hypervisor from the host, which allows
> us to use pmu in protected mode in a subsequent patch.
> 
> No visible side effects in behavior intended.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 ++------
>  arch/arm64/kvm/arm.c              | 15 +++++++++++++++
>  arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
>  arch/arm64/kvm/pmu.c              | 12 ++++--------
>  include/kvm/arm_pmu.h             |  6 ++++++
>  5 files changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index dfd360404dd8..90476e713643 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -273,14 +273,8 @@ struct kvm_cpu_context {
>  	struct kvm_vcpu *__hyp_running_vcpu;
>  };
>  
> -struct kvm_pmu_events {
> -	u32 events_host;
> -	u32 events_guest;
> -};
> -

Looks like you're moving this to arm_pmu.h as well. Probably a better
home for it, but unclear why it is done in this patch.

>  struct kvm_host_data {
>  	struct kvm_cpu_context host_ctxt;
> -	struct kvm_pmu_events pmu_events;
>  };
>  

Are we going to need this struct any more since it now has a single
member?

>  struct kvm_host_psci_config {
> @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
> +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);

Why do you need this declaration? I don't see the percpu data being
accessed outside of pmu.c.

>  static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
>  {
> @@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
>  void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
>  void kvm_clr_pmu_events(u32 clr);
>  
> +struct kvm_pmu_events *kvm_get_pmu_events(void);
>  void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>  #else
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 2adb5832a756..86bcdb2a23a8 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu)
>  	return ret;
>  }
>  
> +/*
> + * Updates the vcpu's view of the pmu events for this cpu.
> + * Must be called before every vcpu run after disabling interrupts, to ensure
> + * that an interrupt cannot fire and update the structure.
> + */
> +static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu)
> +{
> +	if (has_vhe() || !kvm_vcpu_has_pmu(vcpu))
> +		return;
> +
> +	vcpu->arch.pmu.events = *kvm_get_pmu_events();
> +}
> +
>  /**
>   * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
>   * @vcpu:	The VCPU pointer
> @@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  
>  		kvm_vgic_flush_hwstate(vcpu);
>  
> +		kvm_pmu_update_vcpu_events(vcpu);
> +
>  		/*
>  		 * Ensure we set mode to IN_GUEST_MODE after we disable
>  		 * interrupts and before the final VCPU requests check.
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 0716163313d6..c61120ec8d1a 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
>  /*
>   * Disable host events, enable guest events
>   */
> -static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
> +static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu)
>  {
> -	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;
> +	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
>  
>  	if (pmu->events_host)
>  		write_sysreg(pmu->events_host, pmcntenclr_el0);
> @@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
>  /*
>   * Disable guest events, enable host events
>   */
> -static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
> +static void __pmu_switch_to_host(struct kvm_vcpu *vcpu)
>  {
> -	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;
> +	struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
>  
>  	if (pmu->events_guest)
>  		write_sysreg(pmu->events_guest, pmcntenclr_el0);
> @@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  	host_ctxt->__hyp_running_vcpu = vcpu;
>  	guest_ctxt = &vcpu->arch.ctxt;
>  
> -	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
> +	pmu_switch_needed = __pmu_switch_to_guest(vcpu);
>  
>  	__sysreg_save_state_nvhe(host_ctxt);
>  	/*
> @@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  	__debug_restore_host_buffers_nvhe(vcpu);
>  
>  	if (pmu_switch_needed)
> -		__pmu_switch_to_host(host_ctxt);
> +		__pmu_switch_to_host(vcpu);
>  
>  	/* Returning to host will clear PSR.I, remask PMR if needed */
>  	if (system_uses_irq_prio_masking())
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index 4bd38ff34221..c19bf6e4969e 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
> @@ -5,7 +5,8 @@
>   */
>  #include <linux/kvm_host.h>
>  #include <linux/perf_event.h>
> -#include <asm/kvm_hyp.h>
> +
> +DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
>  
>  /*
>   * Given the perf event attributes and system type, determine
> @@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
>  	return (attr->exclude_host != attr->exclude_guest);
>  }
>  
> -static struct kvm_pmu_events *kvm_get_pmu_events(void)
> +struct kvm_pmu_events *kvm_get_pmu_events(void)

Why not make this function visible in patch 1? It seems benign even
though there are no other users at that moment outside of the
compilation unit.

--
Thanks,
Oliver

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

Re: [PATCH v3 4/4] KVM: arm64: Reenable pmu in Protected Mode

[Oliver Upton]

Hi Fuad,

On Mon, May 09, 2022 at 12:03:30PM +0000, Fuad Tabba wrote:
> Now that the pmu code does not access hyp data, reenable it in
> protected mode.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/kvm/pmu-emul.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 3dc990ac4f44..11c43bed5f97 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -774,8 +774,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
>  {
>  	struct arm_pmu_entry *entry;
>  
> -	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF ||
> -	    is_protected_kvm_enabled())
> +	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF)

IIUC, you intend to enable the PMU for non-pVM guests, right? Based on
the pKVM feature registers + trap handlers I can see that pVM use of the
PMU is in fact disabled. It'd be good to state in the commit message
what is exactly intended for pVMs and non-pVMs.

Since the PMU counters are disabled outright for pVMs, is there any
value in context switching between host/guest in this case?

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 4/4] KVM: arm64: Reenable pmu in Protected Mode

[Oliver Upton]

Hi Fuad,

On Mon, May 09, 2022 at 12:03:30PM +0000, Fuad Tabba wrote:
> Now that the pmu code does not access hyp data, reenable it in
> protected mode.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/kvm/pmu-emul.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 3dc990ac4f44..11c43bed5f97 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -774,8 +774,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
>  {
>  	struct arm_pmu_entry *entry;
>  
> -	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF ||
> -	    is_protected_kvm_enabled())
> +	if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF)

IIUC, you intend to enable the PMU for non-pVM guests, right? Based on
the pKVM feature registers + trap handlers I can see that pVM use of the
PMU is in fact disabled. It'd be good to state in the commit message
what is exactly intended for pVMs and non-pVMs.

Since the PMU counters are disabled outright for pVMs, is there any
value in context switching between host/guest in this case?

--
Thanks,
Oliver

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

Re: [PATCH v3 2/4] KVM: arm64: Repack struct kvm_pmu to reduce size

[Fuad Tabba]

Hi Oliver,

Thanks for your reviews.

On Mon, May 9, 2022 at 8:57 PM Oliver Upton <oupton@google.com> wrote:
>
> On Mon, May 09, 2022 at 12:03:28PM +0000, Fuad Tabba wrote:
> > struct kvm_pmu has 2 holes using 10 bytes. This is instantiated
> > in all vcpus, so it adds up. This repacking removes all holes.
>
> nit: commit messages should be written in the imperative. So maybe
> 'Repack the structure to remove all holes'.

Will fix that when I respin this.

Cheers,
/fuad

>
> > No functional change intended.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
>
> Otherwise:
>
> Reviewed-by: Oliver Upton <oupton@google.com>
>
> > ---
> >  include/kvm/arm_pmu.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> > index 20193416d214..eaa8290b116f 100644
> > --- a/include/kvm/arm_pmu.h
> > +++ b/include/kvm/arm_pmu.h
> > @@ -21,12 +21,12 @@ struct kvm_pmc {
> >  };
> >
> >  struct kvm_pmu {
> > -     int irq_num;
> > +     struct irq_work overflow_work;
> >       struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
> >       DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> > +     int irq_num;
> >       bool created;
> >       bool irq_level;
> > -     struct irq_work overflow_work;
> >  };
> >
> >  struct arm_pmu_entry {
> > --
> > 2.36.0.512.ge40c2bad7a-goog
> >
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm@lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 2/4] KVM: arm64: Repack struct kvm_pmu to reduce size

[Fuad Tabba]

Hi Oliver,

Thanks for your reviews.

On Mon, May 9, 2022 at 8:57 PM Oliver Upton <oupton@google.com> wrote:
>
> On Mon, May 09, 2022 at 12:03:28PM +0000, Fuad Tabba wrote:
> > struct kvm_pmu has 2 holes using 10 bytes. This is instantiated
> > in all vcpus, so it adds up. This repacking removes all holes.
>
> nit: commit messages should be written in the imperative. So maybe
> 'Repack the structure to remove all holes'.

Will fix that when I respin this.

Cheers,
/fuad

>
> > No functional change intended.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
>
> Otherwise:
>
> Reviewed-by: Oliver Upton <oupton@google.com>
>
> > ---
> >  include/kvm/arm_pmu.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> > index 20193416d214..eaa8290b116f 100644
> > --- a/include/kvm/arm_pmu.h
> > +++ b/include/kvm/arm_pmu.h
> > @@ -21,12 +21,12 @@ struct kvm_pmc {
> >  };
> >
> >  struct kvm_pmu {
> > -     int irq_num;
> > +     struct irq_work overflow_work;
> >       struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
> >       DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> > +     int irq_num;
> >       bool created;
> >       bool irq_level;
> > -     struct irq_work overflow_work;
> >  };
> >
> >  struct arm_pmu_entry {
> > --
> > 2.36.0.512.ge40c2bad7a-goog
> >
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm@lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Fuad Tabba]

Hi Oliver,


On Mon, May 9, 2022 at 9:21 PM Oliver Upton <oupton@google.com> wrote:
>
> On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote:
> > Instead of the host accessing hyp data directly, pass the pmu
> > events of the current cpu to hyp via the vcpu.
> >
> > This adds 64 bits (in two fields) to the vcpu that need to be
> > synced before every vcpu run in nvhe and protected modes.
> > However, it isolates the hypervisor from the host, which allows
> > us to use pmu in protected mode in a subsequent patch.
> >
> > No visible side effects in behavior intended.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_host.h |  8 ++------
> >  arch/arm64/kvm/arm.c              | 15 +++++++++++++++
> >  arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
> >  arch/arm64/kvm/pmu.c              | 12 ++++--------
> >  include/kvm/arm_pmu.h             |  6 ++++++
> >  5 files changed, 33 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index dfd360404dd8..90476e713643 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -273,14 +273,8 @@ struct kvm_cpu_context {
> >       struct kvm_vcpu *__hyp_running_vcpu;
> >  };
> >
> > -struct kvm_pmu_events {
> > -     u32 events_host;
> > -     u32 events_guest;
> > -};
> > -
>
> Looks like you're moving this to arm_pmu.h as well. Probably a better
> home for it, but unclear why it is done in this patch.

Like you said, I thought it was a better home, and it's not needed
here anymore. I could maybe move it to the repacking patch and make it
as a general "cleanup" patch, if you think that would be clearer.
>
> >  struct kvm_host_data {
> >       struct kvm_cpu_context host_ctxt;
> > -     struct kvm_pmu_events pmu_events;
> >  };
> >
>
> Are we going to need this struct any more since it now has a single
> member?

I thought about removing it, but it would cause a bit of code churn.
That said, I could remove it in a new patch that I have as the last
one, and leave it to the maintainer to decide whether to take it.

>
> >  struct kvm_host_psci_config {
> > @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> >  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> >
> >  DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
> > +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
>
> Why do you need this declaration? I don't see the percpu data being
> accessed outside of pmu.c.

You're right. At one previous iteration I did need it, which is why
it's here, but not anymore. Will remove it.

> >  static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
> >  {
> > @@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
> >  void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
> >  void kvm_clr_pmu_events(u32 clr);
> >
> > +struct kvm_pmu_events *kvm_get_pmu_events(void);
> >  void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
> >  void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
> >  #else
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 2adb5832a756..86bcdb2a23a8 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu)
> >       return ret;
> >  }
> >
> > +/*
> > + * Updates the vcpu's view of the pmu events for this cpu.
> > + * Must be called before every vcpu run after disabling interrupts, to ensure
> > + * that an interrupt cannot fire and update the structure.
> > + */
> > +static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu)
> > +{
> > +     if (has_vhe() || !kvm_vcpu_has_pmu(vcpu))
> > +             return;
> > +
> > +     vcpu->arch.pmu.events = *kvm_get_pmu_events();
> > +}
> > +
> >  /**
> >   * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
> >   * @vcpu:    The VCPU pointer
> > @@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> >
> >               kvm_vgic_flush_hwstate(vcpu);
> >
> > +             kvm_pmu_update_vcpu_events(vcpu);
> > +
> >               /*
> >                * Ensure we set mode to IN_GUEST_MODE after we disable
> >                * interrupts and before the final VCPU requests check.
> > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> > index 0716163313d6..c61120ec8d1a 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> > @@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
> >  /*
> >   * Disable host events, enable guest events
> >   */
> > -static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
> > +static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu)
> >  {
> > -     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;
> > +     struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
> >
> >       if (pmu->events_host)
> >               write_sysreg(pmu->events_host, pmcntenclr_el0);
> > @@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
> >  /*
> >   * Disable guest events, enable host events
> >   */
> > -static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
> > +static void __pmu_switch_to_host(struct kvm_vcpu *vcpu)
> >  {
> > -     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;
> > +     struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
> >
> >       if (pmu->events_guest)
> >               write_sysreg(pmu->events_guest, pmcntenclr_el0);
> > @@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> >       host_ctxt->__hyp_running_vcpu = vcpu;
> >       guest_ctxt = &vcpu->arch.ctxt;
> >
> > -     pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
> > +     pmu_switch_needed = __pmu_switch_to_guest(vcpu);
> >
> >       __sysreg_save_state_nvhe(host_ctxt);
> >       /*
> > @@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> >       __debug_restore_host_buffers_nvhe(vcpu);
> >
> >       if (pmu_switch_needed)
> > -             __pmu_switch_to_host(host_ctxt);
> > +             __pmu_switch_to_host(vcpu);
> >
> >       /* Returning to host will clear PSR.I, remask PMR if needed */
> >       if (system_uses_irq_prio_masking())
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > index 4bd38ff34221..c19bf6e4969e 100644
> > --- a/arch/arm64/kvm/pmu.c
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -5,7 +5,8 @@
> >   */
> >  #include <linux/kvm_host.h>
> >  #include <linux/perf_event.h>
> > -#include <asm/kvm_hyp.h>
> > +
> > +DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
> >
> >  /*
> >   * Given the perf event attributes and system type, determine
> > @@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
> >       return (attr->exclude_host != attr->exclude_guest);
> >  }
> >
> > -static struct kvm_pmu_events *kvm_get_pmu_events(void)
> > +struct kvm_pmu_events *kvm_get_pmu_events(void)
>
> Why not make this function visible in patch 1? It seems benign even
> though there are no other users at that moment outside of the
> compilation unit.

Because I wanted every patch to be self-standing, but no preference
really. I can change that to reduce code churn.

Cheers,
/fuad

>
> --
> Thanks,
> Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Fuad Tabba]

Hi Oliver,


On Mon, May 9, 2022 at 9:21 PM Oliver Upton <oupton@google.com> wrote:
>
> On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote:
> > Instead of the host accessing hyp data directly, pass the pmu
> > events of the current cpu to hyp via the vcpu.
> >
> > This adds 64 bits (in two fields) to the vcpu that need to be
> > synced before every vcpu run in nvhe and protected modes.
> > However, it isolates the hypervisor from the host, which allows
> > us to use pmu in protected mode in a subsequent patch.
> >
> > No visible side effects in behavior intended.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_host.h |  8 ++------
> >  arch/arm64/kvm/arm.c              | 15 +++++++++++++++
> >  arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
> >  arch/arm64/kvm/pmu.c              | 12 ++++--------
> >  include/kvm/arm_pmu.h             |  6 ++++++
> >  5 files changed, 33 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index dfd360404dd8..90476e713643 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -273,14 +273,8 @@ struct kvm_cpu_context {
> >       struct kvm_vcpu *__hyp_running_vcpu;
> >  };
> >
> > -struct kvm_pmu_events {
> > -     u32 events_host;
> > -     u32 events_guest;
> > -};
> > -
>
> Looks like you're moving this to arm_pmu.h as well. Probably a better
> home for it, but unclear why it is done in this patch.

Like you said, I thought it was a better home, and it's not needed
here anymore. I could maybe move it to the repacking patch and make it
as a general "cleanup" patch, if you think that would be clearer.
>
> >  struct kvm_host_data {
> >       struct kvm_cpu_context host_ctxt;
> > -     struct kvm_pmu_events pmu_events;
> >  };
> >
>
> Are we going to need this struct any more since it now has a single
> member?

I thought about removing it, but it would cause a bit of code churn.
That said, I could remove it in a new patch that I have as the last
one, and leave it to the maintainer to decide whether to take it.

>
> >  struct kvm_host_psci_config {
> > @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> >  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> >
> >  DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
> > +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
>
> Why do you need this declaration? I don't see the percpu data being
> accessed outside of pmu.c.

You're right. At one previous iteration I did need it, which is why
it's here, but not anymore. Will remove it.

> >  static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
> >  {
> > @@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
> >  void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
> >  void kvm_clr_pmu_events(u32 clr);
> >
> > +struct kvm_pmu_events *kvm_get_pmu_events(void);
> >  void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
> >  void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
> >  #else
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 2adb5832a756..86bcdb2a23a8 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu)
> >       return ret;
> >  }
> >
> > +/*
> > + * Updates the vcpu's view of the pmu events for this cpu.
> > + * Must be called before every vcpu run after disabling interrupts, to ensure
> > + * that an interrupt cannot fire and update the structure.
> > + */
> > +static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu)
> > +{
> > +     if (has_vhe() || !kvm_vcpu_has_pmu(vcpu))
> > +             return;
> > +
> > +     vcpu->arch.pmu.events = *kvm_get_pmu_events();
> > +}
> > +
> >  /**
> >   * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
> >   * @vcpu:    The VCPU pointer
> > @@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> >
> >               kvm_vgic_flush_hwstate(vcpu);
> >
> > +             kvm_pmu_update_vcpu_events(vcpu);
> > +
> >               /*
> >                * Ensure we set mode to IN_GUEST_MODE after we disable
> >                * interrupts and before the final VCPU requests check.
> > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> > index 0716163313d6..c61120ec8d1a 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> > @@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu)
> >  /*
> >   * Disable host events, enable guest events
> >   */
> > -static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
> > +static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu)
> >  {
> > -     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;
> > +     struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
> >
> >       if (pmu->events_host)
> >               write_sysreg(pmu->events_host, pmcntenclr_el0);
> > @@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
> >  /*
> >   * Disable guest events, enable host events
> >   */
> > -static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
> > +static void __pmu_switch_to_host(struct kvm_vcpu *vcpu)
> >  {
> > -     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;
> > +     struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events;
> >
> >       if (pmu->events_guest)
> >               write_sysreg(pmu->events_guest, pmcntenclr_el0);
> > @@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> >       host_ctxt->__hyp_running_vcpu = vcpu;
> >       guest_ctxt = &vcpu->arch.ctxt;
> >
> > -     pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
> > +     pmu_switch_needed = __pmu_switch_to_guest(vcpu);
> >
> >       __sysreg_save_state_nvhe(host_ctxt);
> >       /*
> > @@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> >       __debug_restore_host_buffers_nvhe(vcpu);
> >
> >       if (pmu_switch_needed)
> > -             __pmu_switch_to_host(host_ctxt);
> > +             __pmu_switch_to_host(vcpu);
> >
> >       /* Returning to host will clear PSR.I, remask PMR if needed */
> >       if (system_uses_irq_prio_masking())
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > index 4bd38ff34221..c19bf6e4969e 100644
> > --- a/arch/arm64/kvm/pmu.c
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -5,7 +5,8 @@
> >   */
> >  #include <linux/kvm_host.h>
> >  #include <linux/perf_event.h>
> > -#include <asm/kvm_hyp.h>
> > +
> > +DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
> >
> >  /*
> >   * Given the perf event attributes and system type, determine
> > @@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
> >       return (attr->exclude_host != attr->exclude_guest);
> >  }
> >
> > -static struct kvm_pmu_events *kvm_get_pmu_events(void)
> > +struct kvm_pmu_events *kvm_get_pmu_events(void)
>
> Why not make this function visible in patch 1? It seems benign even
> though there are no other users at that moment outside of the
> compilation unit.

Because I wanted every patch to be self-standing, but no preference
really. I can change that to reduce code churn.

Cheers,
/fuad

>
> --
> Thanks,
> Oliver

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

Re: [PATCH v3 4/4] KVM: arm64: Reenable pmu in Protected Mode

[Fuad Tabba]

Hi Oliver,


On Mon, May 9, 2022 at 11:36 PM Oliver Upton <oupton@google.com> wrote:
>
> Hi Fuad,
>
> On Mon, May 09, 2022 at 12:03:30PM +0000, Fuad Tabba wrote:
> > Now that the pmu code does not access hyp data, reenable it in
> > protected mode.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/kvm/pmu-emul.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> > index 3dc990ac4f44..11c43bed5f97 100644
> > --- a/arch/arm64/kvm/pmu-emul.c
> > +++ b/arch/arm64/kvm/pmu-emul.c
> > @@ -774,8 +774,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
> >  {
> >       struct arm_pmu_entry *entry;
> >
> > -     if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF ||
> > -         is_protected_kvm_enabled())
> > +     if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF)
>
> IIUC, you intend to enable the PMU for non-pVM guests, right? Based on
> the pKVM feature registers + trap handlers I can see that pVM use of the
> PMU is in fact disabled. It'd be good to state in the commit message
> what is exactly intended for pVMs and non-pVMs.
>
> Since the PMU counters are disabled outright for pVMs, is there any
> value in context switching between host/guest in this case?

I will make this clearer in the commit message. PMU won't be supported
for protected VMs but should be for non-protected VMs in protected
mode.

Thanks,
/fuad

> --
> Thanks,
> Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 4/4] KVM: arm64: Reenable pmu in Protected Mode

[Fuad Tabba]

Hi Oliver,


On Mon, May 9, 2022 at 11:36 PM Oliver Upton <oupton@google.com> wrote:
>
> Hi Fuad,
>
> On Mon, May 09, 2022 at 12:03:30PM +0000, Fuad Tabba wrote:
> > Now that the pmu code does not access hyp data, reenable it in
> > protected mode.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/kvm/pmu-emul.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> > index 3dc990ac4f44..11c43bed5f97 100644
> > --- a/arch/arm64/kvm/pmu-emul.c
> > +++ b/arch/arm64/kvm/pmu-emul.c
> > @@ -774,8 +774,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
> >  {
> >       struct arm_pmu_entry *entry;
> >
> > -     if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF ||
> > -         is_protected_kvm_enabled())
> > +     if (pmu->pmuver == 0 || pmu->pmuver == ID_AA64DFR0_PMUVER_IMP_DEF)
>
> IIUC, you intend to enable the PMU for non-pVM guests, right? Based on
> the pKVM feature registers + trap handlers I can see that pVM use of the
> PMU is in fact disabled. It'd be good to state in the commit message
> what is exactly intended for pVMs and non-pVMs.
>
> Since the PMU counters are disabled outright for pVMs, is there any
> value in context switching between host/guest in this case?

I will make this clearer in the commit message. PMU won't be supported
for protected VMs but should be for non-protected VMs in protected
mode.

Thanks,
/fuad

> --
> Thanks,
> Oliver

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

Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Marc Zyngier]

On Tue, 10 May 2022 07:43:45 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> Hi Oliver,
> 
> 
> On Mon, May 9, 2022 at 9:21 PM Oliver Upton <oupton@google.com> wrote:
> >
> > On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote:
> > > Instead of the host accessing hyp data directly, pass the pmu
> > > events of the current cpu to hyp via the vcpu.
> > >
> > > This adds 64 bits (in two fields) to the vcpu that need to be
> > > synced before every vcpu run in nvhe and protected modes.
> > > However, it isolates the hypervisor from the host, which allows
> > > us to use pmu in protected mode in a subsequent patch.
> > >
> > > No visible side effects in behavior intended.
> > >
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > >  arch/arm64/include/asm/kvm_host.h |  8 ++------
> > >  arch/arm64/kvm/arm.c              | 15 +++++++++++++++
> > >  arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
> > >  arch/arm64/kvm/pmu.c              | 12 ++++--------
> > >  include/kvm/arm_pmu.h             |  6 ++++++
> > >  5 files changed, 33 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > index dfd360404dd8..90476e713643 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -273,14 +273,8 @@ struct kvm_cpu_context {
> > >       struct kvm_vcpu *__hyp_running_vcpu;
> > >  };
> > >
> > > -struct kvm_pmu_events {
> > > -     u32 events_host;
> > > -     u32 events_guest;
> > > -};
> > > -
> >
> > Looks like you're moving this to arm_pmu.h as well. Probably a better
> > home for it, but unclear why it is done in this patch.
> 
> Like you said, I thought it was a better home, and it's not needed
> here anymore. I could maybe move it to the repacking patch and make it
> as a general "cleanup" patch, if you think that would be clearer.
> >
> > >  struct kvm_host_data {
> > >       struct kvm_cpu_context host_ctxt;
> > > -     struct kvm_pmu_events pmu_events;
> > >  };
> > >
> >
> > Are we going to need this struct any more since it now has a single
> > member?
> 
> I thought about removing it, but it would cause a bit of code churn.
> That said, I could remove it in a new patch that I have as the last
> one, and leave it to the maintainer to decide whether to take it.

Unless there is a compelling reason for getting rid of kvm_host_data,
I'd keep it. Because next thing you know, we'll need another bit in
there, and we'll reintroduce it. This costs us nothing, so let's not
worry about it.

> 
> >
> > >  struct kvm_host_psci_config {
> > > @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> > >  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> > >
> > >  DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
> > > +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
> >
> > Why do you need this declaration? I don't see the percpu data being
> > accessed outside of pmu.c.
> 
> You're right. At one previous iteration I did need it, which is why
> it's here, but not anymore. Will remove it.

And then make the definition static?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu

[Marc Zyngier]

On Tue, 10 May 2022 07:43:45 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> Hi Oliver,
> 
> 
> On Mon, May 9, 2022 at 9:21 PM Oliver Upton <oupton@google.com> wrote:
> >
> > On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote:
> > > Instead of the host accessing hyp data directly, pass the pmu
> > > events of the current cpu to hyp via the vcpu.
> > >
> > > This adds 64 bits (in two fields) to the vcpu that need to be
> > > synced before every vcpu run in nvhe and protected modes.
> > > However, it isolates the hypervisor from the host, which allows
> > > us to use pmu in protected mode in a subsequent patch.
> > >
> > > No visible side effects in behavior intended.
> > >
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > >  arch/arm64/include/asm/kvm_host.h |  8 ++------
> > >  arch/arm64/kvm/arm.c              | 15 +++++++++++++++
> > >  arch/arm64/kvm/hyp/nvhe/switch.c  | 20 ++++++--------------
> > >  arch/arm64/kvm/pmu.c              | 12 ++++--------
> > >  include/kvm/arm_pmu.h             |  6 ++++++
> > >  5 files changed, 33 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > index dfd360404dd8..90476e713643 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -273,14 +273,8 @@ struct kvm_cpu_context {
> > >       struct kvm_vcpu *__hyp_running_vcpu;
> > >  };
> > >
> > > -struct kvm_pmu_events {
> > > -     u32 events_host;
> > > -     u32 events_guest;
> > > -};
> > > -
> >
> > Looks like you're moving this to arm_pmu.h as well. Probably a better
> > home for it, but unclear why it is done in this patch.
> 
> Like you said, I thought it was a better home, and it's not needed
> here anymore. I could maybe move it to the repacking patch and make it
> as a general "cleanup" patch, if you think that would be clearer.
> >
> > >  struct kvm_host_data {
> > >       struct kvm_cpu_context host_ctxt;
> > > -     struct kvm_pmu_events pmu_events;
> > >  };
> > >
> >
> > Are we going to need this struct any more since it now has a single
> > member?
> 
> I thought about removing it, but it would cause a bit of code churn.
> That said, I could remove it in a new patch that I have as the last
> one, and leave it to the maintainer to decide whether to take it.

Unless there is a compelling reason for getting rid of kvm_host_data,
I'd keep it. Because next thing you know, we'll need another bit in
there, and we'll reintroduce it. This costs us nothing, so let's not
worry about it.

> 
> >
> > >  struct kvm_host_psci_config {
> > > @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> > >  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> > >
> > >  DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
> > > +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events);
> >
> > Why do you need this declaration? I don't see the percpu data being
> > accessed outside of pmu.c.
> 
> You're right. At one previous iteration I did need it, which is why
> it's here, but not anymore. Will remove it.

And then make the definition static?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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