From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 1/2] xen/arm: Add support of PSCI v1.0 for the host Date: Fri, 9 Oct 2015 16:08:17 +0100 Message-ID: <1444403297.1410.417.camel@citrix.com> References: <1444329901-19055-1-git-send-email-julien.grall@citrix.com> <1444329901-19055-2-git-send-email-julien.grall@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZkZHS-0000rF-2B for xen-devel@lists.xenproject.org; Fri, 09 Oct 2015 15:08:22 +0000 In-Reply-To: <1444329901-19055-2-git-send-email-julien.grall@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall , xen-devel@lists.xenproject.org Cc: Mark Rutland , Andre Przywara , stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org 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. (which begs the question why there is not a "arm,psci-1.x" compat string, Mark/Andre?) > > Signed-off-by: Julien Grall > > --- > > Cc: Andre Przywara > Cc: Mark Rutland > --- > 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; > }