[kvm-devel] [PATCH] qemu-kvm: fix monitor and gdbstub deadlocks

[Jan K. <jan...@si...>]

Some monitor commands as well as the vm_stop() issued by the gdbstub on
external interruption so far deadlock on some vcpu locks in the kernel.
Patch below resolve the issue by temporarily or permanently stopping all
vcpu threads before issuing the related KVM IOCTLs.

Among other things, this patch now allows to break into the BIOS or
other guest code spinning in the vcpu (*) and to use things like "info
cpus" in the monitor.

Signed-off-by: Jan Kiszka <jan...@si...>
---
 qemu/gdbstub.c  |   12 ++++--------
 qemu/monitor.c  |    3 +--
 qemu/qemu-kvm.c |   39 +++++++++++++++++++++++++--------------
 qemu/vl.c       |    2 +-
 4 files changed, 31 insertions(+), 25 deletions(-)

Index: b/qemu/monitor.c
===================================================================
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -286,8 +286,7 @@ static CPUState *mon_get_cpu(void)
         mon_set_cpu(0);
     }
 
-    if (kvm_enabled())
-	kvm_save_registers(mon_cpu);
+    kvm_save_registers(mon_cpu);
 
     return mon_cpu;
 }
Index: b/qemu/qemu-kvm.c
===================================================================
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -130,18 +130,6 @@ static int pre_kvm_run(void *opaque, int
     return 0;
 }
 
-void kvm_load_registers(CPUState *env)
-{
-    if (kvm_enabled())
-	kvm_arch_load_regs(env);
-}
-
-void kvm_save_registers(CPUState *env)
-{
-    if (kvm_enabled())
-	kvm_arch_save_regs(env);
-}
-
 int kvm_cpu_exec(CPUState *env)
 {
     int r;
@@ -290,6 +278,24 @@ static void kvm_vm_state_change_handler(
 	pause_all_threads();
 }
 
+void kvm_load_registers(CPUState *env)
+{
+    if (kvm_enabled())
+	kvm_arch_load_regs(env);
+}
+
+void kvm_save_registers(CPUState *env)
+{
+    if (!kvm_enabled())
+        return;
+
+    if (vm_running)
+        pause_all_threads();
+    kvm_arch_save_regs(env);
+    if (vm_running)
+        resume_all_threads();
+}
+
 static void update_regs_for_sipi(CPUState *env)
 {
     kvm_arch_update_regs_for_sipi(env);
@@ -698,7 +704,7 @@ int kvm_qemu_init_env(CPUState *cenv)
 int kvm_update_debugger(CPUState *env)
 {
     struct kvm_debug_guest dbg;
-    int i;
+    int i, r;
 
     dbg.enabled = 0;
     if (env->nb_breakpoints || env->singlestep_enabled) {
@@ -709,7 +715,12 @@ int kvm_update_debugger(CPUState *env)
 	}
 	dbg.singlestep = env->singlestep_enabled;
     }
-    return kvm_guest_debug(kvm_context, env->cpu_index, &dbg);
+    if (vm_running)
+        pause_all_threads();
+    r = kvm_guest_debug(kvm_context, env->cpu_index, &dbg);
+    if (vm_running)
+        resume_all_threads();
+    return r;
 }
 
 
Index: b/qemu/vl.c
===================================================================
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -7165,12 +7165,12 @@ void vm_stop(int reason)
 {
     if (vm_running) {
         cpu_disable_ticks();
-        vm_running = 0;
         if (reason != 0) {
             if (vm_stop_cb) {
                 vm_stop_cb(vm_stop_opaque, reason);
             }
         }
+        vm_running = 0;
         vm_state_notify(0);
     }
 }
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -904,8 +904,7 @@ static int gdb_handle_packet(GDBState *s
             addr = strtoull(p, (char **)&p, 16);
 #if defined(TARGET_I386)
             env->eip = addr;
-	    if (kvm_enabled())
-		kvm_load_registers(env);
+            kvm_load_registers(env);
 #elif defined (TARGET_PPC)
             env->nip = addr;
 #elif defined (TARGET_SPARC)
@@ -928,8 +927,7 @@ static int gdb_handle_packet(GDBState *s
             addr = strtoull(p, (char **)&p, 16);
 #if defined(TARGET_I386)
             env->eip = addr;
-	    if (kvm_enabled())
-		kvm_load_registers(env);
+            kvm_load_registers(env);
 #elif defined (TARGET_PPC)
             env->nip = addr;
 #elif defined (TARGET_SPARC)
@@ -973,8 +971,7 @@ static int gdb_handle_packet(GDBState *s
         }
         break;
     case 'g':
-	if (kvm_enabled())
-	    kvm_save_registers(env);
+        kvm_save_registers(env);
         reg_size = cpu_gdb_read_registers(env, mem_buf);
         memtohex(buf, mem_buf, reg_size);
         put_packet(s, buf);
@@ -984,8 +981,7 @@ static int gdb_handle_packet(GDBState *s
         len = strlen(p) / 2;
         hextomem((uint8_t *)registers, p, len);
         cpu_gdb_write_registers(env, mem_buf, len);
-	if (kvm_enabled())
-	    kvm_load_registers(env);
+        kvm_load_registers(env);
         put_packet(s, "OK");
         break;
     case 'm':


(*) Fully functional guest debugging and guest debug register support is 
also working here, patches just wait to be reformatted...