From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54928) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zug3C-0003VC-FZ for qemu-devel@nongnu.org; Fri, 06 Nov 2015 07:23:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zug38-0002Yz-EC for qemu-devel@nongnu.org; Fri, 06 Nov 2015 07:23:26 -0500 Received: from mail-lb0-x22e.google.com ([2a00:1450:4010:c04::22e]:34687) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zug38-0002Yj-76 for qemu-devel@nongnu.org; Fri, 06 Nov 2015 07:23:22 -0500 Received: by lbbwb3 with SMTP id wb3so56518709lbb.1 for ; Fri, 06 Nov 2015 04:23:21 -0800 (PST) References: <1446726361-18328-1-git-send-email-serge.fdrv@gmail.com> From: Sergey Fedorov Message-ID: <563C9BB7.9080404@gmail.com> Date: Fri, 6 Nov 2015 15:23:19 +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: Fix non-CPU breakpoint handling in arm_debug_excp_handler() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers On 06.11.2015 14:57, Peter Maydell wrote: > On 5 November 2015 at 12:26, Sergey Fedorov wrote: >> Do not raise a CPU exception if no CPU breakpoint has fired, since >> singlestep is also done by generating a debug internal exception. This >> fixes a bug with singlestepping in gdbstub. >> >> Signed-off-by: Sergey Fedorov >> --- >> This is a v2 of 'target-arm: Fix arm_debug_excp_handler() for singlestep >> enabled.' >> >> Changes in v2: >> * Commit subject and body changed >> * Instead of checking for singlestep enabled, CPU breakpoint match checked > Isn't this fixing singlestep, not non-CPU breakpoints? > (GDB breakpoints were already being checked for and early-returned.) Sorry for the confusing commit subject, actually, I meant "fix handling of EXCP_DEBUG in case of no CPU breakpoint fired" :) > > I'll tweak the commit subject line and apply to target-arm.next. > > Incidentally, this only affects gdbstub singlestep for aarch64 > in practice, because gdb for 32-bit ARM uses set-breakpoint-and-continue > rather than the singlestep gdbstub protocol command. > >> target-arm/op_helper.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c >> index b5db345..6cd54c8 100644 >> --- a/target-arm/op_helper.c >> +++ b/target-arm/op_helper.c >> @@ -917,7 +917,13 @@ void arm_debug_excp_handler(CPUState *cs) >> uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; >> bool same_el = (arm_debug_target_el(env) == arm_current_el(env)); >> >> - if (cpu_breakpoint_test(cs, pc, BP_GDB)) { >> + /* (1) GDB breakpoints should be handled first. >> + * (2) Do not raise a CPU exception if no CPU breakpoint has fired, >> + * since singlestep is also done by generating a debug internal >> + * exception. >> + */ >> + if (cpu_breakpoint_test(cs, pc, BP_GDB) >> + || !cpu_breakpoint_test(cs, pc, BP_CPU)) { >> return; >> } >> >> -- >> 1.9.1 >> > Thanks, Sergey