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

[Andre Przywara <andre.przywara@linaro.org>]

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? I was under the assumption that the semantics of cpu_on 
is to start executing code at the given address, whatever this takes 
internally.
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, ARM32 non-PSCI platforms call this now 
explicitly by pointing to that function in their platforms/foo.c file.

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