[RFC PATCH 0/7] ARM: SMP: common "pen" secondary release method

[RFC PATCH 0/7] ARM: SMP: common "pen" secondary release method

[Alex Elder]

This series proposes creating a single implementation of the "pen
release" (or spin-table) mechanism for starting secondary CPUs on
ARM SMP systems.  This mechanism is currently implemented at least 5
times in the kernel, each essentially identical to the other.  There
are other machines that use this method, but slight differences make
it necessary to incorporate further work in order to accomodate them
with a single set of functions.

This work began with integrating SMP support for a Broadcom SoC.
The code I started was another copy of the "pen release" code.
Looking at how existing systems handled this, the duplication of
code was obvious, but the fact that implementations were identical
was less so.  In addition, the arm64 (though not 32-bit arm) code
supports the ability to define an "enable-method" property for CPUs
in a machine's device tree (one of which is "spin-table"), allowing
the secondary startup mechanism to be specified abstractly.

My broader goal then was to set up the SMP support for the Broadcom
SoC such that it could be defined in its device tree using a central
"spin-table" enable method for secondary CPUs.  The first step was
to create a central spin-table implementation.  I've tried to do
this generally so it's usable by as many other platforms as
possible.  (An alternative would be to define a custom enable method
for this platform, but I'd prefer to avoid that if I can.)

I already have some code for the second step--which supports
specifying the "spin-table" CPU enable method in device tree files.
But before presenting that I wanted to post this series to get
feedback on the approach.

I've provided a little more information for each patch below.

This code is available in the "review/common-spin-code" branch of
the git repository here:
    http://git.linaro.org/landing-teams/working/broadcom/kernel.git


					-Alex

Alex Elder (7):
  ARM: allow <asm/cputype.h> inclusion from assembly
    This patch is done specifically so the next one can use the
    value of MPIDR_HWID_BITMASK in secondary_holding_pen(), which is
    defined in asssembly.
  ARM: SMP: generic SMP spin-table method routines
    Define the common code (based on any of the others, but
    reportedly they all derive from ARM Realview.)  Names are
    changed to be more like arch/arm64/kernel/smp_spin_table.c.

  ARM: realview: use central SMP spin-table routines
  ARM: vexpress: use central SMP spin-table routines
  ARM: versatile: kill off SMP support code
  	The SMP code for realview and vexpress both use SMP code
	defined under mach-versatile, so these three go together.

  ARM: ux500: use generic SMP spin-table routines
    This one is a pretty straightforward switch to using
    the common code.
  	
  ARM: spear: use central SMP spin-table routines
    This is straightforward as well, but it depends on the
    two prerequisite patches mentioned above (which together
    make this code roughly identical to the rest).

 arch/arm/include/asm/cputype.h                 |    4 ++
 arch/arm/include/asm/smp.h                     |    5 ++
 arch/arm/kernel/head.S                         |   33 +++++++++
 arch/arm/kernel/smp.c                          |   77 ++++++++++++++++++++
 arch/arm/mach-realview/platsmp.c               |    8 +--
 arch/arm/mach-spear/Makefile                   |    2 +-
 arch/arm/mach-spear/generic.h                  |    1 -
 arch/arm/mach-spear/headsmp.S                  |   47 ------------
 arch/arm/mach-spear/platsmp.c                  |   75 +------------------
 arch/arm/mach-ux500/Makefile                   |    2 +-
 arch/arm/mach-ux500/headsmp.S                  |   37 ----------
 arch/arm/mach-ux500/platsmp.c                  |   72 +------------------
 arch/arm/mach-vexpress/platsmp.c               |    8 +--
 arch/arm/mach-vexpress/v2m.c                   |    1 -
 arch/arm/plat-versatile/Makefile               |    1 -
 arch/arm/plat-versatile/headsmp.S              |   41 -----------
 arch/arm/plat-versatile/include/plat/platsmp.h |   14 ----
 arch/arm/plat-versatile/platsmp.c              |   91 ------------------------
 18 files changed, 133 insertions(+), 386 deletions(-)
 delete mode 100644 arch/arm/mach-spear/headsmp.S
 delete mode 100644 arch/arm/mach-ux500/headsmp.S
 delete mode 100644 arch/arm/plat-versatile/headsmp.S
 delete mode 100644 arch/arm/plat-versatile/include/plat/platsmp.h
 delete mode 100644 arch/arm/plat-versatile/platsmp.c

-- 
1.7.9.5

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

[RFC PATCH 1/7] ARM: allow <asm/cputype.h> inclusion from assembly

[Alex Elder]

Conditionally exclude some portions of ARM <asm/cputype.h>
to allow that file to be included from assembly source.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/include/asm/cputype.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index acdde76..a74c671 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -2,7 +2,9 @@
 #define __ASM_ARM_CPUTYPE_H
 
 #include <linux/stringify.h>
+#ifndef __ASSEMBLY__
 #include <linux/kernel.h>
+#endif
 
 #define CPUID_ID	0
 #define CPUID_CACHETYPE	1
@@ -77,6 +79,7 @@
 #define ARM_CPU_XSCALE_ARCH_V2		0x4000
 #define ARM_CPU_XSCALE_ARCH_V3		0x6000
 
+#ifndef __ASSEMBLY__
 extern unsigned int processor_id;
 
 #ifdef CONFIG_CPU_CP15
@@ -219,5 +222,6 @@ static inline int cpu_is_xsc3(void)
 #else
 #define	cpu_is_xscale()	1
 #endif
+#endif /* !__ASSEMBLY__ */
 
 #endif
-- 
1.7.9.5

[RFC PATCH 2/7] ARM: SMP: generic SMP spin-table method routines

[Alex Elder]

Implement a centralized version of the spin table (a.k.a. "holding
pen") method of secondary CPU initialization.  This is the first
step in removing a number of duplicate implementations of this code.

The eventual goal is to allow "enable-method" properties in device
tree nodes for CPUs to select and use this common code.  As such,
some of the names are selected to match the names used in the SMP
spin-table code for ARM64.

Note:
Most implementations examine only the bottom 4 bits of the MPIDR in
order to determine a CPU's id.  This version looks at the bottom 24
bits instead, based on MPIDR_HWID_BITMASK.  If using only 4 bits is
a requirement for most of the platforms that might use it I'll
switch this use 4 bits instead.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/include/asm/smp.h |    5 +++
 arch/arm/kernel/head.S     |   33 +++++++++++++++++++
 arch/arm/kernel/smp.c      |   77 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)

diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 22a3b9b..83064d1 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -75,6 +75,11 @@ struct secondary_data {
 extern struct secondary_data secondary_data;
 extern volatile int pen_release;
 
+extern volatile u32 secondary_holding_pen_release;
+extern void secondary_holding_pen(void);
+extern int smp_boot_secondary(unsigned int cpu, struct task_struct *idle);
+extern void smp_secondary_init(unsigned int cpu);
+
 extern int __cpu_disable(void);
 
 extern void __cpu_die(unsigned int cpu);
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index f5f381d..3340f94 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -22,6 +22,7 @@
 #include <asm/memory.h>
 #include <asm/thread_info.h>
 #include <asm/pgtable.h>
+#include <asm/cputype.h>
 
 #if defined(CONFIG_DEBUG_LL) && !defined(CONFIG_DEBUG_SEMIHOSTING)
 #include CONFIG_DEBUG_LL_INCLUDE
@@ -402,6 +403,38 @@ __secondary_data:
 	.long	.
 	.long	secondary_data
 	.long	__secondary_switched
+
+
+	/*
+	 * Secondary cores spin in this "holding pen" until they are
+	 * signaled to proceed by jumping to secondary_startup
+	 * (above).  A core knows to proceed when it finds that the
+	 * value of the secondary_holding_pen_release global matches
+	 * its (hardware) CPU ID.  The secondary core acknowledges
+	 * it has begun executing by writing an invalid value (-1)
+	 * back into secondary_holding_pen_release (in
+	 * smp_operations->smp_secondary_init).
+	 */
+ENTRY(secondary_holding_pen)
+ ARM_BE8(setend	be)
+	mrc	p15, 0, r0, c0, c0, 5	@ Get MPIDR and extract CPU id from it
+	and	r0, r0, #MPIDR_HWID_BITMASK
+	adr	r4, 1f			@ Get secondary_holding_pen_release
+	ldmia	r4, {r5, r6}		@ and compute its physical address
+	sub	r4, r4, r5
+	add	r6, r6, r4
+pen:	ldr	r7, [r6]		@ while secondary_holding_pen_release
+	cmp	r7, r0			@ doesn't hold our CPU id, spin
+	bne	pen
+	/*
+	 * At this point we have been released from the holding pen;
+	 * secondary_stack now contains the SVC stack for this core.
+	 */
+	b	secondary_startup
+ENDPROC(secondary_holding_pen)
+	.align
+1:	.long	.
+	.long	secondary_holding_pen_release
 #endif /* defined(CONFIG_SMP) */
 
 
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index b7b4c86..e18151a 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -59,6 +59,8 @@ struct secondary_data secondary_data;
  * boot "holding pen"
  */
 volatile int pen_release = -1;
+volatile u32 secondary_holding_pen_release = -1;
+static DEFINE_RAW_SPINLOCK(boot_lock);
 
 enum ipi_msg_type {
 	IPI_WAKEUP,
@@ -386,6 +388,81 @@ asmlinkage void secondary_start_kernel(void)
 	cpu_startup_entry(CPUHP_ONLINE);
 }
 
+static void write_pen_release(int val)
+{
+	secondary_holding_pen_release = val;
+	smp_wmb();
+	sync_cache_w(&secondary_holding_pen_release);
+}
+
+/*
+ * This is a smp_operations->smp_boot_secondary function, called by
+ * boot_secondary() to signal a secondary core spinning in
+ * secondary_holding_pen() that it should proceed.  The current
+ * (boot) core writes the secondary's (hardware) CPU ID into
+ * secondary_holding_pen_release.  The secondary core signals it has
+ * started running by rewriting an invalid value (-1) back
+ * into secondary_holding_pen_release.
+ */
+int smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	unsigned long timeout;
+
+	/*
+	 * The secondary core will wait for this lock after
+	 * signaling it has started.  That way we know it won't
+	 * proceed until we've recognized the acknowledgement.
+	 */
+	raw_spin_lock(&boot_lock);
+
+	/*
+	 * Release the secondary core from its holding pen by
+	 * writing its CPU ID into secondary_holding_pen_release.
+	 */
+	write_pen_release(cpu_logical_map(cpu));
+
+	/*
+	 * Send the secondary CPU a soft interrupt, thereby causing
+	 * it to jump to its secondary entry point.
+	 */
+	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+	/* Give it some time to start running. */
+	timeout = jiffies + (1 * HZ);
+	while (time_before(jiffies, timeout)) {
+		smp_rmb();
+		if (secondary_holding_pen_release == -1)
+			break;
+
+		udelay(10);
+	}
+
+	/*
+	 * We now know that the secondary core is running (or it
+	 * timed out).  Release the lock so it can proceed.
+	 */
+	raw_spin_unlock(&boot_lock);
+
+	return secondary_holding_pen_release == -1 ? 0 : -ENOSYS;
+}
+
+/*
+ * This is a smp_operations->secondary_init function, called by
+ * secondary_start_kernel() on a newly-booted secondary cpu to do
+ * some initialization activity.  All we need to do is write
+ * secondary_holding_pen_release with an invalid value to signal
+ * we've started executing.  We synchronize with the boot core by
+ * waiting to acquire the boot lock before continuing.
+ */
+void smp_secondary_init(unsigned int cpu)
+{
+	/* Let the primary processor know we're out of the pen. */
+	write_pen_release(-1);
+
+	raw_spin_lock(&boot_lock);
+	raw_spin_unlock(&boot_lock);
+}
+
 void __init smp_cpus_done(unsigned int max_cpus)
 {
 	printk(KERN_INFO "SMP: Total of %d processors activated.\n",
-- 
1.7.9.5

[RFC PATCH 3/7] ARM: realview: use central SMP spin-table routines

[Alex Elder]

This is the first of three patches involved replacing the use of smp
operations defined under arch/arm/mach-versatile.  Both realview and
vexpress use the same smp_boot_secondary and smp_secondary_init
functions, and those are equivalent to the common ones now available
in arch/arm/kernel/smp.c.

We can't get rid of the mach-versatile functions yet, so this patch
just switches mach-realview to use the common routines.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/mach-realview/platsmp.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-realview/platsmp.c b/arch/arm/mach-realview/platsmp.c
index 98e3052..b27dd07 100644
--- a/arch/arm/mach-realview/platsmp.c
+++ b/arch/arm/mach-realview/platsmp.c
@@ -21,8 +21,6 @@
 #include <mach/board-pb11mp.h>
 #include <mach/board-pbx.h>
 
-#include <plat/platsmp.h>
-
 #include "core.h"
 
 static void __iomem *scu_base_addr(void)
@@ -71,15 +69,15 @@ static void __init realview_smp_prepare_cpus(unsigned int max_cpus)
 	 * until it receives a soft interrupt, and then the
 	 * secondary CPU branches to this address.
 	 */
-	__raw_writel(virt_to_phys(versatile_secondary_startup),
+	__raw_writel(virt_to_phys(secondary_holding_pen),
 		     __io_address(REALVIEW_SYS_FLAGSSET));
 }
 
 struct smp_operations realview_smp_ops __initdata = {
 	.smp_init_cpus		= realview_smp_init_cpus,
 	.smp_prepare_cpus	= realview_smp_prepare_cpus,
-	.smp_secondary_init	= versatile_secondary_init,
-	.smp_boot_secondary	= versatile_boot_secondary,
+	.smp_secondary_init	= smp_secondary_init,
+	.smp_boot_secondary	= smp_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= realview_cpu_die,
 #endif
-- 
1.7.9.5

[RFC PATCH 4/7] ARM: vexpress: use central SMP spin-table routines

[Alex Elder]

This is the second patch involved replacing the use of smp
operations defined under arch/arm/mach-versatile.  Both realview and
vexpress use the same smp_boot_secondary and smp_secondary_init
functions, and those are equivalent to the common ones now available
in arch/arm/kernel/smp.c.

This converts mach-vexpress to use the common routines.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/mach-vexpress/platsmp.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-vexpress/platsmp.c b/arch/arm/mach-vexpress/platsmp.c
index 993c9ae..1ed4078 100644
--- a/arch/arm/mach-vexpress/platsmp.c
+++ b/arch/arm/mach-vexpress/platsmp.c
@@ -22,8 +22,6 @@
 
 #include <mach/motherboard.h>
 
-#include <plat/platsmp.h>
-
 #include "core.h"
 
 #if defined(CONFIG_OF)
@@ -193,14 +191,14 @@ static void __init vexpress_smp_prepare_cpus(unsigned int max_cpus)
 	 * until it receives a soft interrupt, and then the
 	 * secondary CPU branches to this address.
 	 */
-	vexpress_flags_set(virt_to_phys(versatile_secondary_startup));
+	vexpress_flags_set(virt_to_phys(secondary_holding_pen));
 }
 
 struct smp_operations __initdata vexpress_smp_ops = {
 	.smp_init_cpus		= vexpress_smp_init_cpus,
 	.smp_prepare_cpus	= vexpress_smp_prepare_cpus,
-	.smp_secondary_init	= versatile_secondary_init,
-	.smp_boot_secondary	= versatile_boot_secondary,
+	.smp_secondary_init	= smp_secondary_init,
+	.smp_boot_secondary	= smp_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= vexpress_cpu_die,
 #endif
-- 
1.7.9.5

[RFC PATCH 5/7] ARM: versatile: kill off SMP support code

[Alex Elder]

The two users of the SMP support code under arch/arm/mach-versatile
are now using common code.  So a bunch of stuff under mach-versatile
can just go away.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/mach-vexpress/v2m.c                   |    1 -
 arch/arm/plat-versatile/Makefile               |    1 -
 arch/arm/plat-versatile/headsmp.S              |   41 -----------
 arch/arm/plat-versatile/include/plat/platsmp.h |   14 ----
 arch/arm/plat-versatile/platsmp.c              |   91 ------------------------
 5 files changed, 148 deletions(-)
 delete mode 100644 arch/arm/plat-versatile/headsmp.S
 delete mode 100644 arch/arm/plat-versatile/include/plat/platsmp.h
 delete mode 100644 arch/arm/plat-versatile/platsmp.c

diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index 4f8b8cb..c2dc99b 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -35,7 +35,6 @@
 #include <mach/motherboard.h>
 
 #include <plat/sched_clock.h>
-#include <plat/platsmp.h>
 
 #include "core.h"
 
diff --git a/arch/arm/plat-versatile/Makefile b/arch/arm/plat-versatile/Makefile
index f88d448..d71d1b4 100644
--- a/arch/arm/plat-versatile/Makefile
+++ b/arch/arm/plat-versatile/Makefile
@@ -4,4 +4,3 @@ obj-$(CONFIG_PLAT_VERSATILE_CLOCK) += clock.o
 obj-$(CONFIG_PLAT_VERSATILE_CLCD) += clcd.o
 obj-$(CONFIG_PLAT_VERSATILE_LEDS) += leds.o
 obj-$(CONFIG_PLAT_VERSATILE_SCHED_CLOCK) += sched-clock.o
-obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/plat-versatile/headsmp.S b/arch/arm/plat-versatile/headsmp.S
deleted file mode 100644
index 40f27e5..0000000
--- a/arch/arm/plat-versatile/headsmp.S
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- *  linux/arch/arm/plat-versatile/headsmp.S
- *
- *  Copyright (c) 2003 ARM Limited
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/linkage.h>
-#include <linux/init.h>
-#include <asm/assembler.h>
-
-/*
- * Realview/Versatile Express specific entry point for secondary CPUs.
- * This provides a "holding pen" into which all secondary cores are held
- * until we're ready for them to initialise.
- */
-ENTRY(versatile_secondary_startup)
- ARM_BE8(setend	be)
-	mrc	p15, 0, r0, c0, c0, 5
-	bic	r0, #0xff000000
-	adr	r4, 1f
-	ldmia	r4, {r5, r6}
-	sub	r4, r4, r5
-	add	r6, r6, r4
-pen:	ldr	r7, [r6]
-	cmp	r7, r0
-	bne	pen
-
-	/*
-	 * we've been released from the holding pen: secondary_stack
-	 * should now contain the SVC stack for this core
-	 */
-	b	secondary_startup
-
-	.align
-1:	.long	.
-	.long	pen_release
-ENDPROC(versatile_secondary_startup)
diff --git a/arch/arm/plat-versatile/include/plat/platsmp.h b/arch/arm/plat-versatile/include/plat/platsmp.h
deleted file mode 100644
index 50fb830..0000000
--- a/arch/arm/plat-versatile/include/plat/platsmp.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- *  linux/arch/arm/plat-versatile/include/plat/platsmp.h
- *
- *  Copyright (C) 2011 ARM Ltd.
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-extern void versatile_secondary_startup(void);
-extern void versatile_secondary_init(unsigned int cpu);
-extern int  versatile_boot_secondary(unsigned int cpu, struct task_struct *idle);
diff --git a/arch/arm/plat-versatile/platsmp.c b/arch/arm/plat-versatile/platsmp.c
deleted file mode 100644
index 53feb90..0000000
--- a/arch/arm/plat-versatile/platsmp.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- *  linux/arch/arm/plat-versatile/platsmp.c
- *
- *  Copyright (C) 2002 ARM Ltd.
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/init.h>
-#include <linux/errno.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/jiffies.h>
-#include <linux/smp.h>
-
-#include <asm/cacheflush.h>
-#include <asm/smp_plat.h>
-
-/*
- * Write pen_release in a way that is guaranteed to be visible to all
- * observers, irrespective of whether they're taking part in coherency
- * or not.  This is necessary for the hotplug code to work reliably.
- */
-static void write_pen_release(int val)
-{
-	pen_release = val;
-	smp_wmb();
-	sync_cache_w(&pen_release);
-}
-
-static DEFINE_SPINLOCK(boot_lock);
-
-void versatile_secondary_init(unsigned int cpu)
-{
-	/*
-	 * let the primary processor know we're out of the
-	 * pen, then head off into the C entry point
-	 */
-	write_pen_release(-1);
-
-	/*
-	 * Synchronise with the boot thread.
-	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
-}
-
-int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
-{
-	unsigned long timeout;
-
-	/*
-	 * Set synchronisation state between this boot processor
-	 * and the secondary one
-	 */
-	spin_lock(&boot_lock);
-
-	/*
-	 * This is really belt and braces; we hold unintended secondary
-	 * CPUs in the holding pen until we're ready for them.  However,
-	 * since we haven't sent them a soft interrupt, they shouldn't
-	 * be there.
-	 */
-	write_pen_release(cpu_logical_map(cpu));
-
-	/*
-	 * Send the secondary CPU a soft interrupt, thereby causing
-	 * the boot monitor to read the system wide flags register,
-	 * and branch to the address found there.
-	 */
-	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
-
-	timeout = jiffies + (1 * HZ);
-	while (time_before(jiffies, timeout)) {
-		smp_rmb();
-		if (pen_release == -1)
-			break;
-
-		udelay(10);
-	}
-
-	/*
-	 * now the secondary core is starting up let it run its
-	 * calibrations, then wait for it to finish
-	 */
-	spin_unlock(&boot_lock);
-
-	return pen_release != -1 ? -ENOSYS : 0;
-}
-- 
1.7.9.5

[RFC PATCH 6/7] ARM: ux500: use generic SMP spin-table routines

[Alex Elder]

Get rid of the ux500-specific code that implements the "holding pen"
for secondary CPUs.  Use the code defined in "arch/arm/kernel/smp.c"
instead.

Note:
The original "holding pen" code for this machine used only the
bottom 4 bits of the MPIDR to identify the processor id.  The
common code looks at the bottom 24 bits.  The validity of this
change needs to be verified.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/mach-ux500/Makefile  |    2 +-
 arch/arm/mach-ux500/headsmp.S |   37 ---------------------
 arch/arm/mach-ux500/platsmp.c |   72 ++---------------------------------------
 3 files changed, 4 insertions(+), 107 deletions(-)
 delete mode 100644 arch/arm/mach-ux500/headsmp.S

diff --git a/arch/arm/mach-ux500/Makefile b/arch/arm/mach-ux500/Makefile
index d05ba75..0308fe1 100644
--- a/arch/arm/mach-ux500/Makefile
+++ b/arch/arm/mach-ux500/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_MACH_MOP500)	+= board-mop500-sdi.o \
 				board-mop500-regulators.o \
 				board-mop500-pins.o \
 				board-mop500-audio.o
-obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
+obj-$(CONFIG_SMP)		+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
 
 CFLAGS_hotplug.o		+= -march=armv7-a
diff --git a/arch/arm/mach-ux500/headsmp.S b/arch/arm/mach-ux500/headsmp.S
deleted file mode 100644
index 9cdea04..0000000
--- a/arch/arm/mach-ux500/headsmp.S
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- *  Copyright (c) 2009 ST-Ericsson
- *	This file is based  ARM Realview platform
- *  Copyright (c) 2003 ARM Limited
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/linkage.h>
-#include <linux/init.h>
-
-/*
- * U8500 specific entry point for secondary CPUs.
- */
-ENTRY(u8500_secondary_startup)
-	mrc	p15, 0, r0, c0, c0, 5
-	and	r0, r0, #15
-	adr	r4, 1f
-	ldmia	r4, {r5, r6}
-	sub	r4, r4, r5
-	add	r6, r6, r4
-pen:	ldr	r7, [r6]
-	cmp	r7, r0
-	bne	pen
-
-	/*
-	 * we've been released from the holding pen: secondary_stack
-	 * should now contain the SVC stack for this core
-	 */
-	b	secondary_startup
-ENDPROC(u8500_secondary_startup)
-
-	.align 2
-1:	.long	.
-	.long	pen_release
diff --git a/arch/arm/mach-ux500/platsmp.c b/arch/arm/mach-ux500/platsmp.c
index a44967f..6812aac 100644
--- a/arch/arm/mach-ux500/platsmp.c
+++ b/arch/arm/mach-ux500/platsmp.c
@@ -26,21 +26,6 @@
 #include "db8500-regs.h"
 #include "id.h"
 
-/* This is called from headsmp.S to wakeup the secondary core */
-extern void u8500_secondary_startup(void);
-
-/*
- * Write pen_release in a way that is guaranteed to be visible to all
- * observers, irrespective of whether they're taking part in coherency
- * or not.  This is necessary for the hotplug code to work reliably.
- */
-static void write_pen_release(int val)
-{
-	pen_release = val;
-	smp_wmb();
-	sync_cache_w(&pen_release);
-}
-
 static void __iomem *scu_base_addr(void)
 {
 	if (cpu_is_u8500_family() || cpu_is_ux540_family())
@@ -51,57 +36,6 @@ static void __iomem *scu_base_addr(void)
 	return NULL;
 }
 
-static DEFINE_SPINLOCK(boot_lock);
-
-static void ux500_secondary_init(unsigned int cpu)
-{
-	/*
-	 * let the primary processor know we're out of the
-	 * pen, then head off into the C entry point
-	 */
-	write_pen_release(-1);
-
-	/*
-	 * Synchronise with the boot thread.
-	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
-}
-
-static int ux500_boot_secondary(unsigned int cpu, struct task_struct *idle)
-{
-	unsigned long timeout;
-
-	/*
-	 * set synchronisation state between this boot processor
-	 * and the secondary one
-	 */
-	spin_lock(&boot_lock);
-
-	/*
-	 * The secondary processor is waiting to be released from
-	 * the holding pen - release it, then wait for it to flag
-	 * that it has been released by resetting pen_release.
-	 */
-	write_pen_release(cpu_logical_map(cpu));
-
-	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
-
-	timeout = jiffies + (1 * HZ);
-	while (time_before(jiffies, timeout)) {
-		if (pen_release == -1)
-			break;
-	}
-
-	/*
-	 * now the secondary core is starting up let it run its
-	 * calibrations, then wait for it to finish
-	 */
-	spin_unlock(&boot_lock);
-
-	return pen_release != -1 ? -ENOSYS : 0;
-}
-
 static void __init wakeup_secondary(void)
 {
 	void __iomem *backupram;
@@ -118,7 +52,7 @@ static void __init wakeup_secondary(void)
 	 * is waiting for. This would wake up the secondary core from WFE
 	 */
 #define UX500_CPU1_JUMPADDR_OFFSET 0x1FF4
-	__raw_writel(virt_to_phys(u8500_secondary_startup),
+	__raw_writel(virt_to_phys(secondary_holding_pen),
 		     backupram + UX500_CPU1_JUMPADDR_OFFSET);
 
 #define UX500_CPU1_WAKEMAGIC_OFFSET 0x1FF0
@@ -161,8 +95,8 @@ static void __init ux500_smp_prepare_cpus(unsigned int max_cpus)
 struct smp_operations ux500_smp_ops __initdata = {
 	.smp_init_cpus		= ux500_smp_init_cpus,
 	.smp_prepare_cpus	= ux500_smp_prepare_cpus,
-	.smp_secondary_init	= ux500_secondary_init,
-	.smp_boot_secondary	= ux500_boot_secondary,
+	.smp_boot_secondary	= smp_boot_secondary,
+	.smp_secondary_init	= smp_secondary_init,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= ux500_cpu_die,
 #endif
-- 
1.7.9.5

[RFC PATCH 7/7] ARM: spear: use central SMP spin-table routines

[Alex Elder]

Get rid of the spear-specific code that implements the "holding pen"
for secondary CPUs.  Use the code defined in "arch/arm/kernel/smp.c"
instead.

Note:
The original "holding pen" code for this machine used only the
bottom 4 bits of the MPIDR to identify the processor id.  The
common code looks at the bottom 24 bits.  The validity of this
change needs to be verified.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/mach-spear/Makefile  |    2 +-
 arch/arm/mach-spear/generic.h |    1 -
 arch/arm/mach-spear/headsmp.S |   47 --------------------------
 arch/arm/mach-spear/platsmp.c |   75 ++---------------------------------------
 4 files changed, 4 insertions(+), 121 deletions(-)
 delete mode 100644 arch/arm/mach-spear/headsmp.S

diff --git a/arch/arm/mach-spear/Makefile b/arch/arm/mach-spear/Makefile
index a946c19..974a417 100644
--- a/arch/arm/mach-spear/Makefile
+++ b/arch/arm/mach-spear/Makefile
@@ -7,7 +7,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include
 # Common support
 obj-y	:= restart.o time.o
 
-smp-$(CONFIG_SMP)		+= headsmp.o platsmp.o
+smp-$(CONFIG_SMP)		+= platsmp.o
 smp-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
 
 obj-$(CONFIG_ARCH_SPEAR13XX)	+= spear13xx.o $(smp-y)
diff --git a/arch/arm/mach-spear/generic.h b/arch/arm/mach-spear/generic.h
index a99d90a..86189ec 100644
--- a/arch/arm/mach-spear/generic.h
+++ b/arch/arm/mach-spear/generic.h
@@ -36,7 +36,6 @@ void __init spear13xx_l2x0_init(void);
 
 void spear_restart(enum reboot_mode, const char *);
 
-void spear13xx_secondary_startup(void);
 void spear13xx_cpu_die(unsigned int cpu);
 
 extern struct smp_operations spear13xx_smp_ops;
diff --git a/arch/arm/mach-spear/headsmp.S b/arch/arm/mach-spear/headsmp.S
deleted file mode 100644
index ed85473..0000000
--- a/arch/arm/mach-spear/headsmp.S
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * arch/arm/mach-spear13XX/headsmp.S
- *
- * Picked from realview
- * Copyright (c) 2012 ST Microelectronics Limited
- * Shiraz Hashim <shiraz.hashim@st.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/linkage.h>
-#include <linux/init.h>
-
-	__INIT
-
-/*
- * spear13xx specific entry point for secondary CPUs. This provides
- * a "holding pen" into which all secondary cores are held until we're
- * ready for them to initialise.
- */
-ENTRY(spear13xx_secondary_startup)
-	mrc	p15, 0, r0, c0, c0, 5
-	and	r0, r0, #15
-	adr	r4, 1f
-	ldmia	r4, {r5, r6}
-	sub	r4, r4, r5
-	add	r6, r6, r4
-pen:	ldr	r7, [r6]
-	cmp	r7, r0
-	bne	pen
-
-	/* re-enable coherency */
-	mrc	p15, 0, r0, c1, c0, 1
-	orr	r0, r0, #(1 << 6) | (1 << 0)
-	mcr	p15, 0, r0, c1, c0, 1
-	/*
-	 * we've been released from the holding pen: secondary_stack
-	 * should now contain the SVC stack for this core
-	 */
-	b	secondary_startup
-
-	.align
-1:	.long	.
-	.long	pen_release
-ENDPROC(spear13xx_secondary_startup)
diff --git a/arch/arm/mach-spear/platsmp.c b/arch/arm/mach-spear/platsmp.c
index e16f779..e97af7d 100644
--- a/arch/arm/mach-spear/platsmp.c
+++ b/arch/arm/mach-spear/platsmp.c
@@ -20,77 +20,8 @@
 #include <mach/spear.h>
 #include "generic.h"
 
-/*
- * Write pen_release in a way that is guaranteed to be visible to all
- * observers, irrespective of whether they're taking part in coherency
- * or not.  This is necessary for the hotplug code to work reliably.
- */
-static void write_pen_release(int val)
-{
-	pen_release = val;
-	smp_wmb();
-	sync_cache_w(&pen_release);
-}
-
-static DEFINE_SPINLOCK(boot_lock);
-
 static void __iomem *scu_base = IOMEM(VA_SCU_BASE);
 
-static void spear13xx_secondary_init(unsigned int cpu)
-{
-	/*
-	 * let the primary processor know we're out of the
-	 * pen, then head off into the C entry point
-	 */
-	write_pen_release(-1);
-
-	/*
-	 * Synchronise with the boot thread.
-	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
-}
-
-static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
-{
-	unsigned long timeout;
-
-	/*
-	 * set synchronisation state between this boot processor
-	 * and the secondary one
-	 */
-	spin_lock(&boot_lock);
-
-	/*
-	 * The secondary processor is waiting to be released from
-	 * the holding pen - release it, then wait for it to flag
-	 * that it has been released by resetting pen_release.
-	 *
-	 * Note that "pen_release" is the hardware CPU ID, whereas
-	 * "cpu" is Linux's internal ID.
-	 */
-	write_pen_release(cpu);
-
-	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
-
-	timeout = jiffies + (1 * HZ);
-	while (time_before(jiffies, timeout)) {
-		smp_rmb();
-		if (pen_release == -1)
-			break;
-
-		udelay(10);
-	}
-
-	/*
-	 * now the secondary core is starting up let it run its
-	 * calibrations, then wait for it to finish
-	 */
-	spin_unlock(&boot_lock);
-
-	return pen_release != -1 ? -ENOSYS : 0;
-}
-
 /*
  * Initialise the CPU possible map early - this describes the CPUs
  * which may be present or become present in the system.
@@ -119,14 +50,14 @@ static void __init spear13xx_smp_prepare_cpus(unsigned int max_cpus)
 	 * (presently it is in SRAM). The BootMonitor waits until it receives a
 	 * soft interrupt, and then the secondary CPU branches to this address.
 	 */
-	__raw_writel(virt_to_phys(spear13xx_secondary_startup), SYS_LOCATION);
+	__raw_writel(virt_to_phys(secondary_holding_pen), SYS_LOCATION);
 }
 
 struct smp_operations spear13xx_smp_ops __initdata = {
        .smp_init_cpus		= spear13xx_smp_init_cpus,
        .smp_prepare_cpus	= spear13xx_smp_prepare_cpus,
-       .smp_secondary_init	= spear13xx_secondary_init,
-       .smp_boot_secondary	= spear13xx_boot_secondary,
+       .smp_secondary_init	= smp_secondary_init,
+       .smp_boot_secondary	= smp_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
        .cpu_die			= spear13xx_cpu_die,
 #endif
-- 
1.7.9.5

Re: [RFC PATCH 0/7] ARM: SMP: common "pen" secondary release method

[Alex Elder]

On 03/28/2014 04:12 PM, Alex Elder wrote:
> This series proposes creating a single implementation of the "pen
> release" (or spin-table) mechanism for starting secondary CPUs on

. . .

>   	
>   ARM: spear: use central SMP spin-table routines
>     This is straightforward as well, but it depends on the
>     two prerequisite patches mentioned above (which together
>     make this code roughly identical to the rest).

Here are the patches, which I neglected to mention after all:

    http://www.spinics.net/lists/arm-kernel/msg315833.html
    http://www.spinics.net/lists/arm-kernel/msg318735.html

Re: [RFC PATCH 0/7] ARM: SMP: common "pen" secondary release method

[Russell King - ARM Linux]

On Fri, Mar 28, 2014 at 04:12:53PM -0500, Alex Elder wrote:
> This series proposes creating a single implementation of the "pen
> release" (or spin-table) mechanism for starting secondary CPUs on
> ARM SMP systems.  This mechanism is currently implemented at least 5
> times in the kernel, each essentially identical to the other.  There
> are other machines that use this method, but slight differences make
> it necessary to incorporate further work in order to accomodate them
> with a single set of functions.

As I've already said, I have strong objections against anything which
"officialises" this holding-pen approach to secondary CPUs and encourages
it's cargo cult programming with respect to it.  I've already made my
views abundantly clear on this subject many times, and they haven't
changed.  I've indicated some clear technical reasons why it should
not be encouraged.

By moving this crap it into core code, you're giving it a stronger
endorsement that it's a right way to deal with bringing CPUs online.
It isn't.  It must not be encouraged.

> This work began with integrating SMP support for a Broadcom SoC.
> The code I started was another copy of the "pen release" code.

... which just goes to show how people cargo-cult program all the
time - and don't really have a clue what the hell they're doing.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/7] ARM: SMP: generic SMP spin-table method routines

[Mark Rutland]

Hi Alex,

On Fri, Mar 28, 2014 at 09:12:55PM +0000, Alex Elder wrote:
> Implement a centralized version of the spin table (a.k.a. "holding
> pen") method of secondary CPU initialization.  This is the first
> step in removing a number of duplicate implementations of this code.
> 
> The eventual goal is to allow "enable-method" properties in device
> tree nodes for CPUs to select and use this common code.  As such,
> some of the names are selected to match the names used in the SMP
> spin-table code for ARM64.

Given that there is a fundamental difference to the spin-table protocol
in use on arm64 (in that here we are required to poke an arbitrary
interrupt controller to send an SGI rather than just issuing a SEV), I
would prefer that this had a name other than "spin-table" to
disambiguate the two protocols.

> 
> Note:
> Most implementations examine only the bottom 4 bits of the MPIDR in
> order to determine a CPU's id.  This version looks at the bottom 24
> bits instead, based on MPIDR_HWID_BITMASK.  If using only 4 bits is
> a requirement for most of the platforms that might use it I'll
> switch this use 4 bits instead.

Given that we require people to describe all of the MPIDR Aff* fields in
the DT, and can update any board files as necessary, is this a problem?

IMO Using all the Aff bits is preferable.

Cheers,
Mark.

> 
> Signed-off-by: Alex Elder <elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/include/asm/smp.h |    5 +++
>  arch/arm/kernel/head.S     |   33 +++++++++++++++++++
>  arch/arm/kernel/smp.c      |   77 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 115 insertions(+)
> 
> diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
> index 22a3b9b..83064d1 100644
> --- a/arch/arm/include/asm/smp.h
> +++ b/arch/arm/include/asm/smp.h
> @@ -75,6 +75,11 @@ struct secondary_data {
>  extern struct secondary_data secondary_data;
>  extern volatile int pen_release;
>  
> +extern volatile u32 secondary_holding_pen_release;
> +extern void secondary_holding_pen(void);
> +extern int smp_boot_secondary(unsigned int cpu, struct task_struct *idle);
> +extern void smp_secondary_init(unsigned int cpu);
> +
>  extern int __cpu_disable(void);
>  
>  extern void __cpu_die(unsigned int cpu);
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index f5f381d..3340f94 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -22,6 +22,7 @@
>  #include <asm/memory.h>
>  #include <asm/thread_info.h>
>  #include <asm/pgtable.h>
> +#include <asm/cputype.h>
>  
>  #if defined(CONFIG_DEBUG_LL) && !defined(CONFIG_DEBUG_SEMIHOSTING)
>  #include CONFIG_DEBUG_LL_INCLUDE
> @@ -402,6 +403,38 @@ __secondary_data:
>  	.long	.
>  	.long	secondary_data
>  	.long	__secondary_switched
> +
> +
> +	/*
> +	 * Secondary cores spin in this "holding pen" until they are
> +	 * signaled to proceed by jumping to secondary_startup
> +	 * (above).  A core knows to proceed when it finds that the
> +	 * value of the secondary_holding_pen_release global matches
> +	 * its (hardware) CPU ID.  The secondary core acknowledges
> +	 * it has begun executing by writing an invalid value (-1)
> +	 * back into secondary_holding_pen_release (in
> +	 * smp_operations->smp_secondary_init).
> +	 */
> +ENTRY(secondary_holding_pen)
> + ARM_BE8(setend	be)
> +	mrc	p15, 0, r0, c0, c0, 5	@ Get MPIDR and extract CPU id from it
> +	and	r0, r0, #MPIDR_HWID_BITMASK
> +	adr	r4, 1f			@ Get secondary_holding_pen_release
> +	ldmia	r4, {r5, r6}		@ and compute its physical address
> +	sub	r4, r4, r5
> +	add	r6, r6, r4
> +pen:	ldr	r7, [r6]		@ while secondary_holding_pen_release
> +	cmp	r7, r0			@ doesn't hold our CPU id, spin
> +	bne	pen
> +	/*
> +	 * At this point we have been released from the holding pen;
> +	 * secondary_stack now contains the SVC stack for this core.
> +	 */
> +	b	secondary_startup
> +ENDPROC(secondary_holding_pen)
> +	.align
> +1:	.long	.
> +	.long	secondary_holding_pen_release
>  #endif /* defined(CONFIG_SMP) */
>  
>  
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index b7b4c86..e18151a 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -59,6 +59,8 @@ struct secondary_data secondary_data;
>   * boot "holding pen"
>   */
>  volatile int pen_release = -1;
> +volatile u32 secondary_holding_pen_release = -1;
> +static DEFINE_RAW_SPINLOCK(boot_lock);
>  
>  enum ipi_msg_type {
>  	IPI_WAKEUP,
> @@ -386,6 +388,81 @@ asmlinkage void secondary_start_kernel(void)
>  	cpu_startup_entry(CPUHP_ONLINE);
>  }
>  
> +static void write_pen_release(int val)
> +{
> +	secondary_holding_pen_release = val;
> +	smp_wmb();
> +	sync_cache_w(&secondary_holding_pen_release);
> +}
> +
> +/*
> + * This is a smp_operations->smp_boot_secondary function, called by
> + * boot_secondary() to signal a secondary core spinning in
> + * secondary_holding_pen() that it should proceed.  The current
> + * (boot) core writes the secondary's (hardware) CPU ID into
> + * secondary_holding_pen_release.  The secondary core signals it has
> + * started running by rewriting an invalid value (-1) back
> + * into secondary_holding_pen_release.
> + */
> +int smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	unsigned long timeout;
> +
> +	/*
> +	 * The secondary core will wait for this lock after
> +	 * signaling it has started.  That way we know it won't
> +	 * proceed until we've recognized the acknowledgement.
> +	 */
> +	raw_spin_lock(&boot_lock);
> +
> +	/*
> +	 * Release the secondary core from its holding pen by
> +	 * writing its CPU ID into secondary_holding_pen_release.
> +	 */
> +	write_pen_release(cpu_logical_map(cpu));
> +
> +	/*
> +	 * Send the secondary CPU a soft interrupt, thereby causing
> +	 * it to jump to its secondary entry point.
> +	 */
> +	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> +
> +	/* Give it some time to start running. */
> +	timeout = jiffies + (1 * HZ);
> +	while (time_before(jiffies, timeout)) {
> +		smp_rmb();
> +		if (secondary_holding_pen_release == -1)
> +			break;
> +
> +		udelay(10);
> +	}
> +
> +	/*
> +	 * We now know that the secondary core is running (or it
> +	 * timed out).  Release the lock so it can proceed.
> +	 */
> +	raw_spin_unlock(&boot_lock);
> +
> +	return secondary_holding_pen_release == -1 ? 0 : -ENOSYS;
> +}
> +
> +/*
> + * This is a smp_operations->secondary_init function, called by
> + * secondary_start_kernel() on a newly-booted secondary cpu to do
> + * some initialization activity.  All we need to do is write
> + * secondary_holding_pen_release with an invalid value to signal
> + * we've started executing.  We synchronize with the boot core by
> + * waiting to acquire the boot lock before continuing.
> + */
> +void smp_secondary_init(unsigned int cpu)
> +{
> +	/* Let the primary processor know we're out of the pen. */
> +	write_pen_release(-1);
> +
> +	raw_spin_lock(&boot_lock);
> +	raw_spin_unlock(&boot_lock);
> +}
> +
>  void __init smp_cpus_done(unsigned int max_cpus)
>  {
>  	printk(KERN_INFO "SMP: Total of %d processors activated.\n",
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/7] ARM: SMP: generic SMP spin-table method routines

[Alex Elder]

On 03/31/2014 10:21 AM, Mark Rutland wrote:
> Hi Alex,
> 
> On Fri, Mar 28, 2014 at 09:12:55PM +0000, Alex Elder wrote:
>> Implement a centralized version of the spin table (a.k.a. "holding
>> pen") method of secondary CPU initialization.  This is the first
>> step in removing a number of duplicate implementations of this code.
>>
>> The eventual goal is to allow "enable-method" properties in device
>> tree nodes for CPUs to select and use this common code.  As such,
>> some of the names are selected to match the names used in the SMP
>> spin-table code for ARM64.

Thanks for reviewing this Mark.  I'll respond below, but given
that Russell King is not interested in incorporating my changes
into the core code I think it may be moot.  Russell believes
centralizing this code encourages people who don't have a clue
what the hell they're doing to cargo-cult program.

> Given that there is a fundamental difference to the spin-table protocol
> in use on arm64 (in that here we are required to poke an arbitrary
> interrupt controller to send an SGI rather than just issuing a SEV), I
> would prefer that this had a name other than "spin-table" to
> disambiguate the two protocols.

It's possible (but I have no way of knowing) that a SEV is
sufficient to wake up the processor as well.  It is on the
platform I'm working on.  But in any case I would happily
use a different name, maybe "holding-pen" or something.

>> Note:
>> Most implementations examine only the bottom 4 bits of the MPIDR in
>> order to determine a CPU's id.  This version looks at the bottom 24
>> bits instead, based on MPIDR_HWID_BITMASK.  If using only 4 bits is
>> a requirement for most of the platforms that might use it I'll
>> switch this use 4 bits instead.
> 
> Given that we require people to describe all of the MPIDR Aff* fields in
> the DT, and can update any board files as necessary, is this a problem?

You're right, it should not be a problem.  I'm just a little
nervous about changing *any* behavior when I don't have the
hardware available to test the result.  So I wanted to be
sure to call attention to this.

> IMO Using all the Aff bits is preferable.

I agree, absolutely.

Thanks again.

					-Alex

> Cheers,
> Mark.
> 
>>
>> Signed-off-by: Alex Elder <elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm/include/asm/smp.h |    5 +++
>>  arch/arm/kernel/head.S     |   33 +++++++++++++++++++
>>  arch/arm/kernel/smp.c      |   77 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 115 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
>> index 22a3b9b..83064d1 100644
>> --- a/arch/arm/include/asm/smp.h
>> +++ b/arch/arm/include/asm/smp.h
>> @@ -75,6 +75,11 @@ struct secondary_data {
>>  extern struct secondary_data secondary_data;
>>  extern volatile int pen_release;
>>  
>> +extern volatile u32 secondary_holding_pen_release;
>> +extern void secondary_holding_pen(void);
>> +extern int smp_boot_secondary(unsigned int cpu, struct task_struct *idle);
>> +extern void smp_secondary_init(unsigned int cpu);
>> +
>>  extern int __cpu_disable(void);
>>  
>>  extern void __cpu_die(unsigned int cpu);
>> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
>> index f5f381d..3340f94 100644
>> --- a/arch/arm/kernel/head.S
>> +++ b/arch/arm/kernel/head.S
>> @@ -22,6 +22,7 @@
>>  #include <asm/memory.h>
>>  #include <asm/thread_info.h>
>>  #include <asm/pgtable.h>
>> +#include <asm/cputype.h>
>>  
>>  #if defined(CONFIG_DEBUG_LL) && !defined(CONFIG_DEBUG_SEMIHOSTING)
>>  #include CONFIG_DEBUG_LL_INCLUDE
>> @@ -402,6 +403,38 @@ __secondary_data:
>>  	.long	.
>>  	.long	secondary_data
>>  	.long	__secondary_switched
>> +
>> +
>> +	/*
>> +	 * Secondary cores spin in this "holding pen" until they are
>> +	 * signaled to proceed by jumping to secondary_startup
>> +	 * (above).  A core knows to proceed when it finds that the
>> +	 * value of the secondary_holding_pen_release global matches
>> +	 * its (hardware) CPU ID.  The secondary core acknowledges
>> +	 * it has begun executing by writing an invalid value (-1)
>> +	 * back into secondary_holding_pen_release (in
>> +	 * smp_operations->smp_secondary_init).
>> +	 */
>> +ENTRY(secondary_holding_pen)
>> + ARM_BE8(setend	be)
>> +	mrc	p15, 0, r0, c0, c0, 5	@ Get MPIDR and extract CPU id from it
>> +	and	r0, r0, #MPIDR_HWID_BITMASK
>> +	adr	r4, 1f			@ Get secondary_holding_pen_release
>> +	ldmia	r4, {r5, r6}		@ and compute its physical address
>> +	sub	r4, r4, r5
>> +	add	r6, r6, r4
>> +pen:	ldr	r7, [r6]		@ while secondary_holding_pen_release
>> +	cmp	r7, r0			@ doesn't hold our CPU id, spin
>> +	bne	pen
>> +	/*
>> +	 * At this point we have been released from the holding pen;
>> +	 * secondary_stack now contains the SVC stack for this core.
>> +	 */
>> +	b	secondary_startup
>> +ENDPROC(secondary_holding_pen)
>> +	.align
>> +1:	.long	.
>> +	.long	secondary_holding_pen_release
>>  #endif /* defined(CONFIG_SMP) */
>>  
>>  
>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>> index b7b4c86..e18151a 100644
>> --- a/arch/arm/kernel/smp.c
>> +++ b/arch/arm/kernel/smp.c
>> @@ -59,6 +59,8 @@ struct secondary_data secondary_data;
>>   * boot "holding pen"
>>   */
>>  volatile int pen_release = -1;
>> +volatile u32 secondary_holding_pen_release = -1;
>> +static DEFINE_RAW_SPINLOCK(boot_lock);
>>  
>>  enum ipi_msg_type {
>>  	IPI_WAKEUP,
>> @@ -386,6 +388,81 @@ asmlinkage void secondary_start_kernel(void)
>>  	cpu_startup_entry(CPUHP_ONLINE);
>>  }
>>  
>> +static void write_pen_release(int val)
>> +{
>> +	secondary_holding_pen_release = val;
>> +	smp_wmb();
>> +	sync_cache_w(&secondary_holding_pen_release);
>> +}
>> +
>> +/*
>> + * This is a smp_operations->smp_boot_secondary function, called by
>> + * boot_secondary() to signal a secondary core spinning in
>> + * secondary_holding_pen() that it should proceed.  The current
>> + * (boot) core writes the secondary's (hardware) CPU ID into
>> + * secondary_holding_pen_release.  The secondary core signals it has
>> + * started running by rewriting an invalid value (-1) back
>> + * into secondary_holding_pen_release.
>> + */
>> +int smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
>> +{
>> +	unsigned long timeout;
>> +
>> +	/*
>> +	 * The secondary core will wait for this lock after
>> +	 * signaling it has started.  That way we know it won't
>> +	 * proceed until we've recognized the acknowledgement.
>> +	 */
>> +	raw_spin_lock(&boot_lock);
>> +
>> +	/*
>> +	 * Release the secondary core from its holding pen by
>> +	 * writing its CPU ID into secondary_holding_pen_release.
>> +	 */
>> +	write_pen_release(cpu_logical_map(cpu));
>> +
>> +	/*
>> +	 * Send the secondary CPU a soft interrupt, thereby causing
>> +	 * it to jump to its secondary entry point.
>> +	 */
>> +	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>> +
>> +	/* Give it some time to start running. */
>> +	timeout = jiffies + (1 * HZ);
>> +	while (time_before(jiffies, timeout)) {
>> +		smp_rmb();
>> +		if (secondary_holding_pen_release == -1)
>> +			break;
>> +
>> +		udelay(10);
>> +	}
>> +
>> +	/*
>> +	 * We now know that the secondary core is running (or it
>> +	 * timed out).  Release the lock so it can proceed.
>> +	 */
>> +	raw_spin_unlock(&boot_lock);
>> +
>> +	return secondary_holding_pen_release == -1 ? 0 : -ENOSYS;
>> +}
>> +
>> +/*
>> + * This is a smp_operations->secondary_init function, called by
>> + * secondary_start_kernel() on a newly-booted secondary cpu to do
>> + * some initialization activity.  All we need to do is write
>> + * secondary_holding_pen_release with an invalid value to signal
>> + * we've started executing.  We synchronize with the boot core by
>> + * waiting to acquire the boot lock before continuing.
>> + */
>> +void smp_secondary_init(unsigned int cpu)
>> +{
>> +	/* Let the primary processor know we're out of the pen. */
>> +	write_pen_release(-1);
>> +
>> +	raw_spin_lock(&boot_lock);
>> +	raw_spin_unlock(&boot_lock);
>> +}
>> +
>>  void __init smp_cpus_done(unsigned int max_cpus)
>>  {
>>  	printk(KERN_INFO "SMP: Total of %d processors activated.\n",
>> -- 
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

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

Re: [RFC PATCH 0/7] ARM: SMP: common "pen" secondary release method

[Alex Elder]

On 03/28/2014 04:12 PM, Alex Elder wrote:
> This series proposes creating a single implementation of the "pen
> release" (or spin-table) mechanism for starting secondary CPUs on
> ARM SMP systems.  This mechanism is currently implemented at least 5
> times in the kernel, each essentially identical to the other.  There
> are other machines that use this method, but slight differences make
> it necessary to incorporate further work in order to accomodate them
> with a single set of functions.

Given Russell King's reluctance to include this sort of
thing in common code I'm withdrawing this series from
consideration.

					-Alex

> 
> This work began with integrating SMP support for a Broadcom SoC.
> The code I started was another copy of the "pen release" code.
> Looking at how existing systems handled this, the duplication of
> code was obvious, but the fact that implementations were identical
> was less so.  In addition, the arm64 (though not 32-bit arm) code
> supports the ability to define an "enable-method" property for CPUs
> in a machine's device tree (one of which is "spin-table"), allowing
> the secondary startup mechanism to be specified abstractly.
> 
> My broader goal then was to set up the SMP support for the Broadcom
> SoC such that it could be defined in its device tree using a central
> "spin-table" enable method for secondary CPUs.  The first step was
> to create a central spin-table implementation.  I've tried to do
> this generally so it's usable by as many other platforms as
> possible.  (An alternative would be to define a custom enable method
> for this platform, but I'd prefer to avoid that if I can.)
> 
> I already have some code for the second step--which supports
> specifying the "spin-table" CPU enable method in device tree files.
> But before presenting that I wanted to post this series to get
> feedback on the approach.
> 
> I've provided a little more information for each patch below.
> 
> This code is available in the "review/common-spin-code" branch of
> the git repository here:
>     http://git.linaro.org/landing-teams/working/broadcom/kernel.git
> 
> 
> 					-Alex
> 
> Alex Elder (7):
>   ARM: allow <asm/cputype.h> inclusion from assembly
>     This patch is done specifically so the next one can use the
>     value of MPIDR_HWID_BITMASK in secondary_holding_pen(), which is
>     defined in asssembly.
>   ARM: SMP: generic SMP spin-table method routines
>     Define the common code (based on any of the others, but
>     reportedly they all derive from ARM Realview.)  Names are
>     changed to be more like arch/arm64/kernel/smp_spin_table.c.
> 
>   ARM: realview: use central SMP spin-table routines
>   ARM: vexpress: use central SMP spin-table routines
>   ARM: versatile: kill off SMP support code
>   	The SMP code for realview and vexpress both use SMP code
> 	defined under mach-versatile, so these three go together.
> 
>   ARM: ux500: use generic SMP spin-table routines
>     This one is a pretty straightforward switch to using
>     the common code.
>   	
>   ARM: spear: use central SMP spin-table routines
>     This is straightforward as well, but it depends on the
>     two prerequisite patches mentioned above (which together
>     make this code roughly identical to the rest).
> 
>  arch/arm/include/asm/cputype.h                 |    4 ++
>  arch/arm/include/asm/smp.h                     |    5 ++
>  arch/arm/kernel/head.S                         |   33 +++++++++
>  arch/arm/kernel/smp.c                          |   77 ++++++++++++++++++++
>  arch/arm/mach-realview/platsmp.c               |    8 +--
>  arch/arm/mach-spear/Makefile                   |    2 +-
>  arch/arm/mach-spear/generic.h                  |    1 -
>  arch/arm/mach-spear/headsmp.S                  |   47 ------------
>  arch/arm/mach-spear/platsmp.c                  |   75 +------------------
>  arch/arm/mach-ux500/Makefile                   |    2 +-
>  arch/arm/mach-ux500/headsmp.S                  |   37 ----------
>  arch/arm/mach-ux500/platsmp.c                  |   72 +------------------
>  arch/arm/mach-vexpress/platsmp.c               |    8 +--
>  arch/arm/mach-vexpress/v2m.c                   |    1 -
>  arch/arm/plat-versatile/Makefile               |    1 -
>  arch/arm/plat-versatile/headsmp.S              |   41 -----------
>  arch/arm/plat-versatile/include/plat/platsmp.h |   14 ----
>  arch/arm/plat-versatile/platsmp.c              |   91 ------------------------
>  18 files changed, 133 insertions(+), 386 deletions(-)
>  delete mode 100644 arch/arm/mach-spear/headsmp.S
>  delete mode 100644 arch/arm/mach-ux500/headsmp.S
>  delete mode 100644 arch/arm/plat-versatile/headsmp.S
>  delete mode 100644 arch/arm/plat-versatile/include/plat/platsmp.h
>  delete mode 100644 arch/arm/plat-versatile/platsmp.c
> 

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