[PATCH 0/4] KVM: arm64: Consistently use 'generic' vCPU target across all uarches

[PATCH 0/4] KVM: arm64: Consistently use 'generic' vCPU target across all uarches

[Oliver Upton]

kvm_vcpu_init::target is quite useless at this point. We don't do any
uarch-specific emulation in the first place, and require userspace
select the 'generic' vCPU target on all but a few implementations.

Small series to (1) clean up usage of the target value in the kernel and
(2) switch to the 'generic' target on implementations that previously
had their own target values. The implementation-specific values are
still tolerated, though, to avoid UAPI breakage.

Oliver Upton (4):
  KVM: arm64: Delete pointless switch statement in kvm_reset_vcpu()
  KVM: arm64: Remove pointless check for changed init target
  KVM: arm64: Replace vCPU target with a configuration flag
  KVM: arm64: Always return generic v8 as the preferred target

 arch/arm64/include/asm/kvm_host.h |  5 +++--
 arch/arm64/kvm/arm.c              | 18 ++++++++----------
 arch/arm64/kvm/guest.c            |  4 +---
 arch/arm64/kvm/hyp/nvhe/switch.c  |  2 +-
 arch/arm64/kvm/reset.c            | 25 ++++++++++---------------
 5 files changed, 23 insertions(+), 31 deletions(-)


base-commit: 192df2aa0113ddddee2a93e453ff46610807b425
-- 
2.41.0.178.g377b9f9a00-goog

[PATCH 1/4] KVM: arm64: Delete pointless switch statement in kvm_reset_vcpu()

[Oliver Upton]

The vCPU target hasn't mattered for quite a long time now. Delete the
useless switch statement in kvm_reset_vcpu(), which hilariously only had
a default case in it.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/reset.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index bc8556b6f459..7a65a35ee4ac 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -248,21 +248,16 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	switch (vcpu->arch.target) {
-	default:
-		if (vcpu_el1_is_32bit(vcpu)) {
-			pstate = VCPU_RESET_PSTATE_SVC;
-		} else if (vcpu_has_nv(vcpu)) {
-			pstate = VCPU_RESET_PSTATE_EL2;
-		} else {
-			pstate = VCPU_RESET_PSTATE_EL1;
-		}
-
-		if (kvm_vcpu_has_pmu(vcpu) && !kvm_arm_support_pmu_v3()) {
-			ret = -EINVAL;
-			goto out;
-		}
-		break;
+	if (vcpu_el1_is_32bit(vcpu))
+		pstate = VCPU_RESET_PSTATE_SVC;
+	else if (vcpu_has_nv(vcpu))
+		pstate = VCPU_RESET_PSTATE_EL2;
+	else
+		pstate = VCPU_RESET_PSTATE_EL1;
+
+	if (kvm_vcpu_has_pmu(vcpu) && !kvm_arm_support_pmu_v3()) {
+		ret = -EINVAL;
+		goto out;
 	}
 
 	/* Reset core registers */
-- 
2.41.0.178.g377b9f9a00-goog

[PATCH 2/4] KVM: arm64: Remove pointless check for changed init target

[Oliver Upton]

At any time there is only a single valid value for KVM_ARM_VCPU_INIT,
depending on the current CPU implementation. In all likelihood, this
will be the generic ARMv8 target.

Drop the pointless check for a changed target value between calls to
KVM_ARM_VCPU_INIT and instead rely on the check against
kvm_target_cpu().

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/arm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index c2c14059f6a8..3f844934b9f3 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1212,8 +1212,7 @@ static bool kvm_vcpu_init_changed(struct kvm_vcpu *vcpu,
 {
 	unsigned long features = init->features[0];
 
-	return !bitmap_equal(vcpu->arch.features, &features, KVM_VCPU_MAX_FEATURES) ||
-			vcpu->arch.target != init->target;
+	return !bitmap_equal(vcpu->arch.features, &features, KVM_VCPU_MAX_FEATURES);
 }
 
 static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
-- 
2.41.0.178.g377b9f9a00-goog

[PATCH 3/4] KVM: arm64: Replace vCPU target with a configuration flag

[Oliver Upton]

The value of kvm_vcpu_arch::target has been used to determine if a vCPU
has actually been initialized. Storing this as an integer is needless at
this point, as KVM doesn't do any microarch-specific emulation in the
first place. Instead, all we care about is whether or not the vCPU has
been initialized.

Delete the field in favor of a vCPU configuration flag indicating if
KVM_ARM_VCPU_INIT has completed for the vCPU.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  5 +++--
 arch/arm64/kvm/arm.c              | 12 +++++-------
 arch/arm64/kvm/hyp/nvhe/switch.c  |  2 +-
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bad7dfe9c16d..2d321b02234b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -562,8 +562,7 @@ struct kvm_vcpu_arch {
 	/* Cache some mmu pages needed inside spinlock regions */
 	struct kvm_mmu_memory_cache mmu_page_cache;
 
-	/* Target CPU and feature flags */
-	int target;
+	/* feature flags */
 	DECLARE_BITMAP(features, KVM_VCPU_MAX_FEATURES);
 
 	/* Virtual SError ESR to restore when HCR_EL2.VSE is set */
@@ -664,6 +663,8 @@ struct kvm_vcpu_arch {
 #define VCPU_SVE_FINALIZED	__vcpu_single_flag(cflags, BIT(1))
 /* PTRAUTH exposed to guest */
 #define GUEST_HAS_PTRAUTH	__vcpu_single_flag(cflags, BIT(2))
+/* KVM_ARM_VCPU_INIT completed */
+#define VCPU_INITIALIZED	__vcpu_single_flag(cflags, BIT(3))
 
 /* Exception pending */
 #define PENDING_EXCEPTION	__vcpu_single_flag(iflags, BIT(0))
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3f844934b9f3..3fa63fdbbf34 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -360,7 +360,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 #endif
 
 	/* Force users to call KVM_ARM_VCPU_INIT */
-	vcpu->arch.target = -1;
+	vcpu_clear_flag(vcpu, VCPU_INITIALIZED);
 	bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
 
 	vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
@@ -569,7 +569,7 @@ unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu)
 
 static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.target >= 0;
+	return vcpu_get_flag(vcpu, VCPU_INITIALIZED);
 }
 
 /*
@@ -1051,7 +1051,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 			 * invalid. The VMM can try and fix it by issuing  a
 			 * KVM_ARM_VCPU_INIT if it really wants to.
 			 */
-			vcpu->arch.target = -1;
+			vcpu_clear_flag(vcpu, VCPU_INITIALIZED);
 			ret = ARM_EXCEPTION_IL;
 		}
 
@@ -1228,20 +1228,18 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 	    !bitmap_equal(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES))
 		goto out_unlock;
 
-	vcpu->arch.target = init->target;
 	bitmap_copy(vcpu->arch.features, &features, KVM_VCPU_MAX_FEATURES);
 
 	/* Now we know what it is, we can reset it. */
 	ret = kvm_reset_vcpu(vcpu);
 	if (ret) {
-		vcpu->arch.target = -1;
 		bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
 		goto out_unlock;
 	}
 
 	bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES);
 	set_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED, &kvm->arch.flags);
-
+	vcpu_set_flag(vcpu, VCPU_INITIALIZED);
 out_unlock:
 	mutex_unlock(&kvm->arch.config_lock);
 	return ret;
@@ -1259,7 +1257,7 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 	if (ret)
 		return ret;
 
-	if (vcpu->arch.target == -1)
+	if (!vcpu_get_flag(vcpu, VCPU_INITIALIZED))
 		return __kvm_vcpu_set_target(vcpu, init);
 
 	if (kvm_vcpu_init_changed(vcpu, init))
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 5fa0b1c9ee8d..6832c9905811 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -234,7 +234,7 @@ static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code)
 		 * KVM_ARM_VCPU_INIT, however, this is likely not possible for
 		 * protected VMs.
 		 */
-		vcpu->arch.target = -1;
+		vcpu_clear_flag(vcpu, VCPU_INITIALIZED);
 		*exit_code &= BIT(ARM_EXIT_WITH_SERROR_BIT);
 		*exit_code |= ARM_EXCEPTION_IL;
 	}
-- 
2.41.0.178.g377b9f9a00-goog

[PATCH 4/4] KVM: arm64: Always return generic v8 as the preferred target

[Oliver Upton]

Userspace selecting an implementation-specific vCPU target has been
completely useless for a very long time. Let's go whole hog and start
returning the generic v8 target across all implementations as the
preferred target.

Uphold the pre-existing behavior by tolerating either the generic target
or an implementation-specific target if the vCPU happens to be running
on one of the lucky few parts.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/arm.c   | 3 ++-
 arch/arm64/kvm/guest.c | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3fa63fdbbf34..5a3b8b2e779b 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1250,7 +1250,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 {
 	int ret;
 
-	if (init->target != kvm_target_cpu())
+	if (init->target != KVM_ARM_TARGET_GENERIC_V8 &&
+	    init->target != kvm_target_cpu())
 		return -EINVAL;
 
 	ret = kvm_vcpu_init_check_features(vcpu, init);
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 20280a5233f6..6b099a914fd4 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -886,8 +886,6 @@ u32 __attribute_const__ kvm_target_cpu(void)
 
 void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
 {
-	u32 target = kvm_target_cpu();
-
 	memset(init, 0, sizeof(*init));
 
 	/*
@@ -896,7 +894,7 @@ void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
 	 * specific features available for the preferred
 	 * target type.
 	 */
-	init->target = (__u32)target;
+	init->target = KVM_ARM_TARGET_GENERIC_V8;
 }
 
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
-- 
2.41.0.178.g377b9f9a00-goog

Re: [PATCH 4/4] KVM: arm64: Always return generic v8 as the preferred target

[Marc Zyngier]

On Fri, 23 Jun 2023 20:42:58 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Userspace selecting an implementation-specific vCPU target has been
> completely useless for a very long time. Let's go whole hog and start
> returning the generic v8 target across all implementations as the
> preferred target.
> 
> Uphold the pre-existing behavior by tolerating either the generic target
> or an implementation-specific target if the vCPU happens to be running
> on one of the lucky few parts.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/arm.c   | 3 ++-
>  arch/arm64/kvm/guest.c | 4 +---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 3fa63fdbbf34..5a3b8b2e779b 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1250,7 +1250,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  {
>  	int ret;
>  
> -	if (init->target != kvm_target_cpu())
> +	if (init->target != KVM_ARM_TARGET_GENERIC_V8 &&
> +	    init->target != kvm_target_cpu())
>  		return -EINVAL;
>  
>  	ret = kvm_vcpu_init_check_features(vcpu, init);
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 20280a5233f6..6b099a914fd4 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -886,8 +886,6 @@ u32 __attribute_const__ kvm_target_cpu(void)
>  
>  void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
>  {
> -	u32 target = kvm_target_cpu();
> -
>  	memset(init, 0, sizeof(*init));
>  
>  	/*
> @@ -896,7 +894,7 @@ void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
>  	 * specific features available for the preferred
>  	 * target type.
>  	 */
> -	init->target = (__u32)target;
> +	init->target = KVM_ARM_TARGET_GENERIC_V8;
>  }

Should we take the opportunity to simply get rid of this function
altogether? Something like the untested hack below.

Thanks,

	M.

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bad7dfe9c16d..67038b4e97e4 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -890,7 +890,6 @@ struct kvm_vcpu_stat {
 	u64 exits;
 };
 
-void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
 unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
 int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index c2c14059f6a8..6fc147bd8f05 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1588,9 +1588,9 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 		return kvm_vm_ioctl_set_device_addr(kvm, &dev_addr);
 	}
 	case KVM_ARM_PREFERRED_TARGET: {
-		struct kvm_vcpu_init init;
-
-		kvm_vcpu_preferred_target(&init);
+		struct kvm_vcpu_init init = {
+			.target = KVM_ARM_TARGET_GENERIC_V8;
+		};
 
 		if (copy_to_user(argp, &init, sizeof(init)))
 			return -EFAULT;
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 20280a5233f6..95f6945c4432 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -884,21 +884,6 @@ u32 __attribute_const__ kvm_target_cpu(void)
 	return KVM_ARM_TARGET_GENERIC_V8;
 }
 
-void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
-{
-	u32 target = kvm_target_cpu();
-
-	memset(init, 0, sizeof(*init));
-
-	/*
-	 * For now, we don't return any features.
-	 * In future, we might use features to return target
-	 * specific features available for the preferred
-	 * target type.
-	 */
-	init->target = (__u32)target;
-}
-
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
 	return -EINVAL;

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

Re: [PATCH 4/4] KVM: arm64: Always return generic v8 as the preferred target

[Oliver Upton]

On Sat, Jun 24, 2023 at 01:45:43AM +0100, Marc Zyngier wrote:
> On Fri, 23 Jun 2023 20:42:58 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > Userspace selecting an implementation-specific vCPU target has been
> > completely useless for a very long time. Let's go whole hog and start
> > returning the generic v8 target across all implementations as the
> > preferred target.
> > 
> > Uphold the pre-existing behavior by tolerating either the generic target
> > or an implementation-specific target if the vCPU happens to be running
> > on one of the lucky few parts.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/arm.c   | 3 ++-
> >  arch/arm64/kvm/guest.c | 4 +---
> >  2 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 3fa63fdbbf34..5a3b8b2e779b 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -1250,7 +1250,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> >  {
> >  	int ret;
> >  
> > -	if (init->target != kvm_target_cpu())
> > +	if (init->target != KVM_ARM_TARGET_GENERIC_V8 &&
> > +	    init->target != kvm_target_cpu())
> >  		return -EINVAL;
> >  
> >  	ret = kvm_vcpu_init_check_features(vcpu, init);
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index 20280a5233f6..6b099a914fd4 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -886,8 +886,6 @@ u32 __attribute_const__ kvm_target_cpu(void)
> >  
> >  void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> >  {
> > -	u32 target = kvm_target_cpu();
> > -
> >  	memset(init, 0, sizeof(*init));
> >  
> >  	/*
> > @@ -896,7 +894,7 @@ void kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> >  	 * specific features available for the preferred
> >  	 * target type.
> >  	 */
> > -	init->target = (__u32)target;
> > +	init->target = KVM_ARM_TARGET_GENERIC_V8;
> >  }
> 
> Should we take the opportunity to simply get rid of this function
> altogether? Something like the untested hack below.

Yeah, I should've thought about it a little more and done as you
suggest. Thanks for the diff, I'll use it in v2.

--
Thanks,
Oliver

Re: [PATCH 4/4] KVM: arm64: Always return generic v8 as the preferred target

[Zenghui Yu]

Hi Oliver,

On 2023/6/24 3:42, Oliver Upton wrote:
> Userspace selecting an implementation-specific vCPU target has been
> completely useless for a very long time. Let's go whole hog and start
> returning the generic v8 target across all implementations as the
> preferred target.
> 
> Uphold the pre-existing behavior by tolerating either the generic target
> or an implementation-specific target if the vCPU happens to be running
> on one of the lucky few parts.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

I love the idea.

Acked-by: Zenghui Yu <yuzenghui@huawei.com>

> ---
>  arch/arm64/kvm/arm.c   | 3 ++-
>  arch/arm64/kvm/guest.c | 4 +---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 3fa63fdbbf34..5a3b8b2e779b 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1250,7 +1250,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  {
>  	int ret;
>  
> -	if (init->target != kvm_target_cpu())
> +	if (init->target != KVM_ARM_TARGET_GENERIC_V8 &&
> +	    init->target != kvm_target_cpu())
>  		return -EINVAL;

I can see that user-space can now select GENERIC_V8 or CORTEX_A53 (for
example) between two KVM_ARM_VCPU_INIT calls on an A53 host, which was
forbidden before this change. I don't think it's a big deal though.

Thanks,
Zenghui

Re: [PATCH 1/4] KVM: arm64: Delete pointless switch statement in kvm_reset_vcpu()

[Zenghui Yu]

On 2023/6/24 3:42, Oliver Upton wrote:
> The vCPU target hasn't mattered for quite a long time now. Delete the
> useless switch statement in kvm_reset_vcpu(), which hilariously only had
> a default case in it.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>

Re: [PATCH 3/4] KVM: arm64: Replace vCPU target with a configuration flag

[Zenghui Yu]

On 2023/6/24 3:42, Oliver Upton wrote:
> The value of kvm_vcpu_arch::target has been used to determine if a vCPU
> has actually been initialized. Storing this as an integer is needless at
> this point, as KVM doesn't do any microarch-specific emulation in the
> first place. Instead, all we care about is whether or not the vCPU has
> been initialized.
> 
> Delete the field in favor of a vCPU configuration flag indicating if
> KVM_ARM_VCPU_INIT has completed for the vCPU.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

[...]

> @@ -1259,7 +1257,7 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  	if (ret)
>  		return ret;
>  
> -	if (vcpu->arch.target == -1)
> +	if (!vcpu_get_flag(vcpu, VCPU_INITIALIZED))

if (!kvm_vcpu_initialized(vcpu)) ?

Regardless,

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>

Re: [PATCH 4/4] KVM: arm64: Always return generic v8 as the preferred target

[Oliver Upton]

On Mon, Jul 10, 2023 at 11:13:34PM +0800, Zenghui Yu wrote:
> Hi Oliver,
> 
> On 2023/6/24 3:42, Oliver Upton wrote:
> > Userspace selecting an implementation-specific vCPU target has been
> > completely useless for a very long time. Let's go whole hog and start
> > returning the generic v8 target across all implementations as the
> > preferred target.
> > 
> > Uphold the pre-existing behavior by tolerating either the generic target
> > or an implementation-specific target if the vCPU happens to be running
> > on one of the lucky few parts.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> 
> I love the idea.
> 
> Acked-by: Zenghui Yu <yuzenghui@huawei.com>

Thanks!

> > ---
> >  arch/arm64/kvm/arm.c   | 3 ++-
> >  arch/arm64/kvm/guest.c | 4 +---
> >  2 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 3fa63fdbbf34..5a3b8b2e779b 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -1250,7 +1250,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> >  {
> >  	int ret;
> > -	if (init->target != kvm_target_cpu())
> > +	if (init->target != KVM_ARM_TARGET_GENERIC_V8 &&
> > +	    init->target != kvm_target_cpu())
> >  		return -EINVAL;
> 
> I can see that user-space can now select GENERIC_V8 or CORTEX_A53 (for
> example) between two KVM_ARM_VCPU_INIT calls on an A53 host, which was
> forbidden before this change. I don't think it's a big deal though.

Agreed, I thought the slight relaxation on invariance was OK. If needed,
we can always change the rules for a new target value, but I doubt we will
need to do that anytime soon.

-- 
Thanks,
Oliver