[Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[David Gibson]

The pseries machine type doesn't usually use the 'pvpanic' device as such,
because it has a firmware/hypervisor facility with roughly the same
purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
guest has crashed.

Our implementation of this call was sending a GUEST_PANICKED qmp event;
however, it was not doing the other usual panic actions, making its
behaviour different from pvpanic for no good reason.

To correct this, we should call qemu_system_guest_panicked() rather than
directly sending the panic event.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_rtas.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 707c4d4..94a2799 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
                             target_ulong args,
                             uint32_t nret, target_ulong rets)
 {
-    target_ulong ret = 0;
+    qemu_system_guest_panicked(NULL);
 
-    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
-                                   &error_abort);
-
-    rtas_st(rets, 0, ret);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 
 static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-- 
2.9.4

Re: [Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[Paolo Bonzini]

----- Original Message -----
> From: "David Gibson" <david@gibson.dropbear.id.au>
> To: mdroth@linux.vnet.ibm.com, thuth@redhat.com, lvivier@redhat.com
> Cc: pbonzini@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, "David Gibson" <david@gibson.dropbear.id.au>
> Sent: Wednesday, June 7, 2017 9:07:32 AM
> Subject: [PATCH] pseries: Correct panic behaviour for pseries machine type
> 
> The pseries machine type doesn't usually use the 'pvpanic' device as such,
> because it has a firmware/hypervisor facility with roughly the same
> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
> guest has crashed.
> 
> Our implementation of this call was sending a GUEST_PANICKED qmp event;
> however, it was not doing the other usual panic actions, making its
> behaviour different from pvpanic for no good reason.
> 
> To correct this, we should call qemu_system_guest_panicked() rather than
> directly sending the panic event.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_rtas.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 707c4d4..94a2799 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>                              target_ulong args,
>                              uint32_t nret, target_ulong rets)
>  {
> -    target_ulong ret = 0;
> +    qemu_system_guest_panicked(NULL);
>  
> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
> -                                   &error_abort);
> -
> -    rtas_st(rets, 0, ret);
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }

It's possible to "cont" a panicked guest, so I think you should keep
the rtas_st.

Paolo

Re: [Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[Thomas Huth]

On 07.06.2017 09:07, David Gibson wrote:
> The pseries machine type doesn't usually use the 'pvpanic' device as such,
> because it has a firmware/hypervisor facility with roughly the same
> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
> guest has crashed.
> 
> Our implementation of this call was sending a GUEST_PANICKED qmp event;
> however, it was not doing the other usual panic actions, making its
> behaviour different from pvpanic for no good reason.
> 
> To correct this, we should call qemu_system_guest_panicked() rather than
> directly sending the panic event.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_rtas.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 707c4d4..94a2799 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>                              target_ulong args,
>                              uint32_t nret, target_ulong rets)
>  {
> -    target_ulong ret = 0;
> +    qemu_system_guest_panicked(NULL);
>  
> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
> -                                   &error_abort);
> -
> -    rtas_st(rets, 0, ret);
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
>  
>  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> 

If I get that qemu_system_guest_panicked() function right, it will stop
the VM, won't it? That contradicts the LoPAPR spec that says that the
RTAS call returns if the "ibm,extended-os-term" property is available in
the device tree. And we currently present this property in the device
tree. So either the guest should not be stopped here, or we've got to
remove the property from the device tree again.

 Thomas

Re: [Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2310 bytes --]

On Wed, Jun 07, 2017 at 03:24:56AM -0400, Paolo Bonzini wrote:
> 
> 
> ----- Original Message -----
> > From: "David Gibson" <david@gibson.dropbear.id.au>
> > To: mdroth@linux.vnet.ibm.com, thuth@redhat.com, lvivier@redhat.com
> > Cc: pbonzini@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, "David Gibson" <david@gibson.dropbear.id.au>
> > Sent: Wednesday, June 7, 2017 9:07:32 AM
> > Subject: [PATCH] pseries: Correct panic behaviour for pseries machine type
> > 
> > The pseries machine type doesn't usually use the 'pvpanic' device as such,
> > because it has a firmware/hypervisor facility with roughly the same
> > purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
> > guest has crashed.
> > 
> > Our implementation of this call was sending a GUEST_PANICKED qmp event;
> > however, it was not doing the other usual panic actions, making its
> > behaviour different from pvpanic for no good reason.
> > 
> > To correct this, we should call qemu_system_guest_panicked() rather than
> > directly sending the panic event.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr_rtas.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 707c4d4..94a2799 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
> >                              target_ulong args,
> >                              uint32_t nret, target_ulong rets)
> >  {
> > -    target_ulong ret = 0;
> > +    qemu_system_guest_panicked(NULL);
> >  
> > -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
> > -                                   &error_abort);
> > -
> > -    rtas_st(rets, 0, ret);
> > +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >  }
> 
> It's possible to "cont" a panicked guest, so I think you should keep
> the rtas_st.

I did keep the rtas_st(), I just changed it to using a constant
instead of an always-0 variable.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

On Wed, Jun 07, 2017 at 09:33:21AM +0200, Thomas Huth wrote:
> On 07.06.2017 09:07, David Gibson wrote:
> > The pseries machine type doesn't usually use the 'pvpanic' device as such,
> > because it has a firmware/hypervisor facility with roughly the same
> > purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
> > guest has crashed.
> > 
> > Our implementation of this call was sending a GUEST_PANICKED qmp event;
> > however, it was not doing the other usual panic actions, making its
> > behaviour different from pvpanic for no good reason.
> > 
> > To correct this, we should call qemu_system_guest_panicked() rather than
> > directly sending the panic event.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr_rtas.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 707c4d4..94a2799 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
> >                              target_ulong args,
> >                              uint32_t nret, target_ulong rets)
> >  {
> > -    target_ulong ret = 0;
> > +    qemu_system_guest_panicked(NULL);
> >  
> > -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
> > -                                   &error_abort);
> > -
> > -    rtas_st(rets, 0, ret);
> > +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >  }
> >  
> >  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > 
> 
> If I get that qemu_system_guest_panicked() function right, it will stop
> the VM, won't it? That contradicts the LoPAPR spec that says that the
> RTAS call returns if the "ibm,extended-os-term" property is available in
> the device tree. And we currently present this property in the device
> tree. So either the guest should not be stopped here, or we've got to
> remove the property from the device tree again.

Hrm.  However in the case of this "extended behaviour" it's not clear
what we should do next.  I'm include to think that the preference set
by the qemu user / management layer should take precedence over PAPR.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] pseries: Correct panic behaviour for pseries machine type

[Paolo Bonzini]

On 07/06/2017 09:33, Thomas Huth wrote:
> On 07.06.2017 09:07, David Gibson wrote:
>> The pseries machine type doesn't usually use the 'pvpanic' device as such,
>> because it has a firmware/hypervisor facility with roughly the same
>> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
>> guest has crashed.
>>
>> Our implementation of this call was sending a GUEST_PANICKED qmp event;
>> however, it was not doing the other usual panic actions, making its
>> behaviour different from pvpanic for no good reason.
>>
>> To correct this, we should call qemu_system_guest_panicked() rather than
>> directly sending the panic event.
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  hw/ppc/spapr_rtas.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 707c4d4..94a2799 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>>                              target_ulong args,
>>                              uint32_t nret, target_ulong rets)
>>  {
>> -    target_ulong ret = 0;
>> +    qemu_system_guest_panicked(NULL);
>>  
>> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
>> -                                   &error_abort);
>> -
>> -    rtas_st(rets, 0, ret);
>> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>  }
>>  
>>  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>
> 
> If I get that qemu_system_guest_panicked() function right, it will stop
> the VM, won't it? That contradicts the LoPAPR spec that says that the
> RTAS call returns if the "ibm,extended-os-term" property is available in
> the device tree.

It does return... but only after the user starts the guest again with
"cont".

Paolo

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct panic behaviour for pseries machine type

[Thomas Huth]

On 07.06.2017 16:34, Paolo Bonzini wrote:
> 
> 
> On 07/06/2017 09:33, Thomas Huth wrote:
>> On 07.06.2017 09:07, David Gibson wrote:
>>> The pseries machine type doesn't usually use the 'pvpanic' device as such,
>>> because it has a firmware/hypervisor facility with roughly the same
>>> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
>>> guest has crashed.
>>>
>>> Our implementation of this call was sending a GUEST_PANICKED qmp event;
>>> however, it was not doing the other usual panic actions, making its
>>> behaviour different from pvpanic for no good reason.
>>>
>>> To correct this, we should call qemu_system_guest_panicked() rather than
>>> directly sending the panic event.
>>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>>  hw/ppc/spapr_rtas.c | 7 ++-----
>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>> index 707c4d4..94a2799 100644
>>> --- a/hw/ppc/spapr_rtas.c
>>> +++ b/hw/ppc/spapr_rtas.c
>>> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>>>                              target_ulong args,
>>>                              uint32_t nret, target_ulong rets)
>>>  {
>>> -    target_ulong ret = 0;
>>> +    qemu_system_guest_panicked(NULL);
>>>  
>>> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
>>> -                                   &error_abort);
>>> -
>>> -    rtas_st(rets, 0, ret);
>>> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>>  }
>>>  
>>>  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>>
>>
>> If I get that qemu_system_guest_panicked() function right, it will stop
>> the VM, won't it? That contradicts the LoPAPR spec that says that the
>> RTAS call returns if the "ibm,extended-os-term" property is available in
>> the device tree.
> 
> It does return... but only after the user starts the guest again with
> "cont".

OK, I guess that's enough to say that the "ibm,extended-os-term"
property can stay ... so I think the patch is fine as it is right now.

 Thomas

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct panic behaviour for pseries machine type

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2537 bytes --]

On Wed, Jun 07, 2017 at 07:10:55PM +0200, Thomas Huth wrote:
> On 07.06.2017 16:34, Paolo Bonzini wrote:
> > 
> > 
> > On 07/06/2017 09:33, Thomas Huth wrote:
> >> On 07.06.2017 09:07, David Gibson wrote:
> >>> The pseries machine type doesn't usually use the 'pvpanic' device as such,
> >>> because it has a firmware/hypervisor facility with roughly the same
> >>> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
> >>> guest has crashed.
> >>>
> >>> Our implementation of this call was sending a GUEST_PANICKED qmp event;
> >>> however, it was not doing the other usual panic actions, making its
> >>> behaviour different from pvpanic for no good reason.
> >>>
> >>> To correct this, we should call qemu_system_guest_panicked() rather than
> >>> directly sending the panic event.
> >>>
> >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >>> ---
> >>>  hw/ppc/spapr_rtas.c | 7 ++-----
> >>>  1 file changed, 2 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >>> index 707c4d4..94a2799 100644
> >>> --- a/hw/ppc/spapr_rtas.c
> >>> +++ b/hw/ppc/spapr_rtas.c
> >>> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
> >>>                              target_ulong args,
> >>>                              uint32_t nret, target_ulong rets)
> >>>  {
> >>> -    target_ulong ret = 0;
> >>> +    qemu_system_guest_panicked(NULL);
> >>>  
> >>> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
> >>> -                                   &error_abort);
> >>> -
> >>> -    rtas_st(rets, 0, ret);
> >>> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >>>  }
> >>>  
> >>>  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >>>
> >>
> >> If I get that qemu_system_guest_panicked() function right, it will stop
> >> the VM, won't it? That contradicts the LoPAPR spec that says that the
> >> RTAS call returns if the "ibm,extended-os-term" property is available in
> >> the device tree.
> > 
> > It does return... but only after the user starts the guest again with
> > "cont".
> 
> OK, I guess that's enough to say that the "ibm,extended-os-term"
> property can stay ... so I think the patch is fine as it is right now.

So.. can I have an R-b?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct panic behaviour for pseries machine type

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 2438 bytes --]

