From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JvWDq-0000DC-G7 for qemu-devel@nongnu.org; Mon, 12 May 2008 07:29:38 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JvWDo-0000Bb-0X for qemu-devel@nongnu.org; Mon, 12 May 2008 07:29:38 -0400 Received: from [199.232.76.173] (port=40167 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JvWDn-0000BQ-RE for qemu-devel@nongnu.org; Mon, 12 May 2008 07:29:35 -0400 Received: from miranda.se.axis.com ([193.13.178.8]:36216) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JvWDn-0004M8-Av for qemu-devel@nongnu.org; Mon, 12 May 2008 07:29:35 -0400 Received: from axis.com (edgar.se.axis.com [10.93.151.1]) by miranda.se.axis.com (8.13.4/8.13.4/Debian-3sarge3) with ESMTP id m4CBTSIF030995 for ; Mon, 12 May 2008 13:29:28 +0200 Date: Mon, 12 May 2008 13:29:28 +0200 From: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] [PATCH] gdbstub improvements for gdb kill/detach/quit Message-ID: <20080512112928.GC21455@edgar.se.axis.com> References: <482456EA.8020108@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <482456EA.8020108@windriver.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wessel Cc: qemu-devel@nongnu.org On Fri, May 09, 2008 at 08:51:38AM -0500, Jason Wessel wrote: > Attached is a patch to improve the gdb stub 'D' 'k' and '?' packets. > See the patch header for details. > > Jason. > From: Jason Wessel > Subject: [PATCH] support for gdb "detach/kill/quit" > > Implement the 'k' gdbserial packet which kills the qemu instance via > the debugger stub. > > Implement the 'D' detach packet for the gdb stub such that you can > disconnect gdb with the "detach" command. This required implementing > a cpu_breakpoint_remove_all function to cleanup all the breakpoints > prior to leaving the gdb stub else simulation can stop with no > debugger attached. > > On a '?' packet remove all the breakpoints. This is considered more > of a safety net in case you force killed gdb or it crashed and you are > reconnecting. The identical behavior exists for kgdb in the linux > kernel. > > Signed-off-by: Jason Wessel Thanks, a few comments inlined. > --- > cpu-all.h | 1 + > exec.c | 14 ++++++++++++++ > gdbstub.c | 16 ++++++++++++++++ > 3 files changed, 31 insertions(+) > > --- a/cpu-all.h > +++ b/cpu-all.h > @@ -759,6 +759,7 @@ void cpu_interrupt(CPUState *s, int mask > void cpu_reset_interrupt(CPUState *env, int mask); > > int cpu_watchpoint_insert(CPUState *env, target_ulong addr); > +int cpu_breakpoint_remove_all(CPUState *env); > int cpu_watchpoint_remove(CPUState *env, target_ulong addr); > int cpu_breakpoint_insert(CPUState *env, target_ulong pc); > int cpu_breakpoint_remove(CPUState *env, target_ulong pc); > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -901,6 +901,11 @@ static int gdb_handle_packet(GDBState *s > /* TODO: Make this return the correct value for user-mode. */ > snprintf(buf, sizeof(buf), "S%02x", SIGTRAP); > put_packet(s, buf); > + /* Remove all the breakpoints when this query is issued, > + * because gdb is doing and initial connect and the state > + * should be cleaned up. > + */ > + cpu_breakpoint_remove_all(env); > break; > case 'c': > if (*p != '\0') { > @@ -924,6 +929,17 @@ static int gdb_handle_packet(GDBState *s > } > gdb_continue(s); > return RS_IDLE; > + case 'k': > + /* Kill the target */ > + fprintf(stderr, "\nQEMU: Terminated via GDBstub\n"); > + exit(0); Should we consider qemu_system_shutdown_request() here? > + case 'D': > + /* Detach packet */ > + if (!cpu_breakpoint_remove_all(env)) { > + gdb_continue(s); > + put_packet(s, "OK"); > + break; > + } > case 's': > if (*p != '\0') { > addr = strtoull(p, (char **)&p, 16); > --- a/exec.c > +++ b/exec.c > @@ -1150,6 +1150,20 @@ int cpu_breakpoint_insert(CPUState *env, > #endif > } > > +/* remove all breakpoints */ > +int cpu_breakpoint_remove_all(CPUState *env) { > +#if defined(TARGET_HAS_ICE) > + int i; > + for(i = 0; i < env->nb_breakpoints; i++) { > + breakpoint_invalidate(env, env->breakpoints[i]); > + } > + env->nb_breakpoints = 0; > + return 0; > +#else > + return -1; > +#endif > +} Why not just drop the return value and make the 'D' command always succeed? Best regards -- Edgar E. Iglesias Axis Communications AB