Patchwork [UPDATED,21/27] sparc32, smpboot: Use generic SMP booting infrastructure

login
register
mail settings
Submitter Srivatsa S. Bhat
Date June 1, 2012, 10:47 p.m.
Message ID <4FC94693.5050707@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/162380/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Srivatsa S. Bhat - June 1, 2012, 10:47 p.m.
Convert sparc32 to use the generic framework to boot secondary CPUs.

Notes:
While removing the call to cpu_idle(), we can also remove the call to
cpu_panic() because we are calling BUG() in the generic code anyway.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Konrad Eisele <konrad@gaisler.com>
Cc: Tkhai Kirill <tkhai@yandex.ru>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: sparclinux@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/sparc/kernel/leon_smp.c      |   25 +++++++++++++++----------
 arch/sparc/kernel/sun4d_smp.c     |   26 +++++++++++++-------------
 arch/sparc/kernel/sun4m_smp.c     |   26 +++++++++++++++-----------
 arch/sparc/kernel/trampoline_32.S |   12 ------------
 4 files changed, 43 insertions(+), 46 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - June 1, 2012, 10:53 p.m.
From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
Date: Sat, 02 Jun 2012 04:17:47 +0530

> 
> Convert sparc32 to use the generic framework to boot secondary CPUs.
> 
> Notes:
> While removing the call to cpu_idle(), we can also remove the call to
> cpu_panic() because we are calling BUG() in the generic code anyway.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Almost there, but not quite yet.

You can't get rid of the cpu_panic invocations, those have to be there
after the *_callin calls to catch if they accidently return.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - June 2, 2012, 6:52 a.m.
Hi Srivatsa.

I cannot see how this would work for sparc32...
[Did not notice before...]

> --- a/arch/sparc/kernel/leon_smp.c
> +++ b/arch/sparc/kernel/leon_smp.c

