[PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0

* [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0
@ 2016-04-11 15:37 Will Deacon
  2016-04-12 10:03 ` Julien Grall
  2016-04-14  1:57 ` Balbir Singh
  0 siblings, 2 replies; 4+ messages in thread
From: Will Deacon @ 2016-04-11 15:37 UTC (permalink / raw)
  To: kvm; +Cc: mpe, Will Deacon, Julien Grall, Balbir Singh

Our exit/reboot code is a bit of a mess:

  - Both kvm__reboot and kvm_cpu_exit send SIGKVMEXIT to running vcpus
  - When vcpu0 exits, the main thread starts executing destructors
    (exitcalls) whilst other vcpus may be running
  - The pause_lock isn't always held when inspecting is_running for
    a vcpu

This patch attempts to fix these issues by restricting the exit/reboot
path to vcpu0 and the main thread. In particular, a KVM_SYSTEM_EVENT
will signal SIGKVMEXIT to vcpu0, which will join with the main thread
and then tear down the other vcpus before invoking any destructor code.

Cc: Julien Grall <julien.grall@arm.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---

Balbir -- does this work on ppc?

 builtin-run.c |  6 ++++--
 kvm-cpu.c     |  3 ++-
 kvm.c         | 28 ++++++----------------------
 3 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/builtin-run.c b/builtin-run.c
index edcaf3ecde01..72b878dcff38 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -630,7 +630,6 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 static int kvm_cmd_run_work(struct kvm *kvm)
 {
 	int i;
-	void *ret = NULL;
 
 	for (i = 0; i < kvm->nrcpus; i++) {
 		if (pthread_create(&kvm->cpus[i]->thread, NULL, kvm_cpu_thread, kvm->cpus[i]) != 0)
@@ -638,7 +637,10 @@ static int kvm_cmd_run_work(struct kvm *kvm)
 	}
 
 	/* Only VCPU #0 is going to exit by itself when shutting down */
-	return pthread_join(kvm->cpus[0]->thread, &ret);
+	if (pthread_join(kvm->cpus[0]->thread, NULL) != 0)
+		die("unable to join with vcpu 0");
+
+	return kvm_cpu__exit(kvm);
 }
 
 static void kvm_cmd_run_exit(struct kvm *kvm, int guest_ret)
diff --git a/kvm-cpu.c b/kvm-cpu.c
index 2af459b34807..cc8385f6143e 100644
--- a/kvm-cpu.c
+++ b/kvm-cpu.c
@@ -305,6 +305,7 @@ int kvm_cpu__exit(struct kvm *kvm)
 	kvm_cpu__delete(kvm->cpus[0]);
 	kvm->cpus[0] = NULL;
 
+	kvm__pause(kvm);
 	for (i = 1; i < kvm->nrcpus; i++) {
 		if (kvm->cpus[i]->is_running) {
 			pthread_kill(kvm->cpus[i]->thread, SIGKVMEXIT);
@@ -315,6 +316,7 @@ int kvm_cpu__exit(struct kvm *kvm)
 		if (ret == NULL)
 			r = 0;
 	}
+	kvm__continue(kvm);
 
 	free(kvm->cpus);
 
@@ -324,4 +326,3 @@ int kvm_cpu__exit(struct kvm *kvm)
 
 	return r;
 }
-core_exit(kvm_cpu__exit);
diff --git a/kvm.c b/kvm.c
index 18b46068271e..7fa76f784de7 100644
--- a/kvm.c
+++ b/kvm.c
@@ -396,22 +396,15 @@ void kvm__dump_mem(struct kvm *kvm, unsigned long addr, unsigned long size, int
 
 void kvm__reboot(struct kvm *kvm)
 {
-	int i;
-
 	/* Check if the guest is running */
 	if (!kvm->cpus[0] || kvm->cpus[0]->thread == 0)
 		return;
 
-	mutex_lock(&pause_lock);
-
-	/* The kvm->cpus array contains a null pointer in the last location */
-	for (i = 0; ; i++) {
-		if (kvm->cpus[i])
-			pthread_kill(kvm->cpus[i]->thread, SIGKVMEXIT);
-		else
-			break;
-	}
+	pthread_kill(kvm->cpus[0]->thread, SIGKVMEXIT);
+}
 
+void kvm__continue(struct kvm *kvm)
+{
 	mutex_unlock(&pause_lock);
 }
 
@@ -419,12 +412,12 @@ void kvm__pause(struct kvm *kvm)
 {
 	int i, paused_vcpus = 0;
 
+	mutex_lock(&pause_lock);
+
 	/* Check if the guest is running */
 	if (!kvm->cpus[0] || kvm->cpus[0]->thread == 0)
 		return;
 
-	mutex_lock(&pause_lock);
-
 	pause_event = eventfd(0, 0);
 	if (pause_event < 0)
 		die("Failed creating pause notification event");
@@ -445,15 +438,6 @@ void kvm__pause(struct kvm *kvm)
 	close(pause_event);
 }
 
-void kvm__continue(struct kvm *kvm)
-{
-	/* Check if the guest is running */
-	if (!kvm->cpus[0] || kvm->cpus[0]->thread == 0)
-		return;
-
-	mutex_unlock(&pause_lock);
-}
-
 void kvm__notify_paused(void)
 {
 	u64 p = 1;
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread