> Am 01.07.2014 um 05:20 schrieb Nikunj A Dadhania <nik...@linux.vnet.ibm.com>:
>
> Alexander Graf <ag...@suse.de> writes:
>
>>> On 30.06.2014, at 17:49, Tyrel Datwyler <turtle.in.the.ker...@gmail.com>
>>> wrote:
>>>
>>>> On 06/30/2014 01:35 AM, Nikunj A Dadhania wrote:
>>>> PAPR compliant guest calls this in absence of kdump. This finally
>>>> reaches the guest and can be handled according to the policies set by
>>>> higher level tools(like taking dump) for further analysis by tools like
>>>> crash.
>>>>
>>>> Linux kernel calls ibm,os-term when extended property of os-term is set.
>>>> This makes sure that a return to the linux kernel is gauranteed.
>>>>
>>>> CC: Benjamin Herrenschmidt <b...@au1.ibm.com>
>>>> CC: Anton Blanchard <an...@samba.org>
>>>> CC: Alexander Graf <ag...@suse.de>
>>>> CC: Tyrel Datwyler <turtle.in.the.ker...@gmail.com>
>>>> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
>>>>
>>>> ---
>>>>
>>>> v2: rebase to ppcnext
>>>> v3: Do not stop the VM, and update comments
>>>> v4: update spapr_register_rtas and qapi_event changes
>>>> v5: set ibm,extended-os-term as null encoded property
>>>> ---
>>>> hw/ppc/spapr.c | 9 +++++++++
>>>> hw/ppc/spapr_rtas.c | 15 +++++++++++++++
>>>> include/hw/ppc/spapr.h | 1 -
>>>> 3 files changed, 24 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index 307c58d..e6c9014 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -520,6 +520,15 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>>
>>>> _FDT((fdt_property_cell(fdt, "rtas-error-log-max",
>>>> RTAS_ERROR_LOG_MAX)));
>>>>
>>>> + /*
>>>> + * According to PAPR, rtas ibm,os-term, does not gaurantee a return
>>>> + * back to the guest cpu.
>>>> + *
>>>> + * While an additional ibm,extended-os-term property indicates that
>>>> + * rtas call return will always occur. Set this property.
>>>> + */
>>>> + _FDT((fdt_property(fdt, "ibm,extended-os-term", NULL, 0)));
>>>> +
>>>> _FDT((fdt_end_node(fdt)));
>>>>
>>>> /* interrupt controller */
>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>> index 9ba1ba6..2ec2a8e 100644
>>>> --- a/hw/ppc/spapr_rtas.c
>>>> +++ b/hw/ppc/spapr_rtas.c
>>>> @@ -277,6 +277,19 @@ 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;
>>>> +
>>>> + qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE,
>>>> &error_abort);
>>>> +
>>>> + rtas_st(rets, 0, ret);
>>>> +}
>>>> +
>>>> static struct rtas_call {
>>>> const char *name;
>>>> spapr_rtas_fn fn;
>>>> @@ -404,6 +417,8 @@ static void core_rtas_register_types(void)
>>>> spapr_rtas_register(RTAS_IBM_SET_SYSTEM_PARAMETER,
>>>> "ibm,set-system-parameter",
>>>> rtas_ibm_set_system_parameter);
>>>> + spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
>>>> + rtas_ibm_os_term);
>>>> }
>>>>
>>>> type_init(core_rtas_register_types)
>>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>> index bbba51a..4e96381 100644
>>>> --- a/include/hw/ppc/spapr.h
>>>> +++ b/include/hw/ppc/spapr.h
>>>> @@ -382,7 +382,6 @@ int spapr_allocate_irq_block(int num, bool lsi, bool
>>>> msi);
>>>> #define RTAS_GET_SENSOR_STATE (RTAS_TOKEN_BASE + 0x1D)
>>>> #define RTAS_IBM_CONFIGURE_CONNECTOR (RTAS_TOKEN_BASE + 0x1E)
>>>> #define RTAS_IBM_OS_TERM (RTAS_TOKEN_BASE + 0x1F)
>>>> -#define RTAS_IBM_EXTENDED_OS_TERM (RTAS_TOKEN_BASE + 0x20)
>>>>
>>>> #define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x21)
>>>
>>> With the removal of RTAS_IBM_EXTENDED_OS_TERM doesn't RTAS_TOKEN_MAX
>>> need to be decremented to (RTAS_TOKEN_BASE + 0x20)?
>
> You are right.
>
>> Good catch. Fixed in my queue.
>
> Thanks, Any reason we dont have that as enum?
We need to maintain number stability for live migration compatibility with
older machine types. So we have to make 100% sure that removing (or adding) one
entry does not have any guest visible impact.
This is a lot easier to guarantee with a lot of defines, as you'd have to touch
multiple lines rather than only a single one to change unrelated entries. Hence
it makes review easier.
Alex
