Re: [PATCH] qemu-kvm: fix guest resetting

[Marcelo Tosatti <mtosatti@redhat.com>]

On Fri, May 09, 2008 at 10:13:33AM +0200, Jan Kiszka wrote:
> Hmm, need to think a bit more about it as I don't see the benefit yet
> (code suggestions are welcome in the meantime :)!).

Doing the reset from two different contexes is confusing and more
complicated.

> The changes to vl.c are actually cleanups, as pause_all_threads() is now
> context-agnostic and we no longer need to go through the qemu way of
> raising reset. This new property of pause_all_threads() is something we
> should desire for simplicity and robustness reasons anyway (e.g. to
> simplify guest debugging later on). 

You can signal the IO thread (main_loop_break in the current
kvm-userspace.git) and then stop.

> And the way qemu_kvm_system_reset_request() is implemented would not
> change, we need this serialization to avoid races between IO and VCPU
> threads.

How about this ?

diff --git a/qemu/qemu-kvm-ia64.c b/qemu/qemu-kvm-ia64.c
index 4d0ddd7..d227d22 100644
--- a/qemu/qemu-kvm-ia64.c
+++ b/qemu/qemu-kvm-ia64.c
@@ -61,3 +61,7 @@ int kvm_arch_try_push_interrupts(void *opaque)
 void kvm_arch_update_regs_for_sipi(CPUState *env)
 {
 }
+
+void kvm_arch_cpu_reset(CPUState *env)
+{
+}
diff --git a/qemu/qemu-kvm-powerpc.c b/qemu/qemu-kvm-powerpc.c
index 14ed945..024b18c 100644
--- a/qemu/qemu-kvm-powerpc.c
+++ b/qemu/qemu-kvm-powerpc.c
@@ -213,3 +213,7 @@ int handle_powerpc_dcr_write(int vcpu, uint32_t dcrn, uint32_t data)
 
     return 0; /* XXX ignore failed DCR ops */
 }
+
+void kvm_arch_cpu_reset(CPUState *env)
+{
+}
diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
index 9a771ff..28eb5c2 100644
--- a/qemu/qemu-kvm-x86.c
+++ b/qemu/qemu-kvm-x86.c
@@ -689,3 +689,19 @@ int handle_tpr_access(void *opaque, int vcpu,
     kvm_tpr_access_report(cpu_single_env, rip, is_write);
     return 0;
 }
+
+void kvm_arch_cpu_reset(CPUState *env)
+{
+    struct kvm_mp_state mp_state = { .mp_state = KVM_MP_STATE_UNINITIALIZED };
+
+    kvm_arch_load_regs(env);
+    if (env->cpu_index != 0) {
+	if (kvm_irqchip_in_kernel(kvm_context))
+	    kvm_set_mpstate(kvm_context, env->cpu_index, &mp_state);
+	else {
+	    env->interrupt_request &= ~CPU_INTERRUPT_HARD;
+	    env->hflags |= HF_HALTED_MASK;
+	    env->exception_index = EXCP_HLT;
+	}
+    }
+}
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 3cc6d8e..1e1f065 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -31,8 +31,6 @@ kvm_context_t kvm_context;
 
 extern int smp_cpus;
 
-static int qemu_kvm_reset_requested;
-
 pthread_mutex_t qemu_mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t qemu_aio_cond = PTHREAD_COND_INITIALIZER;
 pthread_cond_t qemu_vcpu_cond = PTHREAD_COND_INITIALIZER;
@@ -52,7 +50,6 @@ struct vcpu_info {
     int signalled;
     int stop;
     int stopped;
-    int reload_regs;
     int created;
 } vcpu_info[256];
 
