From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 1/2] xen/arm: Add support of PSCI v1.0 for the host Date: Fri, 9 Oct 2015 16:24:51 +0100 Message-ID: <5617DC43.6060603@citrix.com> References: <1444329901-19055-1-git-send-email-julien.grall@citrix.com> <1444329901-19055-2-git-send-email-julien.grall@citrix.com> <1444403297.1410.417.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZkZYn-0002Sv-Ax for xen-devel@lists.xenproject.org; Fri, 09 Oct 2015 15:26:17 +0000 In-Reply-To: <1444403297.1410.417.camel@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: Ian Campbell , xen-devel@lists.xenproject.org Cc: Mark Rutland , Andre Przywara , stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 09/10/15 16:08, 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 first implementation I did was based on the Linux one which is working checking if the PSCI version if >= 0.2. Although I changed my mind before sending the patch because I was worry to see Xen breaking badly when booting on another version of PSCI. > > 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. Right, I will update the check. > (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"? It's just a mistake. When I first wrote the patch the check was: PSCI_MAJOR_VERSION(psci_vers) == 0 && PSCI_MINOR_VERSION(psci_vers) < 2 Although, I was worry about allowing to many version of PSCI. I will use your suggestion. > Also please format the version like you did below with %u.%u. I will do. > >> } >> >> 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; >> } > Regards, -- Julien Grall