public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@web.de>
To: kvm-devel <kvm@vger.kernel.org>
Cc: Avi Kivity <avi@qumranet.com>,
	Hollis Blanchard <hollisb@us.ibm.com>,
	Jerone Young <jyoung5@us.ibm.com>,
	Joerg Roedel <joerg.roedel@amd.com>
Subject: [PATCH 5/11] QEMU: Improve SMP debugging support
Date: Tue, 27 May 2008 00:10:02 +0200	[thread overview]
Message-ID: <483B353A.1010909@web.de> (raw)
In-Reply-To: <4839B14A.3010406@web.de>

This patch enhances QEMU's built-in debugger for SMP guest debugging.
It allows to set the debugger focus explicitly via the monitor command
"cpu", and it automatically switches the focus on breakpoint hit to
the reporting CPU.

Furthermore, the patch propagates breakpoint and watchpoint insertions
or removals to all CPUs, not just the current one as it was the case so
far.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 qemu/cpu-defs.h |    3 +++
 qemu/exec.c     |   30 +++++++++++++++++++++---------
 qemu/gdbstub.c  |   25 ++++++++++++-------------
 qemu/monitor.c  |   19 ++++++++++++-------
 qemu/monitor.h  |    7 +++++++
 qemu/vl.c       |    2 ++
 6 files changed, 57 insertions(+), 29 deletions(-)

Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -35,6 +35,7 @@
 #include "gdbstub.h"
 #include "qemu-kvm.h"
 #endif
+#include "monitor.h"
 
 #include "qemu_socket.h"
 #ifdef _WIN32
