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

[PATCH v2 0/4] KVM: arm64: 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.

v1: https://lore.kernel.org/kvmarm/20230623194258.2648987-1-oliver.upton@linux.dev/

v1 -> v2:
 - Set the generic v8 target in the kvm_vcpu_init struct initializer
   (Marc)
 - Use the kvm_vcpu_initialized() helper (Zenghui)
 - Collect Zenghui's ack/reviews (Thanks!)

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 |  6 +++---
 arch/arm64/kvm/arm.c              | 24 +++++++++++-------------
 arch/arm64/kvm/guest.c            | 15 ---------------
 arch/arm64/kvm/hyp/nvhe/switch.c  |  2 +-
 arch/arm64/kvm/reset.c            | 25 ++++++++++---------------
 5 files changed, 25 insertions(+), 47 deletions(-)


base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 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.

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
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.255.g8b1d071c50-goog

[PATCH v2 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.255.g8b1d071c50-goog

[PATCH v2 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.

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
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 8b6096753740..b53105bccff9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -567,8 +567,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 */
@@ -669,6 +668,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..13f1bde0bc2d 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 (!kvm_vcpu_initialized(vcpu))
 		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 0a6271052def..b9caac3e7b1d 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -236,7 +236,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.255.g8b1d071c50-goog

[PATCH v2 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.

Acked-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  1 -
 arch/arm64/kvm/arm.c              |  9 +++++----
 arch/arm64/kvm/guest.c            | 15 ---------------
 3 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index b53105bccff9..ed60277e834a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -898,7 +898,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 13f1bde0bc2d..9ab17ecd76bb 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);
@@ -1585,9 +1586,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;
-- 
2.41.0.255.g8b1d071c50-goog

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

[Zenghui Yu]

On 2023/7/11 3:31, Oliver Upton wrote:
> 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>

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

Re: [PATCH v2 0/4] KVM: arm64: Use 'generic' vCPU target across all uarches

[Marc Zyngier]

On Mon, 10 Jul 2023 20:31:36 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> 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.
> 
> v1: https://lore.kernel.org/kvmarm/20230623194258.2648987-1-oliver.upton@linux.dev/
> 
> v1 -> v2:
>  - Set the generic v8 target in the kvm_vcpu_init struct initializer
>    (Marc)
>  - Use the kvm_vcpu_initialized() helper (Zenghui)
>  - Collect Zenghui's ack/reviews (Thanks!)
> 
> 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 |  6 +++---
>  arch/arm64/kvm/arm.c              | 24 +++++++++++-------------
>  arch/arm64/kvm/guest.c            | 15 ---------------
>  arch/arm64/kvm/hyp/nvhe/switch.c  |  2 +-
>  arch/arm64/kvm/reset.c            | 25 ++++++++++---------------
>  5 files changed, 25 insertions(+), 47 deletions(-)
> 
> 
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5

Looks great to me. This is 6.6 material, right? If so, I'll earmark it
as such.

Thanks,

	M.

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

Re: [PATCH v2 0/4] KVM: arm64: Use 'generic' vCPU target across all uarches

[Oliver Upton]

On Tue, Jul 11, 2023 at 08:27:22AM +0100, Marc Zyngier wrote:
> On Mon, 10 Jul 2023 20:31:36 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > 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.
> > 
> > v1: https://lore.kernel.org/kvmarm/20230623194258.2648987-1-oliver.upton@linux.dev/
> > 
> > v1 -> v2:
> >  - Set the generic v8 target in the kvm_vcpu_init struct initializer
> >    (Marc)
> >  - Use the kvm_vcpu_initialized() helper (Zenghui)
> >  - Collect Zenghui's ack/reviews (Thanks!)
> > 
> > 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 |  6 +++---
> >  arch/arm64/kvm/arm.c              | 24 +++++++++++-------------
> >  arch/arm64/kvm/guest.c            | 15 ---------------
> >  arch/arm64/kvm/hyp/nvhe/switch.c  |  2 +-
> >  arch/arm64/kvm/reset.c            | 25 ++++++++++---------------
> >  5 files changed, 25 insertions(+), 47 deletions(-)
> > 
> > 
> > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> 
> Looks great to me. This is 6.6 material, right? If so, I'll earmark it
> as such.

Yup, I don't think there's any amount of BS I can say to make this look
like a fix :)

--
Thanks,
Oliver

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

[Shaoqin Huang]

On 7/11/23 03:31, 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.
> 
> Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>   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 */

-- 
Shaoqin

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

[Shaoqin Huang]

On 7/11/23 03:31, 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.
> 
> Acked-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>   arch/arm64/include/asm/kvm_host.h |  1 -
>   arch/arm64/kvm/arm.c              |  9 +++++----
>   arch/arm64/kvm/guest.c            | 15 ---------------
>   3 files changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index b53105bccff9..ed60277e834a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -898,7 +898,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 13f1bde0bc2d..9ab17ecd76bb 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);
> @@ -1585,9 +1586,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;

-- 
Shaoqin

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

[Shaoqin Huang]

On 7/11/23 03:31, 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.
> 
> Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>   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 8b6096753740..b53105bccff9 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -567,8 +567,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 */
> @@ -669,6 +668,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..13f1bde0bc2d 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 (!kvm_vcpu_initialized(vcpu))
>   		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 0a6271052def..b9caac3e7b1d 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -236,7 +236,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;
>   	}

-- 
Shaoqin

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

[Shaoqin Huang]

On 7/11/23 03:31, Oliver Upton wrote:
> 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>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>   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,

-- 
Shaoqin

Re: [PATCH v2 0/4] KVM: arm64: Use 'generic' vCPU target across all uarches

[Marc Zyngier]

On Mon, 10 Jul 2023 19:31:36 +0000, Oliver Upton wrote:
> 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.
> 
> [...]

Applied to next, thanks!

[1/4] KVM: arm64: Delete pointless switch statement in kvm_reset_vcpu()
      commit: ea55d5a2cf7c507a9ac03b41716bf1877edad153
[2/4] KVM: arm64: Remove pointless check for changed init target
      commit: c8a67729b8a36a5f4857de645ee9808fc99d8618
[3/4] KVM: arm64: Replace vCPU target with a configuration flag
      commit: ef98406036769107d5c49a519b31c940910b98d3
[4/4] KVM: arm64: Always return generic v8 as the preferred target
      commit: 5346f7e13e5eb134920a14504b6900c6168dd16e

Cheers,

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