[PATCH 0/2] KVM: arm64: Prevent spurious PMU accesses when no

[PATCH 0/2] KVM: arm64: Prevent spurious PMU accesses when no

[Marc Zyngier]

Yet another PMU bug that is only likely to hit under Nested Virt: we
unconditionally access PMU registers without checking whether it
actually is present.

Given that we already have a predicate for this, promote it to a
static key, and use that in the world switch.

Thanks to Andre for the heads up!

Marc Zyngier (2):
  KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key
  KVM: arm64: Don't access PMSELR_EL0/PMUSERENR_EL0 when no PMU is
    available

 arch/arm64/kernel/image-vars.h          |  3 +++
 arch/arm64/kvm/hyp/include/hyp/switch.h |  9 ++++++---
 arch/arm64/kvm/perf.c                   | 10 ++++++++++
 arch/arm64/kvm/pmu-emul.c               | 10 ----------
 include/kvm/arm_pmu.h                   |  9 +++++++--
 5 files changed, 26 insertions(+), 15 deletions(-)

-- 
2.29.2

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

[PATCH 0/2] KVM: arm64: Prevent spurious PMU accesses when no

[Marc Zyngier]

Yet another PMU bug that is only likely to hit under Nested Virt: we
unconditionally access PMU registers without checking whether it
actually is present.

Given that we already have a predicate for this, promote it to a
static key, and use that in the world switch.

Thanks to Andre for the heads up!

Marc Zyngier (2):
  KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key
  KVM: arm64: Don't access PMSELR_EL0/PMUSERENR_EL0 when no PMU is
    available

 arch/arm64/kernel/image-vars.h          |  3 +++
 arch/arm64/kvm/hyp/include/hyp/switch.h |  9 ++++++---
 arch/arm64/kvm/perf.c                   | 10 ++++++++++
 arch/arm64/kvm/pmu-emul.c               | 10 ----------
 include/kvm/arm_pmu.h                   |  9 +++++++--
 5 files changed, 26 insertions(+), 15 deletions(-)

-- 
2.29.2


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

[PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Marc Zyngier]

We currently find out about the presence of a HW PMU (or the handling
of that PMU by perf, which amounts to the same thing) in a fairly
roundabout way, by checking the number of counters available to perf.
That's good enough for now, but we will soon need to find about about
that on paths where perf is out of reach (in the world switch).

Instead, let's turn kvm_arm_support_pmu_v3() into a static key.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/perf.c     | 10 ++++++++++
 arch/arm64/kvm/pmu-emul.c | 10 ----------
 include/kvm/arm_pmu.h     |  9 +++++++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index d45b8b9a4415..198fa4266b2d 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -11,6 +11,8 @@
 
 #include <asm/kvm_emulate.h>
 
+DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+
 static int kvm_is_in_guest(void)
 {
         return kvm_get_running_vcpu() != NULL;
@@ -48,6 +50,14 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
 
 int kvm_perf_init(void)
 {
+	/*
+	 * Check if HW_PERF_EVENTS are supported by checking the number of
+	 * hardware performance counters. This could ensure the presence of
+	 * a physical PMU and CONFIG_PERF_EVENT is selected.
+	 */
+	if (perf_num_counters() > 0)
+		static_branch_enable(&kvm_arm_pmu_available);
+
 	return perf_register_guest_info_callbacks(&kvm_guest_cbs);
 }
 
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 4ad66a532e38..44d500706ab9 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -813,16 +813,6 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
 	return val & mask;
 }
 
