Re: [Xen-devel] [PATCH 1/2] xen/arm: Add support of PSCI v1.0 for the host

[Mark Rutland <mark.rutland@xxxxxxx>]

On Fri, Oct 09, 2015 at 04:08:17PM +0100, Ian Campbell wrote:
> On Thu, 2015-10-08 at 19:45 +0100, Julien Grall wrote:
> > From Xen point of view, PSCI v0.2 and PSCI v1.0 are very similar. All
> > the PSCI calls used within Xen (PSCI_VERSION, CPU_ON, SYSTEM_OFF and
> > SYSTEM_RESET) behaves exactly the same.
> > 
> > While there is no compatible string to represent PSCI v1.0 in the DT,
> > it's possible to detect it using the function PSCI_VERSION.
> > 
> > The compatible string is now used to detect if the platform may support
> > PSCI v0.2 or higher.
> 
> The actual implementation here looks for precisely 0.2 or 1.0, not >= 0.2
> as suggested by this statement.
> 
> The PSCI 1.0 spec says (section 5.3.1, intended use of PSCI_VERSION) that
> for any 1.y version must be compatible with 1.x when y>x (for those
> functions which existed in 1.x, y might have more).
> 
> IOW an OS supporting 1.0 should work with any 1.x.

This _should_ be the case. Linux assumes future versions are compatible
in this manner.

There was some minor incompatibility going from 0.2 to 1.0 (the new
state parameter format) that could trip up existing PSCI 0.2 aware OSs,
but hopefully we won't have that kind of thing happen again.

> (which begs the question why there is not a "arm,psci-1.x" compat string,
> Mark/Andre?)

There will be one, in fact (see Lorenzo's pull request [1]), but it's
only necessary to allow us to describe PSCI 1.0 implementations which
are not strictly PSCI 0.2 compatible. Otherwise it's harmless to add for
PSCI 1.0 compatible systems, but "arm,psci-0.2" should be sufficient.

Thanks,
Mark.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/374547.html

> 
> > 
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > 
> > ---
> > 
> > Cc: Andre Przywara <andre.przywara@xxxxxxx>
> > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > ---
> >  xen/arch/arm/psci.c        |  9 +++++----
> >  xen/include/asm-arm/psci.h | 13 +++++++++++++
> >  2 files changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> > index 172c6e7..53ee2e4 100644
> > --- a/xen/arch/arm/psci.c
> > +++ b/xen/arch/arm/psci.c
> > @@ -122,15 +122,16 @@ int __init psci_init_0_2(void)
> >  
> >      psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> >  
> > -    if ( psci_ver != XEN_PSCI_V_0_2 )
> > +    if ( psci_ver != PSCI_VERSION(0, 2) && psci_ver != PSCI_VERSION(1, 0) )
> 
> Based on the above I think this should read:
> 
>     if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_MAJOR_VERSION(psci_ver) != 1 )
> 
> >      {
> > -        printk("Error: PSCI version %#x is not supported.\n", psci_ver);
> > -        return -EOPNOTSUPP;
> > +        printk("Error: Conflicting PSCI version detected (%#x)\n", 
> > psci_ver);
> 
> Conflicting with what?
> 
> I think perhaps you meant "Unrecognised" or "Unsupported"?
> 
> Also please format the version like you did below with %u.%u.
> 
> >      }
> >  
> >      psci_cpu_on_nr = PSCI_0_2_FN_NATIVE(CPU_ON);
> >  
> > -    printk(XENLOG_INFO "Using PSCI-0.2 for SMP bringup\n");
> > +    printk(XENLOG_INFO "Using PSCI-%u.%u for SMP bringup\n",
> > +           PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
> >  
> >      return 0;
> >  }
> 

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