From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49147) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZufeW-0005Nl-QZ for qemu-devel@nongnu.org; Fri, 06 Nov 2015 06:57:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZufeV-0004qv-RE for qemu-devel@nongnu.org; Fri, 06 Nov 2015 06:57:56 -0500 Received: from mail-vk0-x230.google.com ([2607:f8b0:400c:c05::230]:34289) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZufeV-0004qo-Lo for qemu-devel@nongnu.org; Fri, 06 Nov 2015 06:57:55 -0500 Received: by vkgs66 with SMTP id s66so12371918vkg.1 for ; Fri, 06 Nov 2015 03:57:55 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1446726361-18328-1-git-send-email-serge.fdrv@gmail.com> References: <1446726361-18328-1-git-send-email-serge.fdrv@gmail.com> From: Peter Maydell Date: Fri, 6 Nov 2015 11:57:35 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 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: Sergey Fedorov Cc: QEMU Developers 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.) 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 -- PMM