-bool kvm_arm_support_pmu_v3(void)
-{
-	/*
-	 * Check if HW_PERF_EVENTS are supported by checking the number of
-	 * hardware performance counters. This could ensure the presence of
-	 * a physical PMU and CONFIG_PERF_EVENT is selected.
-	 */
-	return (perf_num_counters() > 0);
-}
-
 int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
 {
 	if (!kvm_vcpu_has_pmu(vcpu))
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 8dcb3e1477bc..6fd3cda608e4 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -13,6 +13,13 @@
 #define ARMV8_PMU_CYCLE_IDX		(ARMV8_PMU_MAX_COUNTERS - 1)
 #define ARMV8_PMU_MAX_COUNTER_PAIRS	((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
 
+DECLARE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+
+static __always_inline bool kvm_arm_support_pmu_v3(void)
+{
+	return static_branch_likely(&kvm_arm_pmu_available);
+}
+
 #ifdef CONFIG_HW_PERF_EVENTS
 
 struct kvm_pmc {
@@ -47,7 +54,6 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
 void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
 void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
 				    u64 select_idx);
-bool kvm_arm_support_pmu_v3(void);
 int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
 			    struct kvm_device_attr *attr);
 int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu,
@@ -87,7 +93,6 @@ static inline void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
 static inline void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val) {}
 static inline void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
 						  u64 data, u64 select_idx) {}
-static inline bool kvm_arm_support_pmu_v3(void) { return false; }
 static inline int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
 					  struct kvm_device_attr *attr)
 {
-- 
2.29.2

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

[PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Marc Zyngier]

We currently find out about the presence of a HW PMU (or the handling
of that PMU by perf, which amounts to the same thing) in a fairly
roundabout way, by checking the number of counters available to perf.
That's good enough for now, but we will soon need to find about about
that on paths where perf is out of reach (in the world switch).

Instead, let's turn kvm_arm_support_pmu_v3() into a static key.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/perf.c     | 10 ++++++++++
 arch/arm64/kvm/pmu-emul.c | 10 ----------
 include/kvm/arm_pmu.h     |  9 +++++++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index d45b8b9a4415..198fa4266b2d 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -11,6 +11,8 @@
 
 #include <asm/kvm_emulate.h>
 
+DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+
 static int kvm_is_in_guest(void)
 {
         return kvm_get_running_vcpu() != NULL;
@@ -48,6 +50,14 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
 
 int kvm_perf_init(void)
 {
+	/*
+	 * Check if HW_PERF_EVENTS are supported by checking the number of
+	 * hardware performance counters. This could ensure the presence of
+	 * a physical PMU and CONFIG_PERF_EVENT is selected.
+	 */
+	if (perf_num_counters() > 0)
+		static_branch_enable(&kvm_arm_pmu_available);
+
 	return perf_register_guest_info_callbacks(&kvm_guest_cbs);
 }
 
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 4ad66a532e38..44d500706ab9 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -813,16 +813,6 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
 	return val & mask;
 }
 
-bool kvm_arm_support_pmu_v3(void)
-{
-	/*
-	 * Check if HW_PERF_EVENTS are supported by checking the number of
-	 * hardware performance counters. This could ensure the presence of
-	 * a physical PMU and CONFIG_PERF_EVENT is selected.
-	 */
-	return (perf_num_counters() > 0);
-}
-
 int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
 {
 	if (!kvm_vcpu_has_pmu(vcpu))
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 8dcb3e1477bc..6fd3cda608e4 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -13,6 +13,13 @@
 #define ARMV8_PMU_CYCLE_IDX		(ARMV8_PMU_MAX_COUNTERS - 1)
 #define ARMV8_PMU_MAX_COUNTER_PAIRS	((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
 
+DECLARE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+
+static __always_inline bool kvm_arm_support_pmu_v3(void)
+{
+	return static_branch_likely(&kvm_arm_pmu_available);
+}
+
 #ifdef CONFIG_HW_PERF_EVENTS
 
 struct kvm_pmc {
@@ -47,7 +54,6 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
 void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
 void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
 				    u64 select_idx);
-bool kvm_arm_support_pmu_v3(void);
 int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
 			    struct kvm_device_attr *attr);
 int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu,
@@ -87,7 +93,6 @@ static inline void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
 static inline void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val) {}
 static inline void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
 						  u64 data, u64 select_idx) {}
