Re: [PATCH v16 89/99] target/arm: cpu64: some final cleanup on aarch64_cpu_finalize_features

[Richard Henderson]

On 6/4/21 8:53 AM, Alex Bennée wrote:
> From: Claudio Fontana <cfontana@suse.de>
>
> bail out immediately if ARM_FEATURE_AARCH64 is not set,
> and add an else statement when checking for accelerators.
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   target/arm/cpu64.c | 33 ++++++++++++++++-----------------
>   1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 3058e2c273..ecce8c4308 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -473,26 +473,25 @@ void aarch64_cpu_finalize_features(ARMCPU *cpu, Error 
> **errp)
>   {
>       Error *local_err = NULL;
>   
> -    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> -        if (!cpu_sve_finalize_features(cpu, &local_err)) {
> +    if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> +        return;
> +    }

I'm not sure this is correct, either before or after.
What about nonsensical combinations such as

  -cpu max,aarch64=off,sve-vq-max=4

Don't we want cpu_sve_finalize_features and friends to produce an error about 
enabling sve without aarch64.

r~