big.LITTLE low-level CPU and cluster power management

Quoted 8 lines · Nicolas Pitre

arch/arm/Kconfig +0 -0

index f95ba14ae3..2271f02e8e 100644
@@ -1579,6 +1579,12 @@ config HAVE_ARM_TWD
 	help
 	  This options enables support for the ARM timer and watchdog unit
 

arch/arm/Kconfig config HAVE_ARM_TWD · line 1584

Quoted 3 lines

arch/arm/Kconfig +3 -0

+config BIG_LITTLE
+	bool "big.LITTLE support (Experimental)"
+	depends on CPU_V7 && SMP && EXPERIMENTAL

I thought EXPERIMENTAL was being phased out?

Quoted 4 lines

arch/arm/common/Makefile +0 -0

index e8a4e58f1b..50880c494f 100644

arch/arm/common/Makefile obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o · line 17

Quoted 6 lines

arch/arm/common/Makefile +2 -0

@@ -13,3 +13,6 @@ obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
 obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
+obj-$(CONFIG_FIQ_GLUE)		+= fiq_glue.o fiq_glue_setup.o
+obj-$(CONFIG_FIQ_DEBUGGER)	+= fiq_debugger.o

This looks like non-related stuff?

arch/arm/common/bL_entry.c new file +0 -0

+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

arch/arm/common/bL_head.S new file +0 -0

Hi Nico,

Quoted 2 lines · Nicolas Pitre

arch/arm/common/bL_entry.c new file +0 -0

...

Quoted 1 line

+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

IMHO, we should keep this array linear and ignore the cluster grouping at this stage. This information could be added to latter patches that actually need to know about the b.L topology.

This would also imply that we treat the MPIDR just as an ID without digging into its bit layout.

Quoted 4 lines

arch/arm/common/bL_entry.c new file +4 -0

+void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
+{
+       unsigned long val = ptr ? virt_to_phys(ptr) : 0;
+       bL_entry_vectors[cluster][cpu] = val;

arch/arm/common/bL_entry.c line 29

Quoted 4 lines

arch/arm/common/bL_entry.c new file +4 -0

+       smp_wmb();
+       __cpuc_flush_dcache_area((void *)&bL_entry_vectors[cluster][cpu], 4);
+       outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
+                         __pa(&bL_entry_vectors[cluster][cpu + 1]));

Why are you using the smp_wmb() here? We don't need any barrier since data cache ops by MVA are automatically ordered in relation to stores to the same MVA (as long as the MVA is in Normal Cacheable memory).

Quoted 2 lines

arch/arm/common/bL_head.S new file +0 -0

...

arch/arm/common/bL_head.S line 39

Quoted 7 lines

arch/arm/common/bL_head.S new file +7 -0

+ENTRY(bL_entry_point)
+
+ THUMB(        adr     r12, BSYM(1f)   )
+ THUMB(        bx      r12             )
+ THUMB(        .thumb                  )
+1:
+       mrc     p15, 0, r0, c0, c0, 5

Minor thing, maybe a comment for this line like @ MPIDR.

Quoted 9 lines · Catalin Marinas

Quoted 2 lines · Nicolas Pitre

arch/arm/common/bL_entry.c new file +0 -0

...

Quoted 1 line

+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

IMHO, we should keep this array linear and ignore the cluster grouping at this stage. This information could be added to latter patches that actually need to know about the b.L topology.

That's virtually all of them. Everything b.L related is always expressed in terms of a cpu,cluster tuple at the low level.

Quoted 2 lines

This would also imply that we treat the MPIDR just as an ID without digging into its bit layout.

That makes for too large an index space. We always end up needing to break the MPIDR into a cpu,cluster thing as the MPIDR bits are too sparse.

Quoted 17 lines · Nicolas Pitre

Quoted 9 lines · Catalin Marinas

Quoted 2 lines · Nicolas Pitre

arch/arm/common/bL_entry.c new file +0 -0

...

Quoted 1 line

+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

IMHO, we should keep this array linear and ignore the cluster grouping at this stage. This information could be added to latter patches that actually need to know about the b.L topology.

That's virtually all of them. Everything b.L related is always expressed in terms of a cpu,cluster tuple at the low level.

Quoted 2 lines

This would also imply that we treat the MPIDR just as an ID without digging into its bit layout.

Quoted 3 lines

That makes for too large an index space. We always end up needing to break the MPIDR into a cpu,cluster thing as the MPIDR bits are too sparse.

You could find a way to compress this with some mask and shifts. We can look at this later if we are to generalise this to non-b.L systems.

Quoted 17 lines · Nicolas Pitre

Quoted 9 lines · Catalin Marinas

Quoted 2 lines · Nicolas Pitre

arch/arm/common/bL_entry.c new file +0 -0

...

Quoted 1 line

+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

IMHO, we should keep this array linear and ignore the cluster grouping at this stage. This information could be added to latter patches that actually need to know about the b.L topology.

That's virtually all of them. Everything b.L related is always expressed in terms of a cpu,cluster tuple at the low level.

Quoted 2 lines

This would also imply that we treat the MPIDR just as an ID without digging into its bit layout.

Quoted 3 lines

That makes for too large an index space. We always end up needing to break the MPIDR into a cpu,cluster thing as the MPIDR bits are too sparse.

You could find a way to compress this with some mask and shifts. We can look at this later if we are to generalise this to non-b.L systems.

CPUs in a big.LITTLE systems have special needs when entering the kernel due to a hotplug event, or when resuming from a deep sleep mode.

This is vectorized so multiple CPUs can enter the kernel in parallel without serialization.

Only the basic structure is introduced here. This will be extended later.

TODO: MPIDR based indexing should eventually be made runtime adjusted.

arch/arm/common/bL_entry.c new file +10 -0

new file mode 100644
index 0000000000..80fff49417
@@ -0,0 +1,30 @@
+/*
+ * arch/arm/common/bL_entry.c -- big.LITTLE kernel re-entry point
+ *
+ * Created by:  Nicolas Pitre, March 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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.
+ */

arch/arm/common/bL_entry.c new file +10 -0

+
+void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
+{
+	unsigned long val = ptr ? virt_to_phys(ptr) : 0;
+	bL_entry_vectors[cluster][cpu] = val;
+	smp_wmb();
+	__cpuc_flush_dcache_area((void *)&bL_entry_vectors[cluster][cpu], 4);
+	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
+			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
+}

arch/arm/common/bL_head.S new file +40 -0