-static inline bool kvm_arm_support_pmu_v3(void) { return false; }
 static inline int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
 					  struct kvm_device_attr *attr)
 {
-- 
2.29.2


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

Re: [PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Andre Przywara]

On Tue, 26 Jan 2021 15:15:20 +0000
Marc Zyngier <maz@kernel.org> wrote:

Hi Marc,

> We currently find out about the presence of a HW PMU (or the handling
> of that PMU by perf, which amounts to the same thing) in a fairly
> roundabout way, by checking the number of counters available to perf.
> That's good enough for now, but we will soon need to find about about
> that on paths where perf is out of reach (in the world switch).
> 
> Instead, let's turn kvm_arm_support_pmu_v3() into a static key.

I am sure the pesky build bot has told you about it already, but this
fails when ARM_PMU is not defined, as perf_num_counters() is not
defined. It's  bit nasty, since it's a generic function, so we
can't easily stub it in its original header.

Shall we find a place somewhere in arch/arm64 and provide a stub
implementation there, #ifndef CONFIG_ARM_PMU? Sounds ugly, though.

Or something else entirely?

Cheers,
Andre

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/perf.c     | 10 ++++++++++
>  arch/arm64/kvm/pmu-emul.c | 10 ----------
>  include/kvm/arm_pmu.h     |  9 +++++++--
>  3 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
> index d45b8b9a4415..198fa4266b2d 100644
> --- a/arch/arm64/kvm/perf.c
> +++ b/arch/arm64/kvm/perf.c
> @@ -11,6 +11,8 @@
>  
>  #include <asm/kvm_emulate.h>
>  
> +DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
> +
>  static int kvm_is_in_guest(void)
>  {
>          return kvm_get_running_vcpu() != NULL;
> @@ -48,6 +50,14 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
>  
>  int kvm_perf_init(void)
>  {
> +	/*
> +	 * Check if HW_PERF_EVENTS are supported by checking the number of
> +	 * hardware performance counters. This could ensure the presence of
> +	 * a physical PMU and CONFIG_PERF_EVENT is selected.
> +	 */
> +	if (perf_num_counters() > 0)
> +		static_branch_enable(&kvm_arm_pmu_available);
> +
>  	return perf_register_guest_info_callbacks(&kvm_guest_cbs);
>  }
>  
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 4ad66a532e38..44d500706ab9 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -813,16 +813,6 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>  	return val & mask;
>  }
>  
> -bool kvm_arm_support_pmu_v3(void)
> -{
> -	/*
> -	 * Check if HW_PERF_EVENTS are supported by checking the number of
> -	 * hardware performance counters. This could ensure the presence of
> -	 * a physical PMU and CONFIG_PERF_EVENT is selected.
> -	 */
> -	return (perf_num_counters() > 0);
> -}
> -
>  int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
>  {
>  	if (!kvm_vcpu_has_pmu(vcpu))
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 8dcb3e1477bc..6fd3cda608e4 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -13,6 +13,13 @@
>  #define ARMV8_PMU_CYCLE_IDX		(ARMV8_PMU_MAX_COUNTERS - 1)
>  #define ARMV8_PMU_MAX_COUNTER_PAIRS	((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
>  
> +DECLARE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
> +
> +static __always_inline bool kvm_arm_support_pmu_v3(void)
> +{
> +	return static_branch_likely(&kvm_arm_pmu_available);
> +}
> +
>  #ifdef CONFIG_HW_PERF_EVENTS
>  
>  struct kvm_pmc {
> @@ -47,7 +54,6 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>  				    u64 select_idx);
> -bool kvm_arm_support_pmu_v3(void);
>  int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
>  			    struct kvm_device_attr *attr);
>  int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu,
> @@ -87,7 +93,6 @@ static inline void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
>  static inline void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val) {}
>  static inline void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
>  						  u64 data, u64 select_idx) {}
> -static inline bool kvm_arm_support_pmu_v3(void) { return false; }
>  static inline int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
>  					  struct kvm_device_attr *attr)
>  {

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

Re: [PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Andre Przywara]

On Tue, 26 Jan 2021 15:15:20 +0000
Marc Zyngier <maz@kernel.org> wrote:

Hi Marc,

> We currently find out about the presence of a HW PMU (or the handling
> of that PMU by perf, which amounts to the same thing) in a fairly
> roundabout way, by checking the number of counters available to perf.
> That's good enough for now, but we will soon need to find about about
> that on paths where perf is out of reach (in the world switch).
> 
> Instead, let's turn kvm_arm_support_pmu_v3() into a static key.

I am sure the pesky build bot has told you about it already, but this
fails when ARM_PMU is not defined, as perf_num_counters() is not
defined. It's  bit nasty, since it's a generic function, so we
can't easily stub it in its original header.

Shall we find a place somewhere in arch/arm64 and provide a stub
implementation there, #ifndef CONFIG_ARM_PMU? Sounds ugly, though.

Or something else entirely?

Cheers,
Andre

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/perf.c     | 10 ++++++++++
>  arch/arm64/kvm/pmu-emul.c | 10 ----------
>  include/kvm/arm_pmu.h     |  9 +++++++--
>  3 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
> index d45b8b9a4415..198fa4266b2d 100644
> --- a/arch/arm64/kvm/perf.c
> +++ b/arch/arm64/kvm/perf.c
> @@ -11,6 +11,8 @@
>  
>  #include <asm/kvm_emulate.h>
>  
> +DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
> +
>  static int kvm_is_in_guest(void)
>  {
>          return kvm_get_running_vcpu() != NULL;
> @@ -48,6 +50,14 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
>  
>  int kvm_perf_init(void)
>  {
> +	/*
> +	 * Check if HW_PERF_EVENTS are supported by checking the number of
> +	 * hardware performance counters. This could ensure the presence of
> +	 * a physical PMU and CONFIG_PERF_EVENT is selected.
> +	 */
> +	if (perf_num_counters() > 0)
> +		static_branch_enable(&kvm_arm_pmu_available);
> +
>  	return perf_register_guest_info_callbacks(&kvm_guest_cbs);
>  }
>  
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 4ad66a532e38..44d500706ab9 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -813,16 +813,6 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>  	return val & mask;
>  }
>  
> -bool kvm_arm_support_pmu_v3(void)
> -{
> -	/*
> -	 * Check if HW_PERF_EVENTS are supported by checking the number of
> -	 * hardware performance counters. This could ensure the presence of
> -	 * a physical PMU and CONFIG_PERF_EVENT is selected.
> -	 */
> -	return (perf_num_counters() > 0);
> -}
> -
>  int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
>  {
>  	if (!kvm_vcpu_has_pmu(vcpu))
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 8dcb3e1477bc..6fd3cda608e4 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -13,6 +13,13 @@
>  #define ARMV8_PMU_CYCLE_IDX		(ARMV8_PMU_MAX_COUNTERS - 1)
>  #define ARMV8_PMU_MAX_COUNTER_PAIRS	((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
>  
> +DECLARE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
> +
> +static __always_inline bool kvm_arm_support_pmu_v3(void)
> +{
> +	return static_branch_likely(&kvm_arm_pmu_available);
> +}
> +
>  #ifdef CONFIG_HW_PERF_EVENTS
>  
>  struct kvm_pmc {
> @@ -47,7 +54,6 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
>  void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>  				    u64 select_idx);
> -bool kvm_arm_support_pmu_v3(void);
>  int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
>  			    struct kvm_device_attr *attr);
>  int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu,
> @@ -87,7 +93,6 @@ static inline void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
>  static inline void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val) {}
>  static inline void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
>  						  u64 data, u64 select_idx) {}
> -static inline bool kvm_arm_support_pmu_v3(void) { return false; }
>  static inline int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
>  					  struct kvm_device_attr *attr)
>  {


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

Re: [PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Marc Zyngier]

On 2021-01-28 15:16, Andre Przywara wrote:
> On Tue, 26 Jan 2021 15:15:20 +0000
> Marc Zyngier <maz@kernel.org> wrote:
> 
> Hi Marc,
> 
>> We currently find out about the presence of a HW PMU (or the handling
>> of that PMU by perf, which amounts to the same thing) in a fairly
>> roundabout way, by checking the number of counters available to perf.
>> That's good enough for now, but we will soon need to find about about
>> that on paths where perf is out of reach (in the world switch).
>> 
>> Instead, let's turn kvm_arm_support_pmu_v3() into a static key.
> 
> I am sure the pesky build bot has told you about it already, but this
> fails when ARM_PMU is not defined, as perf_num_counters() is not
> defined. It's  bit nasty, since it's a generic function, so we
> can't easily stub it in its original header.

No sign from the bot yet, but that's indeed a problem. Well spotted.

> Shall we find a place somewhere in arch/arm64 and provide a stub
> implementation there, #ifndef CONFIG_ARM_PMU? Sounds ugly, though.
> 
> Or something else entirely?

How about:

diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index 198fa4266b2d..739164324afe 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -55,7 +55,7 @@ int kvm_perf_init(void)
  	 * hardware performance counters. This could ensure the presence of
  	 * a physical PMU and CONFIG_PERF_EVENT is selected.
  	 */
-	if (perf_num_counters() > 0)
+	if (IS_ENABLED(CONFIG_ARM_PMU) && perf_num_counters() > 0)
  		static_branch_enable(&kvm_arm_pmu_available);

  	return perf_register_guest_info_callbacks(&kvm_guest_cbs);

It certainly compiles here.

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Marc Zyngier]

