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

Marcelo Tosatti
2008-05-09T16:59:35Z

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)