[PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Andrew Cooper]

While the triple fault action on native hardware will result in a system
reset, any modern operating system can and will make use of less violent
reboot methods.  As a result, the most likely cause of a triple fault is a
fatal software bug.

This patch allows the toolstack to indicate that a triple fault should mean a
crash rather than a reboot.  The default of reboot still remains the same.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
 void hvm_triple_fault(void)
 {
     struct vcpu *v = current;
+    struct domain * d = v->domain;
+    u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
+        ? SHUTDOWN_crash : SHUTDOWN_reboot;
+
     gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
-             "invoking HVM system reset.\n", v->vcpu_id);
-    domain_shutdown(v->domain, SHUTDOWN_reboot);
+             "invoking HVM system %s.\n", v->vcpu_id,
+             reason == SHUTDOWN_crash ? "crash" : "reboot");
+    domain_shutdown(v->domain, reason);
 }
 
 void hvm_inject_trap(struct hvm_trap *trap)
diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/include/public/hvm/params.h
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -142,6 +142,9 @@
 #define HVM_PARAM_ACCESS_RING_PFN   28
 #define HVM_PARAM_SHARING_RING_PFN  29
 
-#define HVM_NR_PARAMS          31
+/* Boolean: Should a triple fault imply crash rather than reboot? */
+#define HVM_PARAM_TRIPLE_FAULT_CRASH 31
+
+#define HVM_NR_PARAMS          32
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Jan Beulich]

>>> On 04.02.13 at 15:25, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> While the triple fault action on native hardware will result in a system
> reset, any modern operating system can and will make use of less violent
> reboot methods.  As a result, the most likely cause of a triple fault is a
> fatal software bug.
> 
> This patch allows the toolstack to indicate that a triple fault should mean 
> a
> crash rather than a reboot.  The default of reboot still remains the same.

Makes sense to me; minor nits below (no need to resend just
because of that, but would be nice to be addressed if you had
to rev the patch anyway).

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
>  void hvm_triple_fault(void)
>  {
>      struct vcpu *v = current;
> +    struct domain * d = v->domain;

Stray blank.

> +    u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
> +        ? SHUTDOWN_crash : SHUTDOWN_reboot;
> +
>      gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
> -             "invoking HVM system reset.\n", v->vcpu_id);
> -    domain_shutdown(v->domain, SHUTDOWN_reboot);
> +             "invoking HVM system %s.\n", v->vcpu_id,
> +             reason == SHUTDOWN_crash ? "crash" : "reboot");
> +    domain_shutdown(v->domain, reason);

So you have d cached in a local variable now, yet you still use
v->domain here?

Also, I'd prefer for the message to continue to say "reset".

Jan

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Andrew Cooper]

On 04/02/13 14:46, Jan Beulich wrote:
>>>> On 04.02.13 at 15:25, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> While the triple fault action on native hardware will result in a system
>> reset, any modern operating system can and will make use of less violent
>> reboot methods.  As a result, the most likely cause of a triple fault is a
>> fatal software bug.
>>
>> This patch allows the toolstack to indicate that a triple fault should mean 
>> a
>> crash rather than a reboot.  The default of reboot still remains the same.
> Makes sense to me; minor nits below (no need to resend just
> because of that, but would be nice to be addressed if you had
> to rev the patch anyway).
>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
>>  void hvm_triple_fault(void)
>>  {
>>      struct vcpu *v = current;
>> +    struct domain * d = v->domain;
> Stray blank.

Space between * and d ?

>
>> +    u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
>> +        ? SHUTDOWN_crash : SHUTDOWN_reboot;
>> +
>>      gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
>> -             "invoking HVM system reset.\n", v->vcpu_id);
>> -    domain_shutdown(v->domain, SHUTDOWN_reboot);
>> +             "invoking HVM system %s.\n", v->vcpu_id,
>> +             reason == SHUTDOWN_crash ? "crash" : "reboot");
>> +    domain_shutdown(v->domain, reason);
> So you have d cached in a local variable now, yet you still use
> v->domain here?

Doh - missed that.

