On Fri, Mar 29, 2019 at 01:00:32PM +0000, Dave Martin wrote:
> Due to the way the effective SVE vector length is controlled and
> trapped at different exception levels, certain mismatches in the
> sets of vector lengths supported by different physical CPUs in the
> system may prevent straightforward virtualisation of SVE at parity
> with the host.
> 
> This patch analyses the extent to which SVE can be virtualised
> safely without interfering with migration of vcpus between physical
> CPUs, and rejects late secondary CPUs that would erode the
> situation further.
> 
> It is left up to KVM to decide what to do with this information.
> 
> Signed-off-by: Dave Martin <dave.mar...@arm.com>
> Reviewed-by: Julien Thierry <julien.thie...@arm.com>
> Tested-by: zhang.lei <zhang....@jp.fujitsu.com>
> 
> ---
> 
> QUESTION: The final structure of this code makes it quite natural to
> clamp the vector length for KVM guests to the maximum value supportable
> across all CPUs; such a value is guaranteed to exist, but may be
> surprisingly small on a given hardware platform.
> 
> Should we be stricter and actually refuse to support KVM at all on such
> hardware?  This may help to force people to fix Linux (or the
> architecture) if/when this issue comes up.

Blocking KVM would be too harsh, since the users of the host may not
care about guests with SVE, but still care about guests.

> 
> For now, I stick with pr_warn() and make do with a limited SVE vector
> length for guests.

I think that's probably the best we can do.