On 2021-01-28 15:16, Andre Przywara wrote:
> On Tue, 26 Jan 2021 15:15:20 +0000
> Marc Zyngier <maz@kernel.org> wrote:
> 
> Hi Marc,
> 
>> We currently find out about the presence of a HW PMU (or the handling
>> of that PMU by perf, which amounts to the same thing) in a fairly
>> roundabout way, by checking the number of counters available to perf.
>> That's good enough for now, but we will soon need to find about about
>> that on paths where perf is out of reach (in the world switch).
>> 
>> Instead, let's turn kvm_arm_support_pmu_v3() into a static key.
> 
> I am sure the pesky build bot has told you about it already, but this
> fails when ARM_PMU is not defined, as perf_num_counters() is not
> defined. It's  bit nasty, since it's a generic function, so we
> can't easily stub it in its original header.

No sign from the bot yet, but that's indeed a problem. Well spotted.

> Shall we find a place somewhere in arch/arm64 and provide a stub
> implementation there, #ifndef CONFIG_ARM_PMU? Sounds ugly, though.
> 
> Or something else entirely?

How about:

diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index 198fa4266b2d..739164324afe 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -55,7 +55,7 @@ int kvm_perf_init(void)
  	 * hardware performance counters. This could ensure the presence of
  	 * a physical PMU and CONFIG_PERF_EVENT is selected.
  	 */
