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 <da...@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 <th...@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
signature.asc
Description: PGP signature