diff mbox

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

Message ID 1385982538-17855-3-git-send-email-andre.przywara@linaro.org
State New
Headers show

Commit Message

Andre Przywara Dec. 2, 2013, 11:08 a.m. UTC
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.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/arm64/smpboot.c      |  2 +-
 xen/arch/arm/platform.c           |  2 +-
 xen/arch/arm/platforms/exynos5.c  | 11 +----------
 xen/arch/arm/platforms/omap5.c    | 11 +----------
 xen/arch/arm/platforms/vexpress.c | 10 +---------
 xen/arch/arm/smpboot.c            | 15 ++++++++++-----
 xen/include/asm-arm/smp.h         |  2 ++
 7 files changed, 17 insertions(+), 36 deletions(-)

Comments

Julien Grall Dec. 2, 2013, 1:16 p.m. UTC | #1
On 12/02/2013 11:08 AM, 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.
>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>   xen/arch/arm/arm64/smpboot.c      |  2 +-
>   xen/arch/arm/platform.c           |  2 +-
>   xen/arch/arm/platforms/exynos5.c  | 11 +----------
>   xen/arch/arm/platforms/omap5.c    | 11 +----------
>   xen/arch/arm/platforms/vexpress.c | 10 +---------
>   xen/arch/arm/smpboot.c            | 15 ++++++++++-----
>   xen/include/asm-arm/smp.h         |  2 ++
>   7 files changed, 17 insertions(+), 36 deletions(-)
>

[..]

> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index a7f9ee4..056d462 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -112,7 +112,7 @@ int __init platform_cpu_up(int cpu)
>       if ( platform && platform->cpu_up )
>           return platform->cpu_up(cpu);
>
> -    return -EAGAIN;
> +    return -ENODEV;

This change seems unrelated to this patch.
Andre Przywara Dec. 2, 2013, 1:24 p.m. UTC | #2
On 12/02/2013 02:16 PM, Julien Grall wrote:
>
>
> On 12/02/2013 11:08 AM, 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.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/arch/arm/arm64/smpboot.c      |  2 +-
>>   xen/arch/arm/platform.c           |  2 +-
>>   xen/arch/arm/platforms/exynos5.c  | 11 +----------
>>   xen/arch/arm/platforms/omap5.c    | 11 +----------
>>   xen/arch/arm/platforms/vexpress.c | 10 +---------
>>   xen/arch/arm/smpboot.c            | 15 ++++++++++-----
>>   xen/include/asm-arm/smp.h         |  2 ++
>>   7 files changed, 17 insertions(+), 36 deletions(-)
>>
>
> [..]
>
>> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
>> index a7f9ee4..056d462 100644
>> --- a/xen/arch/arm/platform.c
>> +++ b/xen/arch/arm/platform.c
>> @@ -112,7 +112,7 @@ int __init platform_cpu_up(int cpu)
>>       if ( platform && platform->cpu_up )
>>           return platform->cpu_up(cpu);
>>
>> -    return -EAGAIN;
>> +    return -ENODEV;
>
> This change seems unrelated to this patch.

That is disputable. If I read the original code directly, the intention 
of EAGAIN was like: "There is no platform specific code, try something 
else." Now with PSCI and the GIC kicking already folded into this 
function, the answer to "no PSCI and no platform code" is simply: I 
don't know how to enable SMP, so there is no SMP => ENODEV.

I can add a comment if this helps.

Regards,
Andre.
Julien Grall Dec. 2, 2013, 1:57 p.m. UTC | #3
On 12/02/2013 01:24 PM, Andre Przywara wrote:
> On 12/02/2013 02:16 PM, Julien Grall wrote:
>>
>>
>> On 12/02/2013 11:08 AM, 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.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>> ---
>>>   xen/arch/arm/arm64/smpboot.c      |  2 +-
>>>   xen/arch/arm/platform.c           |  2 +-
>>>   xen/arch/arm/platforms/exynos5.c  | 11 +----------
>>>   xen/arch/arm/platforms/omap5.c    | 11 +----------
>>>   xen/arch/arm/platforms/vexpress.c | 10 +---------
>>>   xen/arch/arm/smpboot.c            | 15 ++++++++++-----
>>>   xen/include/asm-arm/smp.h         |  2 ++
>>>   7 files changed, 17 insertions(+), 36 deletions(-)
>>>
>>
>> [..]
>>
>>> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
>>> index a7f9ee4..056d462 100644
>>> --- a/xen/arch/arm/platform.c
>>> +++ b/xen/arch/arm/platform.c
>>> @@ -112,7 +112,7 @@ int __init platform_cpu_up(int cpu)
>>>       if ( platform && platform->cpu_up )
>>>           return platform->cpu_up(cpu);
>>>
>>> -    return -EAGAIN;
>>> +    return -ENODEV;
>>
>> This change seems unrelated to this patch.
>
> That is disputable. If I read the original code directly, the intention
> of EAGAIN was like: "There is no platform specific code, try something
> else." Now with PSCI and the GIC kicking already folded into this
> function, the answer to "no PSCI and no platform code" is simply: I
> don't know how to enable SMP, so there is no SMP => ENODEV.
>
> I can add a comment if this helps.

