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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxx>
> >> ---
> >>  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.


> 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.


> >> +/* PSCI return values (inclusive of all PSCI versions) */
> >> +#define PSCI_RET_SUCCESS                   0
> >> +#define PSCI_RET_NOT_SUPPORTED                    -1
> >> +#define PSCI_RET_INVALID_PARAMS                   -2
> >> +#define PSCI_RET_DENIED                           -3
> >> +#define PSCI_RET_ALREADY_ON                       -4
> >> +#define PSCI_RET_ON_PENDING                       -5
> >> +#define PSCI_RET_INTERNAL_FAILURE         -6
> >> +#define PSCI_RET_NOT_PRESENT                      -7
> >> +#define PSCI_RET_DISABLED                 -8
> > 
> > Be careful: at the moment the functions in vpsci.c return the error
> > codes specified in xen/include/asm-arm/psci.h.
> > We should define the PSCI return codes only in one place.
> 
> It looks like this has been cut & paste from Linux patch (see
> http://www.spinics.net/lists/arm-kernel/msg319712.html).
> 
> But this should not be exposed to neither the toolstack nor the guest.

Indeed

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel