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

[David Gibson]

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 <address@hidden>
> > ---
> >  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

signature.asc
Description: PGP signature