>
> Also, I'd prefer for the message to continue to say "reset".
>
> Jan
>
Ok - I will respin and send as non-rfc.

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Ian Campbell]

On Mon, 2013-02-04 at 14:25 +0000, Andrew Cooper wrote:
> While the triple fault action on native hardware will result in a system
> reset, any modern operating system can and will make use of less violent
> reboot methods.  As a result, the most likely cause of a triple fault is a
> fatal software bug.
> 
> This patch allows the toolstack to indicate that a triple fault should mean a
> crash rather than a reboot.  The default of reboot still remains the same.

Just a random thought -- what about adding SHUTDOWN_triple_fault as an
explicit thing, then the toolstack can decide what to do?

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
>  void hvm_triple_fault(void)
>  {
>      struct vcpu *v = current;
> +    struct domain * d = v->domain;
> +    u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
> +        ? SHUTDOWN_crash : SHUTDOWN_reboot;
> +
>      gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
> -             "invoking HVM system reset.\n", v->vcpu_id);
> -    domain_shutdown(v->domain, SHUTDOWN_reboot);
> +             "invoking HVM system %s.\n", v->vcpu_id,
> +             reason == SHUTDOWN_crash ? "crash" : "reboot");
> +    domain_shutdown(v->domain, reason);
>  }
>  
>  void hvm_inject_trap(struct hvm_trap *trap)
> diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/include/public/hvm/params.h
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -142,6 +142,9 @@
>  #define HVM_PARAM_ACCESS_RING_PFN   28
>  #define HVM_PARAM_SHARING_RING_PFN  29
>  
> -#define HVM_NR_PARAMS          31
> +/* Boolean: Should a triple fault imply crash rather than reboot? */
> +#define HVM_PARAM_TRIPLE_FAULT_CRASH 31
> +
> +#define HVM_NR_PARAMS          32
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Keir Fraser]

On 04/02/2013 15:26, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

> On Mon, 2013-02-04 at 14:25 +0000, Andrew Cooper wrote:
>> While the triple fault action on native hardware will result in a system
>> reset, any modern operating system can and will make use of less violent
>> reboot methods.  As a result, the most likely cause of a triple fault is a
>> fatal software bug.
>> 
>> This patch allows the toolstack to indicate that a triple fault should mean a
>> crash rather than a reboot.  The default of reboot still remains the same.
> 
> Just a random thought -- what about adding SHUTDOWN_triple_fault as an
> explicit thing, then the toolstack can decide what to do?

I kind of prefer that, although it will require changes to every toolstack.

An alternative would be to do that, *and* still have the new HVM_PARAM, so
that any SHUTDOWN_* code can be generated by a triple fault (including new
SHUTDOWN_triple_fault) -- but defaulting to SHUTDOWN_reboot so that the
default behaviour is still unchanged.

Or, in any case, I'm not dead against the existing patch, it just seems less
flexible than it could be. But maybe that flexibility is pointless.

 -- Keir

>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/arch/x86/hvm/hvm.c
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
>>  void hvm_triple_fault(void)
>>  {
>>      struct vcpu *v = current;
>> +    struct domain * d = v->domain;
>> +    u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
>> +        ? SHUTDOWN_crash : SHUTDOWN_reboot;
>> +
>>      gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
>> -             "invoking HVM system reset.\n", v->vcpu_id);
>> -    domain_shutdown(v->domain, SHUTDOWN_reboot);
>> +             "invoking HVM system %s.\n", v->vcpu_id,
>> +             reason == SHUTDOWN_crash ? "crash" : "reboot");
>> +    domain_shutdown(v->domain, reason);
>>  }
>>  
>>  void hvm_inject_trap(struct hvm_trap *trap)
>> diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/include/public/hvm/params.h
>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -142,6 +142,9 @@
>>  #define HVM_PARAM_ACCESS_RING_PFN   28
>>  #define HVM_PARAM_SHARING_RING_PFN  29
>>  
>> -#define HVM_NR_PARAMS          31
>> +/* Boolean: Should a triple fault imply crash rather than reboot? */
>> +#define HVM_PARAM_TRIPLE_FAULT_CRASH 31
>> +
>> +#define HVM_NR_PARAMS          32
>>  
>>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> 

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Andrew Cooper]

