[RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Michael Ellerman]

Currently we use a cpumask called of_spin_map to keep track of which threads
have been spun up. We basically guess that OF has spun up all even numbered
threads, and so all the odd numbered threads need to be brought up.

That's a bit of a dicey assumption at best, and is totally incorrect for
kexec. Luckily we have an rtas call which can tell us whether a cpu is up
or not, so let's use it?

I notice this code is duplicated in platforms/cell, do we want to move it into
kernel/rtas_smp.c or something?

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/platforms/pseries/smp.c |   48 +++++++----------------------------
 1 file changed, 10 insertions(+), 38 deletions(-)

Index: to-merge/arch/powerpc/platforms/pseries/smp.c
===================================================================
--- to-merge.orig/arch/powerpc/platforms/pseries/smp.c
+++ to-merge/arch/powerpc/platforms/pseries/smp.c
@@ -57,16 +57,6 @@
 #define DBG(fmt...)
 #endif
 
-/*
- * The primary thread of each non-boot processor is recorded here before
- * smp init.
- */
-static cpumask_t of_spin_map;
-
-extern void pSeries_secondary_smp_init(unsigned long);
-
-#ifdef CONFIG_HOTPLUG_CPU
-
 /* Get state of physical CPU.
  * Return codes:
  *	0	- The processor is in the RTAS stopped state
@@ -93,6 +83,8 @@ static int query_cpu_stopped(unsigned in
 	return cpu_status;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+
 static int pSeries_cpu_disable(void)
 {
 	int cpu = smp_processor_id();
@@ -269,22 +261,23 @@ static struct notifier_block pSeries_smp
  */
 static inline int __devinit smp_startup_cpu(unsigned int lcpu)
 {
-	int status;
+	extern void pSeries_secondary_smp_init(unsigned long);
 	unsigned long start_here = __pa((u32)*((unsigned long *)
 					       pSeries_secondary_smp_init));
 	unsigned int pcpu;
-	int start_cpu;
-
-	if (cpu_isset(lcpu, of_spin_map))
-		/* Already started by OF and sitting in spin loop */
-		return 1;
+	int start_cpu, status;
 
 	pcpu = get_hard_smp_processor_id(lcpu);
 
+	if (query_cpu_stopped(pcpu) == 0) {
+		DBG("CPU %d (%d) already running.\n", lcpu, pcpu);
+		return 1;
+	}
+
 	/* Fixup atomic count: it exited inside IRQ handler. */
 	task_thread_info(paca[lcpu].__current)->preempt_count	= 0;
 
-	/* 
+	/*
 	 * If the RTAS start-cpu token does not exist then presume the
 	 * cpu is already spinning.
 	 */
@@ -339,9 +332,6 @@ static void __devinit smp_xics_setup_cpu
 
 	if (firmware_has_feature(FW_FEATURE_SPLPAR))
 		vpa_init(cpu);
-
-	cpu_clear(cpu, of_spin_map);
-
 }
 #endif /* CONFIG_XICS */
 
@@ -419,8 +409,6 @@ static struct smp_ops_t pSeries_xics_smp
 /* This is called very early */
 void __init smp_init_pSeries(void)
 {
-	int i;
-
 	DBG(" -> smp_init_pSeries()\n");
 
 	switch (ppc64_interrupt_controller) {
@@ -447,22 +435,6 @@ void __init smp_init_pSeries(void)
 		pSeries_reconfig_notifier_register(&pSeries_smp_nb);
 #endif
 
-	/* Mark threads which are still spinning in hold loops. */
-	if (cpu_has_feature(CPU_FTR_SMT)) {
-		for_each_present_cpu(i) { 
-			if (i % 2 == 0)
-				/*
-				 * Even-numbered logical cpus correspond to
-				 * primary threads.
-				 */
-				cpu_set(i, of_spin_map);
-		}
-	} else {
-		of_spin_map = cpu_present_map;
-	}
-
-	cpu_clear(boot_cpuid, of_spin_map);
-
 	/* Non-lpar has additional take/give timebase */
 	if (rtas_token("freeze-time-base") != RTAS_UNKNOWN_SERVICE) {
 		smp_ops->give_timebase = pSeries_give_timebase;

Re: [RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Anton Blanchard]

Hi,

> Currently we use a cpumask called of_spin_map to keep track of which threads
> have been spun up. We basically guess that OF has spun up all even numbered
> threads, and so all the odd numbered threads need to be brought up.
> 
> That's a bit of a dicey assumption at best, and is totally incorrect for
> kexec. Luckily we have an rtas call which can tell us whether a cpu is up
> or not, so let's use it?

I have a bad feeling its broken on some older (POWER3) machines.

Anton

Re: [RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On Tue, 2006-04-04 at 22:42 +1000, Anton Blanchard wrote:
> Hi,
> 
> > Currently we use a cpumask called of_spin_map to keep track of which threads
> > have been spun up. We basically guess that OF has spun up all even numbered
> > threads, and so all the odd numbered threads need to be brought up.
> > 
> > That's a bit of a dicey assumption at best, and is totally incorrect for
> > kexec. Luckily we have an rtas call which can tell us whether a cpu is up
> > or not, so let's use it?
> 
> I have a bad feeling its broken on some older (POWER3) machines.

Ok, we'll have to see _how_ it's broken. But if it falsely reports that
the thread is started that will suck. I haven't been able to test on my
Power3 because of another breakage *cough benh cough*. I'll try it
sometime.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: [RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Nathan Lynch]

Michael Ellerman wrote:
> Currently we use a cpumask called of_spin_map to keep track of which threads
> have been spun up. We basically guess that OF has spun up all even numbered
> threads, and so all the odd numbered threads need to be brought up.
> 
> That's a bit of a dicey assumption at best, and is totally incorrect for
> kexec.
>
> Luckily we have an rtas call which can tell us whether a cpu is up
> or not, so let's use it?

1. query-cpu-stopped-state isn't available on all RTAS systems (I
   believe it's required only on systems that support cpu offline).

2. I've tried it before.  Hope I'm remembering this correctly, but I
   think my experience was that query-cpu-stopped-state reported
   nonsense for cpus that were started by OF.  This was on Power5,
   btw.

3. This isn't how query-cpu-stopped-state was intended to be used.
   It's meant to be used by the OS to determine when a thread has
   stopped itself using the stop-self method.  Which might partially
   explain (2).

I'm fine with getting rid of the assumptions about thread numbering,
but I don't think this is gonna work, sorry.

Re: [RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Tue, 2006-04-04 at 12:25 -0500, Nathan Lynch wrote:
> Michael Ellerman wrote:
> > Currently we use a cpumask called of_spin_map to keep track of which threads
> > have been spun up. We basically guess that OF has spun up all even numbered
> > threads, and so all the odd numbered threads need to be brought up.
> > 
> > That's a bit of a dicey assumption at best, and is totally incorrect for
> > kexec.
> >
> > Luckily we have an rtas call which can tell us whether a cpu is up
> > or not, so let's use it?
> 
> 1. query-cpu-stopped-state isn't available on all RTAS systems (I
>    believe it's required only on systems that support cpu offline).

That should be ok, I'm pretty sure the way I've written it if
query-cpu-stopped-state isn't there we try and start it the old way
anyway.

> 2. I've tried it before.  Hope I'm remembering this correctly, but I
>    think my experience was that query-cpu-stopped-state reported
>    nonsense for cpus that were started by OF.  This was on Power5,
>    btw.

That's not what I see, perhaps I have newer firmware?

> 3. This isn't how query-cpu-stopped-state was intended to be used.
>    It's meant to be used by the OS to determine when a thread has
>    stopped itself using the stop-self method.  Which might partially
>    explain (2).

Perhaps, PAPR says: "The query-cpu-stopped-state primitive is used to
query a different processor thread to determine its status with respect
to the RTAS stopped state". I guess I assumed that a thread that's not
started would be in the "RTAS stopped state", which seems logical if not
correct.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: [RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Nathan Lynch]

Michael Ellerman wrote:
> On Tue, 2006-04-04 at 12:25 -0500, Nathan Lynch wrote:
> 
> > 2. I've tried it before.  Hope I'm remembering this correctly, but I
> >    think my experience was that query-cpu-stopped-state reported
> >    nonsense for cpus that were started by OF.  This was on Power5,
> >    btw.
> 
> That's not what I see, perhaps I have newer firmware?

I tried your patch (non-kexec boot) on a 1-way SMT power5 with
2.6.17-rc1 and got the badness below, and I think it matches what I
ran into in the past.  Maybe doing query-cpu-stopped-state on a thread
that has yet to be started by RTAS mucks things up?

Memory: 2047640k/2097152k available (5356k kernel code, 49512k
reserved, 1412k data, 900k bss, 240k init)
Calibrating delay loop... 375.80 BogoMIPS (lpj=751616)
Security Framework v1.0.0 initialized
SELinux:  Disabled at boot.
Mount-cache hash table entries: 256
Processor 1 is stuck.               <<<<<<<<
Brought up 1 CPUs
Node 0 CPUs: 0
migration_cost=0

Re: [RFC/PATCH] powerpc: Use rtas query-cpu-stopped-state in smp spinup

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Thu, 2006-04-06 at 20:10 -0500, Nathan Lynch wrote:
> I tried your patch (non-kexec boot) on a 1-way SMT power5 with
> 2.6.17-rc1 and got the badness below, and I think it matches what I
> ran into in the past.  Maybe doing query-cpu-stopped-state on a thread
> that has yet to be started by RTAS mucks things up?
> 
> Memory: 2047640k/2097152k available (5356k kernel code, 49512k
> reserved, 1412k data, 900k bss, 240k init)
> Calibrating delay loop... 375.80 BogoMIPS (lpj=751616)
> Security Framework v1.0.0 initialized
> SELinux:  Disabled at boot.
> Mount-cache hash table entries: 256
> Processor 1 is stuck.               <<<<<<<<

OK, thanks for testing it. This is clearly not going to fly.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]