> 
> Changes since v5:
> 
>  * pr_info() about the presence of unvirtualisable vector lengths in
>    sve_setup() upgrade to pr_warn(), for consistency with
>    sve_verify_vq_map().
> ---
>  arch/arm64/include/asm/fpsimd.h |  1 +
>  arch/arm64/kernel/cpufeature.c  |  2 +-
>  arch/arm64/kernel/fpsimd.c      | 86 
> ++++++++++++++++++++++++++++++++++-------
>  3 files changed, 73 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index dd1ad39..964adc9 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -87,6 +87,7 @@ extern void sve_kernel_enable(const struct 
> arm64_cpu_capabilities *__unused);
>  extern u64 read_zcr_features(void);
>  
>  extern int __ro_after_init sve_max_vl;
> +extern int __ro_after_init sve_max_virtualisable_vl;
>  
>  #ifdef CONFIG_ARM64_SVE
>  
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 4061de1..7f8cc51 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1863,7 +1863,7 @@ static void verify_sve_features(void)
>       unsigned int len = zcr & ZCR_ELx_LEN_MASK;
>  
>       if (len < safe_len || sve_verify_vq_map()) {
> -             pr_crit("CPU%d: SVE: required vector length(s) missing\n",
> +             pr_crit("CPU%d: SVE: vector length support mismatch\n",
>                       smp_processor_id());
>               cpu_die_early();
>       }
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index f59ea67..b219796a 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include <linux/bitmap.h>
> +#include <linux/bitops.h>
>  #include <linux/bottom_half.h>
>  #include <linux/bug.h>
>  #include <linux/cache.h>
> @@ -48,6 +49,7 @@
>  #include <asm/sigcontext.h>
>  #include <asm/sysreg.h>
>  #include <asm/traps.h>
> +#include <asm/virt.h>
>  
>  #define FPEXC_IOF    (1 << 0)
>  #define FPEXC_DZF    (1 << 1)
> @@ -130,14 +132,18 @@ static int sve_default_vl = -1;
>  
>  /* Maximum supported vector length across all CPUs (initially poisoned) */
>  int __ro_after_init sve_max_vl = SVE_VL_MIN;
> +int __ro_after_init sve_max_virtualisable_vl = SVE_VL_MIN;
>  /* Set of available vector lengths, as vq_to_bit(vq): */
>  static __ro_after_init DECLARE_BITMAP(sve_vq_map, SVE_VQ_MAX);
> +/* Set of vector lengths present on at least one cpu: */
> +static __ro_after_init DECLARE_BITMAP(sve_vq_partial_map, SVE_VQ_MAX);
>  static void __percpu *efi_sve_state;
>  
>  #else /* ! CONFIG_ARM64_SVE */
>  
>  /* Dummy declaration for code that will be optimised out: */
>  extern __ro_after_init DECLARE_BITMAP(sve_vq_map, SVE_VQ_MAX);
> +extern __ro_after_init DECLARE_BITMAP(sve_vq_partial_map, SVE_VQ_MAX);
>  extern void __percpu *efi_sve_state;
>  
>  #endif /* ! CONFIG_ARM64_SVE */
> @@ -623,12 +629,6 @@ int sve_get_current_vl(void)
>       return sve_prctl_status(0);
>  }
>  
> -/*
> - * Bitmap for temporary storage of the per-CPU set of supported vector 
> lengths
> - * during secondary boot.
> - */
> -static DECLARE_BITMAP(sve_secondary_vq_map, SVE_VQ_MAX);
> -
>  static void sve_probe_vqs(DECLARE_BITMAP(map, SVE_VQ_MAX))
>  {
>       unsigned int vq, vl;
> @@ -654,6 +654,7 @@ static void sve_probe_vqs(DECLARE_BITMAP(map, SVE_VQ_MAX))
>  void __init sve_init_vq_map(void)
>  {
>       sve_probe_vqs(sve_vq_map);
> +     bitmap_copy(sve_vq_partial_map, sve_vq_map, SVE_VQ_MAX);
>  }
>  
>  /*
> @@ -663,8 +664,11 @@ void __init sve_init_vq_map(void)
>   */
>  void sve_update_vq_map(void)
>  {
> -     sve_probe_vqs(sve_secondary_vq_map);
> -     bitmap_and(sve_vq_map, sve_vq_map, sve_secondary_vq_map, SVE_VQ_MAX);
> +     DECLARE_BITMAP(tmp_map, SVE_VQ_MAX);
> +
> +     sve_probe_vqs(tmp_map);
> +     bitmap_and(sve_vq_map, sve_vq_map, tmp_map, SVE_VQ_MAX);
> +     bitmap_or(sve_vq_partial_map, sve_vq_partial_map, tmp_map, SVE_VQ_MAX);
>  }
>  
>  /*
> @@ -673,18 +677,48 @@ void sve_update_vq_map(void)
>   */
>  int sve_verify_vq_map(void)
>  {
> -     int ret = 0;
> +     DECLARE_BITMAP(tmp_map, SVE_VQ_MAX);
> +     unsigned long b;
>  
> -     sve_probe_vqs(sve_secondary_vq_map);
> -     bitmap_andnot(sve_secondary_vq_map, sve_vq_map, sve_secondary_vq_map,
> -                   SVE_VQ_MAX);
> -     if (!bitmap_empty(sve_secondary_vq_map, SVE_VQ_MAX)) {
> +     sve_probe_vqs(tmp_map);
> +
> +     bitmap_complement(tmp_map, tmp_map, SVE_VQ_MAX);
> +     if (bitmap_intersects(tmp_map, sve_vq_map, SVE_VQ_MAX)) {
>               pr_warn("SVE: cpu%d: Required vector length(s) missing\n",
>                       smp_processor_id());
> -             ret = -EINVAL;
> +             return -EINVAL;
>       }
>  
> -     return ret;
> +     if (!IS_ENABLED(CONFIG_KVM) || !is_hyp_mode_available())
> +             return 0;
> +
> +     /*
> +      * For KVM, it is necessary to ensure that this CPU doesn't
> +      * support any vector length that guests may have probed as
> +      * unsupported.
> +      */
> +
> +     /* Recover the set of supported VQs: */
> +     bitmap_complement(tmp_map, tmp_map, SVE_VQ_MAX);
> +     /* Find VQs supported that are not globally supported: */
> +     bitmap_andnot(tmp_map, tmp_map, sve_vq_map, SVE_VQ_MAX);
> +
> +     /* Find the lowest such VQ, if any: */
> +     b = find_last_bit(tmp_map, SVE_VQ_MAX);
> +     if (b >= SVE_VQ_MAX)
> +             return 0; /* no mismatches */
> +
> +     /*
> +      * Mismatches above sve_max_virtualisable_vl are fine, since
> +      * no guest is allowed to configure ZCR_EL2.LEN to exceed this:
> +      */
> +     if (sve_vl_from_vq(bit_to_vq(b)) <= sve_max_virtualisable_vl) {
> +             pr_warn("SVE: cpu%d: Unsupported vector length(s) present\n",
> +                     smp_processor_id());
> +             return -EINVAL;
> +     }
> +
> +     return 0;
>  }
>  
>  static void __init sve_efi_setup(void)
> @@ -751,6 +785,8 @@ u64 read_zcr_features(void)
>  void __init sve_setup(void)
>  {
>       u64 zcr;
> +     DECLARE_BITMAP(tmp_map, SVE_VQ_MAX);
> +     unsigned long b;
>  
>       if (!system_supports_sve())
>               return;
> @@ -779,11 +815,31 @@ void __init sve_setup(void)
>        */
>       sve_default_vl = find_supported_vector_length(64);
>  
> +     bitmap_andnot(tmp_map, sve_vq_partial_map, sve_vq_map,
> +                   SVE_VQ_MAX);
> +
> +     b = find_last_bit(tmp_map, SVE_VQ_MAX);
> +     if (b >= SVE_VQ_MAX)
> +             /* No non-virtualisable VLs found */
> +             sve_max_virtualisable_vl = SVE_VQ_MAX;
> +     else if (WARN_ON(b == SVE_VQ_MAX - 1))
> +             /* No virtualisable VLs?  This is architecturally forbidden. */
> +             sve_max_virtualisable_vl = SVE_VQ_MIN;
> +     else /* b + 1 < SVE_VQ_MAX */
> +             sve_max_virtualisable_vl = sve_vl_from_vq(bit_to_vq(b + 1));
> +
> +     if (sve_max_virtualisable_vl > sve_max_vl)
> +             sve_max_virtualisable_vl = sve_max_vl;
> +
>       pr_info("SVE: maximum available vector length %u bytes per vector\n",
>               sve_max_vl);
>       pr_info("SVE: default vector length %u bytes per vector\n",
>               sve_default_vl);
>  
> +     /* KVM decides whether to support mismatched systems. Just warn here: */
> +     if (sve_max_virtualisable_vl < sve_max_vl)
> +             pr_warn("SVE: unvirtualisable vector lengths present\n");
> +
>       sve_efi_setup();
>  }
>  
> -- 
> 2.1.4
>

Reviewed-by: Andrew Jones <drjo...@redhat.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to