On 08.06.2017 02:18, David Gibson wrote:
> On Wed, Jun 07, 2017 at 07:10:55PM +0200, Thomas Huth wrote:
>> On 07.06.2017 16:34, Paolo Bonzini wrote:
>>>
>>>
>>> On 07/06/2017 09:33, Thomas Huth wrote:
>>>> On 07.06.2017 09:07, David Gibson wrote:
>>>>> The pseries machine type doesn't usually use the 'pvpanic' device as such,
>>>>> because it has a firmware/hypervisor facility with roughly the same
>>>>> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
>>>>> guest has crashed.
>>>>>
>>>>> Our implementation of this call was sending a GUEST_PANICKED qmp event;
>>>>> however, it was not doing the other usual panic actions, making its
>>>>> behaviour different from pvpanic for no good reason.
>>>>>
>>>>> To correct this, we should call qemu_system_guest_panicked() rather than
>>>>> directly sending the panic event.
>>>>>
>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>>> ---
>>>>>  hw/ppc/spapr_rtas.c | 7 ++-----
>>>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>>> index 707c4d4..94a2799 100644
>>>>> --- a/hw/ppc/spapr_rtas.c
>>>>> +++ b/hw/ppc/spapr_rtas.c
>>>>> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>>>>>                              target_ulong args,
>>>>>                              uint32_t nret, target_ulong rets)
>>>>>  {
>>>>> -    target_ulong ret = 0;
>>>>> +    qemu_system_guest_panicked(NULL);
>>>>>  
>>>>> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
>>>>> -                                   &error_abort);
>>>>> -
>>>>> -    rtas_st(rets, 0, ret);
>>>>> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>>>>  }
>>>>>  
>>>>>  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>>>>
>>>>
>>>> If I get that qemu_system_guest_panicked() function right, it will stop
>>>> the VM, won't it? That contradicts the LoPAPR spec that says that the
>>>> RTAS call returns if the "ibm,extended-os-term" property is available in
>>>> the device tree.
>>>
>>> It does return... but only after the user starts the guest again with
>>> "cont".
>>
>> OK, I guess that's enough to say that the "ibm,extended-os-term"
>> property can stay ... so I think the patch is fine as it is right now.
> 
> So.. can I have an R-b?

Reviewed-by: Thomas Huth <thuth@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct panic behaviour for pseries machine type

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

On Thu, Jun 08, 2017 at 06:33:57AM +0200, Thomas Huth wrote:
> On 08.06.2017 02:18, David Gibson wrote:
> > On Wed, Jun 07, 2017 at 07:10:55PM +0200, Thomas Huth wrote:
> >> On 07.06.2017 16:34, Paolo Bonzini wrote:
> >>>
> >>>
> >>> On 07/06/2017 09:33, Thomas Huth wrote:
> >>>> On 07.06.2017 09:07, David Gibson wrote:
> >>>>> The pseries machine type doesn't usually use the 'pvpanic' device as such,
> >>>>> because it has a firmware/hypervisor facility with roughly the same
> >>>>> purpose.  The 'ibm,os-term' RTAS call notifies the hypervisor that the
> >>>>> guest has crashed.
> >>>>>
> >>>>> Our implementation of this call was sending a GUEST_PANICKED qmp event;
> >>>>> however, it was not doing the other usual panic actions, making its
> >>>>> behaviour different from pvpanic for no good reason.
> >>>>>
> >>>>> To correct this, we should call qemu_system_guest_panicked() rather than
> >>>>> directly sending the panic event.
> >>>>>
> >>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >>>>> ---
> >>>>>  hw/ppc/spapr_rtas.c | 7 ++-----
> >>>>>  1 file changed, 2 insertions(+), 5 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >>>>> index 707c4d4..94a2799 100644
> >>>>> --- a/hw/ppc/spapr_rtas.c
> >>>>> +++ b/hw/ppc/spapr_rtas.c
> >>>>> @@ -293,12 +293,9 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
> >>>>>                              target_ulong args,
> >>>>>                              uint32_t nret, target_ulong rets)
> >>>>>  {
> >>>>> -    target_ulong ret = 0;
> >>>>> +    qemu_system_guest_panicked(NULL);
> >>>>>  
> >>>>> -    qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, false, NULL,
> >>>>> -                                   &error_abort);
> >>>>> -
> >>>>> -    rtas_st(rets, 0, ret);
> >>>>> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >>>>>  }
> >>>>>  
> >>>>>  static void rtas_set_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >>>>>
> >>>>
> >>>> If I get that qemu_system_guest_panicked() function right, it will stop
> >>>> the VM, won't it? That contradicts the LoPAPR spec that says that the
> >>>> RTAS call returns if the "ibm,extended-os-term" property is available in
> >>>> the device tree.
> >>>
> >>> It does return... but only after the user starts the guest again with
> >>> "cont".
> >>
> >> OK, I guess that's enough to say that the "ibm,extended-os-term"
> >> property can stay ... so I think the patch is fine as it is right now.
> > 
> > So.. can I have an R-b?
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]