[PATCH 0/2] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

[PATCH 0/2] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

[Reiji Watanabe]

On systems where the PMUVer is not uniform across all PEs,
KVM currently does not advertise PMUv3 to the guests,
even if userspace successfully runs KVM_ARM_VCPU_INIT with
KVM_ARM_VCPU_PMU_V3.

In such systems, KVM should either disallow userspace from
configuring vPMU, or advertise PMUv3 to the guest.
This series addresses this inconsistent behavior by implementing
the former, as such systems would be extremely uncommon and
unlikely to even use KVM (according to Marc [1]).

The series is based on v6.4-rc5.

[1] https://lore.kernel.org/all/874jnqp73o.wl-maz@kernel.org/

Reiji Watanabe (2):
  KVM: arm64: PMU: Introduce pmu_v3_is_supported() helper
  KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

 arch/arm64/kvm/arm.c      |  1 +
 arch/arm64/kvm/pmu-emul.c |  6 +-----
 arch/arm64/kvm/sys_regs.c |  2 +-
 include/kvm/arm_pmu.h     | 18 ++++++++++++++++++
 4 files changed, 21 insertions(+), 6 deletions(-)


base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 1/2] KVM: arm64: PMU: Introduce pmu_v3_is_supported() helper

[Reiji Watanabe]

Introduce pmu_v3_is_supported() helper to check if the given
PMUVer supports PMUv3, and use it instead of open coding it.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/pmu-emul.c | 3 +--
 arch/arm64/kvm/sys_regs.c | 2 +-
 include/kvm/arm_pmu.h     | 8 ++++++++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 491ca7eb2a4c..5d2903f52a5f 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -672,8 +672,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
 {
 	struct arm_pmu_entry *entry;
 
-	if (pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_NI ||
-	    pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF)
+	if (!pmu_v3_is_supported(pmu->pmuver))
 		return;
 
 	mutex_lock(&arm_pmus_lock);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 753aa7418149..9200b18127f6 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1408,7 +1408,7 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
 	if ((pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF && pmuver > host_pmuver))
 		return -EINVAL;
 
-	valid_pmu = (pmuver != 0 && pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF);
+	valid_pmu = pmu_v3_is_supported(pmuver);
 
 	/* Make sure view register and PMU support do match */
 	if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 1a6a695ca67a..eef17de966da 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -12,6 +12,14 @@
 
 #define ARMV8_PMU_CYCLE_IDX		(ARMV8_PMU_MAX_COUNTERS - 1)
 
+static inline bool pmu_v3_is_supported(int pmuver)
+{
+	WARN_ON_ONCE(pmuver & ~GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0));
+
+	return (pmuver != ID_AA64DFR0_EL1_PMUVer_NI) &&
+	       (pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF);
+}
+
 #ifdef CONFIG_HW_PERF_EVENTS
 
 struct kvm_pmc {
-- 
2.41.0.162.gfafddb0af9-goog

Re: [PATCH 1/2] KVM: arm64: PMU: Introduce pmu_v3_is_supported() helper

[Oliver Upton]

On Fri, Jun 09, 2023 at 11:15:19PM -0700, Reiji Watanabe wrote:
> Introduce pmu_v3_is_supported() helper to check if the given
> PMUVer supports PMUv3, and use it instead of open coding it.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/kvm/pmu-emul.c | 3 +--
>  arch/arm64/kvm/sys_regs.c | 2 +-
>  include/kvm/arm_pmu.h     | 8 ++++++++
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 491ca7eb2a4c..5d2903f52a5f 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -672,8 +672,7 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
>  {
>  	struct arm_pmu_entry *entry;
>  
> -	if (pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_NI ||
> -	    pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF)
> +	if (!pmu_v3_is_supported(pmu->pmuver))
>  		return;

Why not use pmuv3_implemented()?

--
Thanks,
Oliver

[PATCH 2/2] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

[Reiji Watanabe]

Disallow userspace from configuring vPMU for guests on systems
where the PMUVer is not uniform across all PEs.
KVM has not been advertising PMUv3 to the guests with vPMU on
such systems anyway, and such systems would be extremely
uncommon and unlikely to even use KVM.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/arm.c      |  1 +
 arch/arm64/kvm/pmu-emul.c |  3 ---
 include/kvm/arm_pmu.h     | 10 ++++++++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 14391826241c..21901fbd6e4d 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1960,6 +1960,7 @@ static int __init init_subsystems(void)
 		goto out;
 
 	kvm_register_perf_callbacks(NULL);
+	kvm_arm_set_support_pmu_v3();
 
 out:
 	if (err)
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 5d2903f52a5f..45b84cf22026 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -684,9 +684,6 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
 	entry->arm_pmu = pmu;
 	list_add_tail(&entry->entry, &arm_pmus);
 
-	if (list_is_singular(&arm_pmus))
-		static_branch_enable(&kvm_arm_pmu_available);
-
 out_unlock:
 	mutex_unlock(&arm_pmus_lock);
 }
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index eef17de966da..af1fe2b53fbb 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -105,6 +105,14 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
 
 u8 kvm_arm_pmu_get_pmuver_limit(void);
 
+static inline void kvm_arm_set_support_pmu_v3(void)
+{
+	u8 pmuver = kvm_arm_pmu_get_pmuver_limit();
+
+	if (pmu_v3_is_supported(pmuver))
+		static_branch_enable(&kvm_arm_pmu_available);
+}
+
 #else
 struct kvm_pmu {
 };
@@ -114,6 +122,8 @@ static inline bool kvm_arm_support_pmu_v3(void)
 	return false;
 }
 
+static inline void kvm_arm_set_support_pmu_v3(void) {};
+
 #define kvm_arm_pmu_irq_initialized(v)	(false)
 static inline u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu,
 					    u64 select_idx)
-- 
2.41.0.162.gfafddb0af9-goog

Re: [PATCH 2/2] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

[Oliver Upton]

On Fri, Jun 09, 2023 at 11:15:20PM -0700, Reiji Watanabe wrote:
> Disallow userspace from configuring vPMU for guests on systems
> where the PMUVer is not uniform across all PEs.
> KVM has not been advertising PMUv3 to the guests with vPMU on
> such systems anyway, and such systems would be extremely
> uncommon and unlikely to even use KVM.

Ok... Now your changes are starting to make sense. This patch is rather
relevant context for interpreting the other PMU fix [*], so I'd
recommend you send this as a combined series going forward.

[*]  https://lore.kernel.org/kvmarm/20230610194510.4146549-1-reijiw@google.com/

> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index eef17de966da..af1fe2b53fbb 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -105,6 +105,14 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>  
>  u8 kvm_arm_pmu_get_pmuver_limit(void);
>  
> +static inline void kvm_arm_set_support_pmu_v3(void)
> +{
> +	u8 pmuver = kvm_arm_pmu_get_pmuver_limit();
> +
> +	if (pmu_v3_is_supported(pmuver))
> +		static_branch_enable(&kvm_arm_pmu_available);
> +}
> +
>  #else
>  struct kvm_pmu {
>  };
> @@ -114,6 +122,8 @@ static inline bool kvm_arm_support_pmu_v3(void)
>  	return false;
>  }
>  
> +static inline void kvm_arm_set_support_pmu_v3(void) {};
> +

nit: Give this thing a more generic name (e.g. kvm_pmu_init()) in case
we wind up needing more boot-time PMU initialization.

--
Thanks,
Oliver

Re: [PATCH 2/2] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

[Reiji Watanabe]

Hi Oliver,

On Sat, Jun 10, 2023 at 07:32:42PM -0700, Oliver Upton wrote:
> On Fri, Jun 09, 2023 at 11:15:20PM -0700, Reiji Watanabe wrote:
> > Disallow userspace from configuring vPMU for guests on systems
> > where the PMUVer is not uniform across all PEs.
> > KVM has not been advertising PMUv3 to the guests with vPMU on
> > such systems anyway, and such systems would be extremely
> > uncommon and unlikely to even use KVM.
> 
> Ok... Now your changes are starting to make sense. This patch is rather
> relevant context for interpreting the other PMU fix [*], so I'd
> recommend you send this as a combined series going forward.

Sure, I will include the patch [*] in this series.

> 
> [*]  https://lore.kernel.org/kvmarm/20230610194510.4146549-1-reijiw@google.com/
> 
> > diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> > index eef17de966da..af1fe2b53fbb 100644
> > --- a/include/kvm/arm_pmu.h
> > +++ b/include/kvm/arm_pmu.h
> > @@ -105,6 +105,14 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
> >  
> >  u8 kvm_arm_pmu_get_pmuver_limit(void);
> >  
> > +static inline void kvm_arm_set_support_pmu_v3(void)
> > +{
> > +	u8 pmuver = kvm_arm_pmu_get_pmuver_limit();
> > +
> > +	if (pmu_v3_is_supported(pmuver))
> > +		static_branch_enable(&kvm_arm_pmu_available);
> > +}
> > +
> >  #else
> >  struct kvm_pmu {
> >  };
> > @@ -114,6 +122,8 @@ static inline bool kvm_arm_support_pmu_v3(void)
> >  	return false;
> >  }
> >  
> > +static inline void kvm_arm_set_support_pmu_v3(void) {};
> > +
> 
> nit: Give this thing a more generic name (e.g. kvm_pmu_init()) in case
> we wind up needing more boot-time PMU initialization.

Sure, thank you for the suggestion!

Thank you,
Reiji