[SPARC] Improve "ta 0" shutdown

This patch replace the previous implementation with this simplified and
more complete version (no shutdown when psret == 1).

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 target-sparc/helper.c       |    7 -------
 target-sparc/helper.h       |    1 -
 target-sparc/int32_helper.c |   10 ++++++++--
 target-sparc/translate.c    |    9 +--------
 4 files changed, 9 insertions(+), 18 deletions(-)

On Thu, Nov 3, 2011 at 15:17, Fabien Chouteau <chouteau@adacore.com> wrote:
> This patch replace the previous implementation with this simplified and
> more complete version (no shutdown when psret == 1).

In order to get in 1.0, the change should be limited to adding only a
psret check to current helper_shutdown(). This way it should be also
possible to retain the CPU feature check in translation time instead
of pushing it to do_interrupt path, so it should be a better for
performance anyway.

> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
>  target-sparc/helper.c       |    7 -------
>  target-sparc/helper.h       |    1 -
>  target-sparc/int32_helper.c |   10 ++++++++--
>  target-sparc/translate.c    |    9 +--------
>  4 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index 18609c4..037a72c 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -34,13 +34,6 @@ void helper_debug(CPUState *env)
>     cpu_loop_exit(env);
>  }
>
> -void helper_shutdown(void)
> -{
> -#if !defined(CONFIG_USER_ONLY)
> -    qemu_system_shutdown_request();
> -#endif
> -}
> -
>  #ifdef TARGET_SPARC64
>  target_ulong helper_popc(target_ulong val)
>  {
> diff --git a/target-sparc/helper.h b/target-sparc/helper.h
> index faaf8dc..1f67b08 100644
> --- a/target-sparc/helper.h
> +++ b/target-sparc/helper.h
> @@ -79,7 +79,6 @@ DEF_HELPER_1(fcmpeq_fcc2, void, env)
>  DEF_HELPER_1(fcmpeq_fcc3, void, env)
>  #endif
>  DEF_HELPER_2(raise_exception, void, env, int)
> -DEF_HELPER_0(shutdown, void)
>  #define F_HELPER_0_1(name) DEF_HELPER_1(f ## name, void, env)
>
>  DEF_HELPER_3(faddd, f64, env, f64, f64)
> diff --git a/target-sparc/int32_helper.c b/target-sparc/int32_helper.c
> index 3a749bf..ac9d01e 100644
> --- a/target-sparc/int32_helper.c
> +++ b/target-sparc/int32_helper.c
> @@ -19,6 +19,7 @@
>
>  #include "cpu.h"
>  #include "trace.h"
> +#include "sysemu.h"
>
>  //#define DEBUG_PCALL
>
> @@ -100,8 +101,13 @@ void do_interrupt(CPUState *env)
>  #endif
>  #if !defined(CONFIG_USER_ONLY)
>     if (env->psret == 0) {
> -        cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error state",
> -                  env->exception_index);
> +        if (env->exception_index == 0x80 &&
> +            env->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
> +            qemu_system_shutdown_request();
> +        } else {
> +            cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error state",
> +                      env->exception_index);
> +        }
>         return;
>     }
>  #endif
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 9318540..d261112 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -2518,15 +2518,8 @@ static void disas_sparc_insn(DisasContext * dc)
>                         tcg_gen_andi_tl(cpu_dst, cpu_dst, V8_TRAP_MASK);
>                     tcg_gen_addi_tl(cpu_dst, cpu_dst, TT_TRAP);
>                     tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);
> +                    gen_helper_raise_exception(cpu_env, cpu_tmp32);
>
> -                    if (rs2 == 0 &&
> -                        dc->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
> -
> -                        gen_helper_shutdown();
> -
> -                    } else {
> -                        gen_helper_raise_exception(cpu_env, cpu_tmp32);
> -                    }
>                 } else if (cond != 0) {
>                     TCGv r_cond = tcg_temp_new();
>                     int l1;
> --
> 1.7.4.1
>
>

On 13/11/2011 10:43, Blue Swirl wrote:
> On Thu, Nov 3, 2011 at 15:17, Fabien Chouteau <chouteau@adacore.com> wrote:
>> This patch replace the previous implementation with this simplified and
>> more complete version (no shutdown when psret == 1).
> 
> In order to get in 1.0, the change should be limited to adding only a
> psret check to current helper_shutdown(). This way it should be also
> possible to retain the CPU feature check in translation time instead
> of pushing it to do_interrupt path, so it should be a better for
> performance anyway.
>

The purpose of this feature is to know when we get to this error state
(trap when interrupts disabled), if we abort with an error message or
just shutdown the emulator. In any case it's the end of the emulation,
it happens only once, so performance is not important here.

For 1.0, I can do another intermediate patch.

On Mon, Nov 14, 2011 at 09:37, Fabien Chouteau <chouteau@adacore.com> wrote:
> On 13/11/2011 10:43, Blue Swirl wrote:
>> On Thu, Nov 3, 2011 at 15:17, Fabien Chouteau <chouteau@adacore.com> wrote:
>>> This patch replace the previous implementation with this simplified and
>>> more complete version (no shutdown when psret == 1).
>>
>> In order to get in 1.0, the change should be limited to adding only a
>> psret check to current helper_shutdown(). This way it should be also
>> possible to retain the CPU feature check in translation time instead
>> of pushing it to do_interrupt path, so it should be a better for
>> performance anyway.
>>
>
> The purpose of this feature is to know when we get to this error state
> (trap when interrupts disabled), if we abort with an error message or
> just shutdown the emulator. In any case it's the end of the emulation,
> it happens only once, so performance is not important here.

Right, so I applied this as is, thanks.

> For 1.0, I can do another intermediate patch.
>
> --
> Fabien Chouteau
>