From patchwork Thu May 9 08:13:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Tokarev X-Patchwork-Id: 242721 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 386D62C00EC for ; Thu, 9 May 2013 18:24:51 +1000 (EST) Received: from localhost ([::1]:45046 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaM9h-0006Fh-D7 for incoming@patchwork.ozlabs.org; Thu, 09 May 2013 04:24:49 -0400 Received: from eggs.gnu.org ([208.118.235.92]:58745) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaM93-00064W-Rw for qemu-devel@nongnu.org; Thu, 09 May 2013 04:24:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UaLyq-0005Mc-7s for qemu-devel@nongnu.org; Thu, 09 May 2013 04:13:43 -0400 Received: from isrv.corpit.ru ([86.62.121.231]:34497) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaLyp-0005M4-LP; Thu, 09 May 2013 04:13:35 -0400 Received: from gandalf.tls.msk.ru (mjt.vpn.tls.msk.ru [192.168.177.99]) by isrv.corpit.ru (Postfix) with ESMTP id 16BC940681; Thu, 9 May 2013 12:13:35 +0400 (MSK) Received: by gandalf.tls.msk.ru (Postfix, from userid 1000) id D5B29478; Thu, 9 May 2013 12:13:34 +0400 (MSK) From: Michael Tokarev To: Peter Maydell Date: Thu, 9 May 2013 12:13:25 +0400 Message-Id: <1368087206-17258-3-git-send-email-mjt@msgid.tls.msk.ru> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1368087206-17258-1-git-send-email-mjt@msgid.tls.msk.ru> References: <1368087206-17258-1-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <518B59C3.6070507@msgid.tls.msk.ru> References: <518B59C3.6070507@msgid.tls.msk.ru> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 86.62.121.231 Cc: Anthony Liguori , qemu-stable@nongnu.org, Michael Tokarev , qemu-devel@nongnu.org, Alexander Graf , Blue Swirl , Paul Brook , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Richard Henderson Subject: [Qemu-devel] [PATCH 3/4] Handle CPU interrupts by inline checking of a flag X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Peter Maydell 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 Reviewed-by: Richard Henderson Signed-off-by: Blue Swirl (cherry picked from commit 378df4b23753a11be650af7664ca76bc75cb9f01) Conflicts: cpu-exec.c exec.c include/qom/cpu.h translate-all.c (original code was in exec.c, changed there) Signed-off-by: Michael Tokarev --- cpu-defs.h | 1 + cpu-exec.c | 25 ++++++++++++++++++++++++- exec.c | 6 +++--- gen-icount.h | 11 +++++++++++ tcg/tcg.h | 5 +++++ 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index f49e950..75c8e40 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -173,6 +173,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/cpu-exec.c b/cpu-exec.c index edef121..3049e46 100644 --- 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; } @@ -578,7 +584,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); @@ -602,6 +621,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 0a67f07..54a37b5 100644 --- a/exec.c +++ b/exec.c @@ -1756,7 +1756,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; } } @@ -1767,7 +1767,7 @@ CPUInterruptHandler cpu_interrupt_handler = tcg_handle_interrupt; void cpu_interrupt(CPUArchState *env, int mask) { env->interrupt_request |= mask; - cpu_unlink_tb(env); + env->tcg_exit_req = 1; } #endif /* CONFIG_USER_ONLY */ @@ -1779,7 +1779,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; } const CPULogItem cpu_log_items[] = { diff --git a/gen-icount.h b/gen-icount.h index 3d5c131..8037e53 100644 --- a/gen-icount.h +++ b/gen-icount.h @@ -4,10 +4,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; @@ -26,6 +34,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 af06eb5..9b16add 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -607,6 +607,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 @@ -623,6 +627,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) \