Re: [Xen-devel] [PATCH v2 2/6] arm: move GIC SGI kicking into separate function

[Andre Przywara <andre.przywara@xxxxxxxxxx>]

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);

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