@@ -59,7 +60,6 @@ enum RSState {
     RS_SYSCALL,
 };
 typedef struct GDBState {
-    CPUState *env; /* current CPU */
     enum RSState state; /* parsing state */
     char line_buf[4096];
     int line_buf_index;
@@ -962,7 +962,7 @@ static int gdb_handle_packet(GDBState *s
                 p++;
             type = *p;
             if (gdb_current_syscall_cb)
-                gdb_current_syscall_cb(s->env, ret, err);
+                gdb_current_syscall_cb(mon_get_cpu(), ret, err);
             if (type == 'C') {
                 put_packet(s, "T02");
             } else {
@@ -1092,6 +1092,7 @@ extern void tb_flush(CPUState *env);
 static void gdb_vm_stopped(void *opaque, int reason)
 {
     GDBState *s = opaque;
+    CPUState *env = mon_get_cpu();
     char buf[256];
     char *type;
     int ret;
@@ -1100,11 +1101,11 @@ static void gdb_vm_stopped(void *opaque,
         return;
 
     /* disable single step if it was enable */
-    cpu_single_step(s->env, 0);
+    cpu_single_step(env, 0);
 
     if (reason == EXCP_DEBUG) {
-        if (s->env->watchpoint_hit) {
-            switch (s->env->watchpoint[s->env->watchpoint_hit - 1].type) {
+        if (env->watchpoint_hit) {
+            switch (env->watchpoint[env->watchpoint_hit - 1].type) {
             case GDB_WATCHPOINT_READ:
                 type = "r";
                 break;
@@ -1117,12 +1118,12 @@ static void gdb_vm_stopped(void *opaque,
             }
             snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
                      SIGTRAP, type,
-                     s->env->watchpoint[s->env->watchpoint_hit - 1].vaddr);
+                     env->watchpoint[env->watchpoint_hit - 1].vaddr);
             put_packet(s, buf);
-            s->env->watchpoint_hit = 0;
+            env->watchpoint_hit = 0;
             return;
         }
-	tb_flush(s->env);
+	tb_flush(env);
         ret = SIGTRAP;
     } else if (reason == EXCP_INTERRUPT) {
         ret = SIGINT;
@@ -1192,15 +1193,15 @@ void gdb_do_syscall(gdb_syscall_complete
     va_end(va);
     put_packet(s, buf);
 #ifdef CONFIG_USER_ONLY
-    gdb_handlesig(s->env, 0);
+    gdb_handlesig(mon_get_cpu(), 0);
 #else
-    cpu_interrupt(s->env, CPU_INTERRUPT_EXIT);
+    cpu_interrupt(mon_get_cpu(), CPU_INTERRUPT_EXIT);
 #endif
 }
 
 static void gdb_read_byte(GDBState *s, int ch)
 {
-    CPUState *env = s->env;
+    CPUState *env = mon_get_cpu();
     int i, csum;
     uint8_t reply;
 
@@ -1360,7 +1361,6 @@ static void gdb_accept(void *opaque)
 
     s = &gdbserver_state;
     memset (s, 0, sizeof (GDBState));
-    s->env = first_cpu; /* XXX: allow to change CPU */
     s->fd = fd;
 
     gdb_syscall_state = s;
@@ -1463,7 +1463,6 @@ int gdbserver_start(const char *port)
     if (!s) {
         return -1;
     }
-    s->env = first_cpu; /* XXX: allow to change CPU */
     s->chr = chr;
     qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
                           gdb_chr_event, s);
Index: b/qemu/monitor.c
===================================================================
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -266,8 +266,7 @@ static void do_info_blockstats(void)
     bdrv_info_stats();
 }
 
-/* get the current CPU defined by the user */
-static int mon_set_cpu(int cpu_index)
+static int mon_set_cpu_index(int cpu_index)
 {
     CPUState *env;
 
@@ -280,10 +279,16 @@ static int mon_set_cpu(int cpu_index)
     return -1;
 }
 
-static CPUState *mon_get_cpu(void)
+void mon_set_cpu(CPUState *env)
+{
+    mon_cpu = env;
+}
+
+/* get the current CPU defined by the user or by a breakpoint hit */
+CPUState *mon_get_cpu(void)
 {
     if (!mon_cpu) {
-        mon_set_cpu(0);
+        mon_set_cpu(first_cpu);
     }
 
     kvm_save_registers(mon_cpu);
@@ -310,8 +315,8 @@ static void do_info_cpus(void)
 {
     CPUState *env;
 
-    /* just to set the default cpu if not already done */
-    mon_get_cpu();
+    if (!mon_cpu)
+        mon_set_cpu(first_cpu);
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         kvm_save_registers(env);
@@ -342,7 +347,7 @@ static void do_info_cpus(void)
 
 static void do_cpu_set(int index)
 {
-    if (mon_set_cpu(index) < 0)
+    if (mon_set_cpu_index(index) < 0)
         term_printf("Invalid CPU index\n");
 }
 
Index: b/qemu/monitor.h
===================================================================
--- /dev/null
+++ b/qemu/monitor.h
@@ -0,0 +1,7 @@
+#ifndef QEMU_MONITOR_H
+#define QEMU_MONITOR_H
+
+void mon_set_cpu(CPUState *env);
+CPUState *mon_get_cpu(void);
+
+#endif /* QEMU_MONITOR_H */
Index: b/qemu/vl.c
===================================================================
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -33,6 +33,7 @@
 #include "console.h"
 #include "sysemu.h"
 #include "gdbstub.h"
+#include "monitor.h"
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "block.h"
@@ -7644,6 +7645,7 @@ static int main_loop(void)
                 ret = EXCP_INTERRUPT;
             }
             if (unlikely(ret == EXCP_DEBUG)) {
+                mon_set_cpu(cur_cpu);
                 vm_stop(EXCP_DEBUG);
             }
             /* If all cpus are halted then wait until the next IRQ */
Index: b/qemu/cpu-defs.h
===================================================================
--- a/qemu/cpu-defs.h
+++ b/qemu/cpu-defs.h
@@ -170,3 +170,6 @@ typedef struct CPUTLBEntry {
     const char *cpu_model_str;
 
 #endif
+
+#define foreach_cpu(env) \
+    for(env = first_cpu; env != NULL; env = env->next_cpu)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1107,6 +1107,7 @@ static void breakpoint_invalidate(CPUSta
 int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
                           int type)
 {
+    CPUState *env_iter;
     int i;
 
     if (type != GDB_WATCHPOINT_WRITE)
@@ -1119,8 +1120,10 @@ int cpu_watchpoint_insert(CPUState *env,
     if (env->nb_watchpoints >= MAX_WATCHPOINTS)
         return -ENOBUFS;
 
-    i = env->nb_watchpoints++;
-    env->watchpoint[i].vaddr = addr;
+    foreach_cpu(env_iter) {
+        i = env_iter->nb_watchpoints++;
+        env_iter->watchpoint[i].vaddr = addr;
+    }
     tlb_flush_page(env, addr);
     /* FIXME: This flush is needed because of the hack to make memory ops
        terminate the TB.  It can be removed once the proper IO trap and
@@ -1133,6 +1136,7 @@ int cpu_watchpoint_insert(CPUState *env,
 int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len,
                           int type)
 {
+    CPUState *env_iter;
     int i;
 
     if (type != GDB_WATCHPOINT_WRITE)
@@ -1140,8 +1144,11 @@ int cpu_watchpoint_remove(CPUState *env,
 
     for (i = 0; i < env->nb_watchpoints; i++) {
         if (addr == env->watchpoint[i].vaddr) {
-            env->nb_watchpoints--;
-            env->watchpoint[i] = env->watchpoint[env->nb_watchpoints];
+            foreach_cpu(env_iter) {
+                env_iter->nb_watchpoints--;
+                env_iter->watchpoint[i] =
+                    env_iter->watchpoint[env_iter->nb_watchpoints];
+            }
             tlb_flush_page(env, addr);
             return 0;
         }
@@ -1155,6 +1162,7 @@ int cpu_breakpoint_insert(CPUState *env,
                           int type)
 {
 #if defined(TARGET_HAS_ICE)
+    CPUState *env_iter;
     int i;
 
     for(i = 0; i < env->nb_breakpoints; i++) {
@@ -1164,7 +1172,8 @@ int cpu_breakpoint_insert(CPUState *env,
 
     if (env->nb_breakpoints >= MAX_BREAKPOINTS)
         return -ENOBUFS;
-    env->breakpoints[env->nb_breakpoints++] = pc;
+    foreach_cpu(env_iter)
+        env_iter->breakpoints[env_iter->nb_breakpoints++] = pc;
 
     if (kvm_enabled())
 	kvm_update_debugger(env);
@@ -1181,6 +1190,7 @@ int cpu_breakpoint_remove(CPUState *env,
                           int type)
 {
 #if defined(TARGET_HAS_ICE)
+    CPUState *env_iter;
     int i;
     for(i = 0; i < env->nb_breakpoints; i++) {
         if (env->breakpoints[i] == pc)
@@ -1188,13 +1198,15 @@ int cpu_breakpoint_remove(CPUState *env,
     }
     return -ENOENT;
  found:
-    env->nb_breakpoints--;
-    if (i < env->nb_breakpoints)
-      env->breakpoints[i] = env->breakpoints[env->nb_breakpoints];
+    foreach_cpu(env_iter) {
+        env_iter->nb_breakpoints--;
+        env_iter->breakpoints[i] =
+            env_iter->breakpoints[env_iter->nb_breakpoints];
+    }
 
     if (kvm_enabled())
 	kvm_update_debugger(env);
-    
+
     breakpoint_invalidate(env, pc);
     return 0;
 #else




  parent reply	other threads:[~2008-05-26 22:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-26 22:05 [PATCH 0/11] Rework guest debug interface / x86 debug register support -v2 Jan Kiszka
2008-05-26 22:09 ` [PATCH 1/11] QEMU/KVM: Fix deadlocks in monitor and debugger Jan Kiszka
2008-05-27  9:36   ` Avi Kivity
2008-05-27 13:00     ` Jan Kiszka
2008-05-27 13:09       ` Avi Kivity
2008-05-26 22:09 ` [PATCH 2/11] QEMU/KVM: Cleanup and improve kvm_load/save_registers usage Jan Kiszka
2008-06-09 19:16   ` Anthony Liguori
2008-06-12 12:38     ` Avi Kivity
2008-05-26 22:09 ` [PATCH 3/11] QEMU/KVM: Proper vm_stop on debug events Jan Kiszka
2008-05-26 22:09 ` [PATCH 4/11] QEMU: Enhance cpu_break/watchpoint API and gdbstub integration Jan Kiszka
2008-05-26 22:10 ` Jan Kiszka [this message]
2008-05-26 22:10 ` [PATCH 6/11] QEMU/KVM: Introduce single vcpu pause/resume Jan Kiszka
2008-05-26 22:10 ` [PATCH 7/11] QEMU/KVM: New guest debugging interface Jan Kiszka
2008-05-27 18:31   ` Jan Kiszka
2008-05-26 22:10 ` [PATCH 8/11] QEMU/KVM: Support for SMP guest debugging Jan Kiszka
2008-05-26 22:10 ` [PATCH 9/11] KVM: New guest debugging interface Jan Kiszka
2008-05-26 22:10 ` [PATCH 10/11] KVM-x86: Properly virtualize debug registers Jan Kiszka
2008-05-26 22:10 ` [PATCH 11/11] KVM-x86: Wire up host-managed " Jan Kiszka
2008-05-27  9:50 ` [PATCH 0/11] Rework guest debug interface / x86 debug register support -v2 Avi Kivity
2008-05-27 10:44   ` Jan Kiszka
2008-05-27 18:46   ` Hollis Blanchard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=483B353A.1010909@web.de \
    --to=jan.kiszka@web.de \
    --cc=avi@qumranet.com \
    --cc=hollisb@us.ibm.com \
    --cc=joerg.roedel@amd.com \
    --cc=jyoung5@us.ibm.com \
    --cc=kvm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox