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? -- 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