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>
signature.asc
Description: OpenPGP digital signature