Re: [PATCH kvmtool v3 8/9] arm64: Add SVE support

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 30 May 2019 16:13:13 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> This patch adds --enable-sve/--disable-sve command line options to
> allow the user to control whether the Scalable Vector Extension is
> made available to the guest.

I guess I have a similar concern about this enable/disable pair being
confusing, though there is more sense here for SVE, given the impact of it
being enabled in the guest.

Maybe we can cover both pointer auth and SVE options with the same revised
approach?

Cheers,
Andre.


> This requires use of the new KVM_ARM_VCPU_FINALIZE ioctl before the
> vcpu is runnable, so a new hook kvm_cpu__configure_features() is
> added to provide an appropriate place to do this work.
> 
> By default, SVE is enabled for the guest if the host supports it.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  arm/aarch32/include/kvm/kvm-cpu-arch.h    |  4 +++
>  arm/aarch64/include/kvm/kvm-config-arch.h |  6 ++++-
>  arm/aarch64/include/kvm/kvm-cpu-arch.h    |  1 +
>  arm/aarch64/kvm-cpu.c                     | 41 +++++++++++++++++++++++++++++++
>  arm/include/arm-common/kvm-config-arch.h  |  2 ++
>  arm/kvm-cpu.c                             |  3 +++
>  6 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> index 01983f0..780e0e2 100644
> --- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> @@ -15,5 +15,9 @@
>  
>  static inline void kvm_cpu__select_features(struct kvm *kvm,
>  					    struct kvm_vcpu_init *init) { }
> +static inline int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
> +{
> +	return 0;
> +}
>  
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> index fe1699d..41e9d05 100644
> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -12,7 +12,11 @@
>  	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
>  			"Enable pointer authentication for the guest"),	\
>  	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
> -			"Disable pointer authentication for the guest"),
> +			"Disable pointer authentication for the guest"), \
> +	OPT_BOOLEAN('\0', "enable-sve", &(cfg)->enable_sve,		\
> +			"Enable SVE for the guest"),			\
> +	OPT_BOOLEAN('\0', "disable-sve", &(cfg)->disable_sve,		\
> +			"Disable SVE for the guest"),
>  
>  #include "arm-common/kvm-config-arch.h"
>  
> diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> index e6875fc..8dfb82e 100644
> --- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> @@ -18,5 +18,6 @@
>  #define ARM_CPU_CTRL_SCTLR_EL1	0
>  
>  void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init);
> +int kvm_cpu__configure_features(struct kvm_cpu *vcpu);
>  
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
> index 08e4fd5..cdfb22e 100644
> --- a/arm/aarch64/kvm-cpu.c
> +++ b/arm/aarch64/kvm-cpu.c
> @@ -152,9 +152,50 @@ static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
>  	}
>  }
>  
> +static void select_sve_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
> +{
> +	bool supported;
> +
> +	if (kvm->cfg.arch.enable_sve && kvm->cfg.arch.disable_sve)
> +		die("--enable-sve conflicts with --disable-sve");
> +
> +	supported = kvm__supports_extension(kvm, KVM_CAP_ARM_SVE);
> +
> +	if (kvm->cfg.arch.enable_sve && !supported)
> +		die("--enable-sve not supported on this host");
> +
> +	/* Default SVE to on if available and not explicitly disabled */
> +	if (supported && !kvm->cfg.arch.disable_sve) {
> +		kvm->cfg.arch.enable_sve = true;
> +		init->features[0] |= 1UL << KVM_ARM_VCPU_SVE;
> +	}
> +}
> +
>  void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init)
>  {
>  	select_ptrauth_feature(kvm, init);
> +	select_sve_feature(kvm, init);
> +}
> +
> +static int configure_sve(struct kvm_cpu *vcpu)
> +{
> +	int feature = KVM_ARM_VCPU_SVE;
> +
> +	if (ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_FINALIZE, &feature)) {
> +		pr_err("KVM_ARM_VCPU_FINALIZE: %s", strerror(errno));
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
> +{
> +	if (vcpu->kvm->cfg.arch.enable_sve)
> +		if (configure_sve(vcpu))
> +			return -1;
> +
> +	return 0;
>  }
>  
>  void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu)
> diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
> index 1b4287d..40e3d1f 100644
> --- a/arm/include/arm-common/kvm-config-arch.h
> +++ b/arm/include/arm-common/kvm-config-arch.h
> @@ -10,6 +10,8 @@ struct kvm_config_arch {
>  	bool		aarch32_guest;
>  	bool		has_pmuv3;
>  	u64		kaslr_seed;
> +	bool		enable_sve;
> +	bool		disable_sve;
>  	bool		enable_ptrauth;
>  	bool		disable_ptrauth;
>  	enum irqchip_type irqchip;
> diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
> index 1652f6f..554414f 100644
> --- a/arm/kvm-cpu.c
> +++ b/arm/kvm-cpu.c
> @@ -124,6 +124,9 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  	vcpu->cpu_compatible	= target->compatible;
>  	vcpu->is_running	= true;
>  
> +	if (kvm_cpu__configure_features(vcpu))
> +		die("Unable to configure requested vcpu features");
> +
>  	return vcpu;
>  }
>  

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