> +void __cpuinit __cpu_pre_starting(void *unused)
> +{
> +	unsigned int cpuid = hard_smp_processor_id();
>  
> diff --git a/arch/sparc/kernel/sun4d_smp.c b/arch/sparc/kernel/sun4d_smp.c
> index ddaea31..cd5367a 100644
> --- a/arch/sparc/kernel/sun4d_smp.c
> +++ b/arch/sparc/kernel/sun4d_smp.c

> +void __cpuinit __cpu_pre_starting(void *unused)
> +{
> +	unsigned int cpuid = hard_smp_processor_id();
>  
> diff --git a/arch/sparc/kernel/sun4m_smp.c b/arch/sparc/kernel/sun4m_smp.c
> index 128af73..ed05f54 100644
> --- a/arch/sparc/kernel/sun4m_smp.c
> +++ b/arch/sparc/kernel/sun4m_smp.c

> +void __cpuinit __cpu_pre_starting(void *unused)
> +{
> +	unsigned int cpuid = hard_smp_processor_id();

See how you define a function with the same name three times.
On sparc32 we include all of the above files in the kernel,
and uses various tricks to determine at run-time
which variant to use.

We need to define these general functions in smp_32.c and
then take relevant action depending on sparc_cpu_model.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - June 2, 2012, 7:44 a.m.
On Sat, Jun 02, 2012 at 08:52:49AM +0200, Sam Ravnborg wrote:
> Hi Srivatsa.
> 
> I cannot see how this would work for sparc32...
> [Did not notice before...]
> 
> > --- a/arch/sparc/kernel/leon_smp.c
> > +++ b/arch/sparc/kernel/leon_smp.c
> 
> > +void __cpuinit __cpu_pre_starting(void *unused)
> > +{
> > +	unsigned int cpuid = hard_smp_processor_id();
> >  
> > diff --git a/arch/sparc/kernel/sun4d_smp.c b/arch/sparc/kernel/sun4d_smp.c
> > index ddaea31..cd5367a 100644
> > --- a/arch/sparc/kernel/sun4d_smp.c
> > +++ b/arch/sparc/kernel/sun4d_smp.c
> 
> > +void __cpuinit __cpu_pre_starting(void *unused)
> > +{
> > +	unsigned int cpuid = hard_smp_processor_id();
> >  
> > diff --git a/arch/sparc/kernel/sun4m_smp.c b/arch/sparc/kernel/sun4m_smp.c
> > index 128af73..ed05f54 100644
> > --- a/arch/sparc/kernel/sun4m_smp.c
> > +++ b/arch/sparc/kernel/sun4m_smp.c
> 
> > +void __cpuinit __cpu_pre_starting(void *unused)
> > +{
> > +	unsigned int cpuid = hard_smp_processor_id();
> 
> See how you define a function with the same name three times.
> On sparc32 we include all of the above files in the kernel,
> and uses various tricks to determine at run-time
> which variant to use.
> 
> We need to define these general functions in smp_32.c and
> then take relevant action depending on sparc_cpu_model.

I took a short look at this.
And it is a bit complicated :-(

leon is the odd-one here. For reasons I do not understand
we have a call from head_32.S to leon_smp_cpu_startup:
in trampoline_32.S.

But sun4m and sun4d uses the normal path via start_kernel() etc.

This has the side-effect that sparc_cpu_model is not set
when we call leon_smp_cpu_startup.

I will try to dive into this and see if I on the sparc32 side
can make leon behave like the other platforms, and then 
unify some of the smp cpu boot stuff such that we then
can introduce the generalization from your patch.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srivatsa S. Bhat - June 2, 2012, 8:01 a.m.
On 06/02/2012 01:14 PM, Sam Ravnborg wrote:

> On Sat, Jun 02, 2012 at 08:52:49AM +0200, Sam Ravnborg wrote:
>> Hi Srivatsa.
>>
>> I cannot see how this would work for sparc32...
>> [Did not notice before...]
>>
>>> --- a/arch/sparc/kernel/leon_smp.c
>>> +++ b/arch/sparc/kernel/leon_smp.c
>>
>>> +void __cpuinit __cpu_pre_starting(void *unused)
>>> +{
>>> +	unsigned int cpuid = hard_smp_processor_id();
>>>  
>>> diff --git a/arch/sparc/kernel/sun4d_smp.c b/arch/sparc/kernel/sun4d_smp.c
>>> index ddaea31..cd5367a 100644
>>> --- a/arch/sparc/kernel/sun4d_smp.c
>>> +++ b/arch/sparc/kernel/sun4d_smp.c
>>
>>> +void __cpuinit __cpu_pre_starting(void *unused)
>>> +{
>>> +	unsigned int cpuid = hard_smp_processor_id();
>>>  
>>> diff --git a/arch/sparc/kernel/sun4m_smp.c b/arch/sparc/kernel/sun4m_smp.c
>>> index 128af73..ed05f54 100644
>>> --- a/arch/sparc/kernel/sun4m_smp.c
>>> +++ b/arch/sparc/kernel/sun4m_smp.c
>>
>>> +void __cpuinit __cpu_pre_starting(void *unused)
>>> +{
>>> +	unsigned int cpuid = hard_smp_processor_id();
>>
>> See how you define a function with the same name three times.
>> On sparc32 we include all of the above files in the kernel,
>> and uses various tricks to determine at run-time
>> which variant to use.
>>
>> We need to define these general functions in smp_32.c and
>> then take relevant action depending on sparc_cpu_model.
> 
> I took a short look at this.
> And it is a bit complicated :-(
> 
> leon is the odd-one here. For reasons I do not understand
> we have a call from head_32.S to leon_smp_cpu_startup:
> in trampoline_32.S.
> 
> But sun4m and sun4d uses the normal path via start_kernel() etc.
> 
> This has the side-effect that sparc_cpu_model is not set
> when we call leon_smp_cpu_startup.
> 


Yes, I saw that too now..

> I will try to dive into this and see if I on the sparc32 side
> can make leon behave like the other platforms, and then 
> unify some of the smp cpu boot stuff such that we then
> can introduce the generalization from your patch.
> 


Thanks a lot for that Sam!
 
Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srivatsa S. Bhat - June 2, 2012, 8:14 a.m.
On 06/02/2012 01:31 PM, Srivatsa S. Bhat wrote:

> On 06/02/2012 01:14 PM, Sam Ravnborg wrote:
> 
>> On Sat, Jun 02, 2012 at 08:52:49AM +0200, Sam Ravnborg wrote:
>>> Hi Srivatsa.
>>>
>>> I cannot see how this would work for sparc32...
>>> [Did not notice before...]
>>>
>>>> --- a/arch/sparc/kernel/leon_smp.c
>>>> +++ b/arch/sparc/kernel/leon_smp.c
>>>
>>>> +void __cpuinit __cpu_pre_starting(void *unused)
>>>> +{
>>>> +	unsigned int cpuid = hard_smp_processor_id();
>>>>  
>>>> diff --git a/arch/sparc/kernel/sun4d_smp.c b/arch/sparc/kernel/sun4d_smp.c
>>>> index ddaea31..cd5367a 100644
>>>> --- a/arch/sparc/kernel/sun4d_smp.c
>>>> +++ b/arch/sparc/kernel/sun4d_smp.c
>>>
>>>> +void __cpuinit __cpu_pre_starting(void *unused)
>>>> +{
>>>> +	unsigned int cpuid = hard_smp_processor_id();
>>>>  
>>>> diff --git a/arch/sparc/kernel/sun4m_smp.c b/arch/sparc/kernel/sun4m_smp.c
>>>> index 128af73..ed05f54 100644
>>>> --- a/arch/sparc/kernel/sun4m_smp.c
>>>> +++ b/arch/sparc/kernel/sun4m_smp.c
>>>
>>>> +void __cpuinit __cpu_pre_starting(void *unused)
>>>> +{
>>>> +	unsigned int cpuid = hard_smp_processor_id();
>>>
>>> See how you define a function with the same name three times.
>>> On sparc32 we include all of the above files in the kernel,
>>> and uses various tricks to determine at run-time
>>> which variant to use.
>>>
>>> We need to define these general functions in smp_32.c and
>>> then take relevant action depending on sparc_cpu_model.
>>
>> I took a short look at this.
>> And it is a bit complicated :-(
>>
>> leon is the odd-one here. For reasons I do not understand
>> we have a call from head_32.S to leon_smp_cpu_startup:
>> in trampoline_32.S.
>>
>> But sun4m and sun4d uses the normal path via start_kernel() etc.
>>
>> This has the side-effect that sparc_cpu_model is not set
>> when we call leon_smp_cpu_startup.
>>
> 
> Yes, I saw that too now..
> 


But I'm wondering where will it set sparc_cpu_model to sparc_leon
then.. The only place i saw was setup_arch(), which gets called from
start_kernel(). But if leon doesn't even call start_kernel(), then....?
 
Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/kernel/leon_smp.c b/arch/sparc/kernel/leon_smp.c
index a469090..498ca9b 100644
--- a/arch/sparc/kernel/leon_smp.c
+++ b/arch/sparc/kernel/leon_smp.c
@@ -25,6 +25,7 @@ 
 #include <linux/gfp.h>
 #include <linux/cpu.h>
 #include <linux/clockchips.h>
+#include <linux/smpboot.h>
 
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
@@ -73,13 +74,21 @@  static inline unsigned long do_swap(volatile unsigned long *ptr,
 
 void __cpuinit leon_callin(void)
 {
-	int cpuid = hard_smp_processor_id();
+	smpboot_start_secondary(NULL);
+}
+
+void __cpuinit __cpu_pre_starting(void *unused)
+{
+	unsigned int cpuid = hard_smp_processor_id();
 
 	local_ops->cache_all();
 	local_ops->tlb_all();
 	leon_configure_cache_smp();
+}
 
-	notify_cpu_starting(cpuid);
+void __cpuinit __cpu_pre_online(void *unused)
+{
+	unsigned int cpuid = hard_smp_processor_id();
 
 	/* Get our local ticker going. */
 	register_percpu_ce(cpuid);
@@ -91,11 +100,10 @@  void __cpuinit leon_callin(void)
 	local_ops->tlb_all();
 
 	/*
-	 * Unblock the master CPU _only_ when the scheduler state
-	 * of all secondary CPUs will be up-to-date, so after
-	 * the SMP initialization the master will be just allowed
-	 * to call the scheduler code.
-	 * Allow master to continue.
+	 * Allow master to continue. The master will then give us the
+	 * go-ahead by setting the smp_commenced_mask and will wait without
+	 * timeouts until our setup is completed fully (signified by
+	 * our bit being set in the cpu_online_mask).
 	 */
 	do_swap(&cpu_callin_map[cpuid], 1);
 
@@ -112,9 +120,6 @@  void __cpuinit leon_callin(void)
 
 	while (!cpumask_test_cpu(cpuid, &smp_commenced_mask))
 		mb();
-
-	local_irq_enable();
-	set_cpu_online(cpuid, true);
 }
 
 /*
diff --git a/arch/sparc/kernel/sun4d_smp.c b/arch/sparc/kernel/sun4d_smp.c
index ddaea31..cd5367a 100644
--- a/arch/sparc/kernel/sun4d_smp.c
+++ b/arch/sparc/kernel/sun4d_smp.c
@@ -12,6 +12,7 @@ 
 #include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/cpu.h>
+#include <linux/smpboot.h>
 
 #include <asm/cacheflush.h>
 #include <asm/switch_to.h>
@@ -52,8 +53,12 @@  static inline void show_leds(int cpuid)
 
 void __cpuinit smp4d_callin(void)
 {
-	int cpuid = hard_smp_processor_id();
-	unsigned long flags;
+	smpboot_start_secondary(NULL);
+}
+
+void __cpuinit __cpu_pre_starting(void *unused)
+{
+	unsigned int cpuid = hard_smp_processor_id();
 
 	/* Show we are alive */
 	cpu_leds[cpuid] = 0x6;
@@ -64,14 +69,13 @@  void __cpuinit smp4d_callin(void)
 
 	local_ops->cache_all();
 	local_ops->tlb_all();
+}
+
+void __cpuinit __cpu_pre_online(void *unused)
+{
+	unsigned int cpuid = hard_smp_processor_id();
+	unsigned long flags;
 
-	notify_cpu_starting(cpuid);
-	/*
-	 * Unblock the master CPU _only_ when the scheduler state
-	 * of all secondary CPUs will be up-to-date, so after
-	 * the SMP initialization the master will be just allowed
-	 * to call the scheduler code.
-	 */
 	/* Get our local ticker going. */
 	register_percpu_ce(cpuid);
 
@@ -106,16 +110,12 @@  void __cpuinit smp4d_callin(void)
 	local_ops->cache_all();
 	local_ops->tlb_all();
 
-	local_irq_enable();	/* We don't allow PIL 14 yet */
-
 	while (!cpumask_test_cpu(cpuid, &smp_commenced_mask))
 		barrier();
 
 	spin_lock_irqsave(&sun4d_imsk_lock, flags);
 	cc_set_imsk(cc_get_imsk() & ~0x4000); /* Allow PIL 14 as well */
 	spin_unlock_irqrestore(&sun4d_imsk_lock, flags);
-	set_cpu_online(cpuid, true);
-
 }
 
 /*
diff --git a/arch/sparc/kernel/sun4m_smp.c b/arch/sparc/kernel/sun4m_smp.c
index 128af73..ed05f54 100644
--- a/arch/sparc/kernel/sun4m_smp.c
+++ b/arch/sparc/kernel/sun4m_smp.c
@@ -10,6 +10,7 @@ 
 #include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/cpu.h>
+#include <linux/smpboot.h>
 
 #include <asm/cacheflush.h>
 #include <asm/switch_to.h>
@@ -36,12 +37,20 @@  swap_ulong(volatile unsigned long *ptr, unsigned long val)
 
 void __cpuinit smp4m_callin(void)
 {
-	int cpuid = hard_smp_processor_id();
+	smpboot_start_secondary(NULL);
+}
+
+void __cpuinit __cpu_pre_starting(void *unused)
+{
+	unsigned int cpuid = hard_smp_processor_id();
 
 	local_ops->cache_all();
 	local_ops->tlb_all();
+}
 
-	notify_cpu_starting(cpuid);
+void __cpuinit __cpu_pre_online(void *unused)
+{
+	unsigned int cpuid = hard_smp_processor_id();
 
 	register_percpu_ce(cpuid);
 
@@ -52,12 +61,11 @@  void __cpuinit smp4m_callin(void)
 	local_ops->tlb_all();
 
 	/*
-	 * Unblock the master CPU _only_ when the scheduler state
-	 * of all secondary CPUs will be up-to-date, so after
-	 * the SMP initialization the master will be just allowed
-	 * to call the scheduler code.
+	 * Allow master to continue. The master will then give us the
+	 * go-ahead by setting the smp_commenced_mask and will wait without
+	 * timeouts until our setup is completed fully (signified by
+	 * our bit being set in the cpu_online_mask).
 	 */
-	/* Allow master to continue. */
 	swap_ulong(&cpu_callin_map[cpuid], 1);
 
 	/* XXX: What's up with all the flushes? */
@@ -75,10 +83,6 @@  void __cpuinit smp4m_callin(void)
 
 	while (!cpumask_test_cpu(cpuid, &smp_commenced_mask))
 		mb();
-
-	local_irq_enable();
-
-	set_cpu_online(cpuid, true);
 }
 
 /*
diff --git a/arch/sparc/kernel/trampoline_32.S b/arch/sparc/kernel/trampoline_32.S
index 7364ddc..75b659e 100644
--- a/arch/sparc/kernel/trampoline_32.S
+++ b/arch/sparc/kernel/trampoline_32.S
@@ -82,17 +82,9 @@  cpu3_startup:
 	call	smp4m_callin
 	 nop
 
-	b,a	smp_do_cpu_idle
-
 	.text
 	.align	4
 
-smp_do_cpu_idle:
-	call	cpu_idle
-	 mov	0, %o0
-
-	call	cpu_panic
-	 nop
 
 /* CPUID in bootbus can be found at PA 0xff0140000 */
 #define SUN4D_BOOTBUS_CPUID	0xf0140000
@@ -147,8 +139,6 @@  sun4d_cpu_startup:
 	call	smp4d_callin
 	 nop
 
-	b,a	smp_do_cpu_idle
-
 #ifdef CONFIG_SPARC_LEON
 
 	__CPUINIT
@@ -206,6 +196,4 @@  leon_smp_cpu_startup:
 	call	leon_callin
 	 nop
 
-	b,a	smp_do_cpu_idle
-
 #endif