@@ -242,21 +239,29 @@ static void pause_all_threads(void)
 {
     int i;
 
+    if (cpu_single_env) {
+        fprintf(stderr, "qemu-kvm: pause_all_threads from vcpu context\n");
+        exit(0);
+    }
+
     for (i = 0; i < smp_cpus; ++i) {
 	vcpu_info[i].stop = 1;
 	pthread_kill(vcpu_info[i].thread, SIG_IPI);
     }
-    while (!all_threads_paused()) {
-	CPUState *env = cpu_single_env;
+    while (!all_threads_paused())
 	pthread_cond_wait(&qemu_pause_cond, &qemu_mutex);
-	cpu_single_env = env;
-    }
+    cpu_single_env = NULL;
 }
 
 static void resume_all_threads(void)
 {
     int i;
 
+    if (cpu_single_env) {
+        fprintf(stderr, "qemu-kvm: resume_all_threads from vcpu context\n");
+        exit(0);
+    }
+
     for (i = 0; i < smp_cpus; ++i) {
 	vcpu_info[i].stop = 0;
 	vcpu_info[i].stopped = 0;
@@ -301,15 +306,18 @@ static void setup_kernel_sigmask(CPUState *env)
     kvm_set_signal_mask(kvm_context, env->cpu_index, &set);
 }
 
-void qemu_kvm_system_reset_request(void)
+void qemu_kvm_system_reset(void)
 {
     int i;
 
-    for (i = 0; i < smp_cpus; ++i) {
-	vcpu_info[i].reload_regs = 1;
-	pthread_kill(vcpu_info[i].thread, SIG_IPI);
-    }
+    pause_all_threads();
+
     qemu_system_reset();
+
+    for (i = 0; i < smp_cpus; ++i)
+	kvm_arch_cpu_reset(vcpu_info[i].env);
+
+    resume_all_threads();
 }
 
 static int kvm_main_loop_cpu(CPUState *env)
@@ -342,11 +350,6 @@ static int kvm_main_loop_cpu(CPUState *env)
 	    kvm_cpu_exec(env);
 	env->interrupt_request &= ~CPU_INTERRUPT_EXIT;
 	kvm_main_loop_wait(env, 0);
-        if (info->reload_regs) {
-	    info->reload_regs = 0;
-	    if (env->cpu_index == 0) /* ap needs to be placed in INIT */
-		kvm_arch_load_regs(env);
-	}
     }
     pthread_mutex_unlock(&qemu_mutex);
     return 0;
@@ -529,10 +532,8 @@ int kvm_main_loop(void)
             break;
         else if (qemu_powerdown_requested())
             qemu_system_powerdown();
-        else if (qemu_reset_requested()) {
-            pthread_kill(vcpu_info[0].thread, SIG_IPI);
-            qemu_kvm_reset_requested = 1;
-        }
+        else if (qemu_reset_requested())
+	    qemu_kvm_system_reset();
     }
 
     pause_all_threads();
@@ -641,6 +642,9 @@ static int kvm_halt(void *opaque, int vcpu)
 
 static int kvm_shutdown(void *opaque, int vcpu)
 {
+    /* stop the current vcpu from going back to guest mode */
+    vcpu_info[cpu_single_env->cpu_index].stopped = 1;
+
     qemu_system_reset_request();
     return 1;
 }
diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index 21606e9..7412e20 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -59,6 +59,7 @@ void kvm_arch_post_kvm_run(void *opaque, int vcpu);
 int kvm_arch_has_work(CPUState *env);
 int kvm_arch_try_push_interrupts(void *opaque);
 void kvm_arch_update_regs_for_sipi(CPUState *env);
+void kvm_arch_cpu_reset(CPUState *env);
 
 CPUState *qemu_kvm_cpu_env(int index);
 
diff --git a/qemu/vl.c b/qemu/vl.c
index 45c97af..a38c336 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -7302,11 +7302,7 @@ void qemu_system_reset_request(void)
     }
     if (cpu_single_env)
         cpu_interrupt(cpu_single_env, CPU_INTERRUPT_EXIT);
-#ifdef USE_KVM
-    if (kvm_allowed)
-        if (!no_reboot)
-            qemu_kvm_system_reset_request();
-#endif
+    main_loop_break();
 }
 
 void qemu_system_shutdown_request(void)

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