-	if (perf_num_counters() > 0)
+	if (IS_ENABLED(CONFIG_ARM_PMU) && perf_num_counters() > 0)
  		static_branch_enable(&kvm_arm_pmu_available);

  	return perf_register_guest_info_callbacks(&kvm_guest_cbs);

It certainly compiles here.

         M.
-- 
Jazz is not dead. It just smells funny...

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

Re: [PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Andre Przywara]

On Thu, 28 Jan 2021 16:56:01 +0000
Marc Zyngier <maz@kernel.org> wrote:

> On 2021-01-28 15:16, Andre Przywara wrote:
> > On Tue, 26 Jan 2021 15:15:20 +0000
> > Marc Zyngier <maz@kernel.org> wrote:
> > 
> > Hi Marc,
> >   
> >> We currently find out about the presence of a HW PMU (or the handling
> >> of that PMU by perf, which amounts to the same thing) in a fairly
> >> roundabout way, by checking the number of counters available to perf.
> >> That's good enough for now, but we will soon need to find about about
> >> that on paths where perf is out of reach (in the world switch).
> >> 
> >> Instead, let's turn kvm_arm_support_pmu_v3() into a static key.  
> > 
> > I am sure the pesky build bot has told you about it already, but this
> > fails when ARM_PMU is not defined, as perf_num_counters() is not
> > defined. It's  bit nasty, since it's a generic function, so we
> > can't easily stub it in its original header.  
> 
> No sign from the bot yet, but that's indeed a problem. Well spotted.
> 
> > Shall we find a place somewhere in arch/arm64 and provide a stub
> > implementation there, #ifndef CONFIG_ARM_PMU? Sounds ugly, though.
> > 
> > Or something else entirely?  
> 
> How about:
> 
> diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
> index 198fa4266b2d..739164324afe 100644
> --- a/arch/arm64/kvm/perf.c
> +++ b/arch/arm64/kvm/perf.c
> @@ -55,7 +55,7 @@ int kvm_perf_init(void)
>   	 * hardware performance counters. This could ensure the presence of
>   	 * a physical PMU and CONFIG_PERF_EVENT is selected.
>   	 */
> -	if (perf_num_counters() > 0)
> +	if (IS_ENABLED(CONFIG_ARM_PMU) && perf_num_counters() > 0)

