[PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Paul Durrant]

When Windows writes the CrashNotify bit in the CRASH_CTL MSR then we know
it is crashing, so set the domain shutdown code appropriately.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/viridian.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 486065182c..294cf486cc 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
         if ( !ctl.u.CrashNotify )
             break;
 
+        spin_lock(&d->shutdown_lock);
+        d->shutdown_code = SHUTDOWN_crash;
+        spin_unlock(&d->shutdown_lock);
+
         gprintk(XENLOG_WARNING, "VIRIDIAN CRASH: %lx %lx %lx %lx %lx\n",
                 v->arch.hvm_vcpu.viridian.crash_param[0],
                 v->arch.hvm_vcpu.viridian.crash_param[1],
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Andrew Cooper]

On 10/08/18 16:43, Paul Durrant wrote:
> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then we know
> it is crashing, so set the domain shutdown code appropriately.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/x86/hvm/viridian.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 486065182c..294cf486cc 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
>          if ( !ctl.u.CrashNotify )
>              break;
>  
> +        spin_lock(&d->shutdown_lock);
> +        d->shutdown_code = SHUTDOWN_crash;
> +        spin_unlock(&d->shutdown_lock);

How does the domain eventually shut down?  It feels slightly odd to have
a shutdown code before the domain has finished executing code.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Paul Durrant]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 10 August 2018 16:57
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Jan Beulich <jbeulich@suse.com>
> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to
> CrashNotify
> 
> On 10/08/18 16:43, Paul Durrant wrote:
> > When Windows writes the CrashNotify bit in the CRASH_CTL MSR then we
> know
> > it is crashing, so set the domain shutdown code appropriately.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> >  xen/arch/x86/hvm/viridian.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> > index 486065182c..294cf486cc 100644
> > --- a/xen/arch/x86/hvm/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian.c
> > @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
> >          if ( !ctl.u.CrashNotify )
> >              break;
> >
> > +        spin_lock(&d->shutdown_lock);
> > +        d->shutdown_code = SHUTDOWN_crash;
> > +        spin_unlock(&d->shutdown_lock);
> 
> How does the domain eventually shut down?

I assume it shuts down when the guest writes to the PIIX.

>  It feels slightly odd to have
> a shutdown code before the domain has finished executing code.
> 

That's the norm. The PV drivers (if they are installed) set it via a schedop. This just makes sure it is set even if the PV drivers aren't there.

  Paul

> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Andrew Cooper]

On 10/08/18 16:59, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper
>> Sent: 10 August 2018 16:57
>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to
>> CrashNotify
>>
>> On 10/08/18 16:43, Paul Durrant wrote:
>>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then we
>> know
>>> it is crashing, so set the domain shutdown code appropriately.
>>>
>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>>> ---
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>>  xen/arch/x86/hvm/viridian.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
>>> index 486065182c..294cf486cc 100644
>>> --- a/xen/arch/x86/hvm/viridian.c
>>> +++ b/xen/arch/x86/hvm/viridian.c
>>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
>>>          if ( !ctl.u.CrashNotify )
>>>              break;
>>>
>>> +        spin_lock(&d->shutdown_lock);
>>> +        d->shutdown_code = SHUTDOWN_crash;
>>> +        spin_unlock(&d->shutdown_lock);
>> How does the domain eventually shut down?
> I assume it shuts down when the guest writes to the PIIX.
>
>>   It feels slightly odd to have
>> a shutdown code before the domain has finished executing code.
>>
> That's the norm. The PV drivers (if they are installed) set it via a schedop. This just makes sure it is set even if the PV drivers aren't there.

What happens if the user has configured windows to reboot after a crash?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Paul Durrant]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 16 August 2018 10:40
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Jan Beulich <jbeulich@suse.com>
> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to
> CrashNotify
> 
> On 10/08/18 16:59, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Andrew Cooper
> >> Sent: 10 August 2018 16:57
> >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
> devel@lists.xenproject.org
> >> Cc: Jan Beulich <jbeulich@suse.com>
> >> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response
> to
> >> CrashNotify
> >>
> >> On 10/08/18 16:43, Paul Durrant wrote:
> >>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then
> we
> >> know
> >>> it is crashing, so set the domain shutdown code appropriately.
> >>>
> >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >>> ---
> >>> Cc: Jan Beulich <jbeulich@suse.com>
> >>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >>> ---
> >>>  xen/arch/x86/hvm/viridian.c | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> >>> index 486065182c..294cf486cc 100644
> >>> --- a/xen/arch/x86/hvm/viridian.c
> >>> +++ b/xen/arch/x86/hvm/viridian.c
> >>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t
> val)
> >>>          if ( !ctl.u.CrashNotify )
> >>>              break;
> >>>
> >>> +        spin_lock(&d->shutdown_lock);
> >>> +        d->shutdown_code = SHUTDOWN_crash;
> >>> +        spin_unlock(&d->shutdown_lock);
> >> How does the domain eventually shut down?
> > I assume it shuts down when the guest writes to the PIIX.
> >
> >>   It feels slightly odd to have
> >> a shutdown code before the domain has finished executing code.
> >>
> > That's the norm. The PV drivers (if they are installed) set it via a schedop.
> This just makes sure it is set even if the PV drivers aren't there.
> 
> What happens if the user has configured windows to reboot after a crash?
> 

That's exactly what this is trying to fix. Without the patch a VM without PV drivers will appear to just shut down (because nothing will set the shutdown code to anything else) so the actions-after-crash will not take effect.

  Paul

> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Paul Durrant]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 23 August 2018 09:38
> To: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen-
> devel@lists.xenproject.org
> Cc: Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in
> response to CrashNotify
> 
> > -----Original Message-----
> > From: Andrew Cooper
> > Sent: 16 August 2018 10:40
> > To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
> devel@lists.xenproject.org
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to
> > CrashNotify
> >
> > On 10/08/18 16:59, Paul Durrant wrote:
> > >> -----Original Message-----
> > >> From: Andrew Cooper
> > >> Sent: 10 August 2018 16:57
> > >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
> > devel@lists.xenproject.org
> > >> Cc: Jan Beulich <jbeulich@suse.com>
> > >> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response
> > to
> > >> CrashNotify
> > >>
> > >> On 10/08/18 16:43, Paul Durrant wrote:
> > >>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then
> > we
> > >> know
> > >>> it is crashing, so set the domain shutdown code appropriately.
> > >>>
> > >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > >>> ---
> > >>> Cc: Jan Beulich <jbeulich@suse.com>
> > >>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > >>> ---
> > >>>  xen/arch/x86/hvm/viridian.c | 4 ++++
> > >>>  1 file changed, 4 insertions(+)
> > >>>
> > >>> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> > >>> index 486065182c..294cf486cc 100644
> > >>> --- a/xen/arch/x86/hvm/viridian.c
> > >>> +++ b/xen/arch/x86/hvm/viridian.c
> > >>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t
> > val)
> > >>>          if ( !ctl.u.CrashNotify )
> > >>>              break;
> > >>>
> > >>> +        spin_lock(&d->shutdown_lock);
> > >>> +        d->shutdown_code = SHUTDOWN_crash;
> > >>> +        spin_unlock(&d->shutdown_lock);
> > >> How does the domain eventually shut down?
> > > I assume it shuts down when the guest writes to the PIIX.
> > >
> > >>   It feels slightly odd to have
> > >> a shutdown code before the domain has finished executing code.
> > >>
> > > That's the norm. The PV drivers (if they are installed) set it via a schedop.
> > This just makes sure it is set even if the PV drivers aren't there.
> >
> > What happens if the user has configured windows to reboot after a crash?
> >
> 
> That's exactly what this is trying to fix. Without the patch a VM without PV
> drivers will appear to just shut down (because nothing will set the shutdown
> code to anything else) so the actions-after-crash will not take effect.
> 

Ping? Are you ok with the patch now?

  Paul

>   Paul
> 
> > ~Andrew
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Andrew Cooper]

On 06/09/18 16:32, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
>> Of Paul Durrant
>> Sent: 23 August 2018 09:38
>> To: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen-
>> devel@lists.xenproject.org
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Subject: Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in
>> response to CrashNotify
>>
>>> -----Original Message-----
>>> From: Andrew Cooper
>>> Sent: 16 August 2018 10:40
>>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
>> devel@lists.xenproject.org
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to
>>> CrashNotify
>>>
>>> On 10/08/18 16:59, Paul Durrant wrote:
>>>>> -----Original Message-----
>>>>> From: Andrew Cooper
>>>>> Sent: 10 August 2018 16:57
>>>>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
>>> devel@lists.xenproject.org
>>>>> Cc: Jan Beulich <jbeulich@suse.com>
>>>>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response
>>> to
>>>>> CrashNotify
>>>>>
>>>>> On 10/08/18 16:43, Paul Durrant wrote:
>>>>>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then
>>> we
>>>>> know
>>>>>> it is crashing, so set the domain shutdown code appropriately.
>>>>>>
>>>>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>>>>>> ---
>>>>>> Cc: Jan Beulich <jbeulich@suse.com>
>>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>>> ---
>>>>>>  xen/arch/x86/hvm/viridian.c | 4 ++++
>>>>>>  1 file changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
>>>>>> index 486065182c..294cf486cc 100644
>>>>>> --- a/xen/arch/x86/hvm/viridian.c
>>>>>> +++ b/xen/arch/x86/hvm/viridian.c
>>>>>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t
>>> val)
>>>>>>          if ( !ctl.u.CrashNotify )
>>>>>>              break;
>>>>>>
>>>>>> +        spin_lock(&d->shutdown_lock);
>>>>>> +        d->shutdown_code = SHUTDOWN_crash;
>>>>>> +        spin_unlock(&d->shutdown_lock);
>>>>> How does the domain eventually shut down?
>>>> I assume it shuts down when the guest writes to the PIIX.
>>>>
>>>>>   It feels slightly odd to have
>>>>> a shutdown code before the domain has finished executing code.
>>>>>
>>>> That's the norm. The PV drivers (if they are installed) set it via a schedop.
>>> This just makes sure it is set even if the PV drivers aren't there.
>>>
>>> What happens if the user has configured windows to reboot after a crash?
>>>
>> That's exactly what this is trying to fix. Without the patch a VM without PV
>> drivers will appear to just shut down (because nothing will set the shutdown
>> code to anything else) so the actions-after-crash will not take effect.
>>
> Ping? Are you ok with the patch now?

