Re: [Qemu-devel] [PATCH v2] ppc: spapr-rtas - implement os-term rtas call

[Alexander Graf <agraf@suse.de>]

On 17.06.14 11:30, Nikunj A Dadhania wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 12.06.14 14:09, Nikunj A Dadhania wrote:
>>> PAPR compliant guest calls this in absence of kdump. After
>>> receiving this call qemu could trigger a guest dump. This guest dump
>>> can be used to analyse using crash tool.
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>> ---
>>>
>>> v2: indentation fixes
>>>
>>>    hw/ppc/spapr_rtas.c | 32 ++++++++++++++++++++++++++++++++
>>>    1 file changed, 32 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>> index ea4a2b2..f030e73 100644
>>> --- a/hw/ppc/spapr_rtas.c
>>> +++ b/hw/ppc/spapr_rtas.c
>>> @@ -29,6 +29,8 @@
>>>    #include "sysemu/char.h"
>>>    #include "hw/qdev.h"
>>>    #include "sysemu/device_tree.h"
>>> +#include "qapi/qmp/qjson.h"
>>> +#include "monitor/monitor.h"
>>>    
>>>    #include "hw/ppc/spapr.h"
>>>    #include "hw/ppc/spapr_vio.h"
>>> @@ -267,6 +269,34 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
>>>        rtas_st(rets, 0, ret);
>>>    }
>>>    
>>> +static void rtas_ibm_os_term(PowerPCCPU *cpu,
>>> +                             sPAPREnvironment *spapr,
>>> +                             uint32_t token, uint32_t nargs,
>>> +                             target_ulong args,
>>> +                             uint32_t nret, target_ulong rets)
>>> +{
>>> +    target_ulong ret = 0;
>>> +    QObject *data;
>>> +
>>> +    data = qobject_from_jsonf("{ 'action': %s }", "pause");
>>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>>> +    qobject_decref(data);
>>> +    vm_stop(RUN_STATE_GUEST_PANICKED);
>>> +
>>> +    rtas_st(rets, 0, ret);
>>> +}
>>> +
>>> +static void rtas_ibm_ext_os_term(PowerPCCPU *cpu,
>>> +                                 sPAPREnvironment *spapr,
>>> +                                 uint32_t token, uint32_t nargs,
>>> +                                 target_ulong args,
>>> +                                 uint32_t nret, target_ulong rets)
>>> +{
>>> +    target_ulong ret = 0;
>>> +
>>> +    rtas_st(rets, 0, ret);
>>> +}
>>> +
>>>    static struct rtas_call {
>>>        const char *name;
>>>        spapr_rtas_fn fn;
>>> @@ -392,6 +422,8 @@ static void core_rtas_register_types(void)
>>>                            rtas_ibm_get_system_parameter);
>>>        spapr_rtas_register("ibm,set-system-parameter",
>>>                            rtas_ibm_set_system_parameter);
>>> +    spapr_rtas_register("ibm,os-term", rtas_ibm_os_term);
>>> +    spapr_rtas_register("ibm,extended-os-term", rtas_ibm_ext_os_term);
>> Why do we need the extended-os-term if we don't do anything with it?
> Linux kernel checks for both of them because of legacy:
>
> arch/powerpc/kernel/rtas.c:
>
> void rtas_os_term(char *str)
> {
> [...]
>          /*
>           * Firmware with the ibm,extended-os-term property is guaranteed
>           * to always return from an ibm,os-term call. Earlier versions without
>           * this property may terminate the partition which we want to avoid
>           * since it interferes with panic_timeout.

But we do not return from the RTAS call, so we don't adhere to the 
extended semantics?


Alex

>           */
>          if (RTAS_UNKNOWN_SERVICE == rtas_token("ibm,os-term") ||
>              RTAS_UNKNOWN_SERVICE == rtas_token("ibm,extended-os-term"))
>                  return;
>
> [...]
> }
>
> Regards,
> Nikunj
>