[PATCH v3 0/3] KVM-userspace: Improved guest debugging / debug register emulation

[PATCH v3 0/3] KVM-userspace: Improved guest debugging / debug register emulation

[Jan Kiszka]

Changes since last round:
 - Fix non-x86 stubs in qemu-kvm-*.c (Christian Ehrhardt)

As usual, find the patches also at git://git.kiszka.org/kvm-userspace.git gdb-queue

Jan

--
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux

[PATCH v3 3/3] kvm-userspace: Provide compat wrapper for set_debugreg

[Jan Kiszka]

Older set_debugreg macros did not allow to pass the register number as
constant (without additional typcasting). Catch this as the latest kvm
debug changes make use of this property.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 kernel/x86/external-module-compat.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/kernel/x86/external-module-compat.h b/kernel/x86/external-module-compat.h
index 1055050..5489f47 100644
--- a/kernel/x86/external-module-compat.h
+++ b/kernel/x86/external-module-compat.h
@@ -333,6 +333,16 @@ struct kvm_desc_ptr {
 #define FEATURE_CONTROL_VMXON_ENABLED	(1<<2)
 #endif
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__)
+
+#undef set_debugreg
+#define set_debugreg(value, register) \
+	__asm__("movq %0,%%db" #register \
+		: /* no output */ \
+		:"r" ((unsigned long)value))
+
+#endif
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,29)
 
 struct mtrr_var_range {

[PATCH v3 2/3] kvm-userspace: Remove obsolete special_reload_dr7 hack

[Jan Kiszka]

Host debug registers are now properly saved and restored before/after
entering the guest.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 kernel/x86/external-module-compat.h |    2 --
 kernel/x86/hack-module.awk          |    4 ----
 kernel/x86/preempt.c                |    6 ------
 3 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/kernel/x86/external-module-compat.h b/kernel/x86/external-module-compat.h
index b5e11e2..1055050 100644
--- a/kernel/x86/external-module-compat.h
+++ b/kernel/x86/external-module-compat.h
@@ -171,7 +171,6 @@ static inline void preempt_notifier_init(struct preempt_notifier *notifier,
 void start_special_insn(void);
 void end_special_insn(void);
 void in_special_section(void);
-void special_reload_dr7(void);
 
 void preempt_notifier_sys_init(void);
 void preempt_notifier_sys_exit(void);
@@ -181,7 +180,6 @@ void preempt_notifier_sys_exit(void);
 static inline void start_special_insn(void) {}
 static inline void end_special_insn(void) {}
 static inline void in_special_section(void) {}
-static inline void special_reload_dr7(void) {}
 
 static inline void preempt_notifier_sys_init(void) {}
 static inline void preempt_notifier_sys_exit(void) {}
diff --git a/kernel/x86/hack-module.awk b/kernel/x86/hack-module.awk
index 2ad0951..f40c972 100644
--- a/kernel/x86/hack-module.awk
+++ b/kernel/x86/hack-module.awk
@@ -77,10 +77,6 @@ BEGIN { split("INIT_WORK tsc_khz desc_struct ldttss_desc64 desc_ptr " \
 
 { print }
 
-/kvm_x86_ops->run/ {
-    print "\tspecial_reload_dr7();"
-}
-
 /unsigned long flags;/ &&  vmx_load_host_state {
     print "\tunsigned long gsbase;"
 }
diff --git a/kernel/x86/preempt.c b/kernel/x86/preempt.c
index 9e4bd2c..3112879 100644
--- a/kernel/x86/preempt.c
+++ b/kernel/x86/preempt.c
@@ -40,12 +40,6 @@ static void preempt_enable_sched_in_notifiers(void * addr)
 #endif
 }
 
-void special_reload_dr7(void)
-{
-	asm volatile ("mov %0, %%db7" : : "r"(0x701ul));
-}
-EXPORT_SYMBOL_GPL(special_reload_dr7);
-
 static void __preempt_disable_notifiers(void)
 {
 	asm volatile ("mov %0, %%db7" : : "r"(0ul));

[PATCH v3 1/3] kvm-userspace: Switch to new guest debug interface

[Jan Kiszka]

This patch switches both libkvm as well as the qemu pieces over to the
new guest debug interface. It comes with full support for software-based
breakpoints (via guest code modification), hardware-assisted breakpoints
and watchpoints (x86-only so far).

Breakpoint management is done inside qemu-kvm, transparently to gdbstub
and also avoiding that the gdb frontend takes over. This allows for
running debuggers inside the guest while guest debugging it active,
because the host can cleanly tell apart host- and guest-originated
breakpoint events.

Yet improvable are x86 corner cases when using single-step ("forgotten"
debug flags on the guest's stack). And, of course, the yet empty non-x86
helper functions have to be populated.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

 libkvm/kvm-common.h     |    2 
 libkvm/libkvm.c         |   18 +++-
 libkvm/libkvm.h         |    9 ++
 qemu/exec.c             |   18 ++--
 qemu/gdbstub.c          |   23 +++--
 qemu/gdbstub.h          |    7 ++
 qemu/qemu-kvm-ia64.c    |   37 +++++++++
 qemu/qemu-kvm-powerpc.c |   37 +++++++++
 qemu/qemu-kvm-x86.c     |  172 +++++++++++++++++++++++++++++++++++++++++
 qemu/qemu-kvm.c         |  199 +++++++++++++++++++++++++++++++++++++++--------
 qemu/qemu-kvm.h         |   33 ++++++++
 user/main.c             |    7 +-
 12 files changed, 500 insertions(+), 62 deletions(-)

diff --git a/libkvm/kvm-common.h b/libkvm/kvm-common.h
index 9dae17b..c5beacc 100644
--- a/libkvm/kvm-common.h
+++ b/libkvm/kvm-common.h
@@ -88,7 +88,7 @@ int handle_shutdown(kvm_context_t kvm, void *env);
 void post_kvm_run(kvm_context_t kvm, void *env);
 int pre_kvm_run(kvm_context_t kvm, void *env);
 int handle_io_window(kvm_context_t kvm);
-int handle_debug(kvm_context_t kvm, void *env);
+int handle_debug(kvm_context_t kvm, int vcpu, void *env);
 int try_push_interrupts(kvm_context_t kvm);
 
 #endif
diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index d4b1a73..096c097 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -738,9 +738,15 @@ static int handle_io(kvm_context_t kvm, struct kvm_run *run, int vcpu)
 	return 0;
 }
 
-int handle_debug(kvm_context_t kvm, void *env)
+int handle_debug(kvm_context_t kvm, int vcpu, void *env)
 {
-	return kvm->callbacks->debug(kvm->opaque, env);
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+    struct kvm_run *run = kvm->run[vcpu];
+
+    return kvm->callbacks->debug(kvm->opaque, env, &run->debug.arch);
+#else
+    return 0;
+#endif
 }
 
 int kvm_get_regs(kvm_context_t kvm, int vcpu, struct kvm_regs *regs)
@@ -937,7 +943,7 @@ again:
 			r = handle_io(kvm, run, vcpu);
 			break;
 		case KVM_EXIT_DEBUG:
-			r = handle_debug(kvm, env);
+			r = handle_debug(kvm, vcpu, env);
 			break;
 		case KVM_EXIT_MMIO:
 			r = handle_mmio(kvm, run);
@@ -982,10 +988,12 @@ int kvm_inject_irq(kvm_context_t kvm, int vcpu, unsigned irq)
 	return ioctl(kvm->vcpu_fd[vcpu], KVM_INTERRUPT, &intr);
 }
 
-int kvm_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_debug_guest *dbg)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+int kvm_set_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_guest_debug *dbg)
 {
-	return ioctl(kvm->vcpu_fd[vcpu], KVM_DEBUG_GUEST, dbg);
+	return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_GUEST_DEBUG, dbg);
 }