Lol - I was literally just trying to find it.  I'll commit it in a moment.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

[Paul Durrant]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 06 September 2018 16:33
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Jan Beulich <jbeulich@suse.com>
> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to
> CrashNotify
> 
> On 06/09/18 16:32, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On
> Behalf
> >> Of Paul Durrant
> >> Sent: 23 August 2018 09:38
> >> To: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen-
> >> devel@lists.xenproject.org
> >> Cc: Jan Beulich <jbeulich@suse.com>
> >> Subject: Re: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code
> in
> >> response to CrashNotify
> >>
> >>> -----Original Message-----
> >>> From: Andrew Cooper
> >>> Sent: 16 August 2018 10:40
> >>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
> >> devel@lists.xenproject.org
> >>> Cc: Jan Beulich <jbeulich@suse.com>
> >>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response
> to
> >>> CrashNotify
> >>>
> >>> On 10/08/18 16:59, Paul Durrant wrote:
> >>>>> -----Original Message-----
> >>>>> From: Andrew Cooper
> >>>>> Sent: 10 August 2018 16:57
> >>>>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
> >>> devel@lists.xenproject.org
> >>>>> Cc: Jan Beulich <jbeulich@suse.com>
> >>>>> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in
> response
> >>> to
> >>>>> CrashNotify
> >>>>>
> >>>>> On 10/08/18 16:43, Paul Durrant wrote:
> >>>>>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR
> then
> >>> we
> >>>>> know
> >>>>>> it is crashing, so set the domain shutdown code appropriately.
> >>>>>>
> >>>>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >>>>>> ---
> >>>>>> Cc: Jan Beulich <jbeulich@suse.com>
> >>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>>>> ---
> >>>>>>  xen/arch/x86/hvm/viridian.c | 4 ++++
> >>>>>>  1 file changed, 4 insertions(+)
> >>>>>>
> >>>>>> diff --git a/xen/arch/x86/hvm/viridian.c
> b/xen/arch/x86/hvm/viridian.c
> >>>>>> index 486065182c..294cf486cc 100644
> >>>>>> --- a/xen/arch/x86/hvm/viridian.c
> >>>>>> +++ b/xen/arch/x86/hvm/viridian.c
> >>>>>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx,
> uint64_t
> >>> val)
> >>>>>>          if ( !ctl.u.CrashNotify )
> >>>>>>              break;
> >>>>>>
> >>>>>> +        spin_lock(&d->shutdown_lock);
> >>>>>> +        d->shutdown_code = SHUTDOWN_crash;
> >>>>>> +        spin_unlock(&d->shutdown_lock);
> >>>>> How does the domain eventually shut down?
> >>>> I assume it shuts down when the guest writes to the PIIX.
> >>>>
> >>>>>   It feels slightly odd to have
> >>>>> a shutdown code before the domain has finished executing code.
> >>>>>
> >>>> That's the norm. The PV drivers (if they are installed) set it via a
> schedop.
> >>> This just makes sure it is set even if the PV drivers aren't there.
> >>>
> >>> What happens if the user has configured windows to reboot after a
> crash?
> >>>
> >> That's exactly what this is trying to fix. Without the patch a VM without PV
> >> drivers will appear to just shut down (because nothing will set the
> shutdown
> >> code to anything else) so the actions-after-crash will not take effect.
> >>
> > Ping? Are you ok with the patch now?
> 
> Lol - I was literally just trying to find it.  I'll commit it in a moment.
> 

:-)

> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel