[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] gdbstub improvements for gdb kill/detach/quit
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] [PATCH] gdbstub improvements for gdb kill/detach/quit |
Date: |
Mon, 12 May 2008 13:29:28 +0200 |
User-agent: |
Mutt/1.5.16 (2007-06-09) |
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 <address@hidden>
> 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 <address@hidden>
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