Neat!

That indeed compiles and works in both cases (w/ and w/o ARM_PMU),
fixing the original BUG I saw when using this as the L1 kernel.

Thanks!
Andre

>   		static_branch_enable(&kvm_arm_pmu_available);
> 
>   	return perf_register_guest_info_callbacks(&kvm_guest_cbs);
> 
> It certainly compiles here.
> 
>          M.

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

Re: [PATCH 1/2] KVM: arm64: Turn kvm_arm_support_pmu_v3() into a static key

[Andre Przywara]

On Thu, 28 Jan 2021 16:56:01 +0000
Marc Zyngier <maz@kernel.org> wrote:

> On 2021-01-28 15:16, Andre Przywara wrote:
> > On Tue, 26 Jan 2021 15:15:20 +0000
> > Marc Zyngier <maz@kernel.org> wrote:
> > 
> > Hi Marc,
> >   
> >> We currently find out about the presence of a HW PMU (or the handling
> >> of that PMU by perf, which amounts to the same thing) in a fairly
> >> roundabout way, by checking the number of counters available to perf.
> >> That's good enough for now, but we will soon need to find about about
> >> that on paths where perf is out of reach (in the world switch).
> >> 
> >> Instead, let's turn kvm_arm_support_pmu_v3() into a static key.  
> > 
> > I am sure the pesky build bot has told you about it already, but this
> > fails when ARM_PMU is not defined, as perf_num_counters() is not
> > defined. It's  bit nasty, since it's a generic function, so we
> > can't easily stub it in its original header.  
> 
> No sign from the bot yet, but that's indeed a problem. Well spotted.
> 
> > Shall we find a place somewhere in arch/arm64 and provide a stub
> > implementation there, #ifndef CONFIG_ARM_PMU? Sounds ugly, though.
> > 
> > Or something else entirely?  
> 
> How about:
> 
> diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
> index 198fa4266b2d..739164324afe 100644
> --- a/arch/arm64/kvm/perf.c
> +++ b/arch/arm64/kvm/perf.c
> @@ -55,7 +55,7 @@ int kvm_perf_init(void)
>   	 * hardware performance counters. This could ensure the presence of
>   	 * a physical PMU and CONFIG_PERF_EVENT is selected.
>   	 */
> -	if (perf_num_counters() > 0)
> +	if (IS_ENABLED(CONFIG_ARM_PMU) && perf_num_counters() > 0)

