KVM-userspace: Improved guest debugging / debug register emulation

PATCH Applied Jan Kiszka 2 participants 5 messages Dec 15, 2008 – Dec 17, 2008 KVM

lore

Share & tools

lore

Jan Kiszka

author

2008-12-15 12:53 UTC #0 lore raw

v3 cover KVM-userspace: Improved guest debugging / debug register emulation

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

Jan Kiszka

author

2008-12-15 12:53 UTC #1 lore raw

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

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 <[email protected]>

Signed-off-by Christian Ehrhardt <[email protected]>

---

 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(-)

libkvm/kvm-common.h +1 -1

index 9dae17b..c5beacc 100644
@@ -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

libkvm/libkvm.c +13 -5

index d4b1a73..096c097 100644
@@ -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)
 {

libkvm/libkvm.h +7 -2

index 392065b..d068fb3 100644
@@ -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__)
 /*!

qemu/exec.c +7 ~~-11~~

index 36ff1f0..f62eb77 100644
@@ -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
 }

qemu/gdbstub.c +14 -9

index b02571a..fd6223a 100644
@@ -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");

qemu/gdbstub.h +7 -0

index 1ba7931..e17ac90 100644
@@ -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);

qemu/qemu-kvm-ia64.c +37 -0

index 8cd3cff..f9e4b4b 100644
@@ -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

qemu/qemu-kvm-powerpc.c +37 -0

index dadcd1c..1dc7d9a 100644
@@ -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)
+{
+}

qemu/qemu-kvm-x86.c +172 -0

184 lines not on this page · raw message

qemu/qemu-kvm.c +167 ~~-32~~

254 lines not on this page · raw message

qemu/qemu-kvm.h +32 -1

index 90fadcd..bff9bac 100644
@@ -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);

user/main.c +6 -1

index 9ba5b5e..a5b12eb 100644
@@ -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,

Jan Kiszka

author

2008-12-15 12:53 UTC #2 lore raw

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

In reply to Jan Kiszka

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

Signed-off-by Jan Kiszka <[email protected]>

---

 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(-)

kernel/x86/external-module-compat.h +0 -2

index b5e11e2..1055050 100644
@@ -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) {}

kernel/x86/hack-module.awk +0 -4

index 2ad0951..f40c972 100644
@@ -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;"
 }

kernel/x86/preempt.c +0 -6

index 9e4bd2c..3112879 100644
@@ -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));

Jan Kiszka

author

2008-12-15 12:53 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---

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

kernel/x86/external-module-compat.h +10 -0

index 1055050..5489f47 100644
@@ -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 {

Avi Kivity

Applied

2008-12-17 12:22 UTC #4 lore raw

In reply to Jan Kiszka

Quoted 6 lines · 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

Applied all, thanks.

Signature · 1 line

error compiling committee.c: too many arguments to function