From patchwork Mon Nov 9 19:37:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Fedorov X-Patchwork-Id: 541968 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id CA71F1413B9 for ; Tue, 10 Nov 2015 06:38:37 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=OA/w44Pu; dkim-atps=neutral Received: from localhost ([::1]:55207 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvsGx-0006vY-QC for incoming@patchwork.ozlabs.org; Mon, 09 Nov 2015 14:38:35 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvsGH-00068n-Bt for qemu-devel@nongnu.org; Mon, 09 Nov 2015 14:37:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZvsGD-0003cb-Ad for qemu-devel@nongnu.org; Mon, 09 Nov 2015 14:37:53 -0500 Received: from mail-lf0-x236.google.com ([2a00:1450:4010:c07::236]:33759) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvsGD-0003cO-0T for qemu-devel@nongnu.org; Mon, 09 Nov 2015 14:37:49 -0500 Received: by lffz63 with SMTP id z63so36249945lff.0 for ; Mon, 09 Nov 2015 11:37:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=zgUVHhfw2rLVCl8+l8nsQrQvr4eRP/ysfigvDkoeh8M=; b=OA/w44Pu/W6Pn0F03a8j2BermkV5vMlekZM5HUT9v7CtjKAOhjkUHOaJ872glns4Mj x6Unxe0AmsVW00HaB4KVse4bTX40Gq3WinVggG2CDs9isU7EJNkq/V6ClARwO3EEPVfG EjA72UccFiLXdrFvcj5V+WUjQ9pGqQq/E5Jyc5TG5GnfWY5jBtBGGxMGgdKASeov7djb JT2SCmQ60KBfDaOzd7NKtvqSLgHg8nZBoKuL2p9GDXQhU3dZIb2X3nn3zikxsIHQ8inD THsTWNQlF/7M92+SznCuXpj3urefAM0Sh9K5+b5pq/9Wbcyc950wb0VCpRzBAW4yusw3 t6nQ== X-Received: by 10.25.44.15 with SMTP id s15mr9056274lfs.37.1447097868106; Mon, 09 Nov 2015 11:37:48 -0800 (PST) Received: from sfedorov-laptop.smware.local ([213.243.91.10]) by smtp.gmail.com with ESMTPSA id um1sm2571865lbb.23.2015.11.09.11.37.46 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 09 Nov 2015 11:37:46 -0800 (PST) From: Sergey Fedorov To: qemu-devel@nongnu.org Date: Mon, 9 Nov 2015 22:37:39 +0300 Message-Id: <1447097859-586-1-git-send-email-serge.fdrv@gmail.com> X-Mailer: git-send-email 1.9.1 X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4010:c07::236 Cc: Sergey Fedorov , Peter Maydell Subject: [Qemu-devel] [PATCH v2] target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code 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 AArch32 translation code does not distinguish between DISAS_UPDATE and DISAS_JUMP. Thus, we cannot use any of them without first updating PC in CPU state. Furthermore, it is too complicated to update PC in CPU state before PC gets updated in disas context. So it is hardly possible to correctly end TB early if is is not likely to be executed before calling disas_*_insn(), e.g. just after calling breakpoint check helper. Modify DISAS_UPDATE and DISAS_JUMP usage in AArch32 translation and apply to them the same semantic as AArch64 translation does: - DISAS_UPDATE: update PC in CPU state when finishing translation - DISAS_JUMP: preserve current PC value in CPU state when finishing translation This patch fixes a bug in AArch32 breakpoint handling: when check_breakpoints helper does not generate an exception, ending the TB early with DISAS_UPDATE couldn't update PC in CPU state and execution hangs. Signed-off-by: Sergey Fedorov --- Though I don't clearly understand how singlestepping is done here, I just do what Peter suggested in his commnets for v1 and send this patch for review. I'm going to get into this while the patch is in review process... Notes: Changes in v2: * 'default' label moved to right place * DISAS_EXC used after gen_exception_internal() * DISAS_UPDATE handling fixed for singlestepping * Breakpoint handling bug to fix by this patch mentioned in commit message target-arm/translate.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index ff262a2..a56f7fe 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -870,7 +870,7 @@ static inline void gen_bx_im(DisasContext *s, uint32_t addr) { TCGv_i32 tmp; - s->is_jmp = DISAS_UPDATE; + s->is_jmp = DISAS_JUMP; if (s->thumb != (addr & 1)) { tmp = tcg_temp_new_i32(); tcg_gen_movi_i32(tmp, addr & 1); @@ -883,7 +883,7 @@ static inline void gen_bx_im(DisasContext *s, uint32_t addr) /* Set PC and Thumb state from var. var is marked as dead. */ static inline void gen_bx(DisasContext *s, TCGv_i32 var) { - s->is_jmp = DISAS_UPDATE; + s->is_jmp = DISAS_JUMP; tcg_gen_andi_i32(cpu_R[15], var, ~1); tcg_gen_andi_i32(var, var, 1); store_cpu_field(var, thumb); @@ -1062,7 +1062,7 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp, static inline void gen_lookup_tb(DisasContext *s) { tcg_gen_movi_i32(cpu_R[15], s->pc & ~1); - s->is_jmp = DISAS_UPDATE; + s->is_jmp = DISAS_JUMP; } static inline void gen_add_data_offset(DisasContext *s, unsigned int insn, @@ -4096,7 +4096,7 @@ static void gen_exception_return(DisasContext *s, TCGv_i32 pc) tmp = load_cpu_field(spsr); gen_set_cpsr(tmp, CPSR_ERET_MASK); tcg_temp_free_i32(tmp); - s->is_jmp = DISAS_UPDATE; + s->is_jmp = DISAS_JUMP; } /* Generate a v6 exception return. Marks both values as dead. */ @@ -4105,7 +4105,7 @@ static void gen_rfe(DisasContext *s, TCGv_i32 pc, TCGv_i32 cpsr) gen_set_cpsr(cpsr, CPSR_ERET_MASK); tcg_temp_free_i32(cpsr); store_reg(s, 15, pc); - s->is_jmp = DISAS_UPDATE; + s->is_jmp = DISAS_JUMP; } static void gen_nop_hint(DisasContext *s, int val) @@ -9035,7 +9035,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) tmp = load_cpu_field(spsr); gen_set_cpsr(tmp, CPSR_ERET_MASK); tcg_temp_free_i32(tmp); - s->is_jmp = DISAS_UPDATE; + s->is_jmp = DISAS_JUMP; } } break; @@ -11355,7 +11355,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) /* We always get here via a jump, so know we are not in a conditional execution block. */ gen_exception_internal(EXCP_KERNEL_TRAP); - dc->is_jmp = DISAS_UPDATE; + dc->is_jmp = DISAS_EXC; break; } #else @@ -11363,7 +11363,7 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) /* We always get here via a jump, so know we are not in a conditional execution block. */ gen_exception_internal(EXCP_EXCEPTION_EXIT); - dc->is_jmp = DISAS_UPDATE; + dc->is_jmp = DISAS_EXC; break; } #endif @@ -11497,7 +11497,8 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) } gen_set_label(dc->condlabel); } - if (dc->condjmp || !dc->is_jmp) { + if (dc->condjmp || dc->is_jmp == DISAS_NEXT || + dc->is_jmp == DISAS_UPDATE) { gen_set_pc_im(dc, dc->pc); dc->condjmp = 0; } @@ -11533,9 +11534,11 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) case DISAS_NEXT: gen_goto_tb(dc, 1, dc->pc); break; - default: - case DISAS_JUMP: case DISAS_UPDATE: + gen_set_pc_im(dc, dc->pc); + /* fall through */ + case DISAS_JUMP: + default: /* indicate that the hash table must be used to find the next TB */ tcg_gen_exit_tb(0); break;