From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH v2 2/6] arm: move GIC SGI kicking into separate function Date: Wed, 04 Dec 2013 13:33:45 +0100 Message-ID: <529F2129.4090308@linaro.org> References: <1385982538-17855-1-git-send-email-andre.przywara@linaro.org> <1385982538-17855-3-git-send-email-andre.przywara@linaro.org> <1385996473.7108.105.camel@kazak.uk.xensource.com> <529F1CD8.8010107@linaro.org> <1386160111.17466.70.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VoBea-0004Yk-0Q for xen-devel@lists.xenproject.org; Wed, 04 Dec 2013 12:34:08 +0000 Received: by mail-ee0-f54.google.com with SMTP id e51so2380215eek.27 for ; Wed, 04 Dec 2013 04:34:06 -0800 (PST) In-Reply-To: <1386160111.17466.70.camel@kazak.uk.xensource.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 Cc: xen-devel@lists.xenproject.org, julien.grall@linaro.org, patches@linaro.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 12/04/2013 01:28 PM, Ian Campbell wrote: > On Wed, 2013-12-04 at 13:15 +0100, Andre Przywara wrote: >> On 12/02/2013 04:01 PM, Ian Campbell wrote: >>> On Mon, 2013-12-02 at 12:08 +0100, Andre Przywara wrote: >>>> Currently we unconditionally send SGIs to all cores on SMP bringup. >>>> PSCI will not need this, so we move this into a function and call it >>>> explicitly from the platforms that need it. This gets us get rid of >>>> the empty cpu_up() platform functions in ARM32 and the comment in >>>> there. >>> >>> I don't think this is quite true -- even on a PSCI system the kick is >>> required to get past the gate in head.S. >> >> Right, but this is the responsibility of the PSCI handler in the >> firmware, right? > > Note that I am talking about a gate which is implemented in Xen's > head.S, not in the firmware. It is Xen's reponsibility to get the CPU > past that point, which is somewhat independent from the firmware wakeup, > except in reality it is intertwined because they use the same mechanism. > > However, I think I was mistaken. In the case of PSCI we are able to wake > up specific targetted CPUs individually and we do so having already > opened the gate in out head.S, so the CPU must necessarily fall through > without waiting. I think this is worth mentioning in the commit log if > you don't mind. > >> I was under the assumption that the semantics of cpu_on >> is to start executing code at the given address, whatever this takes >> internally. > > Right, the issue here is a gate which we have subsequent to that > happening. > >> Calxeda firmware for instance does the SGI kick. >> >>> >>> I wonder how this interacts with PSCI implementations which use an SGI >>> themselves internally... >>> >>>> @@ -376,11 +386,6 @@ int __cpu_up(unsigned int cpu) >>>> return rc; >>>> } >>>> >>>> - /* We don't know the GIC ID of the CPU until it has woken up, so just signal >>>> - * everyone and rely on our own smp_up_cpu gate to ensure only the one we >>>> - * want gets through. */ >>>> - send_SGI_allbutself(GIC_SGI_EVENT_CHECK); >>>> - >>> >>> So, I was saying in the 00 mail I'm not sure we can get rid of this >>> altogether. >> >> Please note that we do not get rid of this, but just move it. ARM64 >> calls it in arm64/smpboot.c, > > How does this interact with the sev() In smp_spin_table_cpu_up I wonder. Me, too ;-) The original code does the sev() in arm/arm64/smpboot.c, then returns to the caller in arm/smpboot.c, which does the GIC kick. I was already wondering if that is correct, I guess you could answer this much better. > I think the GIC send should be only for the non-PSCI case here too. It is. The arm64 code has the GIC kick moved into the spin_table function only. PSCI is a different beast, not using the GIC. Also arm32 does it only in platform specific functions, which don't get called when PSCI is available. Regards, Andre. > >> ARM32 non-PSCI platforms call this now >> explicitly by pointing to that function in their platforms/foo.c file. > > OK. > >> >>> >>> But I suppose it is the intention that the platform code always has both >>> its own logic and this SGI kick (possibly coalesced) in such >>> circumstances? Which is probably ok? >> >> That was my thinking, yes. >> >> Regards, >> Andre. >> >> >>>> while ( !cpu_online(cpu) ) >>>> { >>>> cpu_relax(); >>>> diff --git a/xen/include/asm-arm/smp.h b/xen/include/asm-arm/smp.h >>>> index 1485cc6..a1de03c 100644 >>>> --- a/xen/include/asm-arm/smp.h >>>> +++ b/xen/include/asm-arm/smp.h >>>> @@ -21,6 +21,8 @@ extern int arch_smp_init(void); >>>> extern int arch_cpu_init(int cpu, struct dt_device_node *dn); >>>> extern int arch_cpu_up(int cpu); >>>> >>>> +int cpu_up_send_sgi(int cpu); >>>> + >>>> /* Secondary CPU entry point */ >>>> extern void init_secondary(void); >>>> >>> >>> >> > >