+#endif
 
 int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, const sigset_t *sigset)
 {
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index 392065b..d068fb3 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -55,7 +55,10 @@ struct kvm_callbacks {
 	/// generic memory writes to unmapped memory (For MMIO devices)
     int (*mmio_write)(void *opaque, uint64_t addr, uint8_t *data,
 					int len);
-    int (*debug)(void *opaque, void *env);
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+    int (*debug)(void *opaque, void *env,
+		 struct kvm_debug_exit_arch *arch_info);
+#endif
 	/*!
 	 * \brief Called when the VCPU issues an 'hlt' instruction.
 	 *
@@ -350,7 +353,9 @@ static inline int kvm_reset_mpstate(kvm_context_t kvm, int vcpu)
  */
 int kvm_inject_irq(kvm_context_t kvm, int vcpu, unsigned irq);
 
-int kvm_guest_debug(kvm_context_t, int vcpu, struct kvm_debug_guest *dbg);
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+int kvm_set_guest_debug(kvm_context_t, int vcpu, struct kvm_guest_debug *dbg);
+#endif
 
 #if defined(__i386__) || defined(__x86_64__)
 /*!
diff --git a/qemu/exec.c b/qemu/exec.c
index 36ff1f0..f62eb77 100644
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1415,9 +1415,6 @@ int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
     else
         TAILQ_INSERT_TAIL(&env->breakpoints, bp, entry);
 
-    if (kvm_enabled())
-	kvm_update_debugger(env);
-
     breakpoint_invalidate(env, pc);
 
     if (breakpoint)
@@ -1452,9 +1449,6 @@ void cpu_breakpoint_remove_by_ref(CPUState *env, CPUBreakpoint *breakpoint)
 #if defined(TARGET_HAS_ICE)
     TAILQ_REMOVE(&env->breakpoints, breakpoint, entry);
 
-    if (kvm_enabled())
-	kvm_update_debugger(env);
-
     breakpoint_invalidate(env, breakpoint->pc);
 
     qemu_free(breakpoint);
@@ -1481,12 +1475,14 @@ void cpu_single_step(CPUState *env, int enabled)
 #if defined(TARGET_HAS_ICE)
     if (env->singlestep_enabled != enabled) {
         env->singlestep_enabled = enabled;
-        /* must flush all the translated code to avoid inconsistancies */
-        /* XXX: only flush what is necessary */
-        tb_flush(env);
+        if (kvm_enabled())
+            kvm_update_guest_debug(env, 0);
+        else {
+            /* must flush all the translated code to avoid inconsistancies */
+            /* XXX: only flush what is necessary */
+            tb_flush(env);
+        }
     }
-    if (kvm_enabled())
-	kvm_update_debugger(env);
 #endif
 }
 
