Re: [KVMTOOL][PATCH] Handle KVM_EXIT_SYSTEM_EVENT on any VCPU

[Mark Rutland]

On Thu, Sep 03, 2015 at 11:24:29AM +0100, Suzuki Poulose wrote:
> On Wed, Sep 2, 2015 at 11:50 AM, Mark Rutland  wrote:
> >
> > When VCPU #0 exits (e.g. due to KVM_EXIT_SYSTEM_EVENT), it sends
> > SIGKVMEXIT to all other VCPUs, waits for them to exit, then tears down
> > any remaining context. The signalling of SIGKVMEXIT is critical to
> > forcing VCPUs to shut down in response to a system event (e.g. PSCI
> > SYSTEM_OFF).
> >
> > VCPUs other that VCPU #0 simply exit in kvm_cpu_thread without forcing
> > other CPUs to shut down. Thus if a system event is taken on a VCPU other
> > than VCPU #0, the remaining CPUs are left online. This results in KVM
> > tool not exiting as expected when a system event is taken on a VCPU
> > other than VCPU #0 (as may happen if the guest panics).
> >
> > Fix this by signalling SIGKVMEXIT to VCPU #0 when a system event is
> > taken on other CPUs, so that it may tear things down as usual.
> >
> > Signed-off-by: Mark Rutland 
> > Cc: Will Deacon 
> > Cc: Marc Zyngier 
> > ---
> >  kvm-cpu.c | 15 +++
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/kvm-cpu.c b/kvm-cpu.c
> > index 5d90664..f47e3db 100644
> > --- a/kvm-cpu.c
> > +++ b/kvm-cpu.c
> > @@ -166,13 +166,20 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
> >  * treat all system events as shutdown request.
> >  */
> > switch (cpu->kvm_run->system_event.type) {
> > -   case KVM_SYSTEM_EVENT_RESET:
> > -   /* Fall through for now */
> > -   case KVM_SYSTEM_EVENT_SHUTDOWN:
> > -   goto exit_kvm;
> > default:
> > pr_warning("unknown system event type %d",
> >cpu->kvm_run->system_event.type);
> > +   /* fall through for now */
> > +   case KVM_SYSTEM_EVENT_RESET:
> > +   /* Fall through for now */
> > +   case KVM_SYSTEM_EVENT_SHUTDOWN:
> > +   /*
> > +* Ensure that VCPU #0 tears everything 
> > down if
> > +* the event was taken on a different VCPU.
> > +*/
> > +   if (cpu->cpu_id != 0)
> > +   
> > pthread_kill(cpu->kvm->cpus[0]->thread,
> > +SIGKVMEXIT);
> 
> Could we use kvm_cpu__reboot() instead ?
> 
> That sends SIGKVMEXIT to all the VCPUs, and is used by the other
> paths which trigger a reboot/shutdown.

Sure; I'll spin a v2 with that and an updated comment.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [KVMTOOL][PATCH] Handle KVM_EXIT_SYSTEM_EVENT on any VCPU

[Suzuki Poulose]

On Wed, Sep 2, 2015 at 11:50 AM, Mark Rutland  wrote:
>
> When VCPU #0 exits (e.g. due to KVM_EXIT_SYSTEM_EVENT), it sends
> SIGKVMEXIT to all other VCPUs, waits for them to exit, then tears down
> any remaining context. The signalling of SIGKVMEXIT is critical to
> forcing VCPUs to shut down in response to a system event (e.g. PSCI
> SYSTEM_OFF).
>
> VCPUs other that VCPU #0 simply exit in kvm_cpu_thread without forcing
> other CPUs to shut down. Thus if a system event is taken on a VCPU other
> than VCPU #0, the remaining CPUs are left online. This results in KVM
> tool not exiting as expected when a system event is taken on a VCPU
> other than VCPU #0 (as may happen if the guest panics).
>
> Fix this by signalling SIGKVMEXIT to VCPU #0 when a system event is
> taken on other CPUs, so that it may tear things down as usual.
>
> Signed-off-by: Mark Rutland 
> Cc: Will Deacon 
> Cc: Marc Zyngier 
> ---
>  kvm-cpu.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/kvm-cpu.c b/kvm-cpu.c
> index 5d90664..f47e3db 100644
> --- a/kvm-cpu.c
> +++ b/kvm-cpu.c
> @@ -166,13 +166,20 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
>  * treat all system events as shutdown request.
>  */
> switch (cpu->kvm_run->system_event.type) {
> -   case KVM_SYSTEM_EVENT_RESET:
> -   /* Fall through for now */
> -   case KVM_SYSTEM_EVENT_SHUTDOWN:
> -   goto exit_kvm;
> default:
> pr_warning("unknown system event type %d",
>cpu->kvm_run->system_event.type);
> +   /* fall through for now */
> +   case KVM_SYSTEM_EVENT_RESET:
> +   /* Fall through for now */
> +   case KVM_SYSTEM_EVENT_SHUTDOWN:
> +   /*
> +* Ensure that VCPU #0 tears everything down 
> if
> +* the event was taken on a different VCPU.
> +*/
> +   if (cpu->cpu_id != 0)
> +   
> pthread_kill(cpu->kvm->cpus[0]->thread,
> +SIGKVMEXIT);

Could we use kvm_cpu__reboot() instead ?

That sends SIGKVMEXIT to all the VCPUs, and is used by the other
paths which trigger a reboot/shutdown.

Suzuki
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html