Handle CPU interrupts by inline checking of a flag

Fix some of the nasty TCG race conditions and crashes by implementing
cpu_exit() as setting a flag which is checked at the start of each TB.
This avoids crashes if a thread or signal handler calls cpu_exit()
while the execution thread is itself modifying the TB graph (which
may happen in system emulation mode as well as in linux-user mode
with a multithreaded guest binary).

This fixes the crashes seen in LP:668799; however there are another
class of crashes described in LP:1098729 which stem from the fact
that in linux-user with a multithreaded guest all threads will
use and modify the same global TCG date structures (including the
generated code buffer) without any kind of locking. This means that
multithreaded guest binaries are still in the "unsupported"
category.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
(cherry picked from commit 378df4b23753a11be650af7664ca76bc75cb9f01)

Conflicts:
	exec.c
	include/qom/cpu.h
	translate-all.c
	include/exec/gen-icount.h

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Conflicts:
	cpu-exec.c

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

diff --git a/cpu-exec.c b/cpu-exec.c
index 4ffae22ffe1cb505a1d8bc7a0cf6b0adc1650c04..1c6af242aec310eb496bc4c7930c83b0df5a8e1c 100644 (file)
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -63,6 +63,12 @@ static inline tcg_target_ulong cpu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
         TranslationBlock *tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
         cpu_pc_from_tb(env, tb);
     }
+    if ((next_tb & TB_EXIT_MASK) == TB_EXIT_REQUESTED) {
+        /* We were asked to stop executing TBs (probably a pending
+         * interrupt. We've now stopped, so clear the flag.
+         */
+        env->tcg_exit_req = 0;
+    }
     return next_tb;
 }
 
@@ -606,7 +612,20 @@ int cpu_exec(CPUArchState *env)
                     tc_ptr = tb->tc_ptr;
                     /* execute the generated code */
                     next_tb = cpu_tb_exec(env, tc_ptr);
-                    if ((next_tb & TB_EXIT_MASK) == TB_EXIT_ICOUNT_EXPIRED) {
+                    switch (next_tb & TB_EXIT_MASK) {
+                    case TB_EXIT_REQUESTED:
+                        /* Something asked us to stop executing
+                         * chained TBs; just continue round the main
+                         * loop. Whatever requested the exit will also
+                         * have set something else (eg exit_request or
+                         * interrupt_request) which we will handle
+                         * next time around the loop.
+                         */
+                        tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
+                        next_tb = 0;
+                        break;
+                    case TB_EXIT_ICOUNT_EXPIRED:
+                    {
                         /* Instruction counter expired.  */
                         int insns_left;
                         tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
@@ -630,6 +649,10 @@ int cpu_exec(CPUArchState *env)
                             next_tb = 0;
                             cpu_loop_exit(env);
                         }
+                        break;
+                    }
+                    default:
+                        break;
                     }
                 }
                 env->current_tb = NULL;

diff --git a/exec.c b/exec.c
index b85508ba30d4d5d3714e0eae5f38c2d16e570f25..371713ae995247a077c013ba32b29a4313790a4c 100644 (file)
--- a/exec.c
+++ b/exec.c
@@ -493,7 +493,7 @@ void cpu_reset_interrupt(CPUArchState *env, int mask)
 void cpu_exit(CPUArchState *env)
 {
     env->exit_request = 1;
-    cpu_unlink_tb(env);
+    env->tcg_exit_req = 1;
 }
 
 void cpu_abort(CPUArchState *env, const char *fmt, ...)

diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 2911b9fc908aa792125374371d94a931f3bddb70..07fce6957e9b8624e70c79bb15268e5d07af28f6 100644 (file)
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -161,6 +161,7 @@ typedef struct CPUWatchpoint {
     uint32_t halted; /* Nonzero if the CPU is in suspend state */       \
     uint32_t interrupt_request;                                         \
     volatile sig_atomic_t exit_request;                                 \
+    volatile sig_atomic_t tcg_exit_req;                                 \
     CPU_COMMON_TLB                                                      \
     struct TranslationBlock *tb_jmp_cache[TB_JMP_CACHE_SIZE];           \
     /* buffer for temporaries in the code generator */                  \

diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index c858a738c0b885157cf80ca548f56d1a85670bda..f45f975718bd4ba75bc63fe50f92c7477db22238 100644 (file)
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -7,10 +7,18 @@
 
 static TCGArg *icount_arg;
 static int icount_label;
+static int exitreq_label;
 
 static inline void gen_icount_start(void)
 {
     TCGv_i32 count;
+    TCGv_i32 flag;
+
+    exitreq_label = gen_new_label();
+    flag = tcg_temp_local_new_i32();
+    tcg_gen_ld_i32(flag, cpu_env, offsetof(CPUArchState, tcg_exit_req));
+    tcg_gen_brcondi_i32(TCG_COND_NE, flag, 0, exitreq_label);
+    tcg_temp_free_i32(flag);
 
     if (!use_icount)
         return;
@@ -29,6 +37,9 @@ static inline void gen_icount_start(void)
 
 static void gen_icount_end(TranslationBlock *tb, int num_insns)
 {
+    gen_set_label(exitreq_label);
+    tcg_gen_exit_tb((tcg_target_long)tb + TB_EXIT_REQUESTED);
+
     if (use_icount) {
         *icount_arg = num_insns;
         gen_set_label(icount_label);

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 10eb3f43f842c78b097108e9aa5e5568c9d9a74b..34b2ca8773ddec04523a1bd5383a7da854cee094 100644 (file)
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -686,6 +686,10 @@ extern uint8_t *code_gen_prologue;
  *        would hit zero midway through it. In this case the next-TB pointer
  *        returned is the TB we were about to execute, and the caller must
  *        arrange to execute the remaining count of instructions.
+ *  3:    we stopped because the CPU's exit_request flag was set
+ *        (usually meaning that there is an interrupt that needs to be
+ *        handled). The next-TB pointer returned is the TB we were
+ *        about to execute when we noticed the pending exit request.
  *
  * If the bottom two bits indicate an exit-via-index then the CPU
  * state is correctly synchronised and ready for execution of the next
@@ -702,6 +706,7 @@ extern uint8_t *code_gen_prologue;
 #define TB_EXIT_IDX0 0
 #define TB_EXIT_IDX1 1
 #define TB_EXIT_ICOUNT_EXPIRED 2
+#define TB_EXIT_REQUESTED 3
 
 #if !defined(tcg_qemu_tb_exec)
 # define tcg_qemu_tb_exec(env, tb_ptr) \

diff --git a/translate-all.c b/translate-all.c
index bf1db093c9914a4c5bdc51b14f0fa64c1871f8d2..1288b2ac054f0f453ae72a7154990f81e49ac6a8 100644 (file)
--- a/translate-all.c
+++ b/translate-all.c
@@ -1476,7 +1476,7 @@ static void tcg_handle_interrupt(CPUArchState *env, int mask)
             cpu_abort(env, "Raised interrupt while not in I/O function");
         }
     } else {
-        cpu_unlink_tb(env);
+        env->tcg_exit_req = 1;
     }
 }
 
@@ -1617,7 +1617,7 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
 void cpu_interrupt(CPUArchState *env, int mask)
 {
     env->interrupt_request |= mask;
-    cpu_unlink_tb(env);
+    env->tcg_exit_req = 1;
 }
 
 /*