[Qemu-devel] [PATCH] sh4-linux-user: fix multi-threading regression.

[Cédric VINCENT]

This reverts commit fd4bab10 "target-sh4: optimize exceptions": the
function cpu_restore_state() isn't expected to be called in user-mode,
as a consequence it isn't protected from race conditions.  For
information, syscalls are exceptions on Linux/SH4.

There were two possible fixes: either "tb_lock" is acquired/released
around the call to cpu_restore_state() [1] or the commit that
introduced this regression is reverted [2].  The performance impact of
these two approaches were compared with two different tests:

    +-------------------------+--------+-----------------+-------------------+
    |                         | v1.0.1 | [1]      v1.0.1 | [2]        v1.0.1 |
    | Test                    |        | + tb_lock patch | + reverted commit |
    +-------------------------+--------+-----------------+-------------------+
    | bzip2 17MB.mp3 (v1.0.5) |   ~46s |            ~46s |              ~46s |
    | df_dok/X11 (v1.4.12)    |  crash |           ~257s |             ~256s |
    +-------------------------+--------+-----------------+-------------------+

Since I didn't see any performance impact, I prefered not to add extra
calls to spin_lock/spin_unlock that were specific to the user-mode.

Signed-off-by: Cédric VINCENT <address@hidden>
---
 target-sh4/op_helper.c |   22 ++++++++++------------
 target-sh4/translate.c |    5 +++++
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
index b299576..b8b6e4a 100644
--- a/target-sh4/op_helper.c
+++ b/target-sh4/op_helper.c
@@ -84,31 +84,28 @@ void helper_ldtlb(void)
 #endif
 }
 
-static inline void raise_exception(int index, void *retaddr)
-{
-    env->exception_index = index;
-    cpu_restore_state_from_retaddr(retaddr);
-    cpu_loop_exit(env);
-}
-
 void helper_raise_illegal_instruction(void)
 {
-    raise_exception(0x180, GETPC());
+    env->exception_index = 0x180;
+    cpu_loop_exit(env);
 }
 
 void helper_raise_slot_illegal_instruction(void)
 {
-    raise_exception(0x1a0, GETPC());
+    env->exception_index = 0x1a0;
+    cpu_loop_exit(env);
 }
 
 void helper_raise_fpu_disable(void)
 {
-    raise_exception(0x800, GETPC());
+  env->exception_index = 0x800;
+  cpu_loop_exit(env);
 }
 
 void helper_raise_slot_fpu_disable(void)
 {
-    raise_exception(0x820, GETPC());
+  env->exception_index = 0x820;
+  cpu_loop_exit(env);
 }
 
 void helper_debug(void)
@@ -129,7 +126,8 @@ void helper_sleep(uint32_t next_pc)
 void helper_trapa(uint32_t tra)
 {
     env->tra = tra << 2;
-    raise_exception(0x160, GETPC());
+    env->exception_index = 0x160;
+    cpu_loop_exit(env);
 }
 
 void helper_movcal(uint32_t address, uint32_t value)
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index e04a6e0..5f4ad0e 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -466,6 +466,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define CHECK_NOT_DELAY_SLOT \
   if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL))     \
   {                                                           \
+      tcg_gen_movi_i32(cpu_pc, ctx->pc);                      \
       gen_helper_raise_slot_illegal_instruction();            \
       ctx->bstate = BS_EXCP;                                  \
       return;                                                 \
@@ -473,6 +474,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 
 #define CHECK_PRIVILEGED                                        \
   if (IS_USER(ctx)) {                                           \
+      tcg_gen_movi_i32(cpu_pc, ctx->pc);                        \
       if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
          gen_helper_raise_slot_illegal_instruction();           \
       } else {                                                  \
@@ -484,6 +486,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 
 #define CHECK_FPU_ENABLED                                       \
   if (ctx->flags & SR_FD) {                                     \
+      tcg_gen_movi_i32(cpu_pc, ctx->pc);                        \
       if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
           gen_helper_raise_slot_fpu_disable();                  \
       } else {                                                  \
@@ -1384,6 +1387,7 @@ static void _decode_opc(DisasContext * ctx)
        {
            TCGv imm;
            CHECK_NOT_DELAY_SLOT
+           tcg_gen_movi_i32(cpu_pc, ctx->pc);
            imm = tcg_const_i32(B7_0);
            gen_helper_trapa(imm);
            tcg_temp_free(imm);
@@ -1881,6 +1885,7 @@ static void _decode_opc(DisasContext * ctx)
            ctx->opcode, ctx->pc);
     fflush(stderr);
 #endif
+    tcg_gen_movi_i32(cpu_pc, ctx->pc);
     if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
        gen_helper_raise_slot_illegal_instruction();
     } else {
-- 
1.7.5.1