diff mbox series

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

Message ID 20210604155312.15902-90-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series arm tcg/kvm refactor and split with kvm only support | expand

Commit Message

Alex Bennée June 4, 2021, 3:53 p.m. UTC
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(-)

Comments

Richard Henderson June 5, 2021, 10:20 p.m. UTC | #1
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~
diff mbox series

Patch

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;
+    }
+    if (!cpu_sve_finalize_features(cpu, &local_err)) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /*
+     * KVM does not support modifications to this feature.
+     * We have not registered the cpu properties when KVM
+     * is in use, so the user will not be able to set them.
+     */
+    if (tcg_enabled()) {
+        if (!cpu_pauth_finalize(cpu, &local_err)) {
             error_propagate(errp, local_err);
             return;
         }
-
-        /*
-         * KVM does not support modifications to this feature.
-         * We have not registered the cpu properties when KVM
-         * is in use, so the user will not be able to set them.
-         */
-        if (tcg_enabled()) {
-            if (!cpu_pauth_finalize(cpu, &local_err)) {
-                error_propagate(errp, local_err);
-                return;
-            }
-        }
-    }
-
-    if (kvm_enabled()) {
+    } else if (kvm_enabled()) {
         kvm_arm_steal_time_finalize(cpu, &local_err);
         if (local_err != NULL) {
             error_propagate(errp, local_err);