From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52150) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxLfP-0001hp-FJ for qemu-devel@nongnu.org; Fri, 13 Nov 2015 16:13:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZxLfK-0003P6-CO for qemu-devel@nongnu.org; Fri, 13 Nov 2015 16:13:55 -0500 Received: from mail-lb0-x22a.google.com ([2a00:1450:4010:c04::22a]:34976) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxLfK-0003Ol-4g for qemu-devel@nongnu.org; Fri, 13 Nov 2015 16:13:50 -0500 Received: by lbbsy6 with SMTP id sy6so32306536lbb.2 for ; Fri, 13 Nov 2015 13:13:48 -0800 (PST) References: <1447097859-586-1-git-send-email-serge.fdrv@gmail.com> From: Sergey Fedorov Message-ID: <5646528B.70607@gmail.com> Date: Sat, 14 Nov 2015 00:13:47 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] target-arm: Clean up DISAS_UPDATE usage in AArch32 translation code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers On 10.11.2015 15:15, Peter Maydell wrote: > So the way the 32-bit code works for singlestep is complicated > because of the need to handle the conditional instructions, > which means you get a lot more cases like "this is a conditional > SWI" that need to be handled. A quick summary of some of the > possible cases: > > * unconditional normal instruction: > -- need to write the PC and condexec bits back to the CPU state > -- then take a singlestep insn (either the architectural one > or the EXCP_DEBUG one depending on which sort of step we are doing) > * unconditional exception-generating instruction > -- for architectural step of SWI/HVC/SMC we need to advance the > singlestep state machine so that they behave correctly > -- generate the relevant exception and then no point writing the > code to take EXCP_DEBUG &c because we won't get to it > * conditional instruction (including cond. branches): > -- earlier code has already written back the PC for the > "condition passed" case > -- write out the code which takes the singlestep exception for > the "condition passed" case > -- then do gen_set_label(dc->condlabel) > -- then the code to take the single step exception after > executing for the "condition failed" case > > In particular in this bit: > if (dc->condjmp || !dc->is_jmp) { > gen_set_pc_im(dc, dc->pc); > dc->condjmp = 0; Hi Peter, Thank you a lot for your explanation! It was really helpful for understanding the code :) One thing I wasn't sure of was whether this "dc->condjmp = 0" means that "condition failed" codepath below will also generate an exception whereas it shouldn't? Getting into the way the condexec bits handled I see that gen_set_condexec() should be called before gen_helper_check_breakpoints(), and probably also before gen_helper_access_check_cp_reg() because these helpers can raise an exception. I'm going to prepare patches for that soon. Best regards, Sergey > } > the cases when we need to update the PC are > (a) for the condition-failed codepath of a conditional insn > (the condition-passed codepath will already have written PC) > (b) for a non-conditional insn that hasn't already written PC > > The A64 equivalent is much simpler because the only cases we > need to handle are: > * exception already generated (no point writing anything) > * jumps (PC already written, just write code to take the step exception) > * everything else (write PC then take step exception)