new file mode 100644
index 0000000000..9d351f2b4c
@@ -0,0 +1,81 @@
+/*
+ * arch/arm/common/bL_head.S -- big.LITTLE kernel re-entry point
+ *
+ * Created by:  Nicolas Pitre, March 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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 <asm/bL_entry.h>
+
+	.macro	pr_dbg	cpu, string
+#if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
+	b	1901f
+1902:	.ascii	"CPU 0: \0CPU 1: \0CPU 2: \0CPU 3: \0"
+	.ascii	"CPU 4: \0CPU 5: \0CPU 6: \0CPU 7: \0"
+1903:	.asciz	"\string"
+	.align
+1901:	adr	r0, 1902b
+	add	r0, r0, \cpu, lsl #3
+	bl	printascii
+	adr	r0, 1903b
+	bl	printascii
+#endif
+	.endm
+
+	.arm
+	.align
+
+ENTRY(bL_entry_point)
+
+ THUMB(	adr	r12, BSYM(1f)	)
+ THUMB(	bx	r12		)
+ THUMB(	.thumb			)
+1:
+	mrc	p15, 0, r0, c0, c0, 5
+	ubfx	r9, r0, #0, #4			@ r9 = cpu

arch/arm/common/bL_head.S new file +21 -0

+
+2:	pr_dbg	r4, "kernel bL_entry_point\n"
+
+	/*
+	 * MMU is off so we need to get to bL_entry_vectors in a
+	 * position independent way.
+	 */
+	adr	r5, 3f
+	ldr	r6, [r5]
+	add	r6, r5, r6			@ r6 = bL_entry_vectors
+
+bL_entry_gated:
+	ldr	r5, [r6, r4, lsl #2]		@ r5 = CPU entry vector
+	cmp	r5, #0
+	wfeeq
+	beq	bL_entry_gated
+	pr_dbg	r4, "released\n"
+	bx	r5
+
+	.align	2
+

arch/arm/include/asm/bL_entry.h new file +6 -0

new file mode 100644
index 0000000000..ff623333a1
@@ -0,0 +1,35 @@
+/*
+ * arch/arm/include/asm/bL_entry.h
+ *
+ * Created by:  Nicolas Pitre, April 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *

Quoted 10 lines · Nicolas Pitre

CPUs in a big.LITTLE systems have special needs when entering the kernel due to a hotplug event, or when resuming from a deep sleep mode.

This is vectorized so multiple CPUs can enter the kernel in parallel without serialization.

Only the basic structure is introduced here. This will be extended later.

TODO: MPIDR based indexing should eventually be made runtime adjusted.

Agreed.

Quoted 22 lines

arch/arm/common/bL_entry.c new file +16 -0

new file mode 100644
index 0000000000..80fff49417
@@ -0,0 +1,30 @@
+/*
+ * arch/arm/common/bL_entry.c -- big.LITTLE kernel re-entry point
+ *
+ * Created by:  Nicolas Pitre, March 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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/kernel.h>
+#include <linux/init.h>
+
+#include <asm/bL_entry.h>
+#include <asm/barrier.h>

arch/arm/common/bL_entry.c line 20

Quoted 4 lines

arch/arm/common/bL_entry.c new file +4 -0

+#include <asm/proc-fns.h>
+#include <asm/cacheflush.h>
+
+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

Does this actually need to be volatile? I'd have thought a compiler barrier in place of the smp_wmb below would be enough (following on from Catalin's comments).

Quoted 62 lines

arch/arm/common/bL_entry.c new file +9 -0

+void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
+{
+	unsigned long val = ptr ? virt_to_phys(ptr) : 0;
+	bL_entry_vectors[cluster][cpu] = val;
+	smp_wmb();
+	__cpuc_flush_dcache_area((void *)&bL_entry_vectors[cluster][cpu], 4);
+	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
+			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
+}

arch/arm/common/bL_head.S new file +47 -0

new file mode 100644
index 0000000000..9d351f2b4c
@@ -0,0 +1,81 @@
+/*
+ * arch/arm/common/bL_head.S -- big.LITTLE kernel re-entry point
+ *
+ * Created by:  Nicolas Pitre, March 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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 <asm/bL_entry.h>
+
+	.macro	pr_dbg	cpu, string
+#if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
+	b	1901f
+1902:	.ascii	"CPU 0: \0CPU 1: \0CPU 2: \0CPU 3: \0"
+	.ascii	"CPU 4: \0CPU 5: \0CPU 6: \0CPU 7: \0"
+1903:	.asciz	"\string"
+	.align
+1901:	adr	r0, 1902b
+	add	r0, r0, \cpu, lsl #3
+	bl	printascii
+	adr	r0, 1903b
+	bl	printascii
+#endif
+	.endm
+
+	.arm
+	.align
+
+ENTRY(bL_entry_point)
+
+ THUMB(	adr	r12, BSYM(1f)	)
+ THUMB(	bx	r12		)
+ THUMB(	.thumb			)
+1:
+	mrc	p15, 0, r0, c0, c0, 5
+	ubfx	r9, r0, #0, #4			@ r9 = cpu
+	ubfx	r10, r0, #8, #4			@ r10 = cluster
+	mov	r3, #BL_CPUS_PER_CLUSTER
+	mla	r4, r3, r10, r9			@ r4 = canonical CPU index
+	cmp	r4, #(BL_CPUS_PER_CLUSTER * BL_NR_CLUSTERS)
+	blo	2f
+
+	/* We didn't expect this CPU.  Try to make it quiet. */

arch/arm/common/bL_head.S line 50

Quoted 3 lines

arch/arm/common/bL_head.S new file +3 -0

+1:	wfi
+	wfe
+	b	1b

I realise this CPU is stuck at this point, but you should have a dsb before a wfi instruction. This could be problematic with the CCI this early, so maybe just a comment saying that it doesn't matter because we don't care about this core?

Quoted 25 lines

arch/arm/common/bL_head.S new file +25 -0

+
+2:	pr_dbg	r4, "kernel bL_entry_point\n"
+
+	/*
+	 * MMU is off so we need to get to bL_entry_vectors in a
+	 * position independent way.
+	 */
+	adr	r5, 3f
+	ldr	r6, [r5]
+	add	r6, r5, r6			@ r6 = bL_entry_vectors
+
+bL_entry_gated:
+	ldr	r5, [r6, r4, lsl #2]		@ r5 = CPU entry vector
+	cmp	r5, #0
+	wfeeq
+	beq	bL_entry_gated
+	pr_dbg	r4, "released\n"
+	bx	r5
+
+	.align	2
+
+3:	.word	bL_entry_vectors - .
+
+ENDPROC(bL_entry_point)
+

arch/arm/common/bL_head.S line 81

Quoted 6 lines

arch/arm/common/bL_head.S new file +6 -0

+	.bss
+	.align	5
+
+	.type	bL_entry_vectors, #object
+ENTRY(bL_entry_vectors)
+	.space	4 * BL_NR_CLUSTERS * BL_CPUS_PER_CLUSTER

Is there a particular reason to put this in the bss?

Quoted 18 lines

arch/arm/include/asm/bL_entry.h new file +12 -0

new file mode 100644
index 0000000000..ff623333a1
@@ -0,0 +1,35 @@
+/*
+ * arch/arm/include/asm/bL_entry.h
+ *
+ * Created by:  Nicolas Pitre, April 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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.
+ */
+
+#ifndef BL_ENTRY_H

arch/arm/include/asm/bL_entry.h line 16

Quoted 4 lines

arch/arm/include/asm/bL_entry.h new file +4 -0

+#define BL_ENTRY_H
+
+#define BL_CPUS_PER_CLUSTER	4
+#define BL_NR_CLUSTERS		2

Hmm, I see these have to be constant so you can allocate your space in the assembly file. In which case, I think it's worth changing their names to have MAX or LIMIT in them...

maybe they could even be CONFIG options?

Quoted 7 lines · Will Deacon

Review

Quoted 2 lines · Nicolas Pitre

(quoted diff) +2 -0

+
+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

Does this actually need to be volatile? I'd have thought a compiler barrier in place of the smp_wmb below would be enough (following on from Catalin's comments).

Actually, I did the reverse i.e. I removed the smp_wmb() entirely. A compiler barrier forces the whole world to memory while here we only want this particular assignment to be pushed out.

Furthermore, I like the volatile as it flags that this is a special variable which in this case is also accessed from CPUs with no cache.

Quoted 9 lines

arch/arm/common/bL_head.S line 50

Quoted 4 lines

arch/arm/common/bL_head.S new file +4 -0

+	/* We didn't expect this CPU.  Try to make it quiet. */
+1:	wfi
+	wfe
+	b	1b

I realise this CPU is stuck at this point, but you should have a dsb before a wfi instruction. This could be problematic with the CCI this early, so maybe just a comment saying that it doesn't matter because we don't care about this core?

Why a dsb? No data was even touched at this point. And since this is meant to be a better "b ." kind of loop, I'd rather not try to make it more sophisticated than it already is. And of course it is meant to never be executed in practice.

Quoted 18 lines

Quoted 18 lines

arch/arm/include/asm/bL_entry.h new file +12 -0

new file mode 100644
index 0000000000..ff623333a1
@@ -0,0 +1,35 @@
+/*
+ * arch/arm/include/asm/bL_entry.h
+ *
+ * Created by:  Nicolas Pitre, April 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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.
+ */
+
+#ifndef BL_ENTRY_H

Quoted 8 lines

arch/arm/include/asm/bL_entry.h line 16

Quoted 4 lines

arch/arm/include/asm/bL_entry.h new file +4 -0

+#define BL_ENTRY_H
+
+#define BL_CPUS_PER_CLUSTER	4
+#define BL_NR_CLUSTERS		2

Hmm, I see these have to be constant so you can allocate your space in the assembly file. In which case, I think it's worth changing their names to have MAX or LIMIT in them...

Yes, good point. I'll change them.

Quoted 1 line

maybe they could even be CONFIG options?

Nah. I prefer not adding new config options unless this is really necessary or useful. For the forseeable future, we'll see systems with at most 2 clusters and at most 4 CPUs per cluster. That could easily be revisited later if that becomes unsuitable for some new systems.

Initially I wanted all those things to be runtime sized in relation with the TODO item in the commit log. That too can come later.

Quoted 9 lines · Nicolas Pitre

Quoted 7 lines · Will Deacon

Review

Quoted 2 lines · Nicolas Pitre

(quoted diff) +2 -0

+
+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

Does this actually need to be volatile? I'd have thought a compiler barrier in place of the smp_wmb below would be enough (following on from Catalin's comments).

Quoted 6 lines

Actually, I did the reverse i.e. I removed the smp_wmb() entirely. A compiler barrier forces the whole world to memory while here we only want this particular assignment to be pushed out.

Furthermore, I like the volatile as it flags that this is a special variable which in this case is also accessed from CPUs with no cache.

Ok, fair enough. Given that the smp_wmb isn't needed that sounds better.

Quoted 10 lines

Quoted 9 lines

arch/arm/common/bL_head.S line 50

Quoted 4 lines

arch/arm/common/bL_head.S new file +4 -0

+	/* We didn't expect this CPU.  Try to make it quiet. */
+1:	wfi
+	wfe
+	b	1b

I realise this CPU is stuck at this point, but you should have a dsb before a wfi instruction. This could be problematic with the CCI this early, so maybe just a comment saying that it doesn't matter because we don't care about this core?

Quoted 4 lines

Why a dsb? No data was even touched at this point. And since this is meant to be a better "b ." kind of loop, I'd rather not try to make it more sophisticated than it already is. And of course it is meant to never be executed in practice.

Sure, that's why I think just mentioning that we don't ever plan to boot this CPU is a good idea (so people don't add code here later on).

Quoted 27 lines

Quoted 18 lines

Quoted 18 lines

arch/arm/include/asm/bL_entry.h new file +12 -0

new file mode 100644
index 0000000000..ff623333a1
@@ -0,0 +1,35 @@
+/*
+ * arch/arm/include/asm/bL_entry.h
+ *
+ * Created by:  Nicolas Pitre, April 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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.
+ */
+
+#ifndef BL_ENTRY_H

Quoted 8 lines

arch/arm/include/asm/bL_entry.h line 16

Quoted 4 lines

arch/arm/include/asm/bL_entry.h new file +4 -0

+#define BL_ENTRY_H
+
+#define BL_CPUS_PER_CLUSTER	4
+#define BL_NR_CLUSTERS		2

Hmm, I see these have to be constant so you can allocate your space in the assembly file. In which case, I think it's worth changing their names to have MAX or LIMIT in them...

Quoted 1 line

Yes, good point. I'll change them.

Thanks.

Quoted 6 lines

Quoted 1 line

maybe they could even be CONFIG options?

Nah. I prefer not adding new config options unless this is really necessary or useful. For the forseeable future, we'll see systems with at most 2 clusters and at most 4 CPUs per cluster. That could easily be revisited later if that becomes unsuitable for some new systems.

The current GIC is limited to 8 CPUs, so 4x2 is also a realistic possibility.

Quoted 2 lines

Initially I wanted all those things to be runtime sized in relation with the TODO item in the commit log. That too can come later.

Out of interest: how would you achieve that? I also thought about getting this information from the device tree, but I can't see how to plug that in with static storage.

CPUs in a big.LITTLE systems have special needs when entering the kernel due to a hotplug event, or when resuming from a deep sleep mode.

This is vectorized so multiple CPUs can enter the kernel in parallel without serialization.

Only the basic structure is introduced here. This will be extended later.

TODO: MPIDR based indexing should eventually be made runtime adjusted.

Signed-off-by Nicolas Pitre <[email protected]>

---

arch/arm/common/Makefile +3 -0

index e8a4e58f1b..50880c494f 100644
@@ -13,3 +13,6 @@ obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
  obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
  obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
  obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
+obj-$(CONFIG_FIQ_GLUE)		+= fiq_glue.o fiq_glue_setup.o
+obj-$(CONFIG_FIQ_DEBUGGER)	+= fiq_debugger.o
+obj-$(CONFIG_BIG_LITTLE)	+= bL_head.o bL_entry.o

arch/arm/common/bL_entry.c new file +0 -0

new file mode 100644
index 0000000000..80fff49417

arch/arm/common/bL_entry.c new file +15 -0

+ *
+ * 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/kernel.h>
+#include <linux/init.h>
+
+#include <asm/bL_entry.h>
+#include <asm/barrier.h>
+#include <asm/proc-fns.h>
+#include <asm/cacheflush.h>
+
+extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

arch/arm/common/bL_head.S new file +39 -0

new file mode 100644
index 0000000000..9d351f2b4c
@@ -0,0 +1,81 @@
+/*
+ * arch/arm/common/bL_head.S -- big.LITTLE kernel re-entry point
+ *
+ * Created by:  Nicolas Pitre, March 2012
+ * Copyright:   (C) 2012  Linaro Limited
+ *
+ * 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 <asm/bL_entry.h>
+
+	.macro	pr_dbg	cpu, string
+#if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
+	b	1901f
+1902:	.ascii	"CPU 0: \0CPU 1: \0CPU 2: \0CPU 3: \0"
+	.ascii	"CPU 4: \0CPU 5: \0CPU 6: \0CPU 7: \0"
+1903:	.asciz	"\string"
+	.align
+1901:	adr	r0, 1902b
+	add	r0, r0, \cpu, lsl #3
+	bl	printascii
+	adr	r0, 1903b
+	bl	printascii
+#endif
+	.endm
+
+	.arm
+	.align
+
+ENTRY(bL_entry_point)
+
+ THUMB(	adr	r12, BSYM(1f)	)
+ THUMB(	bx	r12		)
+ THUMB(	.thumb			)
+1:
+	mrc	p15, 0, r0, c0, c0, 5

Quoted 12 lines · Nicolas Pitre

arch/arm/common/bL_head.S new file +12 -0

+ENTRY(bL_entry_point)
+
+ THUMB(	adr	r12, BSYM(1f)	)
+ THUMB(	bx	r12		)
+ THUMB(	.thumb			)
+1:
+	mrc	p15, 0, r0, c0, c0, 5
+	ubfx	r9, r0, #0, #4			@ r9 = cpu
+	ubfx	r10, r0, #8, #4			@ r10 = cluster
+	mov	r3, #BL_CPUS_PER_CLUSTER
+	mla	r4, r3, r10, r9			@ r4 = canonical CPU index
+	cmp	r4, #(BL_CPUS_PER_CLUSTER * BL_NR_CLUSTERS)

arch/arm/common/bL_head.S line 49

Quoted 5 lines

arch/arm/common/bL_head.S new file +5 -0

+	blo	2f
+
+	/* We didn't expect this CPU.  Try to make it quiet. */
+1:	wfi
+	wfe

Why do you need a wfe followed by wif ?
Just curious.

Quoted 13 lines · Santosh Shilimkar

Quoted 12 lines · Nicolas Pitre

arch/arm/common/bL_head.S new file +12 -0

+ENTRY(bL_entry_point)
+
+ THUMB(	adr	r12, BSYM(1f)	)
+ THUMB(	bx	r12		)
+ THUMB(	.thumb			)
+1:
+	mrc	p15, 0, r0, c0, c0, 5
+	ubfx	r9, r0, #0, #4			@ r9 = cpu
+	ubfx	r10, r0, #8, #4			@ r10 = cluster
+	mov	r3, #BL_CPUS_PER_CLUSTER
+	mla	r4, r3, r10, r9			@ r4 = canonical CPU index
+	cmp	r4, #(BL_CPUS_PER_CLUSTER * BL_NR_CLUSTERS)

Quoted 8 lines

arch/arm/common/bL_head.S line 49

Quoted 5 lines

arch/arm/common/bL_head.S new file +5 -0

+	blo	2f
+
+	/* We didn't expect this CPU.  Try to make it quiet. */
+1:	wfi
+	wfe

Why do you need a wfe followed by wif ?
Just curious.

If the WFI doesn't work because an interrupt is pending then the WFE might work better. But as I mentioned before, this is not intended to be used for other purposes than "we're really screwed so at least let's try to cheaply quieten this CPU" case.

Hi!

Quoted 6 lines

arch/arm/Kconfig +0 -0

@@ -1579,6 +1579,12 @@ config HAVE_ARM_TWD
 	help
 	  This options enables support for the ARM timer and watchdog unit
 

arch/arm/Kconfig config HAVE_ARM_TWD · line 1587

Quoted 6 lines

arch/arm/Kconfig +6 -0

+config BIG_LITTLE
+	bool "big.LITTLE support (Experimental)"
+	depends on CPU_V7 && SMP && EXPERIMENTAL
+	help
+	  This option enables support for the big.LITTLE architecture.
+

Perhaps few lines of what big.LITTLE is would be nice here?

It is that "few high-performance cores + few low-power cores" on chip, right?

BTW... is it possible/good idea to run all the cores at the same time for max performance? From descriptions I understood that is not normally done and I'd like to understand why....

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/common/bL_entry.c      | 88 +++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/bL_entry.h | 92 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +7 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/irqflags.h>
 
 #include <asm/bL_entry.h>
 #include <asm/barrier.h>
 #include <asm/proc-fns.h>
 #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
 extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
 

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
 	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
 			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
 }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct bL_platform_power_ops *ops)
+{

arch/arm/common/bL_entry.c +60 -0

+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/* Very similar to bL_cpu_power_down() */
+	setup_mm_for_reboot();
+	platform_ops->suspend(expected_residency);
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+	BUG();
+}
+
+int bL_cpu_powered_up(void)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	if (platform_ops->powered_up)
+		platform_ops->powered_up();
+	return 0;
+}

arch/arm/include/asm/bL_entry.h +72 -0

index ff623333a1..942d7f9f19 100644
@@ -31,5 +31,97 @@ extern void bL_entry_point(void);
  */
 void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr);
 