Neat!

That indeed compiles and works in both cases (w/ and w/o ARM_PMU),
fixing the original BUG I saw when using this as the L1 kernel.

Thanks!
Andre

>   		static_branch_enable(&kvm_arm_pmu_available);
> 
>   	return perf_register_guest_info_callbacks(&kvm_guest_cbs);
> 
> It certainly compiles here.
> 
>          M.


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

[PATCH 2/2] KVM: arm64: Don't access PMSELR_EL0/PMUSERENR_EL0 when no PMU is available

[Marc Zyngier]

When running under a nesting hypervisor, it isn't guaranteed that
the virtual HW will include a PMU. In which case, let's not try
to access the PMU registers in the world switch, as that'd be
deadly.

Reported-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/image-vars.h          | 3 +++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 9 ++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index f676243abac6..32af3c865700 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -102,6 +102,9 @@ KVM_NVHE_ALIAS(__stop___kvm_ex_table);
 /* Array containing bases of nVHE per-CPU memory regions. */
 KVM_NVHE_ALIAS(kvm_arm_hyp_percpu_base);
 
+/* PMU available static key */
+KVM_NVHE_ALIAS(kvm_arm_pmu_available);
+
 #endif /* CONFIG_KVM */
 
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 84473574c2e7..75c0faa3b791 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -90,15 +90,18 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
 	 * counter, which could make a PMXEVCNTR_EL0 access UNDEF at
 	 * EL1 instead of being trapped to EL2.
 	 */
-	write_sysreg(0, pmselr_el0);
-	write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+	if (kvm_arm_support_pmu_v3()) {
+		write_sysreg(0, pmselr_el0);
+		write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+	}
 	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
 }
 
 static inline void __deactivate_traps_common(void)
 {
 	write_sysreg(0, hstr_el2);
-	write_sysreg(0, pmuserenr_el0);
+	if (kvm_arm_support_pmu_v3())
+		write_sysreg(0, pmuserenr_el0);
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)
-- 
2.29.2

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

[PATCH 2/2] KVM: arm64: Don't access PMSELR_EL0/PMUSERENR_EL0 when no PMU is available

[Marc Zyngier]

When running under a nesting hypervisor, it isn't guaranteed that
the virtual HW will include a PMU. In which case, let's not try
to access the PMU registers in the world switch, as that'd be
deadly.

Reported-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/image-vars.h          | 3 +++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 9 ++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index f676243abac6..32af3c865700 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -102,6 +102,9 @@ KVM_NVHE_ALIAS(__stop___kvm_ex_table);
 /* Array containing bases of nVHE per-CPU memory regions. */
 KVM_NVHE_ALIAS(kvm_arm_hyp_percpu_base);
 
+/* PMU available static key */
+KVM_NVHE_ALIAS(kvm_arm_pmu_available);
+
 #endif /* CONFIG_KVM */
 
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 84473574c2e7..75c0faa3b791 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -90,15 +90,18 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
 	 * counter, which could make a PMXEVCNTR_EL0 access UNDEF at
 	 * EL1 instead of being trapped to EL2.
 	 */
-	write_sysreg(0, pmselr_el0);
-	write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+	if (kvm_arm_support_pmu_v3()) {
+		write_sysreg(0, pmselr_el0);
+		write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+	}
 	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
 }
 
 static inline void __deactivate_traps_common(void)
 {
 	write_sysreg(0, hstr_el2);
-	write_sysreg(0, pmuserenr_el0);
+	if (kvm_arm_support_pmu_v3())
+		write_sysreg(0, pmuserenr_el0);
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)
-- 
2.29.2


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