Re: [Qemu-devel] [PATCH v2 06/14] target/arm: Allow SVE to be disabled via a CPU property

[Richard Henderson <richard.henderson@linaro.org>]

On 6/21/19 6:34 PM, Andrew Jones wrote:
> Since 97a28b0eeac14 ("target/arm: Allow VFP and Neon to be disabled via
> a CPU property") we can disable the 'max' cpu model's VFP and neon
> features, but there's no way to disable SVE. Add the 'sve=on|off'
> property to give it that flexibility. We also rename
> cpu_max_get/set_sve_vq to cpu_max_get/set_sve_max_vq in order for them
> to follow the typical *_get/set_<property-name> pattern.

I think perhaps the new property should not be overloaded on cpu->sve_max_vq.

At present you are generating an error for

    -cpu max,sve=off,sve_max_vq=2

but not for

    -cpu max,sve_max_vq=2,sve=off

and then there's the issue of

    -cpu max,sve_max_vq=2,sve=off,sve=on

discarding the earlier sve_max_vq setting.


> @@ -1129,6 +1129,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          cpu->isar.mvfr0 = u;
>      }
>  
> +    if (!cpu->sve_max_vq) {
> +        uint64_t t;
> +
> +        t = cpu->isar.id_aa64pfr0;
> +        t = FIELD_DP64(t, ID_AA64PFR0, SVE, 0);
> +        cpu->isar.id_aa64pfr0 = t;
> +    }


I suppse the isar bits are initialized too late for you to be able to re-use
the ID_AA64PFR0.SVE field *as* the property?


>  static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                        uint64_t value)
>  {
> +    ARMCPU *cpu = env_archcpu(env);
>      int cur_el = arm_current_el(env);
> -    int old_len = sve_zcr_len_for_el(env, cur_el);
> -    int new_len;
> +    int old_len, new_len;
> +
> +    assert(cpu->sve_max_vq);

Certainly there's no reason for this assert, given the above.


r~