+/*
+ * CPU/cluster power operations API for higher subsystems to use.
+ */
+
+/**
+ * bL_cpu_power_up - make given CPU in given cluster runable
+ *
+ * @cpu: CPU number within given cluster
+ * @cluster: cluster number for the CPU
+ *
+ * The identified CPU is brought out of reset.  If the cluster was powered
+ * down then it is brought up as well, taking care not to let the other CPUs
+ * in the cluster run, and ensuring appropriate cluster setup.
+ *
+ * Caller must ensure the appropriate entry vector is initialized with
+ * bL_set_entry_vector() prior to calling this.
+ *
+ * This must be called in a sleepable context.  However, the implementation
+ * is strongly encouraged to return early and let the operation happen
+ * asynchronously, especially when significant delays are expected.
+ *
+ * If the operation cannot be performed then an error code is returned.
+ */
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster);
+
+/**
+ * bL_cpu_power_down - power the calling CPU down
+ *
+ * The calling CPU is powered down.
+ *
+ * If this CPU is found to be the "last man standing" in the cluster
+ * then the cluster is prepared for power-down too.
+ *
+ * This must be called with interrupts disabled.
+ *
+ * This does not return.  Re-entry in the kernel is expected via
+ * bL_entry_point.
+ */
+void bL_cpu_power_down(void);
+
+/**
+ * bL_cpu_suspend - bring the calling CPU in a suspended state
+ *
+ * @expected_residency: duration in microseconds the CPU is expected
+ *			to remain suspended, or 0 if unknown/infinity.
+ *
+ * The calling CPU is suspended.  The expected residency argument is used
+ * as a hint by the platform specific backend to implement the appropriate
+ * sleep state level according to the knowledge it has on wake-up latency
+ * for the given hardware.
+ *
+ * If this CPU is found to be the "last man standing" in the cluster
+ * then the cluster may be prepared for power-down too, if the expected
+ * residency makes it worthwhile.
+ *
+ * This must be called with interrupts disabled.
+ *
+ * This does not return.  Re-entry in the kernel is expected via
+ * bL_entry_point.
+ */
+void bL_cpu_suspend(u64 expected_residency);
+
+/**
+ * bL_cpu_powered_up - housekeeping workafter a CPU has been powered up
+ *
+ * This lets the platform specific backend code perform needed housekeeping
+ * work.  This must be called by the newly activated CPU as soon as it is
+ * fully operational in kernel space, before it enables interrupts.
+ *
+ * If the operation cannot be performed then an error code is returned.
+ */
+int bL_cpu_powered_up(void);

Quoted 42 lines · Nicolas Pitre

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/common/bL_entry.c      | 88 +++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/bL_entry.h | 92 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +11 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/irqflags.h>
 
 #include <asm/bL_entry.h>
 #include <asm/barrier.h>
 #include <asm/proc-fns.h>
 #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
 extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
 

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
 	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
 			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
 }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct bL_platform_power_ops *ops)
+{
+	if (platform_ops)
+		return -EBUSY;
+	platform_ops = ops;
+	return 0;

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 47

Quoted 6 lines

arch/arm/common/bL_entry.c +6 -0

+}
+
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;

Is this the right error code?

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 52

Quoted 5 lines

arch/arm/common/bL_entry.c +5 -0

+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);

Maybe it's worth putting this typedef in a header file somewhere. It's also used by the soft reboot code.

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 58

Quoted 6 lines

arch/arm/common/bL_entry.c +6 -0

+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);

Seems a bit overkill, or are we unrecoverable by this point?

Quoted 146 lines

arch/arm/common/bL_entry.c +60 -0

+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/* Very similar to bL_cpu_power_down() */
+	setup_mm_for_reboot();
+	platform_ops->suspend(expected_residency);
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+	BUG();
+}
+
+int bL_cpu_powered_up(void)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	if (platform_ops->powered_up)
+		platform_ops->powered_up();
+	return 0;
+}

arch/arm/include/asm/bL_entry.h +77 -0

index ff623333a1..942d7f9f19 100644
@@ -31,5 +31,97 @@ extern void bL_entry_point(void);
  */
 void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr);
 
+/*
+ * CPU/cluster power operations API for higher subsystems to use.
+ */
+
+/**
+ * bL_cpu_power_up - make given CPU in given cluster runable
+ *
+ * @cpu: CPU number within given cluster
+ * @cluster: cluster number for the CPU
+ *
+ * The identified CPU is brought out of reset.  If the cluster was powered
+ * down then it is brought up as well, taking care not to let the other CPUs
+ * in the cluster run, and ensuring appropriate cluster setup.
+ *
+ * Caller must ensure the appropriate entry vector is initialized with
+ * bL_set_entry_vector() prior to calling this.
+ *
+ * This must be called in a sleepable context.  However, the implementation
+ * is strongly encouraged to return early and let the operation happen
+ * asynchronously, especially when significant delays are expected.
+ *
+ * If the operation cannot be performed then an error code is returned.
+ */
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster);
+
+/**
+ * bL_cpu_power_down - power the calling CPU down
+ *
+ * The calling CPU is powered down.
+ *
+ * If this CPU is found to be the "last man standing" in the cluster
+ * then the cluster is prepared for power-down too.
+ *
+ * This must be called with interrupts disabled.
+ *
+ * This does not return.  Re-entry in the kernel is expected via
+ * bL_entry_point.
+ */
+void bL_cpu_power_down(void);
+
+/**
+ * bL_cpu_suspend - bring the calling CPU in a suspended state
+ *
+ * @expected_residency: duration in microseconds the CPU is expected
+ *			to remain suspended, or 0 if unknown/infinity.
+ *
+ * The calling CPU is suspended.  The expected residency argument is used
+ * as a hint by the platform specific backend to implement the appropriate
+ * sleep state level according to the knowledge it has on wake-up latency
+ * for the given hardware.
+ *
+ * If this CPU is found to be the "last man standing" in the cluster
+ * then the cluster may be prepared for power-down too, if the expected
+ * residency makes it worthwhile.
+ *
+ * This must be called with interrupts disabled.
+ *
+ * This does not return.  Re-entry in the kernel is expected via
+ * bL_entry_point.
+ */
+void bL_cpu_suspend(u64 expected_residency);
+
+/**
+ * bL_cpu_powered_up - housekeeping workafter a CPU has been powered up
+ *
+ * This lets the platform specific backend code perform needed housekeeping
+ * work.  This must be called by the newly activated CPU as soon as it is
+ * fully operational in kernel space, before it enables interrupts.
+ *
+ * If the operation cannot be performed then an error code is returned.
+ */
+int bL_cpu_powered_up(void);
+
+/*
+ * Platform specific methods used in the implementation of the above API.
+ */
+struct bL_platform_power_ops {

arch/arm/include/asm/bL_entry.h extern void bL_entry_point(void); · line 115

Quoted 5 lines

arch/arm/include/asm/bL_entry.h +5 -0

+	int (*power_up)(unsigned int cpu, unsigned int cluster);
+	void (*power_down)(void);
+	void (*suspend)(u64);
+	void (*powered_up)(void);
+};

It would be good if these prototypes matched the PSCI code, then platforms could just glue them together directly.

Quoted 7 lines · Will Deacon

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 47

Quoted 4 lines · Nicolas Pitre

arch/arm/common/bL_entry.c +4 -0

+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;

Is this the right error code?

It is as good as any other, with some meaning to be distinguished from the traditional ones like -ENOMEM or -EINVAL that the platform backends could return.

Would you prefer another one?

Quoted 8 lines

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 52

Quoted 5 lines

arch/arm/common/bL_entry.c +5 -0

+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);

Maybe it's worth putting this typedef in a header file somewhere. It's also used by the soft reboot code.

Agreed. Maybe separately from this series though.

Quoted 8 lines

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 58

Quoted 6 lines

arch/arm/common/bL_entry.c +6 -0

+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);

Seems a bit overkill, or are we unrecoverable by this point?

We are. The upper layer expects this CPU to be dead and there is no easy recovery possible. This is a "should never happen" condition, and the kernel is badly configured otherwise.

Quoted 4 lines

Quoted 4 lines

arch/arm/include/asm/bL_entry.h +4 -0

+/*
+ * Platform specific methods used in the implementation of the above API.
+ */
+struct bL_platform_power_ops {

Quoted 8 lines

arch/arm/include/asm/bL_entry.h extern void bL_entry_point(void); · line 115

Quoted 5 lines

arch/arm/include/asm/bL_entry.h +5 -0

+	int (*power_up)(unsigned int cpu, unsigned int cluster);
+	void (*power_down)(void);
+	void (*suspend)(u64);
+	void (*powered_up)(void);
+};

It would be good if these prototypes matched the PSCI code, then platforms could just glue them together directly.

No.

I discussed this at length with Charles (the PSCI spec author) already. Even in the PSCI case, a minimum PSCI backend is necessary to do some impedance matching between what the PSCI calls expect as arguments and what this kernel specific API needs to express. For example, the UP method needs to always be provided with the address for bL_entry, irrespective of where the user of this kernel API wants execution to be resumed. There might be some cases where the backend might decide to override the desired power saving state because of other kernel induced constraints (ongoing DMA operation for example) that PSCI doesn't (and should not) know about. And the best place to arbitrate between those platform specific constraints is in this platform specific shim or backend.

Quoted 43 lines · Will Deacon

Quoted 42 lines · Nicolas Pitre

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/common/bL_entry.c      | 88 +++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/bL_entry.h | 92 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +11 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/irqflags.h>
 
 #include <asm/bL_entry.h>
 #include <asm/barrier.h>
 #include <asm/proc-fns.h>
 #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
 extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
 

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
 	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
 			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
 }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct bL_platform_power_ops *ops)
+{
+	if (platform_ops)
+		return -EBUSY;
+	platform_ops = ops;
+	return 0;

Quoted 8 lines

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 47

Quoted 6 lines

arch/arm/common/bL_entry.c +6 -0

+}
+
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;

Is this the right error code?

