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.

This commit reflects following two commits from QEMU ToT:

Main part of the solution:
378df4b Handle CPU interrupts by inline checking of a flag
Code cleanup:
3a808cc translate-all.c: Remove cpu_unlink_tb()

Change-Id: I88d900d2919b00fd4b5cea3c916fb5cdcbaf036c

cpu-exec.c

@@ -594,10 +594,25 @@ int cpu_exec(CPUOldState *env)
                     tc_ptr = tb->tc_ptr;
                 /* execute the generated code */
                     next_tb = tcg_qemu_tb_exec(env, tc_ptr);
-                    if ((next_tb & 3) == 2) {
+                    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.
+                         */
+                        cpu->tcg_exit_req = 0;
+                        tb = (TranslationBlock *)(intptr_t)(next_tb & ~TB_EXIT_MASK);
+                        cpu_pc_from_tb(env, tb);
+                        next_tb = 0;
+                        break;
+                    case TB_EXIT_ICOUNT_EXPIRED:
+                    {
                         /* Instruction counter expired.  */
                         int insns_left;
-                        tb = (TranslationBlock *)(intptr_t)(next_tb & ~3);
+                        tb = (TranslationBlock *)(intptr_t)(next_tb & ~TB_EXIT_MASK);
                         /* Restore PC.  */
                         cpu_pc_from_tb(env, tb);
                         insns_left = env->icount_decr.u32;
@@ -620,6 +635,10 @@ int cpu_exec(CPUOldState *env)
                             next_tb = 0;
                             cpu_loop_exit(env);
                         }
+                        break;
+                    }
+                    default:
+                        break;
                     }
                 }
                 env->current_tb = NULL;

exec.c

@@ -437,26 +437,6 @@ void cpu_set_log_filename(const char *filename)
     cpu_set_log(loglevel);
 }
 
-void cpu_unlink_tb(CPUOldState *env)
-{
-    /* FIXME: TB unchaining isn't SMP safe.  For now just ignore the
-       problem and hope the cpu will stop of its own accord.  For userspace
-       emulation this often isn't actually as bad as it sounds.  Often
-       signals are used primarily to interrupt blocking syscalls.  */
-    TranslationBlock *tb;
-    static spinlock_t interrupt_lock = SPIN_LOCK_UNLOCKED;
-
-    spin_lock(&interrupt_lock);
-    tb = env->current_tb;
-    /* if the cpu is currently executing code, we must unlink it and
-       all the potentially executing TB */
-    if (tb) {
-        env->current_tb = NULL;
-        tb_reset_jump_recursive(tb);
-    }
-    spin_unlock(&interrupt_lock);
-}
-
 void cpu_reset_interrupt(CPUState *cpu, int mask)
 {
     cpu->interrupt_request &= ~mask;
@@ -465,7 +445,7 @@ void cpu_reset_interrupt(CPUState *cpu, int mask)
 void cpu_exit(CPUState *cpu)
 {
     cpu->exit_request = 1;
-    cpu_unlink_tb(cpu->env_ptr);
+    cpu->tcg_exit_req = 1;
 }
 
 void cpu_abort(CPUArchState *env, const char *fmt, ...)

include/exec/exec-all.h

@@ -105,7 +105,6 @@ void tlb_flush(CPUArchState *env, int flush_global);
 void tlb_set_page(CPUArchState *env, target_ulong vaddr,
                   hwaddr paddr, int prot,
                   int mmu_idx, target_ulong size);
-void tb_reset_jump_recursive(TranslationBlock *tb);
 void tb_invalidate_phys_addr(hwaddr addr);
 #else
 static inline void tlb_flush_page(CPUArchState *env, target_ulong addr)

include/exec/gen-icount.h

@@ -7,10 +7,19 @@
 
 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(CPUState, tcg_exit_req) - ENV_OFFSET);
+    tcg_gen_brcondi_i32(TCG_COND_NE, flag, 0, exitreq_label);
+    tcg_temp_free_i32(flag);
 
     if (!use_icount)
         return;
@@ -29,10 +38,13 @@ 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((uintptr_t)tb + TB_EXIT_REQUESTED);
+
     if (use_icount) {
         *icount_arg = num_insns;
         gen_set_label(icount_label);
-        tcg_gen_exit_tb((uintptr_t)tb + 2);
+        tcg_gen_exit_tb((uintptr_t)tb + TB_EXIT_ICOUNT_EXPIRED);
     }
 }
 

include/qom/cpu.h

@@ -69,6 +69,7 @@ struct CPUState {
     uint32_t stopped; /* Artificially stopped */
 
     volatile sig_atomic_t exit_request;
+    volatile sig_atomic_t tcg_exit_req;
     uint32_t interrupt_request;
 
     void *env_ptr; /* CPUArchState */

translate-all.c

@@ -1446,56 +1446,10 @@ void cpu_interrupt(CPUState *cpu, int mask)
             cpu_abort(env, "Raised interrupt while not in I/O function");
         }
     } else {
-        // cpu->tcg_exit_req = 1;
-        cpu_unlink_tb(env);
+        cpu->tcg_exit_req = 1;
     }
 }
 
-static inline void tb_reset_jump_recursive2(TranslationBlock *tb, int n)
-{
-    TranslationBlock *tb1, *tb_next, **ptb;
-    unsigned int n1;
-
-    tb1 = tb->jmp_next[n];
-    if (tb1 != NULL) {
-        /* find head of list */
-        for(;;) {
-            n1 = (uintptr_t)tb1 & 3;
-            tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3);
-            if (n1 == 2)
-                break;
-            tb1 = tb1->jmp_next[n1];
-        }
-        /* we are now sure now that tb jumps to tb1 */
-        tb_next = tb1;
-
-        /* remove tb from the jmp_first list */
-        ptb = &tb_next->jmp_first;
-        for(;;) {
-            tb1 = *ptb;
-            n1 = (uintptr_t)tb1 & 3;
-            tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3);
-            if (n1 == n && tb1 == tb)
-                break;
-            ptb = &tb1->jmp_next[n1];
-        }
-        *ptb = tb->jmp_next[n];
-        tb->jmp_next[n] = NULL;
-
-        /* suppress the jump to next tb in generated code */
-        tb_reset_jump(tb, n);
-
-        /* suppress jumps in the tb on which we could have jumped */
-        tb_reset_jump_recursive(tb_next);
-    }
-}
-
-void tb_reset_jump_recursive(TranslationBlock *tb)
-{
-    tb_reset_jump_recursive2(tb, 0);
-    tb_reset_jump_recursive2(tb, 1);
-}
-
 /* in deterministic execution mode, instructions doing device I/Os
    must be at the end of the TB */
 void cpu_io_recompile(CPUArchState *env, uintptr_t retaddr)

translate-all.h

@@ -31,7 +31,6 @@
 /* translate-all.c */
 bool tcg_enabled(void);
 void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
-void cpu_unlink_tb(CPUOldState *cpu);
 void tb_check_watchpoint(CPUArchState *env);
 
 #endif /* TRANSLATE_ALL_H */