* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 8:35 [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call Nikunj A Dadhania
@ 2014-06-30 9:11 ` Alexander Graf
2014-06-30 9:25 ` Nikunj A Dadhania
2014-06-30 15:49 ` Tyrel Datwyler
2014-07-03 3:41 ` Alexey Kardashevskiy
2 siblings, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2014-06-30 9:11 UTC (permalink / raw)
To: Nikunj A Dadhania
Cc: Benjamin Herrenschmidt, aik@ozlabs.ru, Tyrel Datwyler,
qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Anton Blanchard
> Am 30.06.2014 um 10:35 schrieb Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>:
>
> 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 <benh@au1.ibm.com>
> CC: Anton Blanchard <anton@samba.org>
> CC: Alexander Graf <agraf@suse.de>
> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@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);
The guest doesn't pause. Since the guest will call os-term in a loop, this will also flood the event listener with lots and lots of panic messages.
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 9:11 ` Alexander Graf
@ 2014-06-30 9:25 ` Nikunj A Dadhania
2014-06-30 12:05 ` Alexander Graf
0 siblings, 1 reply; 11+ messages in thread
From: Nikunj A Dadhania @ 2014-06-30 9:25 UTC (permalink / raw)
To: Alexander Graf
Cc: Benjamin Herrenschmidt, aik@ozlabs.ru, Tyrel Datwyler,
qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Anton Blanchard
Alexander Graf <agraf@suse.de> writes:
>> Am 30.06.2014 um 10:35 schrieb Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>:
>>
>> +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);
>
> The guest doesn't pause.
I see the event reaching libvirt and a dump taken and the guest
restarts.
> Since the guest will call os-term in a loop, this will also flood the
> event listener with lots and lots of panic messages.
do {
status = rtas_call(rtas_token("ibm,os-term"), 1, 1, NULL,
__pa(rtas_os_term_buf));
} while (rtas_busy_delay(status));
So when status from the rtas call is success, the loop should exit.
Am I missing something?
Regards
Nikunj
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 9:25 ` Nikunj A Dadhania
@ 2014-06-30 12:05 ` Alexander Graf
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2014-06-30 12:05 UTC (permalink / raw)
To: Nikunj A Dadhania
Cc: Benjamin Herrenschmidt, aik@ozlabs.ru, Tyrel Datwyler,
qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Anton Blanchard
On 30.06.14 11:25, Nikunj A Dadhania wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>>> Am 30.06.2014 um 10:35 schrieb Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>:
>>>
>>> +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);
>> The guest doesn't pause.
> I see the event reaching libvirt and a dump taken and the guest
> restarts.
>
>> Since the guest will call os-term in a loop, this will also flood the
>> event listener with lots and lots of panic messages.
> do {
> status = rtas_call(rtas_token("ibm,os-term"), 1, 1, NULL,
> __pa(rtas_os_term_buf));
> } while (rtas_busy_delay(status));
>
> So when status from the rtas call is success, the loop should exit.
>
> Am I missing something?
No, I think you're right. I'll queue it for 2.2.
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 8:35 [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call Nikunj A Dadhania
2014-06-30 9:11 ` Alexander Graf
@ 2014-06-30 15:49 ` Tyrel Datwyler
2014-06-30 16:24 ` Alexander Graf
2014-07-03 3:41 ` Alexey Kardashevskiy
2 siblings, 1 reply; 11+ messages in thread
From: Tyrel Datwyler @ 2014-06-30 15:49 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-devel
Cc: aik, Benjamin Herrenschmidt, qemu-ppc, Anton Blanchard,
Alexander Graf
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 <benh@au1.ibm.com>
> CC: Anton Blanchard <anton@samba.org>
> CC: Alexander Graf <agraf@suse.de>
> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@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)?
-Tyrel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 15:49 ` Tyrel Datwyler
@ 2014-06-30 16:24 ` Alexander Graf
2014-07-01 3:20 ` Nikunj A Dadhania
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Graf @ 2014-06-30 16:24 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: Benjamin Herrenschmidt, Nikunj A Dadhania, aik, qemu-devel,
qemu-ppc, Anton Blanchard
On 30.06.2014, at 17:49, Tyrel Datwyler <turtle.in.the.kernel@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 <benh@au1.ibm.com>
>> CC: Anton Blanchard <anton@samba.org>
>> CC: Alexander Graf <agraf@suse.de>
>> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@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)?
Good catch. Fixed in my queue.
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 16:24 ` Alexander Graf
@ 2014-07-01 3:20 ` Nikunj A Dadhania
2014-07-01 5:10 ` Alexander Graf
0 siblings, 1 reply; 11+ messages in thread
From: Nikunj A Dadhania @ 2014-07-01 3:20 UTC (permalink / raw)
To: Alexander Graf, Tyrel Datwyler
Cc: aik, Benjamin Herrenschmidt, qemu-ppc, qemu-devel,
Anton Blanchard
Alexander Graf <agraf@suse.de> writes:
> On 30.06.2014, at 17:49, Tyrel Datwyler <turtle.in.the.kernel@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 <benh@au1.ibm.com>
>>> CC: Anton Blanchard <anton@samba.org>
>>> CC: Alexander Graf <agraf@suse.de>
>>> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@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?
Regards
Nikunj
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-07-01 3:20 ` Nikunj A Dadhania
@ 2014-07-01 5:10 ` Alexander Graf
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2014-07-01 5:10 UTC (permalink / raw)
To: Nikunj A Dadhania
Cc: Benjamin Herrenschmidt, aik@ozlabs.ru, Tyrel Datwyler,
qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Anton Blanchard
> Am 01.07.2014 um 05:20 schrieb Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>:
>
> Alexander Graf <agraf@suse.de> writes:
>
>>> On 30.06.2014, at 17:49, Tyrel Datwyler <turtle.in.the.kernel@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 <benh@au1.ibm.com>
>>>> CC: Anton Blanchard <anton@samba.org>
>>>> CC: Alexander Graf <agraf@suse.de>
>>>> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-06-30 8:35 [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call Nikunj A Dadhania
2014-06-30 9:11 ` Alexander Graf
2014-06-30 15:49 ` Tyrel Datwyler
@ 2014-07-03 3:41 ` Alexey Kardashevskiy
2014-07-03 3:48 ` Alexey Kardashevskiy
2014-07-03 5:27 ` Nikunj A Dadhania
2 siblings, 2 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2014-07-03 3:41 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-devel
Cc: Tyrel Datwyler, Benjamin Herrenschmidt, qemu-ppc, Anton Blanchard,
Alexander Graf
On 06/30/2014 06:35 PM, 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 <benh@au1.ibm.com>
> CC: Anton Blanchard <anton@samba.org>
> CC: Alexander Graf <agraf@suse.de>
> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@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)
So we never ever going to implement this RTAS call?
I'd keep the number.
>
> #define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x21)
>
>
--
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-07-03 3:41 ` Alexey Kardashevskiy
@ 2014-07-03 3:48 ` Alexey Kardashevskiy
2014-07-03 5:27 ` Nikunj A Dadhania
1 sibling, 0 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2014-07-03 3:48 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-devel
Cc: Tyrel Datwyler, Benjamin Herrenschmidt, qemu-ppc, Anton Blanchard,
Alexander Graf
On 07/03/2014 01:41 PM, Alexey Kardashevskiy wrote:
> On 06/30/2014 06:35 PM, 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 <benh@au1.ibm.com>
>> CC: Anton Blanchard <anton@samba.org>
>> CC: Alexander Graf <agraf@suse.de>
>> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@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)
>
>
> So we never ever going to implement this RTAS call?
> I'd keep the number.
ah, it is in ppc-next-2.2 already. Never mind :)
>
>
>>
>> #define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x21)
>>
>>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v5] ppc: spapr-rtas - implement os-term rtas call
2014-07-03 3:41 ` Alexey Kardashevskiy
2014-07-03 3:48 ` Alexey Kardashevskiy
@ 2014-07-03 5:27 ` Nikunj A Dadhania
1 sibling, 0 replies; 11+ messages in thread
From: Nikunj A Dadhania @ 2014-07-03 5:27 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel
Cc: Tyrel Datwyler, Benjamin Herrenschmidt, qemu-ppc, Anton Blanchard,
Alexander Graf
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 06/30/2014 06:35 PM, 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 <benh@au1.ibm.com>
>> CC: Anton Blanchard <anton@samba.org>
>> CC: Alexander Graf <agraf@suse.de>
>> CC: Tyrel Datwyler <turtle.in.the.kernel@gmail.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> 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)
>
>
> So we never ever going to implement this RTAS call?
Yeah, as its an RTAS property an not a call.
> I'd keep the number.
Regards
Nikunj
^ permalink raw reply [flat|nested] 11+ messages in thread