diff --git a/qemu/gdbstub.c b/qemu/gdbstub.c
index b02571a..fd6223a 100644
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -1145,13 +1145,6 @@ void gdb_register_coprocessor(CPUState * env,
     }
 }
 
-/* GDB breakpoint/watchpoint types */
-#define GDB_BREAKPOINT_SW        0
-#define GDB_BREAKPOINT_HW        1
-#define GDB_WATCHPOINT_WRITE     2
-#define GDB_WATCHPOINT_READ      3
-#define GDB_WATCHPOINT_ACCESS    4
-
 #ifndef CONFIG_USER_ONLY
 static const int xlat_gdb_type[] = {
     [GDB_WATCHPOINT_WRITE]  = BP_GDB | BP_MEM_WRITE,
@@ -1165,6 +1158,9 @@ static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type)
     CPUState *env;
     int err = 0;
 
+    if (kvm_enabled())
+        return kvm_insert_breakpoint(gdbserver_state->c_cpu, addr, len, type);
+
     switch (type) {
     case GDB_BREAKPOINT_SW:
     case GDB_BREAKPOINT_HW:
@@ -1196,6 +1192,9 @@ static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
     CPUState *env;
     int err = 0;
 
+    if (kvm_enabled())
+        return kvm_remove_breakpoint(gdbserver_state->c_cpu, addr, len, type);
+
     switch (type) {
     case GDB_BREAKPOINT_SW:
     case GDB_BREAKPOINT_HW:
@@ -1225,6 +1224,11 @@ static void gdb_breakpoint_remove_all(void)
 {
     CPUState *env;
 
+    if (kvm_enabled()) {
+        kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
+        return;
+    }
+
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
         cpu_breakpoint_remove_all(env, BP_GDB);
 #ifndef CONFIG_USER_ONLY
@@ -1348,7 +1352,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'g':
-        kvm_save_registers(env);
+        kvm_save_registers(s->g_cpu);
         len = 0;
         for (addr = 0; addr < num_g_regs; addr++) {
             reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr);
@@ -1366,7 +1370,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             len -= reg_size;
             registers += reg_size;
         }
-        kvm_load_registers(env);
+        kvm_load_registers(s->g_cpu);
         put_packet(s, "OK");
         break;
     case 'm':
@@ -1525,6 +1529,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             thread = strtoull(p+16, (char **)&p, 16);
             for (env = first_cpu; env != NULL; env = env->next_cpu)
                 if (env->cpu_index + 1 == thread) {
+                    kvm_save_registers(env);
                     len = snprintf((char *)mem_buf, sizeof(mem_buf),
                                    "CPU#%d [%s]", env->cpu_index,
                                    env->halted ? "halted " : "running");
diff --git a/qemu/gdbstub.h b/qemu/gdbstub.h
index 1ba7931..e17ac90 100644
--- a/qemu/gdbstub.h
+++ b/qemu/gdbstub.h
@@ -3,6 +3,13 @@
 
 #define DEFAULT_GDBSTUB_PORT "1234"
 
+/* GDB breakpoint/watchpoint types */
+#define GDB_BREAKPOINT_SW        0
+#define GDB_BREAKPOINT_HW        1
+#define GDB_WATCHPOINT_WRITE     2
+#define GDB_WATCHPOINT_READ      3
+#define GDB_WATCHPOINT_ACCESS    4
+
 typedef void (*gdb_syscall_complete_cb)(CPUState *env,
                                         target_ulong ret, target_ulong err);
 
diff --git a/qemu/qemu-kvm-ia64.c b/qemu/qemu-kvm-ia64.c
index 8cd3cff..f9e4b4b 100644
--- a/qemu/qemu-kvm-ia64.c
+++ b/qemu/qemu-kvm-ia64.c
@@ -61,6 +61,43 @@ void kvm_arch_update_regs_for_sipi(CPUState *env)
 {
 }
 
+int kvm_arch_insert_sw_breakpoint(CPUState *current_env,
+                                  struct kvm_sw_breakpoint *bp)
+{
+    return -EINVAL;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *current_env,
+                                  struct kvm_sw_breakpoint *bp)
+{
+    return -EINVAL;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+				  target_ulong len, int type)
+{
+    return -ENOSYS;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+				  target_ulong len, int type)
+{
+    return -ENOSYS;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+}
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+{
+    return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
+{
+}
+
 void kvm_save_mpstate(CPUState *env)
 {
 #ifdef KVM_CAP_MP_STATE
diff --git a/qemu/qemu-kvm-powerpc.c b/qemu/qemu-kvm-powerpc.c
index dadcd1c..1dc7d9a 100644
--- a/qemu/qemu-kvm-powerpc.c
+++ b/qemu/qemu-kvm-powerpc.c
@@ -215,3 +215,40 @@ int handle_powerpc_dcr_write(int vcpu, uint32_t dcrn, uint32_t data)
 void kvm_arch_cpu_reset(CPUState *env)
 {
 }
+
+int kvm_arch_insert_sw_breakpoint(CPUState *current_env,
+                                  struct kvm_sw_breakpoint *bp)
+{
+    return -EINVAL;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *current_env,
+                                  struct kvm_sw_breakpoint *bp)
+{
+    return -EINVAL;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+				  target_ulong len, int type)
+{
+    return -ENOSYS;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+				  target_ulong len, int type)
+{
+    return -ENOSYS;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+}
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+{
+    return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
+{
+}
diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
index 616c7c6..ddc0d7e 100644
--- a/qemu/qemu-kvm-x86.c
+++ b/qemu/qemu-kvm-x86.c
@@ -11,6 +11,7 @@
 
 #include <string.h>
 #include "hw/hw.h"
+#include "gdbstub.h"
 #include <sys/io.h>
 
 #include "qemu-kvm.h"
@@ -716,6 +717,177 @@ void kvm_arch_cpu_reset(CPUState *env)
     }
 }
 
+int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
+{
+    uint8_t int3 = 0xcc;
+
+    if (cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 0) ||
+        cpu_memory_rw_debug(env, bp->pc, &int3, 1, 1))
+        return -EINVAL;
+    return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
+{
+    uint8_t int3;
+
+    if (cpu_memory_rw_debug(env, bp->pc, &int3, 1, 0) || int3 != 0xcc ||
+        cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1))
+        return -EINVAL;
+    return 0;
+}
+
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+static struct {
+    target_ulong addr;
+    int len;
+    int type;
+} hw_breakpoint[4];
+
+static int nb_hw_breakpoint;
+
+static int find_hw_breakpoint(target_ulong addr, int len, int type)
+{
+    int n;
+
+    for (n = 0; n < nb_hw_breakpoint; n++)
+	if (hw_breakpoint[n].addr == addr && hw_breakpoint[n].type == type &&
+	    (hw_breakpoint[n].len == len || len == -1))
+	    return n;
+    return -1;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+                                  target_ulong len, int type)
+{
+    switch (type) {
+    case GDB_BREAKPOINT_HW:
+	len = 1;
+	break;
+    case GDB_WATCHPOINT_WRITE:
+    case GDB_WATCHPOINT_ACCESS:
+	switch (len) {
+	case 1:
+	    break;
+	case 2:
+	case 4:
+	case 8:
+	    if (addr & (len - 1))
+		return -EINVAL;
+	    break;
+	default:
+	    return -EINVAL;
+	}
+	break;
+    default:
+	return -ENOSYS;
+    }
+
+    if (nb_hw_breakpoint == 4)
+        return -ENOBUFS;
+
+    if (find_hw_breakpoint(addr, len, type) >= 0)
+        return -EEXIST;
+
+    hw_breakpoint[nb_hw_breakpoint].addr = addr;
+    hw_breakpoint[nb_hw_breakpoint].len = len;
+    hw_breakpoint[nb_hw_breakpoint].type = type;
+    nb_hw_breakpoint++;
+
+    return 0;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+                                  target_ulong len, int type)
+{
+    int n;
+
+    n = find_hw_breakpoint(addr, (type == GDB_BREAKPOINT_HW) ? 1 : len, type);
+    if (n < 0)
+        return -ENOENT;
+
+    nb_hw_breakpoint--;
+    hw_breakpoint[n] = hw_breakpoint[nb_hw_breakpoint];
+
+    return 0;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+    nb_hw_breakpoint = 0;
+}
+
+static CPUWatchpoint hw_watchpoint;
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+{
+    int handle = 0;
+    int n;
+
+    if (arch_info->exception == 1) {
+	if (arch_info->dr6 & (1 << 14)) {
+	    if (cpu_single_env->singlestep_enabled)
+		handle = 1;
+	} else {
+	    for (n = 0; n < 4; n++)
+		if (arch_info->dr6 & (1 << n))
+		    switch ((arch_info->dr7 >> (16 + n*4)) & 0x3) {
+		    case 0x0:
+			handle = 1;
+			break;
+		    case 0x1:
+			handle = 1;
+			cpu_single_env->watchpoint_hit = &hw_watchpoint;
+			hw_watchpoint.vaddr = hw_breakpoint[n].addr;
+			hw_watchpoint.flags = BP_MEM_WRITE;
+			break;
+		    case 0x3:
+			handle = 1;
+			cpu_single_env->watchpoint_hit = &hw_watchpoint;
+			hw_watchpoint.vaddr = hw_breakpoint[n].addr;
+			hw_watchpoint.flags = BP_MEM_ACCESS;
+			break;
+		    }
+	}
+    } else if (kvm_find_sw_breakpoint(arch_info->pc))
+	handle = 1;
+
+    if (!handle)
+	kvm_update_guest_debug(cpu_single_env,
+			(arch_info->exception == 1) ?
+			KVM_GUESTDBG_INJECT_DB : KVM_GUESTDBG_INJECT_BP);
+
+    return handle;
+}
+
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
+{
+    const uint8_t type_code[] = {
+	[GDB_BREAKPOINT_HW] = 0x0,
+	[GDB_WATCHPOINT_WRITE] = 0x1,
+	[GDB_WATCHPOINT_ACCESS] = 0x3
+    };
+    const uint8_t len_code[] = {
+	[1] = 0x0, [2] = 0x1, [4] = 0x3, [8] = 0x2
+    };
+    int n;
+
+    if (!TAILQ_EMPTY(&kvm_sw_breakpoints))
+	dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
+
+    if (nb_hw_breakpoint > 0) {
+	dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
+	dbg->arch.debugreg[7] = 0x0600;
+	for (n = 0; n < nb_hw_breakpoint; n++) {
+	    dbg->arch.debugreg[n] = hw_breakpoint[n].addr;
+	    dbg->arch.debugreg[7] |= (2 << (n * 2)) |
+		(type_code[hw_breakpoint[n].type] << (16 + n*4)) |
+		(len_code[hw_breakpoint[n].len] << (18 + n*4));
+	}
+    }
+}
+#endif
+
 void kvm_arch_do_ioperm(void *_data)
 {
     struct ioperm_data *data = _data;
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 121873e..5e373a0 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -20,6 +20,7 @@ int kvm_pit = 1;
 #include "console.h"
 #include "block.h"
 #include "compatfd.h"
+#include "gdbstub.h"
 
 #include "qemu-kvm.h"
 #include <libkvm.h>
@@ -52,7 +53,7 @@ pthread_t io_thread;
 static int io_thread_fd = -1;
 static int io_thread_sigfd = -1;
 
-static int kvm_debug_stop_requested;
+static CPUState *kvm_debug_cpu_requested;
 
 /* The list of ioperm_data */
 static LIST_HEAD(, ioperm_data) ioperm_head;
@@ -601,9 +602,10 @@ int kvm_main_loop(void)
             qemu_system_powerdown();
         else if (qemu_reset_requested())
 	    qemu_kvm_system_reset();
-	else if (kvm_debug_stop_requested) {
+	else if (kvm_debug_cpu_requested) {
+	    gdb_set_stop_cpu(kvm_debug_cpu_requested);
 	    vm_stop(EXCP_DEBUG);
-	    kvm_debug_stop_requested = 0;
+	    kvm_debug_cpu_requested = NULL;
 	}
     }
 
@@ -613,14 +615,19 @@ int kvm_main_loop(void)
     return 0;
 }
 
-static int kvm_debug(void *opaque, void *data)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+int kvm_debug(void *opaque, void *data, struct kvm_debug_exit_arch *arch_info)
 {
-    struct CPUState *env = (struct CPUState *)data;
+    int handle = kvm_arch_debug(arch_info);
+    struct CPUState *env = data;
 
-    kvm_debug_stop_requested = 1;
-    env->kvm_cpu_state.stopped = 1;
-    return 1;
+    if (handle) {
+	kvm_debug_cpu_requested = env;
+	env->kvm_cpu_state.stopped = 1;
+    }
+    return handle;
 }
+#endif
 
 static int kvm_inb(void *opaque, uint16_t addr, uint8_t *data)
 {
@@ -724,7 +731,9 @@ static int kvm_shutdown(void *opaque, void *data)
 }
  
 static struct kvm_callbacks qemu_kvm_ops = {
+#ifdef KVM_CAP_SET_GUEST_DEBUG
     .debug = kvm_debug,
+#endif
     .inb   = kvm_inb,
     .inw   = kvm_inw,
     .inl   = kvm_inl,
@@ -933,44 +942,170 @@ int kvm_qemu_init_env(CPUState *cenv)
     return kvm_arch_qemu_init_env(cenv);
 }
 
-struct kvm_guest_debug_data {
-    struct kvm_debug_guest dbg;
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+struct kvm_sw_breakpoint_head kvm_sw_breakpoints =
+    TAILQ_HEAD_INITIALIZER(kvm_sw_breakpoints);
+
+struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(target_ulong pc)
+{
+    struct kvm_sw_breakpoint *bp;
+
+    TAILQ_FOREACH(bp, &kvm_sw_breakpoints, entry) {
+	if (bp->pc == pc)
+	    return bp;
+    }
+    return NULL;
+}
+
+struct kvm_set_guest_debug_data {
+    struct kvm_guest_debug dbg;
     int err;
 };
 
-void kvm_invoke_guest_debug(void *data)
+void kvm_invoke_set_guest_debug(void *data)
 {
-    struct kvm_guest_debug_data *dbg_data = data;
+    struct kvm_set_guest_debug_data *dbg_data = data;
 
-    dbg_data->err = kvm_guest_debug(kvm_context, cpu_single_env->cpu_index,
-                                    &dbg_data->dbg);
+    dbg_data->err = kvm_set_guest_debug(kvm_context, cpu_single_env->cpu_index,
+                                        &dbg_data->dbg);
 }
 
-int kvm_update_debugger(CPUState *env)
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
 {
-    struct kvm_guest_debug_data data;
-    CPUBreakpoint *bp;
-    int i;
+    struct kvm_set_guest_debug_data data;
 
-    memset(data.dbg.breakpoints, 0, sizeof(data.dbg.breakpoints));
+    data.dbg.control = 0;
+    if (env->singlestep_enabled)
+	data.dbg.control = KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
 
-    data.dbg.enabled = 0;
-    if (!TAILQ_EMPTY(&env->breakpoints) || env->singlestep_enabled) {
-        bp = TAILQ_FIRST(&env->breakpoints);
-	data.dbg.enabled = 1;
-	for (i = 0; i < 4; ++i) {
-	    data.dbg.breakpoints[i].enabled = bp != NULL;
-            if (bp) {
-                data.dbg.breakpoints[i].address = bp->pc;
-                bp = TAILQ_NEXT(bp, entry);
-            }
+    kvm_arch_update_guest_debug(env, &data.dbg);
+    data.dbg.control |= reinject_trap;
+
+    on_vcpu(env, kvm_invoke_set_guest_debug, &data);
+    return data.err;
+}
+
+int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    struct kvm_sw_breakpoint *bp;
+    CPUState *env;
+    int err;
+
+    if (type == GDB_BREAKPOINT_SW) {
+	bp = kvm_find_sw_breakpoint(addr);
+	if (bp) {
+	    bp->use_count++;
+	    return 0;
+	}
+
+	bp = qemu_malloc(sizeof(struct kvm_sw_breakpoint));
+	if (!bp)
+	    return -ENOMEM;
+
+	bp->pc = addr;
+	bp->use_count = 1;
+	err = kvm_arch_insert_sw_breakpoint(current_env, bp);
+	if (err) {
+	    free(bp);
+	    return err;
 	}
-	data.dbg.singlestep = env->singlestep_enabled;
+
+	TAILQ_INSERT_HEAD(&kvm_sw_breakpoints, bp, entry);
+    } else {
+	err = kvm_arch_insert_hw_breakpoint(addr, len, type);
+	if (err)
+	    return err;
     }
-    on_vcpu(env, kvm_invoke_guest_debug, &data);
-    return data.err;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+	err = kvm_update_guest_debug(env, 0);
+	if (err)
+	    return err;
+    }
+    return 0;
 }
 
+int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    struct kvm_sw_breakpoint *bp;
+    CPUState *env;
+    int err;
+
+    if (type == GDB_BREAKPOINT_SW) {
+	bp = kvm_find_sw_breakpoint(addr);
+	if (!bp)
+	    return -ENOENT;
+
+	if (bp->use_count > 1) {
+	    bp->use_count--;
+	    return 0;
+	}
+
+	err = kvm_arch_remove_sw_breakpoint(current_env, bp);
+	if (err)
+	    return err;
+
+	TAILQ_REMOVE(&kvm_sw_breakpoints, bp, entry);
+	qemu_free(bp);
+    } else {
+	err = kvm_arch_remove_hw_breakpoint(addr, len, type);
+	if (err)
+	    return err;
+    }
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+	err = kvm_update_guest_debug(env, 0);
+	if (err)
+	    return err;
+    }
+    return 0;
+}
+
+void kvm_remove_all_breakpoints(CPUState *current_env)
+{
+    struct kvm_sw_breakpoint *bp, *next;
+    CPUState *env;
+
+    TAILQ_FOREACH_SAFE(bp, &kvm_sw_breakpoints, entry, next) {
+        if (kvm_arch_remove_sw_breakpoint(current_env, bp) != 0) {
+            /* Try harder to find a CPU that currently sees the breakpoint. */
+            for (env = first_cpu; env != NULL; env = env->next_cpu) {
+                if (kvm_arch_remove_sw_breakpoint(env, bp) == 0)
+                    break;
+            }
+        }
+    }
+    kvm_arch_remove_all_hw_breakpoints();
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu)
+	kvm_update_guest_debug(env, 0);
+}
+
+#else /* !KVM_CAP_SET_GUEST_DEBUG */
+
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
+{
+    return -EINVAL;
+}
+
+int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    return -EINVAL;
+}
+
+int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    return -EINVAL;
+}
+
+void kvm_remove_all_breakpoints(CPUState *current_env)
+{
+}
+#endif /* !KVM_CAP_SET_GUEST_DEBUG */
 
 /*
  * dirty pages logging
diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index 90fadcd..bff9bac 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -22,7 +22,12 @@ void kvm_save_registers(CPUState *env);
 void kvm_load_mpstate(CPUState *env);
 void kvm_save_mpstate(CPUState *env);
 int kvm_cpu_exec(CPUState *env);
-int kvm_update_debugger(CPUState *env);
+int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type);
+int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type);
+void kvm_remove_all_breakpoints(CPUState *current_env);
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
 int kvm_qemu_init_env(CPUState *env);
 int kvm_qemu_check_extension(int ext);
 void kvm_apic_init(CPUState *env);
@@ -70,6 +75,32 @@ void kvm_arch_push_nmi(void *opaque);
 void kvm_arch_update_regs_for_sipi(CPUState *env);
 void kvm_arch_cpu_reset(CPUState *env);
 
+struct kvm_guest_debug;
+struct kvm_debug_exit_arch;
+
+struct kvm_sw_breakpoint {
+    target_ulong pc;
+    target_ulong saved_insn;
+    int use_count;
+    TAILQ_ENTRY(kvm_sw_breakpoint) entry;
+};
+TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint);
+
+extern struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
+struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(target_ulong pc);
+int kvm_arch_insert_sw_breakpoint(CPUState *current_env,
+                                  struct kvm_sw_breakpoint *bp);
+int kvm_arch_remove_sw_breakpoint(CPUState *current_env,
+                                  struct kvm_sw_breakpoint *bp);
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+				  target_ulong len, int type);
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+				  target_ulong len, int type);
+void kvm_arch_remove_all_hw_breakpoints(void);
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
+
 void qemu_kvm_aio_wait_start(void);
 void qemu_kvm_aio_wait(void);
 void qemu_kvm_aio_wait_end(void);
diff --git a/user/main.c b/user/main.c
index 9ba5b5e..a5b12eb 100644
--- a/user/main.c
+++ b/user/main.c
@@ -298,11 +298,14 @@ static int test_outl(void *opaque, uint16_t addr, uint32_t value)
 	return 0;
 }
 
-static int test_debug(void *opaque, void *vcpu)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+static int test_debug(void *opaque, void *vcpu,
+		      struct kvm_debug_exit_arch *arch_info)
 {
 	printf("test_debug\n");
 	return 0;
 }
+#endif
 
 static int test_halt(void *opaque, int vcpu)
 {
@@ -371,7 +374,9 @@ static struct kvm_callbacks test_callbacks = {
 	.outl        = test_outl,
 	.mmio_read   = test_mem_read,
 	.mmio_write  = test_mem_write,
+#ifdef KVM_CAP_SET_GUEST_DEBUG
 	.debug       = test_debug,
+#endif
 	.halt        = test_halt,
 	.io_window = test_io_window,
 	.try_push_interrupts = test_try_push_interrupts,

Re: [PATCH v3 0/3] KVM-userspace: Improved guest debugging / debug register emulation

[Avi Kivity]

Jan Kiszka wrote:
> Changes since last round:
>  - Fix non-x86 stubs in qemu-kvm-*.c (Christian Ehrhardt)
>
> As usual, find the patches also at git://git.kiszka.org/kvm-userspace.git gdb-queue
>
>   

Applied all, thanks.

-- 
error compiling committee.c: too many arguments to function