On 04/02/13 16:46, Keir Fraser wrote:
> On 04/02/2013 15:26, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
>
>> On Mon, 2013-02-04 at 14:25 +0000, Andrew Cooper wrote:
>>> While the triple fault action on native hardware will result in a system
>>> reset, any modern operating system can and will make use of less violent
>>> reboot methods.  As a result, the most likely cause of a triple fault is a
>>> fatal software bug.
>>>
>>> This patch allows the toolstack to indicate that a triple fault should mean a
>>> crash rather than a reboot.  The default of reboot still remains the same.
>> Just a random thought -- what about adding SHUTDOWN_triple_fault as an
>> explicit thing, then the toolstack can decide what to do?
> I kind of prefer that, although it will require changes to every toolstack.
>
> An alternative would be to do that, *and* still have the new HVM_PARAM, so
> that any SHUTDOWN_* code can be generated by a triple fault (including new
> SHUTDOWN_triple_fault) -- but defaulting to SHUTDOWN_reboot so that the
> default behaviour is still unchanged.
>
> Or, in any case, I'm not dead against the existing patch, it just seems less
> flexible than it could be. But maybe that flexibility is pointless.
>
>  -- Keir

I considered this approach originally, but decided against it.

SHUTDOWN_triple_fault would be meaningless as a standard SCHOP_shutdown
parameter, and having the toolstack differentiate between _crash and
_triple_fault seems pointless.

I thought that the ideal end result would be specifying

on_triple_fault="reboot"|"crash"

In the vm.cfg file

The on_{crash,reboot} actions would still then take effect as usual.

Having said that, if _triple_fault is preferred, I am not overly
attached to this specific implementation.


If it isn't obvious, the motivation behind this patch is because I am
currently chasing a windows triple fault on Xen-4.2.  It appears machine
specific, but related to our PV driver, and takes a long time to
reproduce.  Having automated tests fail soon with a triple fault is
better than having the domain in question sit in a reboot loop until the
hour long timeout kicks in.

~Andrew

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Keir Fraser]

On 04/02/2013 17:12, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:

>> An alternative would be to do that, *and* still have the new HVM_PARAM, so
>> that any SHUTDOWN_* code can be generated by a triple fault (including new
>> SHUTDOWN_triple_fault) -- but defaulting to SHUTDOWN_reboot so that the
>> default behaviour is still unchanged.
>> 
>> Or, in any case, I'm not dead against the existing patch, it just seems less
>> flexible than it could be. But maybe that flexibility is pointless.
>> 
>>  -- Keir
> 
> I considered this approach originally, but decided against it.
> 
> SHUTDOWN_triple_fault would be meaningless as a standard SCHOP_shutdown
> parameter, and having the toolstack differentiate between _crash and
> _triple_fault seems pointless.

How about letting the HVM_PARAM accept any SHUTDOWN_ code? Rather than being
a boolean? That's a trivial change, just seems a bit cleaner than a boolean
to me.

Also adding the SHUTDOWN_triple_fault seemed like a maybe-nice-to-have. I
don't really care that much, and indeed it probably is pointless.

> I thought that the ideal end result would be specifying
> 
> on_triple_fault="reboot"|"crash"
> 
> In the vm.cfg file
> 
> The on_{crash,reboot} actions would still then take effect as usual.
> 
> Having said that, if _triple_fault is preferred, I am not overly
> attached to this specific implementation.

No let's drop the idea of a SHUTDOWN_triple_fault. :)

> If it isn't obvious, the motivation behind this patch is because I am
> currently chasing a windows triple fault on Xen-4.2.  It appears machine
> specific, but related to our PV driver, and takes a long time to
> reproduce.  Having automated tests fail soon with a triple fault is
> better than having the domain in question sit in a reboot loop until the
> hour long timeout kicks in.

Yep, agreed, a patch along these lines of some sort is a very good idea!

 -- Keir

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Xuquan (Quan Xu)]

