From patchwork Mon Mar 26 13:07:18 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: vincent X-Patchwork-Id: 148771 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 8CC34B6FDD for ; Tue, 27 Mar 2012 03:10:55 +1100 (EST) Received: from localhost ([::1]:43013 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCCIi-0001fZ-9m for incoming@patchwork.ozlabs.org; Mon, 26 Mar 2012 11:57:44 -0400 Received: from eggs.gnu.org ([208.118.235.92]:57986) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SC9hR-00071y-1w for qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:11:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SC9hK-0000zb-AY for qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:11:04 -0400 Received: from eu1sys200aog110.obsmtp.com ([207.126.144.129]:47153) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SC9hK-0000zJ-14 for qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:10:58 -0400 Received: from beta.dmz-eu.st.com ([164.129.1.35]) (using TLSv1) by eu1sys200aob110.postini.com ([207.126.147.11]) with SMTP ID DSNKT3Bq2vv+irvE28Viw3hn95+8GrlpiWtB@postini.com; Mon, 26 Mar 2012 13:10:57 UTC Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 28405173; Mon, 26 Mar 2012 13:10:22 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas1.st.com [10.75.90.14]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id B48FC2253; Mon, 26 Mar 2012 13:10:22 +0000 (GMT) Received: from localhost.localdomain (164.129.122.152) by webmail-eu.st.com (10.75.90.13) with Microsoft SMTP Server (TLS) id 8.3.192.1; Mon, 26 Mar 2012 15:09:27 +0200 From: =?UTF-8?q?C=C3=A9dric=20VINCENT?= To: qemu-devel Date: Mon, 26 Mar 2012 15:07:18 +0200 Message-ID: <1332767238-22730-1-git-send-email-cedric.vincent@st.com> X-Mailer: git-send-email 1.7.5.1 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 207.126.144.129 X-Mailman-Approved-At: Mon, 26 Mar 2012 11:56:19 -0400 Cc: =?UTF-8?q?C=C3=A9dric=20VINCENT?= , Riku Voipio Subject: [Qemu-devel] [PATCH] sh4-linux-user: fix multi-threading regression. 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 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 --- 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 {