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

[Marc Zyngier <maz@kernel.org>]

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.