Re: [PATCH RFC] xen/arm : emulation of arm's psci v0.2 standard

[Julien Grall <julien.grall@linaro.org>]

On 19/06/14 18:22, Stefano Stabellini wrote:
> On Thu, 19 Jun 2014, Julien Grall wrote:
>> On 06/19/2014 05:00 PM, Stefano Stabellini wrote:
>>> On Thu, 19 Jun 2014, Parth Dixit wrote:
>>>> From: parthd <parth.dixit@linaro.org>
>>>>
>>>> Arm based virtual machines dom0/guest will request power related functionality
>>>> from xen through psci interface. This patch implements version 0.2 of
>>>> PSCI standard specified by arm for 64bit and 32 bit arm machines.
>>>>
>>>> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>>>> ---
>>>>   xen/arch/arm/domain_build.c     |  5 ++-
>>>>   xen/arch/arm/traps.c            | 56 ++++++++++++++++++++++--
>>>>   xen/arch/arm/vpsci.c            | 75 ++++++++++++++++++++++++++++++++
>>>>   xen/include/asm-arm/processor.h |  6 +++
>>>>   xen/include/asm-arm/psci.h      | 18 ++++++++
>>>>   xen/include/public/arch-arm.h   | 95 +++++++++++++++++++++++++++++++++++++++--
>>>>   6 files changed, 246 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index c424793..ebd4170 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -388,6 +388,9 @@ static int make_hypervisor_node(struct domain *d,
>>>>   static int make_psci_node(void *fdt, const struct dt_device_node *parent)
>>>>   {
>>>>       int res;
>>>> +    const char compat[] =
>>>> +        "arm,psci-0.2""\0"
>>>> +        "arm,psci";
>>>>
>>>>       DPRINT("Create PSCI node\n");
>>>>
>>>> @@ -396,7 +399,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
>>>>       if ( res )
>>>>           return res;
>>>>
>>>> -    res = fdt_property_string(fdt, "compatible", "arm,psci");
>>>> +    res = fdt_property(fdt, "compatible", compat, sizeof(compat));
>>>>       if ( res )
>>>>           return res;
>>>>
>>>
>>> Even though you are adding the psci-0.2 compatible string, I don't see
>>> the new PSCI_0_2_FN_* function numbers being exposed to the guest yet.
>>
>> These function numbers are defined by the spec. There is no need to
>> expose to the guest.
>
> I disagree, it makes things clearer leaving less margin for errors.

The PSCI spec clearly define standard values (see every functions 
description in the doc). Those values won't change in the PSCI 0.2 version.

The properties function = <id> should only be created if we want PSCI 
0.1 use them.

That made me think, I don't think we need to have specific values for 
PSCI 0.1 function, unless they are not compatible.

>
>> IIRC, the DT binding for PSCI v0.2 only contain the method to call psci
>> and the compatible string.
>
> That is not what the spec uses as example of DT bindings at page 46,
> chapter 5.12.

The properties function = <id> will only be used by guest support only 
PSCI 0.1. I suspect the spec try to make the device node compliant for 
both version (see the compatible string).

Even tho it's not yet merged, it looks like Linux will go to the 
following binding:

http://www.spinics.net/lists/kvm/msg103313.html

Regards,

-- 
Julien Grall