From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JwIAm-0007v0-B2 for qemu-devel@nongnu.org; Wed, 14 May 2008 10:41:40 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JwIAl-0007ts-CW for qemu-devel@nongnu.org; Wed, 14 May 2008 10:41:39 -0400 Received: from [199.232.76.173] (port=60941 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JwIAl-0007td-2y for qemu-devel@nongnu.org; Wed, 14 May 2008 10:41:39 -0400 Received: from mail.windriver.com ([147.11.1.11]:56649 helo=mail.wrs.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1JwIAk-0004Xy-LJ for qemu-devel@nongnu.org; Wed, 14 May 2008 10:41:38 -0400 Message-ID: <48290691.9060505@windriver.com> Date: Mon, 12 May 2008 22:10:09 -0500 From: Jason Wessel MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] gdbstub improvements for gdb kill/detach/quit References: <482456EA.8020108@windriver.com> <20080512112928.GC21455@edgar.se.axis.com> In-Reply-To: <20080512112928.GC21455@edgar.se.axis.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Edgar E. Iglesias" Cc: qemu-devel@nongnu.org More comments below. Edgar E. Iglesias wrote: > 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. >> >> >> --- 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? We desire the same action as if you run "quit" from the monitor. Today the monitor invokes do_quit() which just does exit(). If there is a more graceful way to exit the qemu instance which saves/flushes state, the monitor should probably be changed this way as well. I figure you can use the monitor commands from the gdbstub or directly in the monitor if you want to do any kind of saving of the state of the machine at this point. > > >> + 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? This seems reasonable. Previously I had simply used the same call/return semantics as removing a single break point. Also I noticed the watch points have the exact same problem as the breakpoints with respect to detaching the debugger, so I fixed that in the next version of the patch which will be posted shortly. Jason.