Re: [PATCH v2 3/3] target/arm: Add sve-default-vector-length cpu property

[Andrew Jones <drjones@redhat.com>]

On Fri, Jul 23, 2021 at 10:33:44AM -1000, Richard Henderson wrote:
> Mirror the behavour of /proc/sys/abi/sve_default_vector_length
> under the real linux kernel.  We have no way of passing along
> a real default across exec like the kernel can, but this is a
> decent way of adjusting the startup vector length of a process.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/482
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  docs/system/arm/cpu-features.rst | 11 ++++++
>  target/arm/cpu.h                 |  5 +++
>  target/arm/cpu.c                 | 14 ++++++--
>  target/arm/cpu64.c               | 60 ++++++++++++++++++++++++++++++++
>  4 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
> index c455442eaf..4ff36cc83f 100644
> --- a/docs/system/arm/cpu-features.rst
> +++ b/docs/system/arm/cpu-features.rst
> @@ -376,3 +376,14 @@ verbose command lines.  However, the recommended way to select vector
>  lengths is to explicitly enable each desired length.  Therefore only
>  example's (1), (4), and (6) exhibit recommended uses of the properties.
>  
> +SVE User-mode Default Vector Length Property
> +--------------------------------------------
> +
> +For qemu-aarch64, the cpu property `sve-default-vector-length=N` is
> +defined to mirror the Linux kernel parameter file
> +`/proc/sys/abi/sve_default_vector_length`.  The default length, `N`,
> +is in units of bytes and must be between 16 and 8192.  

Hmm. If a user inputs anything greater than 256, then won't it get
silently reduced to 256?

> +If not specified, the default vector length is 64.
> +
> +If the default length is larger than the maximum vector length enabled
> +with `sve<N>` properties, the actual vector length will be reduced.

Here it's pointed out that the default may be reduced, but it implies that
that only happens if an sve<N> property is also given. Won't users wonder
why they're only getting vectors that are 256 bytes large even when they
ask for more?

Thanks,
drew

> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index be9a4dceae..9f0a5f84d5 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1006,6 +1006,11 @@ struct ARMCPU {
>      /* Used to set the maximum vector length the cpu will support.  */
>      uint32_t sve_max_vq;
>  
> +#ifdef CONFIG_USER_ONLY
> +    /* Used to set the default vector length at process start. */
> +    uint32_t sve_default_vq;
> +#endif
> +
>      /*
>       * In sve_vq_map each set bit is a supported vector length of
>       * (bit-number + 1) * 16 bytes, i.e. each bit number + 1 is the vector
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 752b15bb79..2866dd7658 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -201,7 +201,8 @@ static void arm_cpu_reset(DeviceState *dev)
>          env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 16, 2, 3);
>          /* with reasonable vector length */
>          if (cpu_isar_feature(aa64_sve, cpu)) {
> -            env->vfp.zcr_el[1] = MIN(cpu->sve_max_vq - 1, 3);
> +            env->vfp.zcr_el[1] =
> +                aarch64_sve_zcr_get_valid_len(cpu, cpu->sve_default_vq - 1);
>          }
>          /*
>           * Enable TBI0 but not TBI1.
> @@ -1051,7 +1052,16 @@ static void arm_cpu_initfn(Object *obj)
>      QLIST_INIT(&cpu->pre_el_change_hooks);
>      QLIST_INIT(&cpu->el_change_hooks);
>  
> -#ifndef CONFIG_USER_ONLY
> +#ifdef CONFIG_USER_ONLY
> +# ifdef TARGET_AARCH64
> +    /*
> +     * The linux kernel defaults to 512-bit vectors, when sve is supported.
> +     * See documentation for /proc/sys/abi/sve_default_vector_length, and
> +     * our corresponding sve-default-vector-length cpu property.
> +     */
> +    cpu->sve_default_vq = 4;
> +# endif
> +#else
>      /* Our inbound IRQ and FIQ lines */
>      if (kvm_enabled()) {
>          /* VIRQ and VFIQ are unused with KVM but we add them to maintain
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index c7a1626bec..c690318a9b 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -559,6 +559,59 @@ static void cpu_arm_set_sve(Object *obj, bool value, Error **errp)
>      cpu->isar.id_aa64pfr0 = t;
>  }
>  
> +#ifdef CONFIG_USER_ONLY
> +/* Mirror linux /proc/sys/abi/sve_default_vector_length. */
> +static void cpu_arm_set_sve_default_vec_len(Object *obj, Visitor *v,
> +                                            const char *name, void *opaque,
> +                                            Error **errp)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +    int32_t default_len, default_vq, remainder;
> +
> +    if (!visit_type_int32(v, name, &default_len, errp)) {
> +        return;
> +    }
> +
> +    /* Undocumented, but the kernel allows -1 to indicate "maximum". */
> +    if (default_len == -1) {
> +        cpu->sve_default_vq = ARM_MAX_VQ;
> +        return;
> +    }
> +
> +    default_vq = default_len / 16;
> +    remainder = default_len % 16;
> +
> +    /*
> +     * Note that the 512 max comes from include/uapi/asm/sve_context.h
> +     * and is the maximum architectural width of ZCR_ELx.LEN.
> +     */
> +    if (remainder || default_vq < 1 || default_vq > 512) {
> +        error_setg(errp, "cannot set sve-default-vector-length");
> +        if (remainder) {
> +            error_append_hint(errp, "Vector length not a multiple of 16\n");
> +        } else if (default_vq < 1) {
> +            error_append_hint(errp, "Vector length smaller than 16\n");
> +        } else {
> +            error_append_hint(errp, "Vector length larger than %d\n",
> +                              512 * 16);
> +        }
> +        return;
> +    }
> +
> +    cpu->sve_default_vq = default_vq;
> +}
> +
> +static void cpu_arm_get_sve_default_vec_len(Object *obj, Visitor *v,
> +                                            const char *name, void *opaque,
> +                                            Error **errp)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +    int32_t value = cpu->sve_default_vq * 16;
> +
> +    visit_type_int32(v, name, &value, errp);
> +}
> +#endif
> +
>  void aarch64_add_sve_properties(Object *obj)
>  {
>      uint32_t vq;
> @@ -571,6 +624,13 @@ void aarch64_add_sve_properties(Object *obj)
>          object_property_add(obj, name, "bool", cpu_arm_get_sve_vq,
>                              cpu_arm_set_sve_vq, NULL, NULL);
>      }
> +
> +#ifdef CONFIG_USER_ONLY
> +    /* Mirror linux /proc/sys/abi/sve_default_vector_length. */
> +    object_property_add(obj, "sve-default-vector-length", "int32",
> +                        cpu_arm_get_sve_default_vec_len,
> +                        cpu_arm_set_sve_default_vec_len, NULL, NULL);
> +#endif
>  }
>  
>  void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
> -- 
> 2.25.1
> 
>