* [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
* Re: [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0
2016-04-11 15:37 [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0 Will Deacon
@ 2016-04-12 10:03 ` Julien Grall
2016-04-14 1:57 ` Balbir Singh
1 sibling, 0 replies; 4+ messages in thread
From: Julien Grall @ 2016-04-12 10:03 UTC (permalink / raw)
To: Will Deacon, kvm; +Cc: mpe, Balbir Singh
Hello Will,
On 11/04/2016 16:37, Will Deacon wrote:
> 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>
I've been able to power cycle 1000 guests without any issue. Without
this patch it break at ~50 cycles.
Tested-by: Julien Grall <julien.grall@arm.com>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0
2016-04-11 15:37 [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0 Will Deacon
2016-04-12 10:03 ` Julien Grall
@ 2016-04-14 1:57 ` Balbir Singh
2016-04-14 8:49 ` Will Deacon
1 sibling, 1 reply; 4+ messages in thread
From: Balbir Singh @ 2016-04-14 1:57 UTC (permalink / raw)
To: Will Deacon, kvm; +Cc: mpe, Julien Grall
On 12/04/16 01:37, Will Deacon wrote:
> 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?
Yes, it does
Acked-by: Balbir Singh <bsingharora@gmail.com>
My patch for exit race fixes can be dropped in favour of this change
Balbir Singh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0
2016-04-14 1:57 ` Balbir Singh
@ 2016-04-14 8:49 ` Will Deacon
0 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2016-04-14 8:49 UTC (permalink / raw)
To: Balbir Singh; +Cc: kvm, mpe, Julien Grall
On Thu, Apr 14, 2016 at 11:57:31AM +1000, Balbir Singh wrote:
>
>
> On 12/04/16 01:37, Will Deacon wrote:
> > 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?
>
>
> Yes, it does
>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
>
> My patch for exit race fixes can be dropped in favour of this change
Thanks, Balbir! I've pushed it out, along with your ppc changes.
Will
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-14 8:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-11 15:37 [PATCH] kvmtool: delegate exit/reboot responsibility to vcpu0 Will Deacon
2016-04-12 10:03 ` Julien Grall
2016-04-14 1:57 ` Balbir Singh
2016-04-14 8:49 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.