diff mbox series

[3/5] target/arm: wrap semihosting and psci calls with tcg_enabled

Message ID 20221216212944.28229-4-farosas@suse.de (mailing list archive)
State New, archived
Headers show
Series target/arm: Some CONFIG_TCG code movement | expand

Commit Message

Fabiano Rosas Dec. 16, 2022, 9:29 p.m. UTC 
From: Claudio Fontana <cfontana@suse.de>

for "all" builds (tcg + kvm), we want to avoid doing
the psci and semihosting checks if tcg is built-in, but not enabled.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
Originally from:
[RFC v14 39/80] target/arm: replace CONFIG_TCG with tcg_enabled
https://lore.kernel.org/r/20210416162824.25131-40-cfontana@suse.de
---
 target/arm/helper.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

Comments

Richard Henderson Dec. 17, 2022, 12:13 a.m. UTC | #1 
On 12/16/22 13:29, Fabiano Rosas wrote:
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> +    if (tcg_enabled()) {
> +        if (arm_is_psci_call(cpu, cs->exception_index)) {

This could be

     if (tcg_enabled() && arm_is_psci_call(...))

because...

> -    /*
> -     * Semihosting semantics depend on the register width of the code
> -     * that caused the exception, not the target exception level, so
> -     * must be handled here.
> -     */
> +        /*
> +         * Semihosting semantics depend on the register width of the code
> +         * that caused the exception, not the target exception level, so
> +         * must be handled here.
> +         */
>   #ifdef CONFIG_TCG
> -    if (cs->exception_index == EXCP_SEMIHOST) {
> -        tcg_handle_semihosting(cs);
> -        return;
> -    }
> +        if (cs->exception_index == EXCP_SEMIHOST) {

If you were able to replace the ifdef, that would be one thing, but since you aren't I 
don't think this requires a separate check.  There is no way to generate EXCP_SEMIHOST 
except via TCG.

I guess I don't insist, since you're working toward Claudio's much larger patch set, so

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Fabiano Rosas Dec. 19, 2022, 11:54 a.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 12/16/22 13:29, Fabiano Rosas wrote:
>> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
>> -        arm_handle_psci_call(cpu);
>> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
>> -        return;
>> -    }
>> +    if (tcg_enabled()) {
>> +        if (arm_is_psci_call(cpu, cs->exception_index)) {
>
> This could be
>
>      if (tcg_enabled() && arm_is_psci_call(...))
>
> because...
>
>> -    /*
>> -     * Semihosting semantics depend on the register width of the code
>> -     * that caused the exception, not the target exception level, so
>> -     * must be handled here.
>> -     */
>> +        /*
>> +         * Semihosting semantics depend on the register width of the code
>> +         * that caused the exception, not the target exception level, so
>> +         * must be handled here.
>> +         */
>>   #ifdef CONFIG_TCG
>> -    if (cs->exception_index == EXCP_SEMIHOST) {
>> -        tcg_handle_semihosting(cs);
>> -        return;
>> -    }
>> +        if (cs->exception_index == EXCP_SEMIHOST) {
>
> If you were able to replace the ifdef, that would be one thing, but since you aren't I 
> don't think this requires a separate check.  There is no way to generate EXCP_SEMIHOST 
> except via TCG.

Right, I had to keep the ifdef that was being removed in the original
patch because tcg_handle_semihosting had moved elsewhere in Claudio's
series.

>
> I guess I don't insist, since you're working toward Claudio's much larger patch set, so
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

If you don't mind I'll leave like this then, unless this comes to a v2.

Thank you
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 45bf164a07..9d0a53cb00 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -26,6 +26,7 @@ 
 #include "sysemu/cpus.h"
 #include "sysemu/cpu-timers.h"
 #include "sysemu/kvm.h"
+#include "sysemu/tcg.h"
 #include "qemu/range.h"
 #include "qapi/qapi-commands-machine-target.h"
 #include "qapi/error.h"
@@ -10300,23 +10301,25 @@  void arm_cpu_do_interrupt(CPUState *cs)
                       env->exception.syndrome);
     }
 
-    if (arm_is_psci_call(cpu, cs->exception_index)) {
-        arm_handle_psci_call(cpu);
-        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
-        return;
-    }
+    if (tcg_enabled()) {
+        if (arm_is_psci_call(cpu, cs->exception_index)) {
+            arm_handle_psci_call(cpu);
+            qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
+            return;
+        }
 
-    /*
-     * Semihosting semantics depend on the register width of the code
-     * that caused the exception, not the target exception level, so
-     * must be handled here.
-     */
+        /*
+         * Semihosting semantics depend on the register width of the code
+         * that caused the exception, not the target exception level, so
+         * must be handled here.
+         */
 #ifdef CONFIG_TCG
-    if (cs->exception_index == EXCP_SEMIHOST) {
-        tcg_handle_semihosting(cs);
-        return;
-    }
+        if (cs->exception_index == EXCP_SEMIHOST) {
+            tcg_handle_semihosting(cs);
+            return;
+        }
 #endif
+    }
 
     /* Hooks may change global state so BQL should be held, also the
      * BQL needs to be held for any modification of