From 83b3110899144837f784f6590b1f10225d32ff2f Mon Sep 17 00:00:00 2001 From: Miodrag Dinic Date: Wed, 22 Apr 2015 19:50:04 +0200 Subject: [PATCH] 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 | 23 +++++++++++++++++-- exec.c | 22 +----------------- include/exec/exec-all.h | 1 - include/exec/gen-icount.h | 14 +++++++++++- include/qom/cpu.h | 1 + translate-all.c | 48 +-------------------------------------- translate-all.h | 1 - 7 files changed, 37 insertions(+), 73 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 81c5ae95e7c9..fe2606de128f 100644 --- a/cpu-exec.c +++ b/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; diff --git a/exec.c b/exec.c index 627cff5111a5..e926da4bb367 100644 --- a/exec.c +++ b/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, ...) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 6939cb40f3c7..dac736e8b58f 100644 --- a/include/exec/exec-all.h +++ b/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) diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 97f01d1bac95..52e4bf1612e4 100644 --- a/include/exec/gen-icount.h +++ b/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); } } diff --git a/include/qom/cpu.h b/include/qom/cpu.h index dcab7d4ac4ed..a00aae93c43f 100644 --- a/include/qom/cpu.h +++ b/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 */ diff --git a/translate-all.c b/translate-all.c index cd97573510a0..a77b00af5edb 100644 --- a/translate-all.c +++ b/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) diff --git a/translate-all.h b/translate-all.h index 498db1b8ff78..864ae6504c10 100644 --- a/translate-all.h +++ b/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 */