From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0 Date: Mon, 11 Apr 2016 16:37:41 +0100 Message-ID: <1460389061-19451-1-git-send-email-will.deacon@arm.com> Cc: mpe@ellerman.id.au, Will Deacon , Julien Grall , Balbir Singh To: kvm@vger.kernel.org Return-path: Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]:65160 "EHLO cam-admin0.cambridge.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755459AbcDKPht (ORCPT ); Mon, 11 Apr 2016 11:37:49 -0400 Sender: kvm-owner@vger.kernel.org List-ID: 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 Cc: Balbir Singh Signed-off-by: Will Deacon --- 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