Re: [PATCH v6 01/82] target/arm: Add ID_AA64ZFR0 fields and isar_feature_aa64_sve2

[Peter Maydell]

On Fri, 30 Apr 2021 at 21:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Will be used for SVE2 isa subset enablement.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Do not read zfr0 from kvm unless sve is available.
> ---


> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index dff85f6db9..e8729b53fd 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -567,6 +567,17 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures 
> *ahcf)
>          err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr2,
>                                ARM64_SYS_REG(3, 0, 0, 7, 2));
>
> +        /*
> +         * Before v5.1, KVM did not support SVE and did not expose
> +         * ID_AA64ZFR0_EL1 even as RAZ.  After v5.1, KVM still does
> +         * not expose the register to "user" requests like this
> +         * unless the host supports SVE.
> +         */
> +        if (isar_feature_aa64_sve(&ahcf->isar)) {
> +            err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64zfr0,
> +                                  ARM64_SYS_REG(3, 0, 0, 4, 4));
> +        }
> +

This code is earlier in the function than the place where we
update ahcf->isar to set the "SVE supported bits":

    /* Add feature bits that can't appear until after VCPU init. */
    if (sve_supported) {
        t = ahcf->isar.id_aa64pfr0;
        t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
        ahcf->isar.id_aa64pfr0 = t;
    }

so won't the condition here be always false ?

thanks
-- PMM