On Feb 4, 2013, 6:25 AM, <andrew.cooper3@citrix> wrote:
> While the triple fault action on native hardware will result in a system
> reset, any modern operating system can and will make use of less violent
> reboot methods. As a result, the most likely cause of a triple fault is a
> fatal software bug.
>
> This patch allows the toolstack to indicate that a triple fault should mean a
> crash rather than a reboot. The default of reboot still remains the same.
>

hi, Andrew

this email has been ages ago. I am working for a triple fault related issues, and I have some doubts:

1)  as you mentioned here, 'fatal software bug', is it to xen hypervisor or guest os?
2)  why introduce 'crash'?  would the 'reboot' lead to a fatal problem ( i.e, xen hypervisor panic) ?
3)  are there any differences between __'crash the guest, destroy the guest , create the guest again'__
    And __'reboot guest'__ ?
4)  any ideas to reproduce triple fault?

Thanks
Quan



> Signed-off-by: Andrew Cooper <andrew.cooper3 [at] citrix>
>
> diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
> void hvm_triple_fault(void)
> {
> struct vcpu *v = current;
> + struct domain * d = v->domain;
> + u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
> + ? SHUTDOWN_crash : SHUTDOWN_reboot;
> +
> gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
> - "invoking HVM system reset.\n", v->vcpu_id);
> - domain_shutdown(v->domain, SHUTDOWN_reboot);
> + "invoking HVM system %s.\n", v->vcpu_id,
> + reason == SHUTDOWN_crash ? "crash" : "reboot");
> + domain_shutdown(v->domain, reason);
> }
>
> void hvm_inject_trap(struct hvm_trap *trap)
> diff -r 5af4f2ab06f3 -r 6f8c532df545 xen/include/public/hvm/params.h
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -142,6 +142,9 @@
> #define HVM_PARAM_ACCESS_RING_PFN 28
> #define HVM_PARAM_SHARING_RING_PFN 29
>
> -#define HVM_NR_PARAMS 31
> +/* Boolean: Should a triple fault imply crash rather than reboot? */
> +#define HVM_PARAM_TRIPLE_FAULT_CRASH 31
> +
> +#define HVM_NR_PARAMS 32
>
> #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

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

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Andrew Cooper]

On 07/11/16 12:56, Xuquan (Quan Xu) wrote:
> On Feb 4, 2013, 6:25 AM, <andrew.cooper3@citrix> wrote:
>> While the triple fault action on native hardware will result in a system
>> reset, any modern operating system can and will make use of less violent
>> reboot methods. As a result, the most likely cause of a triple fault is a
>> fatal software bug.
>>
>> This patch allows the toolstack to indicate that a triple fault should mean a
>> crash rather than a reboot. The default of reboot still remains the same.
>>
> hi, Andrew
>
> this email has been ages ago. I am working for a triple fault related issues, and I have some doubts:
>
> 1)  as you mentioned here, 'fatal software bug', is it to xen hypervisor or guest os?

Guest

> 2)  why introduce 'crash'?  would the 'reboot' lead to a fatal problem ( i.e, xen hypervisor panic) ?

The purpose was to change how the shutdown is reported to the toolstack,
and in particular whether the on_reboot or on_crash action gets called.

> 3)  are there any differences between __'crash the guest, destroy the guest , create the guest again'__
>     And __'reboot guest'__ ?

Only which toolstack action gets invoked.

> 4)  any ideas to reproduce triple fault?

What do you mean by reproduce? Causing a triple fault is very easy from
the guest kernel.

~Andrew

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

Re: [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

[Xuquan (Quan Xu)]

On November 07, 2016 9:13 PM, Andrew Cooper wrote:
>On 07/11/16 12:56, Xuquan (Quan Xu) wrote:
>> 4)  any ideas to reproduce triple fault?
>
>What do you mean by reproduce? Causing a triple fault is very easy from the
>guest kernel.

Yes, by reproduce.. indeed, we just implemented one when I sent out the last email. Thanks.

Quan

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