Thanks for the explanation, I don't think it needs a comment.

Acked-by: Julien Grall <julien.grall@linaro.org>
Ian Campbell Dec. 2, 2013, 3:01 p.m. UTC | #4
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.

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.

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?



>      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);
>
Andre Przywara Dec. 4, 2013, 12:15 p.m. UTC | #5
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);
>>
>
>
Ian Campbell Dec. 4, 2013, 12:28 p.m. UTC | #6
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.

I think the GIC send should be only for the non-PSCI case here too.

>  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);
> >>
> >
> >
>
Andre Przywara Dec. 4, 2013, 12:33 p.m. UTC | #7
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);
>>>>
>>>
>>>
>>
>
>
Ian Campbell Dec. 4, 2013, 12:35 p.m. UTC | #8
On Wed, 2013-12-04 at 13:33 +0100, Andre Przywara wrote:
> 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 firmware uses wfe while the Xen gate uses wfi so infact both
are needed.

> > 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.

Great, I should have checked the code before replying.

Thanks,
Ian.

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

Patch

diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
index 8696ed6..6a34bd4 100644
--- a/xen/arch/arm/arm64/smpboot.c
+++ b/xen/arch/arm/arm64/smpboot.c
@@ -38,7 +38,7 @@  static int __init smp_spin_table_cpu_up(int cpu)
 
     sev();
 
-    return 0;
+    return cpu_up_send_sgi(cpu);
 }
 
 static void __init smp_spin_table_init(int cpu, struct dt_device_node *dn)
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index a7f9ee4..056d462 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -112,7 +112,7 @@  int __init platform_cpu_up(int cpu)
     if ( platform && platform->cpu_up )
         return platform->cpu_up(cpu);
 
-    return -EAGAIN;
+    return -ENODEV;
 }
 
 int __init platform_smp_init(void)
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index 0e76cac..7880815 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -85,15 +85,6 @@  static int __init exynos5_smp_init(void)
     return 0;
 }
 
-static int __init exynos5_cpu_up(int cpu)
-{
-    /* Nothing to do here, the generic sev() will suffice to kick CPUs
-     * out of either the firmware or our own smp_up_cpu gate,
-     * depending on where they have ended up. */
-
-    return 0;
-}
-
 static void exynos5_reset(void)
 {
     void __iomem *pmu;
@@ -137,7 +128,7 @@  PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
     .init_time = exynos5_init_time,
     .specific_mapping = exynos5_specific_mapping,
     .smp_init = exynos5_smp_init,
-    .cpu_up = exynos5_cpu_up,
+    .cpu_up = cpu_up_send_sgi,
     .reset = exynos5_reset,
     .quirks = exynos5_quirks,
     .blacklist_dev = exynos5_blacklist_dev,
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index 54fa5ff..4be8e8e 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -144,15 +144,6 @@  static int __init omap5_smp_init(void)
     return 0;
 }
 
-static int __init omap5_cpu_up(int cpu)
-{
-    /* Nothing to do here, the generic sev() will suffice to kick CPUs
-     * out of either the firmware or our own smp_up_cpu gate,
-     * depending on where they have ended up. */
-
-    return 0;
-}
-
 static uint32_t omap5_quirks(void)
 {
     return PLATFORM_QUIRK_DOM0_MAPPING_11;
@@ -169,7 +160,7 @@  PLATFORM_START(omap5, "TI OMAP5")
     .init_time = omap5_init_time,
     .specific_mapping = omap5_specific_mapping,
     .smp_init = omap5_smp_init,
-    .cpu_up = omap5_cpu_up,
+    .cpu_up = cpu_up_send_sgi,
     .quirks = omap5_quirks,
 PLATFORM_END
 
diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c
index 9056366..6132056 100644
--- a/xen/arch/arm/platforms/vexpress.c
+++ b/xen/arch/arm/platforms/vexpress.c
@@ -144,14 +144,6 @@  static int __init vexpress_smp_init(void)
     return 0;
 }
 
-static int __init vexpress_cpu_up(int cpu)
-{
-    /* Nothing to do here, the generic sev() will suffice to kick CPUs
-     * out of either the firmware or our own smp_up_cpu gate,
-     * depending on where they have ended up. */
-
-    return 0;
-}
 #endif
 
 static const char * const vexpress_dt_compat[] __initconst =
@@ -180,7 +172,7 @@  PLATFORM_START(vexpress, "VERSATILE EXPRESS")
     .compatible = vexpress_dt_compat,
 #ifdef CONFIG_ARM_32
     .smp_init = vexpress_smp_init,
-    .cpu_up = vexpress_cpu_up,
+    .cpu_up = cpu_up_send_sgi,
 #endif
     .reset = vexpress_reset,
     .blacklist_dev = vexpress_blacklist_dev,
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 6b4a18c..52cef30 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -343,6 +343,16 @@  void stop_cpu(void)
         wfi();
 }
 
+int __init cpu_up_send_sgi(int cpu)
+{
+    /* 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);
+
+    return 0;
+}
+
 /* Bring up a remote CPU */
 int __cpu_up(unsigned int cpu)
 {
@@ -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);
-
     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);