[PATCH] kvm-qemu: Fix monitor and gdbstub deadlocks v2

[Jan Kiszka <jan.kiszka@web.de>]

Refreshed version of the earlier posted patch, now also adding a safety
check about the correct vm state to kvm_load_registers.

Some monitor commands as well as the vm_stop() issued by the gdbstub on
external interruption so far deadlock on vcpu locks in the kernel. Patch
below resolves the issue by temporarily or permanently stopping all vcpu
threads before issuing the related KVM IOCTLs. At this chance the patch
also removes redundant checks for kvm_enabled.

Among other things, the 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.kiszka@siemens.com>
---
 qemu/gdbstub.c  |   12 ++++--------
 qemu/monitor.c  |    3 +--
 qemu/qemu-kvm.c |   41 +++++++++++++++++++++++++++--------------
 qemu/vl.c       |    2 +-
 4 files changed, 33 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
@@ -137,18 +137,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;
@@ -276,6 +264,26 @@ static void kvm_vm_state_change_handler(
 	pause_all_threads();
 }
 
+void kvm_load_registers(CPUState *env)
+{
+    assert(!vm_running);
+
+    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);
@@ -764,7 +772,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) {
@@ -775,7 +783,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
@@ -7225,12 +7225,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':

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone