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

2018-09-06 Thread Paul Durrant
> -Original Message-
> From: Andrew Cooper
> Sent: 06 September 2018 16:33
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Jan Beulich 
> 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-boun...@lists.xenproject.org] On
> Behalf
> >> Of Paul Durrant
> >> Sent: 23 August 2018 09:38
> >> To: Andrew Cooper ; xen-
> >> de...@lists.xenproject.org
> >> Cc: Jan Beulich 
> >> 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 ; xen-
> >> de...@lists.xenproject.org
> >>> Cc: Jan Beulich 
> >>> 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 ; xen-
> >>> de...@lists.xenproject.org
> >>>>> Cc: Jan Beulich 
> >>>>> 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 
> >>>>>> ---
> >>>>>> Cc: Jan Beulich 
> >>>>>> Cc: Andrew Cooper 
> >>>>>> ---
> >>>>>>  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(>shutdown_lock);
> >>>>>> +d->shutdown_code = SHUTDOWN_crash;
> >>>>>> +spin_unlock(>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: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

2018-09-06 Thread Andrew Cooper
On 06/09/18 16:32, Paul Durrant wrote:
>> -Original Message-
>> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
>> Of Paul Durrant
>> Sent: 23 August 2018 09:38
>> To: Andrew Cooper ; xen-
>> de...@lists.xenproject.org
>> Cc: Jan Beulich 
>> 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 ; xen-
>> de...@lists.xenproject.org
>>> Cc: Jan Beulich 
>>> 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 ; xen-
>>> de...@lists.xenproject.org
>>>>> Cc: Jan Beulich 
>>>>> 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 
>>>>>> ---
>>>>>> Cc: Jan Beulich 
>>>>>> Cc: Andrew Cooper 
>>>>>> ---
>>>>>>  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(>shutdown_lock);
>>>>>> +d->shutdown_code = SHUTDOWN_crash;
>>>>>> +spin_unlock(>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: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

2018-09-06 Thread Paul Durrant
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 23 August 2018 09:38
> To: Andrew Cooper ; xen-
> de...@lists.xenproject.org
> Cc: Jan Beulich 
> 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 ; xen-
> de...@lists.xenproject.org
> > Cc: Jan Beulich 
> > 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 ; xen-
> > de...@lists.xenproject.org
> > >> Cc: Jan Beulich 
> > >> 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 
> > >>> ---
> > >>> Cc: Jan Beulich 
> > >>> Cc: Andrew Cooper 
> > >>> ---
> > >>>  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(>shutdown_lock);
> > >>> +d->shutdown_code = SHUTDOWN_crash;
> > >>> +spin_unlock(>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: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

2018-08-23 Thread Paul Durrant
> -Original Message-
> From: Andrew Cooper
> Sent: 16 August 2018 10:40
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Jan Beulich 
> 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 ; xen-
> de...@lists.xenproject.org
> >> Cc: Jan Beulich 
> >> 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 
> >>> ---
> >>> Cc: Jan Beulich 
> >>> Cc: Andrew Cooper 
> >>> ---
> >>>  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(>shutdown_lock);
> >>> +d->shutdown_code = SHUTDOWN_crash;
> >>> +spin_unlock(>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: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

2018-08-16 Thread 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 ; xen-devel@lists.xenproject.org
>> Cc: Jan Beulich 
>> 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 
>>> ---
>>> Cc: Jan Beulich 
>>> Cc: Andrew Cooper 
>>> ---
>>>  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(>shutdown_lock);
>>> +d->shutdown_code = SHUTDOWN_crash;
>>> +spin_unlock(>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: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

2018-08-10 Thread Paul Durrant
> -Original Message-
> From: Andrew Cooper
> Sent: 10 August 2018 16:57
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Jan Beulich 
> 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 
> > ---
> > Cc: Jan Beulich 
> > Cc: Andrew Cooper 
> > ---
> >  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(>shutdown_lock);
> > +d->shutdown_code = SHUTDOWN_crash;
> > +spin_unlock(>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: [Xen-devel] [PATCH] x86/hvm/viridian: set shutdown_code in response to CrashNotify

2018-08-10 Thread 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 
> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> ---
>  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(>shutdown_lock);
> +d->shutdown_code = SHUTDOWN_crash;
> +spin_unlock(>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

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

2018-08-10 Thread 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 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 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(>shutdown_lock);
+d->shutdown_code = SHUTDOWN_crash;
+spin_unlock(>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