It is as good as any other, with some meaning to be distinguished from the traditional ones like -ENOMEM or -EINVAL that the platform backends could return.

Would you prefer another one?

Quoted 8 lines

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 52

Quoted 5 lines

arch/arm/common/bL_entry.c +5 -0

+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);

Maybe it's worth putting this typedef in a header file somewhere. It's also used by the soft reboot code.

Agreed. Maybe separately from this series though.

Quoted 8 lines

arch/arm/common/bL_entry.c void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr) · line 58

Quoted 6 lines

arch/arm/common/bL_entry.c +6 -0

+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);

Seems a bit overkill, or are we unrecoverable by this point?

We are. The upper layer expects this CPU to be dead and there is no easy recovery possible. This is a "should never happen" condition, and the kernel is badly configured otherwise.

Quoted 147 lines

Quoted 147 lines

arch/arm/common/bL_entry.c +60 -0

+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/* Very similar to bL_cpu_power_down() */
+	setup_mm_for_reboot();
+	platform_ops->suspend(expected_residency);
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+	BUG();
+}
+
+int bL_cpu_powered_up(void)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	if (platform_ops->powered_up)
+		platform_ops->powered_up();
+	return 0;
+}

arch/arm/include/asm/bL_entry.h +77 -0

index ff623333a1..942d7f9f19 100644
@@ -31,5 +31,97 @@ extern void bL_entry_point(void);
  */
 void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr);
 
+/*
+ * CPU/cluster power operations API for higher subsystems to use.
+ */
+
+/**
+ * bL_cpu_power_up - make given CPU in given cluster runable
+ *
+ * @cpu: CPU number within given cluster
+ * @cluster: cluster number for the CPU
+ *
+ * The identified CPU is brought out of reset.  If the cluster was powered
+ * down then it is brought up as well, taking care not to let the other CPUs
+ * in the cluster run, and ensuring appropriate cluster setup.
+ *
+ * Caller must ensure the appropriate entry vector is initialized with
+ * bL_set_entry_vector() prior to calling this.
+ *
+ * This must be called in a sleepable context.  However, the implementation
+ * is strongly encouraged to return early and let the operation happen
+ * asynchronously, especially when significant delays are expected.
+ *
+ * If the operation cannot be performed then an error code is returned.
+ */
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster);
+
+/**
+ * bL_cpu_power_down - power the calling CPU down
+ *
+ * The calling CPU is powered down.
+ *
+ * If this CPU is found to be the "last man standing" in the cluster
+ * then the cluster is prepared for power-down too.
+ *
+ * This must be called with interrupts disabled.
+ *
+ * This does not return.  Re-entry in the kernel is expected via
+ * bL_entry_point.
+ */
+void bL_cpu_power_down(void);
+
+/**
+ * bL_cpu_suspend - bring the calling CPU in a suspended state
+ *
+ * @expected_residency: duration in microseconds the CPU is expected
+ *			to remain suspended, or 0 if unknown/infinity.
+ *
+ * The calling CPU is suspended.  The expected residency argument is used
+ * as a hint by the platform specific backend to implement the appropriate
+ * sleep state level according to the knowledge it has on wake-up latency
+ * for the given hardware.
+ *
+ * If this CPU is found to be the "last man standing" in the cluster
+ * then the cluster may be prepared for power-down too, if the expected
+ * residency makes it worthwhile.
+ *
+ * This must be called with interrupts disabled.
+ *
+ * This does not return.  Re-entry in the kernel is expected via
+ * bL_entry_point.
+ */
+void bL_cpu_suspend(u64 expected_residency);
+
+/**
+ * bL_cpu_powered_up - housekeeping workafter a CPU has been powered up
+ *
+ * This lets the platform specific backend code perform needed housekeeping
+ * work.  This must be called by the newly activated CPU as soon as it is
+ * fully operational in kernel space, before it enables interrupts.
+ *
+ * If the operation cannot be performed then an error code is returned.
+ */
+int bL_cpu_powered_up(void);
+
+/*
+ * Platform specific methods used in the implementation of the above API.
+ */
+struct bL_platform_power_ops {

Quoted 8 lines

arch/arm/include/asm/bL_entry.h extern void bL_entry_point(void); · line 115

Quoted 5 lines

arch/arm/include/asm/bL_entry.h +5 -0

+	int (*power_up)(unsigned int cpu, unsigned int cluster);
+	void (*power_down)(void);
+	void (*suspend)(u64);
+	void (*powered_up)(void);
+};

It would be good if these prototypes matched the PSCI code, then platforms could just glue them together directly.

No.

I discussed this at length with Charles (the PSCI spec author) already. Even in the PSCI case, a minimum PSCI backend is necessary to do some impedance matching between what the PSCI calls expect as arguments and what this kernel specific API needs to express. For example, the UP method needs to always be provided with the address for bL_entry, irrespective of where the user of this kernel API wants execution to be resumed. There might be some cases where the backend might decide to override the desired power saving state because of other kernel induced constraints (ongoing DMA operation for example) that PSCI doesn't (and should not) know about. And the best place to arbitrate between those platform specific constraints is in this platform specific shim or backend.

Because of that, and because one feature of Linux is to not have stable APIs in the kernel so to be free to adapt them to future needs, I think it is best not to even try matching the PSCI interface here.

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/common/bL_entry.c      | 88 +++++++++++++++++++++++++++++++++++++++
  arch/arm/include/asm/bL_entry.h | 92 +++++++++++++++++++++++++++++++++++++++++
  2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +68 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
  #include <linux/kernel.h>
  #include <linux/init.h>
+#include <linux/irqflags.h>
 
  #include <asm/bL_entry.h>
  #include <asm/barrier.h>
  #include <asm/proc-fns.h>
  #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
  extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
 

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
  	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
  			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
  }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct bL_platform_power_ops *ops)
+{
+	if (platform_ops)
+		return -EBUSY;
+	platform_ops = ops;
+	return 0;
+}
+
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);
+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;

Quoted 112 lines · Nicolas Pitre

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/common/bL_entry.c      | 88
+++++++++++++++++++++++++++++++++++++++
  arch/arm/include/asm/bL_entry.h | 92
+++++++++++++++++++++++++++++++++++++++++
  2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +6 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
  #include <linux/kernel.h>
  #include <linux/init.h>
+#include <linux/irqflags.h>
 
  #include <asm/bL_entry.h>
  #include <asm/barrier.h>
  #include <asm/proc-fns.h>
  #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
  extern volatile unsigned long
 bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster,
 void *ptr)
  	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
  			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
  }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct bL_platform_power_ops

*ops)

arch/arm/common/bL_entry.c +70 -0

+{
+	if (platform_ops)
+		return -EBUSY;
+	platform_ops = ops;
+	return 0;
+}
+
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);
+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/* Very similar to bL_cpu_power_down() */
+	setup_mm_for_reboot();
+	platform_ops->suspend(expected_residency);
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);

arch/arm/common/bL_entry.c

Quoted 3 lines

arch/arm/common/bL_entry.c +2 -0

+	phys_reset(virt_to_phys(bL_entry_point));
+	BUG();

I might be missing all the rationales behind not having a recovery for CPUs entering suspend if they actualy come here because of some events. This is pretty much possible in many scenario's and hence letting CPU cpu come out of suspend should be possible. May be switcher code don't have such requirement but it appeared bit off to me.

Quoted 113 lines · Santosh Shilimkar

Quoted 112 lines · Nicolas Pitre

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
   arch/arm/common/bL_entry.c      | 88
+++++++++++++++++++++++++++++++++++++++
   arch/arm/include/asm/bL_entry.h | 92
+++++++++++++++++++++++++++++++++++++++++
   2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +6 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
   #include <linux/kernel.h>
   #include <linux/init.h>
+#include <linux/irqflags.h>
 
   #include <asm/bL_entry.h>
   #include <asm/barrier.h>
   #include <asm/proc-fns.h>
   #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
   extern volatile unsigned long
 bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster,
 void *ptr)
   	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
   			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
   }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct bL_platform_power_ops

*ops)

arch/arm/common/bL_entry.c +70 -0

+{
+	if (platform_ops)
+		return -EBUSY;
+	platform_ops = ops;
+	return 0;
+}
+
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);
+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/* Very similar to bL_cpu_power_down() */
+	setup_mm_for_reboot();
+	platform_ops->suspend(expected_residency);
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);

Quoted 8 lines

arch/arm/common/bL_entry.c

Quoted 3 lines

arch/arm/common/bL_entry.c +2 -0

+	phys_reset(virt_to_phys(bL_entry_point));
+	BUG();

I might be missing all the rationales behind not having a recovery for CPUs entering suspend if they actualy come here because of some events. This is pretty much possible in many scenario's and hence letting CPU cpu come out of suspend should be possible. May be switcher code don't have such requirement but it appeared bit off to me.

There are two things to consider here:

1) The CPU is suspended. CPU state is lost. Next interrupt to wake up the CPU will make it restart from the reset vector and re-entry in the kernel will happen via bL_entry_point to deal with the various cluster issues, to eventually resume kernel code via cpu_resume. Obviously, the machine specific backend code would have set the bL_entry_point address in its machine specific reset vector in advance.

2) An interrupt comes along before the CPU is effectively suspended, say right before the backend code executes a WFI to shut the CPU down. The CPU and possibly cluster state was already set for being powered off. We cannot simply return at this point as caches are off, the CPU is not coherent with the rest of the system anymore, etc. So if the platform specific backend ever returns, say because the final WFI exited, then we have to go through the same arbitration process to restore the CPU and cluster state as if that was a hard reset. Hence the cpu_reset call to loop back into bL_entry_point.

In either cases, we simply cannot ever return from bL_cpu_suspend() directly. Of course, the caller is expected to have used bL_set_entry_vector() beforehand, most probably with cpu_resume as argument.

Quoted 130 lines · Nicolas Pitre

Quoted 117 lines · Santosh Shilimkar

Quoted 116 lines · Nicolas Pitre

This is the basic API used to handle the powering up/down of individual CPUs in a big.LITTLE system. The platform specific backend implementation
has the responsibility to also handle the cluster level power as well when
the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---
   arch/arm/common/bL_entry.c      | 88
+++++++++++++++++++++++++++++++++++++++
   arch/arm/include/asm/bL_entry.h | 92
+++++++++++++++++++++++++++++++++++++++++
   2 files changed, 180 insertions(+)

arch/arm/common/bL_entry.c +6 -0

index 80fff49417..41de0622de 100644
@@ -11,11 +11,13 @@
 
   #include <linux/kernel.h>
   #include <linux/init.h>
+#include <linux/irqflags.h>
 
   #include <asm/bL_entry.h>
   #include <asm/barrier.h>
   #include <asm/proc-fns.h>
   #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 
   extern volatile unsigned long
 bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];

@@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned
 cluster,
 void *ptr)
   	outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
   			  __pa(&bL_entry_vectors[cluster][cpu + 1]));
   }
+
+static const struct bL_platform_power_ops *platform_ops;
+
+int __init bL_platform_power_register(const struct

bL_platform_power_ops
*ops)

arch/arm/common/bL_entry.c +70 -0

+{
+	if (platform_ops)
+		return -EBUSY;
+	platform_ops = ops;
+	return 0;
+}
+
+int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
+{
+	if (!platform_ops)
+		return -EUNATCH;
+	might_sleep();
+	return platform_ops->power_up(cpu, cluster);
+}
+
+typedef void (*phys_reset_t)(unsigned long);
+
+void bL_cpu_power_down(void)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/*
+	 * Do this before calling into the power_down method,
+	 * as it might not always be safe to do afterwards.
+	 */
+	setup_mm_for_reboot();
+
+	platform_ops->power_down();
+
+	/*
+	 * It is possible for a power_up request to happen concurrently
+	 * with a power_down request for the same CPU. In this case the
+	 * power_down method might not be able to actually enter a
+	 * powered down state with the WFI instruction if the power_up
+	 * method has removed the required reset condition.  The
+	 * power_down method is then allowed to return. We must perform
+	 * a re-entry in the kernel as if the power_up method just had
+	 * deasserted reset on the CPU.
+	 *
+	 * To simplify race issues, the platform specific implementation
+	 * must accommodate for the possibility of unordered calls to
+	 * power_down and power_up with a usage count. Therefore, if a
+	 * call to power_up is issued for a CPU that is not down, then
+	 * the next call to power_down must not attempt a full shutdown
+	 * but only do the minimum (normally disabling L1 cache and CPU
+	 * coherency) and return just as if a concurrent power_up request
+	 * had happened as described above.
+	 */
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(bL_entry_point));
+
+	/* should never get here */
+	BUG();
+}
+
+void bL_cpu_suspend(u64 expected_residency)
+{
+	phys_reset_t phys_reset;
+
+	BUG_ON(!platform_ops);
+	BUG_ON(!irqs_disabled());
+
+	/* Very similar to bL_cpu_power_down() */
+	setup_mm_for_reboot();
+	platform_ops->suspend(expected_residency);
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);

Quoted 8 lines

arch/arm/common/bL_entry.c

Quoted 3 lines

arch/arm/common/bL_entry.c +2 -0

+	phys_reset(virt_to_phys(bL_entry_point));
+	BUG();

I might be missing all the rationales behind not having a recovery for CPUs entering suspend if they actualy come here because of some events. This is pretty much possible in many scenario's and hence letting CPU cpu come out of suspend should be possible. May be switcher code don't have such requirement but it appeared bit off to me.

There are two things to consider here:

Quoted 7 lines

1) The CPU is suspended. CPU state is lost. Next interrupt to wake up the CPU will make it restart from the reset vector and re-entry in the kernel will happen via bL_entry_point to deal with the various cluster issues, to eventually resume kernel code via cpu_resume. Obviously, the machine specific backend code would have set the bL_entry_point address in its machine specific reset vector in advance.

This is the successful case and in that case you will anyway not hit the BUG.

Quoted 11 lines

2) An interrupt comes along before the CPU is effectively suspended, say right before the backend code executes a WFI to shut the CPU down. The CPU and possibly cluster state was already set for being powered off. We cannot simply return at this point as caches are off, the CPU is not coherent with the rest of the system anymore, etc. So if the platform specific backend ever returns, say because the final WFI exited, then we have to go through the same arbitration process to restore the CPU and cluster state as if that was a hard reset. Hence the cpu_reset call to loop back into bL_entry_point.

This is the one I was thinking. Enabling C bit and SMP bit should be enough for CPU to get back to right state since the CPU has not lost the context all the registers including SP is intact and CPU should be able to resume.

Quoted 5 lines

In either cases, we simply cannot ever return from bL_cpu_suspend() directly. Of course, the caller is expected to have used bL_set_entry_vector() beforehand, most probably with cpu_resume as argument.

The above might get complicated if when above situation happens on last CPU where even CCI gets disabled and then adding the rever code for all of that may not be worth. You approach is much safer.

Thanks for explaining it further.

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

For use in the power_down() implementation:
* __bL_cpu_going_down(unsigned int cluster, unsigned int cpu)
* __bL_outbound_enter_critical(unsigned int cluster)
* __bL_outbound_leave_critical(unsigned int cluster)
* __bL_cpu_down(unsigned int cluster, unsigned int cpu)

The power_up_setup() helper should do platform-specific setup in preparation for turning the CPU on, such as invalidating local caches or entering coherency. It must be assembler for now, since it must run before the MMU can be switched on. It is passed the affinity level which should be initialized.

Because the bL_cluster_sync_struct content is looked-up and modified with the cache enabled or disabled depending on the code path, it is crucial to always ensure proper cache maintenance to update main memory right away. Therefore, any cached write must be followed by a cache clean operation and any cached read must be preceded by a cache invalidate operation on the accessed memory.

To avoid races where a reader would invalidate the cache and discard the latest update from a writer before that writer had a chance to clean it to RAM, we simply use cache flush (clean+invalidate) operations everywhere.

Also, in order to prevent a cached writer from interfering with an adjacent non-cached writer, we ensure each state variable is located to a separate cache line.

Thanks to Nicolas Pitre and Achin Gupta for the help with this patch.

Signed-off-by Dave Martin <[email protected]>

---
 .../arm/big.LITTLE/cluster-pm-race-avoidance.txt   | 498 +++++++++++++++++++++
 arch/arm/common/bL_entry.c                         | 160 +++++++
 arch/arm/common/bL_head.S                          |  88 +++-
 arch/arm/include/asm/bL_entry.h                    |  62 +++
 4 files changed, 806 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt

arch/arm/common/bL_entry.c +0 -0

+int __init bL_cluster_sync_init(void (*power_up_setup)(void))

Quoted 1 line

+int __init bL_cluster_sync_init(void (*power_up_setup)(void))

The addition of the affinity level parameter for power_up_setup means that this prototype is not correct.

This is not a functional change, since that function is only called from assembler anyway, but it will help avoid confusion.

Quoted 4 lines · Dave Martin

Quoted 1 line

+int __init bL_cluster_sync_init(void (*power_up_setup)(void))

The addition of the affinity level parameter for power_up_setup means that this prototype is not correct.

Indeed.

Quoted 2 lines

This is not a functional change, since that function is only called from assembler anyway, but it will help avoid confusion.

Fixed now, as well as the DCSCB usage.

Nicolas

arch/arm/common/bL_entry.c +0 -0

@@ -116,3 +116,163 @@ int bL_cpu_powered_up(void)
                platform_ops->powered_up();
        return 0;
 }

arch/arm/common/bL_entry.c +15 -0

+#define sync_mem(ptr) __sync_range(ptr, sizeof *(ptr))
+
+/*
+ * __bL_cpu_going_down: Indicates that the cpu is being torn down.
+ *    This must be called at the point of committing to teardown of a CPU.
+ *    The CPU cache (SCTRL.C bit) is expected to still be active.
+ */
+void __bL_cpu_going_down(unsigned int cpu, unsigned int cluster)
+{
+       bL_sync.clusters[cluster].cpus[cpu].cpu = CPU_GOING_DOWN;
+       sync_mem(&bL_sync.clusters[cluster].cpus[cpu].cpu);
+}
+
+/*
+ * __bL_cpu_down: Indicates that cpu teardown is complete and that the

Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt line 2

Quoted 5 lines · Nicolas Pitre

Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt new file +2 -0

@@ -0,0 +1,498 @@
+Big.LITTLE cluster Power-up/power-down race avoidance algorithm
+===============================================================

Nice description and ascii art :).

Quoted 6 lines

arch/arm/common/bL_entry.c +0 -0

@@ -116,3 +116,163 @@ int bL_cpu_powered_up(void)
                platform_ops->powered_up();
        return 0;
 }

arch/arm/common/bL_entry.c int bL_cpu_powered_up(void) · line 128

Quoted 10 lines

arch/arm/common/bL_entry.c +10 -0

+
+struct bL_sync_struct bL_sync;
+
+static void __sync_range(volatile void *p, size_t size)
+{
+       char *_p = (char *)p;
+
+       __cpuc_flush_dcache_area(_p, size);
+       outer_flush_range(__pa(_p), __pa(_p + size));
+       outer_sync();

The outer flush-range operations already contain a cache_sync, so an additional outer_sync() operation is not necessary.

You (well, Dave) said that you use the flush instead of clean/invalidate to avoid races with other CPUs writing the location.

However, on the same CPU you can get a speculative load into L1 after the L1 flush but before the L2 flush, so the reader case can fail.

The sequence for readers is (note *L2* inval first):

L2 inval
L1 inval

The sequence for writers is:

L1 clean
L2 clean

The bi-directional sequence (that's what you need) is:

L1 clean
L2 clean+inval
L1 clean+inval

The last L1 op must be clean+inval in case another CPU writes to this location to avoid discarding the write.

If you don't have an L2, you just end up with two L1 clean ops, so you can probably put some checks.

Quoted 20 lines

arch/arm/common/bL_entry.c +20 -0

+#define sync_mem(ptr) __sync_range(ptr, sizeof *(ptr))
+
+/*
+ * __bL_cpu_going_down: Indicates that the cpu is being torn down.
+ *    This must be called at the point of committing to teardown of a CPU.
+ *    The CPU cache (SCTRL.C bit) is expected to still be active.
+ */
+void __bL_cpu_going_down(unsigned int cpu, unsigned int cluster)
+{
+       bL_sync.clusters[cluster].cpus[cpu].cpu = CPU_GOING_DOWN;
+       sync_mem(&bL_sync.clusters[cluster].cpus[cpu].cpu);
+}
+
+/*
+ * __bL_cpu_down: Indicates that cpu teardown is complete and that the
+ *    cluster can be torn down without disrupting this CPU.
+ *    To avoid deadlocks, this must be called before a CPU is powered down.
+ *    The CPU cache (SCTRL.C bit) is expected to be off.
+ */
+void __bL_cpu_down(unsigned int cpu, unsigned int cluster)

arch/arm/common/bL_entry.c int bL_cpu_powered_up(void) · line 155

Quoted 5 lines

arch/arm/common/bL_entry.c +5 -0

+{
+       dsb();
+       bL_sync.clusters[cluster].cpus[cpu].cpu = CPU_DOWN;
+       sync_mem(&bL_sync.clusters[cluster].cpus[cpu].cpu);
+       sev();

For the sev() here (and other functions in this patch) you need a dsb() before. I'm not sure outer_sync() has one.

Quoted 17 lines · Catalin Marinas

Quoted 16 lines · Nicolas Pitre

arch/arm/common/bL_entry.c +10 -0

@@ -116,3 +116,163 @@ int bL_cpu_powered_up(void)
                platform_ops->powered_up();
        return 0;
 }
+
+struct bL_sync_struct bL_sync;
+
+static void __sync_range(volatile void *p, size_t size)
+{
+       char *_p = (char *)p;
+
+       __cpuc_flush_dcache_area(_p, size);
+       outer_flush_range(__pa(_p), __pa(_p + size));
+       outer_sync();

Quoted 7 lines

However, on the same CPU you can get a speculative load into L1 after the L1 flush but before the L2 flush, so the reader case can fail.

The sequence for readers is (note *L2* inval first):

L2 inval
L1 inval

As you noted below and as I explained above, this can't be an inval operation as that could discard a concurrent writer's update.

Quoted 14 lines

The sequence for writers is:

L1 clean
L2 clean

The bi-directional sequence (that's what you need) is:

L1 clean
L2 clean+inval
L1 clean+inval

The last L1 op must be clean+inval in case another CPU writes to this location to avoid discarding the write.

Quoted 2 lines

If you don't have an L2, you just end up with two L1 clean ops, so you can probably put some checks.

In fact, since this is only used on A7/A15 right now, there is no outer cache and the outer calls are effectively no-ops. I'm wondering if those should simply be removed until/unless there is some system showing up with a need for them.

Quoted 19 lines · Nicolas Pitre

Quoted 17 lines · Catalin Marinas

Quoted 16 lines · Nicolas Pitre

arch/arm/common/bL_entry.c +10 -0

@@ -116,3 +116,163 @@ int bL_cpu_powered_up(void)
                platform_ops->powered_up();
        return 0;
 }
+
+struct bL_sync_struct bL_sync;
+
+static void __sync_range(volatile void *p, size_t size)
+{
+       char *_p = (char *)p;
+
+       __cpuc_flush_dcache_area(_p, size);
+       outer_flush_range(__pa(_p), __pa(_p + size));
+       outer_sync();

...

Quoted 25 lines

Quoted 7 lines

However, on the same CPU you can get a speculative load into L1 after the L1 flush but before the L2 flush, so the reader case can fail.

The sequence for readers is (note *L2* inval first):

L2 inval
L1 inval

As you noted below and as I explained above, this can't be an inval operation as that could discard a concurrent writer's update.

Quoted 14 lines

The sequence for writers is:

L1 clean
L2 clean

The bi-directional sequence (that's what you need) is:

L1 clean
L2 clean+inval
L1 clean+inval

The last L1 op must be clean+inval in case another CPU writes to this location to avoid discarding the write.

Quoted 7 lines

Quoted 2 lines

If you don't have an L2, you just end up with two L1 clean ops, so you can probably put some checks.

In fact, since this is only used on A7/A15 right now, there is no outer cache and the outer calls are effectively no-ops. I'm wondering if those should simply be removed until/unless there is some system showing up with a need for them.

You could. I expect multi-cluster systems to have integrated L2 cache and avoid explicit outer cache maintenance. But is there a chance that your patches could be generalised to existing systems with A9 (not b.L configuration but just hotplug or cpuidle support)? I haven't finished reading all the patches, so maybe that's not the case at all.

Anyway, my point is that if L1 is inner and L2 outer, the correct bi-derectional flushing sequence is slightly different.

Quoted 34 lines · Catalin Marinas

Quoted 19 lines · Nicolas Pitre

Quoted 17 lines · Catalin Marinas

Quoted 16 lines · Nicolas Pitre

arch/arm/common/bL_entry.c +10 -0

@@ -116,3 +116,163 @@ int bL_cpu_powered_up(void)
                platform_ops->powered_up();
        return 0;
 }
+
+struct bL_sync_struct bL_sync;
+
+static void __sync_range(volatile void *p, size_t size)
+{
+       char *_p = (char *)p;
+
+       __cpuc_flush_dcache_area(_p, size);
+       outer_flush_range(__pa(_p), __pa(_p + size));
+       outer_sync();

...

Quoted 13 lines

Quoted 7 lines

However, on the same CPU you can get a speculative load into L1 after the L1 flush but before the L2 flush, so the reader case can fail.

The sequence for readers is (note *L2* inval first):

L2 inval
L1 inval

As you noted below and as I explained above, this can't be an inval operation as that could discard a concurrent writer's update.

Quoted 2 lines

The sequence for writers is:

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

L1 clean
L2 clean

The bi-directional sequence (that's what you need) is:

L1 clean
L2 clean+inval
L1 clean+inval

Quoted 12 lines

Quoted 11 lines

Quoted 6 lines

The last L1 op must be clean+inval in case another CPU writes to this location to avoid discarding the write.

If you don't have an L2, you just end up with two L1 clean ops, so you can probably put some checks.

In fact, since this is only used on A7/A15 right now, there is no outer cache and the outer calls are effectively no-ops. I'm wondering if those should simply be removed until/unless there is some system showing up with a need for them.

Quoted 5 lines

You could. I expect multi-cluster systems to have integrated L2 cache and avoid explicit outer cache maintenance. But is there a chance that your patches could be generalised to existing systems with A9 (not b.L configuration but just hotplug or cpuidle support)? I haven't finished reading all the patches, so maybe that's not the case at all.

I suppose it could, although the special requirements put on the first man / last man exist only for multi-cluster systems. OTOH, existing A9 systems are already served by far less complex code already, so it is really a matter of figuring out if the backend for those A9 systems needed by this cluster code would be simpler than the existing code, in which case that would certainly be beneficial.

Quoted 2 lines

Anyway, my point is that if L1 is inner and L2 outer, the correct bi-derectional flushing sequence is slightly different.

Agreed, I'll make sure to capture that in the code somehow.

Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt line 2

Quoted 5 lines · Nicolas Pitre

Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt new file +2 -0

@@ -0,0 +1,498 @@
+Big.LITTLE cluster Power-up/power-down race avoidance algorithm
+===============================================================

Nice description and ascii art :).

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

arch/arm/common/bL_entry.c +8 -0

+int __init bL_cluster_sync_init(void (*power_up_setup)(void))
+{
+       unsigned int i, j, mpidr, this_cluster;
+
+       BUILD_BUG_ON(BL_SYNC_CLUSTER_SIZE * BL_NR_CLUSTERS != sizeof bL_sync);
+       BUG_ON((unsigned long)&bL_sync & (__CACHE_WRITEBACK_GRANULE - 1));
+
+       /*

+ this_cluster = (mpidr >> 8) & 0xf;

arch/arm/common/bL_entry.c +12 -0

+       for_each_online_cpu(i)
+               bL_sync.clusters[this_cluster].cpus[i].cpu = CPU_UP;
+       bL_sync.clusters[this_cluster].cluster = CLUSTER_UP;
+       sync_mem(&bL_sync);
+
+       if (power_up_setup) {
+               bL_power_up_setup_phys = virt_to_phys(power_up_setup);
+               sync_mem(&bL_power_up_setup_phys);
+       }
+
+       return 0;
+}

arch/arm/common/bL_head.S +13 -2

index 9d351f2b4c..f7a64ac127 100644
@@ -7,11 +7,19 @@
  * 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.
+ *
+ *
+ * Refer to Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
+ * for details of the synchronisation algorithms used here.
  */
 
 #include <linux/linkage.h>
 #include <asm/bL_entry.h>
 
+.if BL_SYNC_CLUSTER_CPUS
+.error "cpus must be the first member of struct bL_cluster_sync_struct"
+.endif
+
        .macro  pr_dbg  cpu, string
 #if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
        b       1901f

@@ -52,12 +60,82 @@ ENTRY(bL_entry_point)
 2:     pr_dbg  r4, "kernel bL_entry_point\n"
 
        /*
-        * MMU is off so we need to get to bL_entry_vectors in a
+        * MMU is off so we need to get to various variables in a
         * position independent way.
         */
        adr     r5, 3f
-       ldr     r6, [r5]
+       ldmia   r5, {r6, r7, r8}
        add     r6, r5, r6                      @ r6 = bL_entry_vectors
+       ldr     r7, [r5, r7]                    @ r7 = bL_power_up_setup_phys
+       add     r8, r5, r8                      @ r8 = bL_sync
+

arch/arm/common/bL_head.S +63 -0

+
+       @ At this point, the cluster cannot unexpectedly enter the GOING_DOWN
+       @ state, because there is at least one active CPU (this CPU).
+
+       @ Check if the cluster has been set up yet:
+       ldrb    r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
+       cmp     r0, #CLUSTER_UP
+       beq     cluster_already_up
+
+       @ Signal that the cluster is being brought up:
+       mov     r0, #INBOUND_COMING_UP
+       strb    r0, [r8, #BL_SYNC_CLUSTER_INBOUND]
+
+       dsb
+
+       @ Any CPU trying to take the cluster into CLUSTER_GOING_DOWN from this
+       @ point onwards will observe INBOUND_COMING_UP and abort.
+
+       @ Wait for any previously-pending cluster teardown operations to abort
+       @ or complete:
+cluster_teardown_wait:
+       ldrb    r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
+       cmp     r0, #CLUSTER_GOING_DOWN
+       wfeeq
+       beq     cluster_teardown_wait
+
+       @ power_up_setup is responsible for setting up the cluster:
+
+       cmp     r7, #0
+       mov     r0, #1          @ second (cluster) affinity level
+       blxne   r7              @ Call power_up_setup if defined
+
+       @ Leave the cluster setup critical section:
+
+       dsb
+       mov     r0, #INBOUND_NOT_COMING_UP
+       strb    r0, [r8, #BL_SYNC_CLUSTER_INBOUND]
+       mov     r0, #CLUSTER_UP
+       strb    r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
+       dsb
+       sev
+
+cluster_already_up:
+       @ If a platform-specific CPU setup hook is needed, it is
+       @ called from here.
+
+       cmp     r7, #0
+       mov     r0, #0          @ first (CPU) affinity level
+       blxne   r7              @ Call power_up_setup if defined
+
+       @ Mark the CPU as up:
+
+       dsb
+       mov     r0, #CPU_UP
+       strb    r0, [r5]
+       dsb
+       sev
 
 bL_entry_gated:
        ldr     r5, [r6, r4, lsl #2]            @ r5 = CPU entry vector

@@ -70,6 +148,8 @@ bL_entry_gated:
        .align  2
 
 3:     .word   bL_entry_vectors - .
+       .word   bL_power_up_setup_phys - 3b
+       .word   bL_sync - 3b
 
 ENDPROC(bL_entry_point)
 

@@ -79,3 +159,7 @@ ENDPROC(bL_entry_point)
        .type   bL_entry_vectors, #object
 ENTRY(bL_entry_vectors)
        .space  4 * BL_NR_CLUSTERS * BL_CPUS_PER_CLUSTER
+
+       .type   bL_power_up_setup_phys, #object
+ENTRY(bL_power_up_setup_phys)
+       .space  4               @ set by bL_cluster_sync_init()

arch/arm/include/asm/bL_entry.h +2 -0

index 942d7f9f19..167394d9a0 100644
@@ -15,8 +15,37 @@
 #define BL_CPUS_PER_CLUSTER    4
 #define BL_NR_CLUSTERS         2
 
+/* Definitions for bL_cluster_sync_struct */
+#define CPU_DOWN               0x11

Quoted 6 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

[...]

Quoted 8 lines

arch/arm/common/bL_entry.c +8 -0

+int __init bL_cluster_sync_init(void (*power_up_setup)(void))
+{
+       unsigned int i, j, mpidr, this_cluster;
+
+       BUILD_BUG_ON(BL_SYNC_CLUSTER_SIZE * BL_NR_CLUSTERS != sizeof bL_sync);
+       BUG_ON((unsigned long)&bL_sync & (__CACHE_WRITEBACK_GRANULE - 1));
+
+       /*

arch/arm/common/bL_entry.c int bL_cpu_powered_up(void) · line 265

Quoted 10 lines

arch/arm/common/bL_entry.c +10 -0

+        * Set initial CPU and cluster states.
+        * Only one cluster is assumed to be active at this point.
+        */
+       for (i = 0; i < BL_NR_CLUSTERS; i++) {
+               bL_sync.clusters[i].cluster = CLUSTER_DOWN;
+               bL_sync.clusters[i].inbound = INBOUND_NOT_COMING_UP;
+               for (j = 0; j < BL_CPUS_PER_CLUSTER; j++)
+                       bL_sync.clusters[i].cpus[j].cpu = CPU_DOWN;
+       }
+       asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));

We have a helper for this...

Quoted 1 line

+ this_cluster = (mpidr >> 8) & 0xf;

... and also this, thanks to Lorenzo's recent patches.

Quoted 57 lines

arch/arm/common/bL_entry.c +12 -0

+       for_each_online_cpu(i)
+               bL_sync.clusters[this_cluster].cpus[i].cpu = CPU_UP;
+       bL_sync.clusters[this_cluster].cluster = CLUSTER_UP;
+       sync_mem(&bL_sync);
+
+       if (power_up_setup) {
+               bL_power_up_setup_phys = virt_to_phys(power_up_setup);
+               sync_mem(&bL_power_up_setup_phys);
+       }
+
+       return 0;
+}

arch/arm/common/bL_head.S +19 -2

index 9d351f2b4c..f7a64ac127 100644
@@ -7,11 +7,19 @@
  * 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.
+ *
+ *
+ * Refer to Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
+ * for details of the synchronisation algorithms used here.
  */
 
 #include <linux/linkage.h>
 #include <asm/bL_entry.h>
 
+.if BL_SYNC_CLUSTER_CPUS
+.error "cpus must be the first member of struct bL_cluster_sync_struct"
+.endif
+
        .macro  pr_dbg  cpu, string
 #if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
        b       1901f

@@ -52,12 +60,82 @@ ENTRY(bL_entry_point)
 2:     pr_dbg  r4, "kernel bL_entry_point\n"
 
        /*
-        * MMU is off so we need to get to bL_entry_vectors in a
+        * MMU is off so we need to get to various variables in a
         * position independent way.
         */
        adr     r5, 3f
-       ldr     r6, [r5]
+       ldmia   r5, {r6, r7, r8}
        add     r6, r5, r6                      @ r6 = bL_entry_vectors
+       ldr     r7, [r5, r7]                    @ r7 = bL_power_up_setup_phys
+       add     r8, r5, r8                      @ r8 = bL_sync
+
+       mov     r0, #BL_SYNC_CLUSTER_SIZE
+       mla     r8, r0, r10, r8                 @ r8 = bL_sync cluster base
+
+       @ Signal that this CPU is coming UP:
+       mov     r0, #CPU_COMING_UP
+       mov     r5, #BL_SYNC_CPU_SIZE