Patchwork [2/2,SPARC] Fix TA0_Shutdown feature

login
register
mail settings
Submitter Julien Grall
Date May 17, 2011, 3:32 p.m.
Message ID <BANLkTi=Bv=+6QgcHC0=jGiRFOc3D3ug2JA@mail.gmail.com>
Download mbox | patch
Permalink /patch/95949/
State New
Headers show

Comments

Julien Grall - May 17, 2011, 3:32 p.m.
Fix TA0_SHUTDOWN feature

Signed-off-by: Julien Grall <julien.grall@gmail.com>
---
 target-sparc/op_helper.c |   13 +++++++++++--
 target-sparc/translate.c |    9 +--------
 2 files changed, 12 insertions(+), 10 deletions(-)
Blue Swirl - May 17, 2011, 7:57 p.m.
On Tue, May 17, 2011 at 6:32 PM, Julien Grall <julien.grall@gmail.com> wrote:
> Fix TA0_SHUTDOWN feature

But what would be the bug?

> Signed-off-by: Julien Grall <julien.grall@gmail.com>
> ---
>  target-sparc/op_helper.c |   13 +++++++++++--
>  target-sparc/translate.c |    9 +--------
>  2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index a6fabad..cb775f5 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -326,8 +326,17 @@ void HELPER(raise_exception)(int tt)
>
>  void HELPER(trap_always)(int tt)
>  {
> -    env->exception_index = tt;
> -    do_interrupt(env);
> +    if (tt == TT_TRAP
> +        && env->def->features & CPU_FEATURE_TA0_SHUTDOWN
> +#ifndef TARGET_SPARC64
> +        && env->psret == 0
> +#endif
> +        ) {
> +        helper_shutdown();
> +    } else {
> +        env->exception_index = tt;
> +        do_interrupt(env);
> +    }
>  }
>
>  void helper_shutdown(void)
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index b30003b..a47a2de 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -2009,14 +2009,7 @@ static void disas_sparc_insn(DisasContext * dc)
>                     tcg_gen_addi_tl(cpu_dst, cpu_dst, TT_TRAP);
>                     tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);
>
> -                    if (rs2 == 0 &&
> -                        dc->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
> -
> -                        gen_helper_shutdown();
> -
> -                    } else {
> -                        gen_helper_trap_always(cpu_tmp32);
> -                    }
> +                    gen_helper_trap_always(cpu_tmp32);

No, this would actually be just opposite to how QEMU works.
Performance comes from doing more work at translation time in order to
save time during executing the generated code.
Julien Grall - May 18, 2011, 1:17 p.m.
On Tue, May 17, 2011 at 9:57 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, May 17, 2011 at 6:32 PM, Julien Grall <julien.grall@gmail.com> wrote:
>> Fix TA0_SHUTDOWN feature
>
> But what would be the bug?

We try to add RTEMS's support on leon platform for QEMU. RTEMS uses
software trap 0 for various syscall.
The current implementation of TA0_SHUTDOWN breaks the RTEMS boot.

> No, this would actually be just opposite to how QEMU works.
> Performance comes from doing more work at translation time in order to
> save time during executing the generated code.

I agree, this solution slows down the execution, but I didn't find
another solution to fix this bug.
Blue Swirl - May 20, 2011, 4:03 p.m.
On Wed, May 18, 2011 at 4:17 PM, Julien Grall <julien.grall@gmail.com> wrote:
> On Tue, May 17, 2011 at 9:57 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, May 17, 2011 at 6:32 PM, Julien Grall <julien.grall@gmail.com> wrote:
>>> Fix TA0_SHUTDOWN feature
>>
>> But what would be the bug?
>
> We try to add RTEMS's support on leon platform for QEMU. RTEMS uses
> software trap 0 for various syscall.
> The current implementation of TA0_SHUTDOWN breaks the RTEMS boot.
>
>> No, this would actually be just opposite to how QEMU works.
>> Performance comes from doing more work at translation time in order to
>> save time during executing the generated code.
>
> I agree, this solution slows down the execution, but I didn't find
> another solution to fix this bug.

You should retain the feature check in translate.c but add a leon
specific helper in place of helper_shutdown, which can check for
psret.

It would be also possible to generate code for the psret check if the
helper overhead were an issue.

Patch

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index a6fabad..cb775f5 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -326,8 +326,17 @@  void HELPER(raise_exception)(int tt)

 void HELPER(trap_always)(int tt)
 {
-    env->exception_index = tt;
-    do_interrupt(env);
+    if (tt == TT_TRAP
+        && env->def->features & CPU_FEATURE_TA0_SHUTDOWN
+#ifndef TARGET_SPARC64
+        && env->psret == 0
+#endif
+        ) {
+        helper_shutdown();
+    } else {
+        env->exception_index = tt;
+        do_interrupt(env);
+    }
 }

 void helper_shutdown(void)
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index b30003b..a47a2de 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2009,14 +2009,7 @@  static void disas_sparc_insn(DisasContext * dc)
                     tcg_gen_addi_tl(cpu_dst, cpu_dst, TT_TRAP);
                     tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);

-                    if (rs2 == 0 &&
-                        dc->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
-
-                        gen_helper_shutdown();
-
-                    } else {
-                        gen_helper_trap_always(cpu_tmp32);
-                    }
+                    gen_helper_trap_always(cpu_tmp32);
                 } else if (cond != 0) {
                     TCGv r_cond = tcg_temp_new();
                     int l1;