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 <cfont...@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 <cfont...@suse.de>
Signed-off-by: Alex Bennée <alex.ben...@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~