ARM: common idle infrastructure

From: Will Deacon<[email protected]>

This patch adds simple definitions of cpu_reset for ARMv6 and ARMv7 cores, which disable the MMU via the SCTLR.

Signed-off-by Will Deacon<[email protected]>

---
  arch/arm/mm/proc-v6.S |    5 +++++
  arch/arm/mm/proc-v7.S |    7 +++++++
  2 files changed, 12 insertions(+), 0 deletions(-)

arch/arm/mm/proc-v6.S +5 -0

index 1d2b845..f3b5232 100644
@@ -56,6 +56,11 @@ ENTRY(cpu_v6_proc_fin)
   */
  	.align	5
  ENTRY(cpu_v6_reset)
+	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
+	bic	r1, r1, #0x1			@ ...............m
+	mcr	p15, 0, r1, c1, c0, 0		@ disable MMU
+	mov	r1, #0
+	mcr	p15, 0, r1, c7, c5, 4		@ ISB
  	mov	pc, r0
 
  /*

arch/arm/mm/proc-v7.S +0 -0

index 089c0b5..15d6191 100644

Minor nit,

Quoted 26 lines · Lorenzo Pieralisi

From: Will Deacon<[email protected]>

This patch adds simple definitions of cpu_reset for ARMv6 and ARMv7 cores, which disable the MMU via the SCTLR.

Signed-off-by Will Deacon<[email protected]>

---
  arch/arm/mm/proc-v6.S |    5 +++++
  arch/arm/mm/proc-v7.S |    7 +++++++
  2 files changed, 12 insertions(+), 0 deletions(-)

arch/arm/mm/proc-v6.S +5 -0

index 1d2b845..f3b5232 100644
@@ -56,6 +56,11 @@ ENTRY(cpu_v6_proc_fin)
   */
  	.align	5
  ENTRY(cpu_v6_reset)
+	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
+	bic	r1, r1, #0x1			@ ...............m
+	mcr	p15, 0, r1, c1, c0, 0		@ disable MMU
+	mov	r1, #0
+	mcr	p15, 0, r1, c7, c5, 4		@ ISB
  	mov	pc, r0
 

arch/arm/mm/proc-v7.S ENDPROC(cpu_v7_proc_fin) · line 63

Quoted 12 lines

arch/arm/mm/proc-v6.S +0 -0

  /*

arch/arm/mm/proc-v7.S +3 -0

index 089c0b5..15d6191 100644
@@ -58,9 +58,16 @@ ENDPROC(cpu_v7_proc_fin)
   *	to what would be the reset vector.
   *
   *	- loc   - location to jump to for soft reset
+ *
+ *	This code must be executed using a flat identity mapping with
+ *      caches disabled.

Align the text body.

From: Will Deacon <[email protected]>

This patch adds simple definitions of cpu_reset for ARMv6 and ARMv7 cores, which disable the MMU via the SCTLR.

Quoted 4 lines · Lorenzo Pieralisi

From: Will Deacon <[email protected]>

This patch adds simple definitions of cpu_reset for ARMv6 and ARMv7 cores, which disable the MMU via the SCTLR.

This really needs fixing properly, so that we have this well defined across all supported ARM cores. Requiring ARMv6 and ARMv7 to have this code called with a flat mapping (which may overlap a section boundary) vs ARMv5 and lower code which doesn't is just silly.

With any API, we need consistency. So if ARMv6 and v7 require a flat mapping, we need to ensure that ARMv5 and lower is happy to deal with that code being also called with a flat mapping.

I've had a look at a bunch of the cpu_*_reset definitions and I can't see any reason why they wouldn't be callable with the flat mapping in place. In fact, there's a scary comment for xscale:

@ CAUTION: MMU turned off from this point. We count on the pipeline @ already containing those two last instructions to survive.

which I think would disappear if the code was called via the ID map.

Quoted 3 lines · Will Deacon

I've had a look at a bunch of the cpu_*_reset definitions and I can't see any reason why they wouldn't be callable with the flat mapping in place. In fact, there's a scary comment for xscale:

However, that flat mapping doesn't save us, because this only covers space below PAGE_OFFSET virtual. We're executing these generally from virtual space at addresses greater than this, which means when the MMU is turned off, the instruction stream disappears.

Quoted 4 lines

@ CAUTION: MMU turned off from this point. We count on the pipeline @ already containing those two last instructions to survive.

which I think would disappear if the code was called via the ID map.

Yes, and everything pre-ARMv6 fundamentally relies upon the instructions following the MCR to turn the MMU off already being in the CPUs pipeline. Pre-ARMv6 relies upon this kind of behaviour from the CPU:

fetch decode execute
mcr
mov pc mcr
nop mov pc mcr
nop nop mov pc
<inst0> <--- flushed --->

where inst0 is the instruction at the target of the "mov pc" branch. May not be well documented in the architecture manual, but there have been documentation of this level against CPUs in the past. Maybe back in ARMv3 times, but the trick has proven to work all the way up to ARMv5, and these are of course CPU specific files rather than architecture specific.

It's curious that with ARMs move to a more relaxed model, that turning the MMU off has visibly changed from a fairly weak operation to an effective strong instruction barrier. These now have visibly this behaviour:

	fetch	decode	execute
	mcr
	mov pc	mcr
	<inst0>	mov pc	mcr
	<------ flushed ------>
	mov pc*

* - attempt to reload this instruction fails because MMU is now off.

I'm not saying there that the pipeline is flushed (it may be) - but this represents the _observed_ behaviour.

From: Colin Cross<[email protected]>

During some CPU power modes entered during idle, hotplug and suspend, peripherals located in the CPU power domain, such as the GIC and VFP, may be powered down. Add a notifier chain that allows drivers for those peripherals to be notified before and after they may be reset.

Signed-off-by Colin Cross<[email protected]>

Tested-by Kevin Hilman<[email protected]>

---
  arch/arm/Kconfig              |    7 ++
  arch/arm/include/asm/cpu_pm.h |   54 ++++++++++++
  arch/arm/kernel/Makefile      |    1 +
  arch/arm/kernel/cpu_pm.c      |  181 +++++++++++++++++++++++++++++++++++++++++
  4 files changed, 243 insertions(+), 0 deletions(-)

create mode 100644 arch/arm/include/asm/cpu_pm.h create mode 100644 arch/arm/kernel/cpu_pm.c

arch/arm/Kconfig +7 -0

index 9adc278..356f266 100644
@@ -183,6 +183,13 @@ config FIQ
  config ARCH_MTD_XIP
  	bool
 
+config ARCH_USES_CPU_PM
+       bool
+
+config CPU_PM
+       def_bool y
+       depends on ARCH_USES_CPU_PM&&  (PM || CPU_IDLE)
+
  config VECTORS_BASE
  	hex
  	default 0xffff0000 if MMU || CPU_HIGH_VECTOR

arch/arm/include/asm/cpu_pm.h new file +43 -0

new file mode 100644
index 0000000..b4bb715
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * Author:
+ *	Colin Cross<[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _ASMARM_CPU_PM_H
+#define _ASMARM_CPU_PM_H
+
+#include<linux/kernel.h>
+#include<linux/notifier.h>
+
+/* Event codes passed as unsigned long val to notifier calls */
+enum cpu_pm_event {
+	/* A single cpu is entering a low power state */
+	CPU_PM_ENTER,
+
+	/* A single cpu failed to enter a low power state */
+	CPU_PM_ENTER_FAILED,
+
+	/* A single cpu is exiting a low power state */
+	CPU_PM_EXIT,
+
+	/* A cpu power domain is entering a low power state */
+	CPU_COMPLEX_PM_ENTER,
+
+	/* A cpu power domain failed to enter a low power state */
+	CPU_COMPLEX_PM_ENTER_FAILED,
+
+	/* A cpu power domain is exiting a low power state */
+	CPU_COMPLEX_PM_EXIT,
+};

During some CPU power modes entered during idle, hotplug and suspend, peripherals located in the CPU power domain, such as the GIC and VFP, may be powered down. Add a notifier chain that allows drivers for those peripherals to be notified before and after they may be reset.

Quoted 5 lines · Lorenzo Pieralisi

During some CPU power modes entered during idle, hotplug and suspend, peripherals located in the CPU power domain, such as the GIC and VFP, may be powered down. ?Add a notifier chain that allows drivers for those peripherals to be notified before and after they may be reset.

I defer comment on this until I see what the CPU_COMPLEX_* stuff is used for.

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
  arch/arm/common/gic.c |  212 +++++++++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

arch/arm/common/gic.c +16 -0

+
+static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
+{
+	int i;
+
+	for (i = 0; i<  MAX_GIC_NR; i++) {
+		switch (cmd) {
+		case CPU_PM_ENTER:
+			gic_cpu_save(i);
+			break;
+		case CPU_PM_ENTER_FAILED:
+		case CPU_PM_EXIT:
+			gic_cpu_restore(i);
+			break;
+		case CPU_COMPLEX_PM_ENTER:
+			gic_dist_save(i);

Quoted 22 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
?arch/arm/common/gic.c | ?212
+++++++++++++++++++++++++++++++++++++++++++++++++
?1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

Quoted 17 lines

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

+
+static int gic_notifier(struct notifier_block *self, unsigned long cmd,

? ? ?void *v)

arch/arm/common/gic.c +14 -0

+{
+ ? ? ? int i;
+
+ ? ? ? for (i = 0; i< ?MAX_GIC_NR; i++) {
+ ? ? ? ? ? ? ? switch (cmd) {
+ ? ? ? ? ? ? ? case CPU_PM_ENTER:
+ ? ? ? ? ? ? ? ? ? ? ? gic_cpu_save(i);
+ ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? case CPU_PM_ENTER_FAILED:
+ ? ? ? ? ? ? ? case CPU_PM_EXIT:
+ ? ? ? ? ? ? ? ? ? ? ? gic_cpu_restore(i);
+ ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? case CPU_COMPLEX_PM_ENTER:
+ ? ? ? ? ? ? ? ? ? ? ? gic_dist_save(i);

arch/arm/common/gic.c

Quoted 10 lines

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

+ ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? case CPU_COMPLEX_PM_ENTER_FAILED:
+ ? ? ? ? ? ? ? case CPU_COMPLEX_PM_EXIT:
+ ? ? ? ? ? ? ? ? ? ? ? gic_dist_restore(i);
+ ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
+ ? ? ? return NOTIFY_OK;
+}

Just to put forth OMAP requirements for GIC and see how much we can leverage these for OMAP.

OMAP support GP(general) and HS(secure) devices and implements the trustzone on these devices. on Secure devices the GIC save and restore is completely done by secure ROM code. There are API's for save and restore is automatic on CPU reset based on the last CPU cluster state.

On GP devices too, very few GIC registers needs to be saved in a pre-defined memory/register layout and restore is again done by boot-ROM code.

OMAP need to enable/disable distributor and CPU interfaces based on CPU power states and that is something can be useful. Would be good if there is a provision to over-write the gic save/restore function using function pointers so that OMAP PM code can use the notifiers.

Any more thoughts how we can handle this? We would like to use common ARM code as much as possible.

Quoted 73 lines · Santosh Shilimkar

Quoted 22 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
  arch/arm/common/gic.c |  212
+++++++++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

Quoted 17 lines

arch/arm/common/gic.c +16 -0

+
+static int gic_notifier(struct notifier_block *self, unsigned long cmd,
      void *v)
+{
+       int i;
+
+       for (i = 0; i<    MAX_GIC_NR; i++) {
+               switch (cmd) {
+               case CPU_PM_ENTER:
+                       gic_cpu_save(i);
+                       break;
+               case CPU_PM_ENTER_FAILED:
+               case CPU_PM_EXIT:
+                       gic_cpu_restore(i);
+                       break;
+               case CPU_COMPLEX_PM_ENTER:
+                       gic_dist_save(i);

arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 536

Quoted 10 lines

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

+                       break;
+               case CPU_COMPLEX_PM_ENTER_FAILED:
+               case CPU_COMPLEX_PM_EXIT:
+                       gic_dist_restore(i);
+                       break;
+               }
+       }
+
+       return NOTIFY_OK;
+}

Just to put forth OMAP requirements for GIC and see how much we can leverage these for OMAP.

OMAP support GP(general) and HS(secure) devices and implements the trustzone on these devices. on Secure devices the GIC save and restore is completely done by secure ROM code. There are API's for save and restore is automatic on CPU reset based on the last CPU cluster state.

On GP devices too, very few GIC registers needs to be saved in a pre-defined memory/register layout and restore is again done by boot-ROM code.

OMAP need to enable/disable distributor and CPU interfaces based on CPU power states and that is something can be useful. Would be good if there is a provision to over-write the gic save/restore function using function pointers so that OMAP PM code can use the notifiers.

Quoted 2 lines

Any more thoughts how we can handle this? We would like to use common ARM code as much as possible.

Is it strictly necessary to use the custom OMAP save and restore?

Anything that was modified by the kernel is obviously writable, and could be saved and restored using the common code. Anything that can only be modified by TrustZone already has to be restored by the custom OMAP code. There aren't many registers in the GIC, so it shouldn't be much of a performance difference.

On 7/7/2011 6:41 PM, Colin Cross wrote:

+/*
+ * Saves the GIC distributor registers during suspend or idle. Must be

+ * with interrupts disabled but before powering down the GIC. After

+ * this function, no interrupts will be delivered by the GIC, and another
+ * platform-specific wakeup source must be enabled.
+ */

From: Colin Cross <[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/common/gic.c |  212 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 212 insertions(+), 0 deletions(-)

arch/arm/common/gic.c +212 -0

index 4ddd0a6..8d62e07 100644
@@ -29,6 +29,7 @@
 #include <linux/cpumask.h>
 #include <linux/io.h>
 
+#include <asm/cpu_pm.h>
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 #include <asm/hardware/gic.h>

@@ -42,6 +43,17 @@ struct gic_chip_data {
 	unsigned int irq_offset;
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
+#ifdef CONFIG_CPU_PM
+	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
+	u32 saved_spi_pri[DIV_ROUND_UP(1020, 4)];
+	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
+	u32 __percpu *saved_ppi_enable;
+	u32 __percpu *saved_ppi_conf;
+	u32 __percpu *saved_ppi_pri;
+#endif
+
+	unsigned int gic_irqs;
 };
 
 /*

@@ -283,6 +295,8 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 	if (gic_irqs > 1020)
 		gic_irqs = 1020;
 
+	gic->gic_irqs = gic_irqs;
+
 	/*
 	 * Set all global interrupts to be level triggered, active low.
 	 */

@@ -350,6 +364,203 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
 	writel_relaxed(1, base + GIC_CPU_CTRL);
 }
 
+#ifdef CONFIG_CPU_PM
+/*
+ * Saves the GIC distributor registers during suspend or idle.  Must be called
+ * with interrupts disabled but before powering down the GIC.  After calling
+ * this function, no interrupts will be delivered by the GIC, and another
+ * platform-specific wakeup source must be enabled.
+ */
+static void gic_dist_save(unsigned int gic_nr)
+{
+	unsigned int gic_irqs;
+	void __iomem *dist_base;
+	int i;
+
+	if (gic_nr >= MAX_GIC_NR)
+		BUG();
+
+	gic_irqs = gic_data[gic_nr].gic_irqs;
+	dist_base = gic_data[gic_nr].dist_base;
+
+	if (!dist_base)
+		return;
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 16); i++)
+		gic_data[gic_nr].saved_spi_conf[i] =
+			readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+		gic_data[gic_nr].saved_spi_pri[i] =
+			readl_relaxed(dist_base + GIC_DIST_PRI + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+		gic_data[gic_nr].saved_spi_target[i] =
+			readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+		gic_data[gic_nr].saved_spi_enable[i] =
+			readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	writel_relaxed(0, dist_base + GIC_DIST_CTRL);
+}
+
+/*
+ * Restores the GIC distributor registers during resume or when coming out of
+ * idle.  Must be called before enabling interrupts.  If a level interrupt
+ * that occured while the GIC was suspended is still present, it will be
+ * handled normally, but any edge interrupts that occured will not be seen by
+ * the GIC and need to be handled by the platform-specific wakeup source.
+ */
+static void gic_dist_restore(unsigned int gic_nr)
+{
+	unsigned int gic_irqs;
+	unsigned int i;
+	void __iomem *dist_base;
+
+	if (gic_nr >= MAX_GIC_NR)
+		BUG();
+
+	gic_irqs = gic_data[gic_nr].gic_irqs;
+	dist_base = gic_data[gic_nr].dist_base;
+
+	if (!dist_base)
+		return;
+
+	writel_relaxed(0, dist_base + GIC_DIST_CTRL);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 16); i++)
+		writel_relaxed(gic_data[gic_nr].saved_spi_conf[i],
+			dist_base + GIC_DIST_CONFIG + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+		writel_relaxed(gic_data[gic_nr].saved_spi_pri[i],
+			dist_base + GIC_DIST_PRI + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+		writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
+			dist_base + GIC_DIST_TARGET + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
+			dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	writel_relaxed(1, dist_base + GIC_DIST_CTRL);
+}
+
+static void gic_cpu_save(unsigned int gic_nr)
+{
+	int i;
+	u32 *ptr;
+	void __iomem *dist_base;
+	void __iomem *cpu_base;
+
+	if (gic_nr >= MAX_GIC_NR)
+		BUG();
+
+	dist_base = gic_data[gic_nr].dist_base;
+	cpu_base = gic_data[gic_nr].cpu_base;
+
+	if (!dist_base || !cpu_base)
+		return;
+
+	ptr = __this_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	ptr = __this_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
+	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
+		ptr[i] = readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
+
+	ptr = __this_cpu_ptr(gic_data[gic_nr].saved_ppi_pri);
+	for (i = 0; i < DIV_ROUND_UP(32, 4); i++)
+		ptr[i] = readl_relaxed(dist_base + GIC_DIST_PRI + i * 4);
+}
+
+static void gic_cpu_restore(unsigned int gic_nr)
+{
+	int i;
+	u32 *ptr;
+	void __iomem *dist_base;
+	void __iomem *cpu_base;
+
+	if (gic_nr >= MAX_GIC_NR)
+		BUG();
+
+	dist_base = gic_data[gic_nr].dist_base;
+	cpu_base = gic_data[gic_nr].cpu_base;
+
+	if (!dist_base || !cpu_base)
+		return;
+
+	ptr = __this_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	ptr = __this_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
+	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
+		writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
+
+	ptr = __this_cpu_ptr(gic_data[gic_nr].saved_ppi_pri);
+	for (i = 0; i < DIV_ROUND_UP(32, 4); i++)
+		writel_relaxed(ptr[i], dist_base + GIC_DIST_PRI + i * 4);
+
+	writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
+	writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+}
+
+static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
+{
+	int i;
+
+	for (i = 0; i < MAX_GIC_NR; i++) {
+		switch (cmd) {
+		case CPU_PM_ENTER:
+			gic_cpu_save(i);
+			break;
+		case CPU_PM_ENTER_FAILED:
+		case CPU_PM_EXIT:
+			gic_cpu_restore(i);
+			break;
+		case CPU_COMPLEX_PM_ENTER:
+			gic_dist_save(i);
+			break;
+		case CPU_COMPLEX_PM_ENTER_FAILED:
+		case CPU_COMPLEX_PM_EXIT:
+			gic_dist_restore(i);
+			break;
+		}
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block gic_notifier_block = {
+	.notifier_call = gic_notifier,
+};
+
+static void __init gic_cpu_pm_init(struct gic_chip_data *gic)
+{
+	gic->saved_ppi_enable = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
+		sizeof(u32));
+	BUG_ON(!gic->saved_ppi_enable);
+
+	gic->saved_ppi_conf = __alloc_percpu(DIV_ROUND_UP(32, 16) * 4,
+		sizeof(u32));
+	BUG_ON(!gic->saved_ppi_conf);
+
+	gic->saved_ppi_pri = __alloc_percpu(DIV_ROUND_UP(32, 4) * 4,
+		sizeof(u32));
+	BUG_ON(!gic->saved_ppi_pri);
+
+	cpu_pm_register_notifier(&gic_notifier_block);
+}
+#else
+static void __init gic_cpu_pm_init(struct gic_chip_data *gic)
+{
+}
+#endif
+
 void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
 	void __iomem *dist_base, void __iomem *cpu_base)
 {

@@ -367,6 +578,7 @@ void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
 
 	gic_dist_init(gic, irq_start);
 	gic_cpu_init(gic);
+	gic_cpu_pm_init(gic);
 }
 
 void __cpuinit gic_secondary_init(unsigned int gic_nr)

Signature · 8 lines

1.7.4.4



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 11 lines · Lorenzo Pieralisi

From: Colin Cross <[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Quoted 1 line

Signed-off-by Colin Cross <[email protected]>

Lost attributations and original author. ?This is based in part on code from Gary King, according to patch 6646/1 in the patch system.

Moreover, how now that we have genirq dealing with the suspend/restore issues, how much of this is actually required. ?And should this be GIC specific or should there be a way of asking genirq to take care of some of this for us?

We need to _reduce_ the amount of code needed to support this stuff, and if core code almost-but-not-quite does what we need then we need to talk to the maintainers of that code to see whether it can be changed.

Because adding 212 lines to save and restore the state of every interrupt controller that we may have just isn't on. ?We need this properly abstracted and dealt with in a generic way.

This is necessary for cpuidle states that lose the GIC registers, not just suspend, because the GIC is in the cpu's power domain. We could avoid saving and restoring all the GIC registers in suspend and idle by reusing the initialization functions, and then having the core irq code call the unmask, set_type, and set_affinity functions on each irq to reconfigure it, but that will be very inefficient - it will convert each register write in the restore functions to a read-modify-write per interrupt in that register. Santosh is already complaining that this commong GIC restore code will be slower than the automatic DMA to restore the GIC registers that OMAP4 supports.

Quoted 10 lines · Colin Cross

This is necessary for cpuidle states that lose the GIC registers, not just suspend, because the GIC is in the cpu's power domain. ?We could avoid saving and restoring all the GIC registers in suspend and idle by reusing the initialization functions, and then having the core irq code call the unmask, set_type, and set_affinity functions on each irq to reconfigure it, but that will be very inefficient - it will convert each register write in the restore functions to a read-modify-write per interrupt in that register. ?Santosh is already complaining that this commong GIC restore code will be slower than the automatic DMA to restore the GIC registers that OMAP4 supports.

Well, we need to come up with something sensible - a way of doing this which doesn't require every interrupt controller driver (of which we as an architecture have many) to have lots of support added.

If the current way is inefficient and is noticably so, then let's talk to Thomas about finding a way around that - maybe having the generic code make one suspend/resume callback per irq gc chip rather than doing it per-IRQ. ?We can then reuse the same paths for suspend/resume as for idle state saving.

Quoted 16 lines · Russell King - ARM Linux

Quoted 10 lines · Colin Cross

This is necessary for cpuidle states that lose the GIC registers, not just suspend, because the GIC is in the cpu's power domain. ?We could avoid saving and restoring all the GIC registers in suspend and idle by reusing the initialization functions, and then having the core irq code call the unmask, set_type, and set_affinity functions on each irq to reconfigure it, but that will be very inefficient - it will convert each register write in the restore functions to a read-modify-write per interrupt in that register. ?Santosh is already complaining that this commong GIC restore code will be slower than the automatic DMA to restore the GIC registers that OMAP4 supports.

Well, we need to come up with something sensible - a way of doing this which doesn't require every interrupt controller driver (of which we as an architecture have many) to have lots of support added.

Quoted 6 lines

If the current way is inefficient and is noticably so, then let's talk to Thomas about finding a way around that - maybe having the generic code make one suspend/resume callback per irq gc chip rather than doing it per-IRQ. ?We can then reuse the same paths for suspend/resume as for idle state saving.

Are you referring to moving the gic driver to be gc chip? Otherwise, I don't understand your suggestion - how is callback per chip any different than what this patch implements? It just gets it's notification through a cpu_pm notifier, which works in idle and suspend, instead of a syscore op like the gc driver does.

This patch does save and restore some registers that are never modified after init, so they don't need to be saved.

Quoted 31 lines · Colin Cross

Quoted 16 lines · Russell King - ARM Linux

Quoted 10 lines · Colin Cross

This is necessary for cpuidle states that lose the GIC registers, not just suspend, because the GIC is in the cpu's power domain. ?We could avoid saving and restoring all the GIC registers in suspend and idle by reusing the initialization functions, and then having the core irq code call the unmask, set_type, and set_affinity functions on each irq to reconfigure it, but that will be very inefficient - it will convert each register write in the restore functions to a read-modify-write per interrupt in that register. ?Santosh is already complaining that this commong GIC restore code will be slower than the automatic DMA to restore the GIC registers that OMAP4 supports.

Well, we need to come up with something sensible - a way of doing this which doesn't require every interrupt controller driver (of which we as an architecture have many) to have lots of support added.

Quoted 6 lines

If the current way is inefficient and is noticably so, then let's talk to Thomas about finding a way around that - maybe having the generic code make one suspend/resume callback per irq gc chip rather than doing it per-IRQ. ?We can then reuse the same paths for suspend/resume as for idle state saving.

Are you referring to moving the gic driver to be gc chip? ?Otherwise, I don't understand your suggestion - how is callback per chip any different than what this patch implements? ?It just gets it's notification through a cpu_pm notifier, which works in idle and suspend, instead of a syscore op like the gc driver does.

Quoted 2 lines

This patch does save and restore some registers that are never modified after init, so they don't need to be saved.

The point is that we should aim to get to the point where, if an interrupt controller supports PM, then it supports _all_ PM out the box and doesn't require additional code for cpu idle PM vs system suspend PM.

In other words, all we should need to do is provide genirq with a couple of functions for 'save state' and 'restore state'.

Quoted 25 lines · Colin Cross

Quoted 16 lines · Russell King - ARM Linux

Quoted 10 lines · Colin Cross

This is necessary for cpuidle states that lose the GIC registers, not just suspend, because the GIC is in the cpu's power domain. We could avoid saving and restoring all the GIC registers in suspend and idle by reusing the initialization functions, and then having the core irq code call the unmask, set_type, and set_affinity functions on each irq to reconfigure it, but that will be very inefficient - it will convert each register write in the restore functions to a read-modify-write per interrupt in that register. Santosh is already complaining that this commong GIC restore code will be slower than the automatic DMA to restore the GIC registers that OMAP4 supports.

Well, we need to come up with something sensible - a way of doing this which doesn't require every interrupt controller driver (of which we as an architecture have many) to have lots of support added.

Quoted 6 lines

If the current way is inefficient and is noticably so, then let's talk to Thomas about finding a way around that - maybe having the generic code make one suspend/resume callback per irq gc chip rather than doing it per-IRQ. We can then reuse the same paths for suspend/resume as for idle state saving.

Quoted 8 lines

Are you referring to moving the gic driver to be gc chip? Otherwise, I don't understand your suggestion - how is callback per chip any different than what this patch implements? It just gets it's notification through a cpu_pm notifier, which works in idle and suspend, instead of a syscore op like the gc driver does.

This patch does save and restore some registers that are never modified after init, so they don't need to be saved.

The point is that we should aim to get to the point where, if an interrupt controller supports PM, then it supports _all_ PM out the box and doesn't require additional code for cpu idle PM vs system suspend PM.

In other words, all we should need to do is provide genirq with a couple of functions for 'save state' and 'restore state'.

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

+			break;
+		case CPU_PM_ENTER_FAILED:
+		case CPU_PM_EXIT:
+			gic_cpu_restore(i);
+			break;
+		case CPU_COMPLEX_PM_ENTER:
+			gic_dist_save(i);
+			break;

Lorenzo, Colin,

arch/arm/common/gic.c

Quoted 20 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
  arch/arm/common/gic.c |  212 +++++++++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

I missed one more comment in the last review.

arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 519

Quoted 8 lines

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

+static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
+{
+	int i;
+
+	for (i = 0; i<  MAX_GIC_NR; i++) {
+		switch (cmd) {
+		case CPU_PM_ENTER:
+			gic_cpu_save(i);

On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down.

Quoted 13 lines

arch/arm/common/gic.c +13 -0

+			break;
+		case CPU_PM_ENTER_FAILED:
+		case CPU_PM_EXIT:
+			gic_cpu_restore(i);
+			break;
+		case CPU_COMPLEX_PM_ENTER:
+			gic_dist_save(i);
+			break;
+		case CPU_COMPLEX_PM_ENTER_FAILED:
+		case CPU_COMPLEX_PM_EXIT:
+			gic_dist_restore(i);
+			break;
+		}

arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 537

Quoted 5 lines

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

+	}
+
+	return NOTIFY_OK;
+}
+

Entire GIC is kept in CPU cluster power domain and hence GIC CPU interface context won't be lost whenever CPU alone enters in the deepest power state.

If it is different on other SOC's then the common notifiers won't match to all the SOC designs.

Looks like exporting these functions directly or adding them to gen irq and then invoking them from platform code based on power sequence need might be better.

Quoted 28 lines · Santosh Shilimkar

Lorenzo, Colin,

arch/arm/common/gic.c

Quoted 20 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
   arch/arm/common/gic.c |  212 +++++++++++++++++++++++++++++++++++++++++++++++++
   1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

I missed one more comment in the last review.

Quoted 10 lines

arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 519

Quoted 8 lines

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

+static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
+{
+	int i;
+
+	for (i = 0; i<   MAX_GIC_NR; i++) {
+		switch (cmd) {
+		case CPU_PM_ENTER:
+			gic_cpu_save(i);

On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down.

Yes, it's true, but that's the only chance we have to save the GIC CPU IF state if the GIC context is lost, right ?
It is a private memory map per processor; I agree, it might be useless if just one CPU is shutdown, but at that point in time you do not know the state of other CPUs. If the cluster moves to a state where GIC context is lost at least you had the GIC CPU IF state saved. If we do not save it, well, there is no way to do that anymore since the last CPU cannot access other CPUs GIC CPU IF registers (or better, banked GIC distributor registers).
If you force hotplug on CPUs other than 0 (that's the way it is done on OMAP4 in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU IF state as per cold boot, so yes, it is useless there.

Quoted 46 lines · Lorenzo Pieralisi

Quoted 31 lines · Santosh Shilimkar

Lorenzo, Colin,

arch/arm/common/gic.c

Quoted 22 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
? arch/arm/common/gic.c | ?212
+++++++++++++++++++++++++++++++++++++++++++++++++
? 1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

I missed one more comment in the last review.

Quoted 12 lines

arch/arm/common/gic.c

Quoted 9 lines

+static int gic_notifier(struct notifier_block *self, unsigned long cmd, ? ? ? ?void *v)

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

+{
+ ? ? ? int i;
+
+ ? ? ? for (i = 0; i< ? MAX_GIC_NR; i++) {
+ ? ? ? ? ? ? ? switch (cmd) {
+ ? ? ? ? ? ? ? case CPU_PM_ENTER:
+ ? ? ? ? ? ? ? ? ? ? ? gic_cpu_save(i);

On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down.

Quoted 16 lines

Yes, it's true, but that's the only chance we have to save the GIC CPU IF state if the GIC context is lost, right ?
It is a private memory map per processor; I agree, it might be useless if just one CPU is shutdown, but at that point in time you do not know the state of other CPUs. If the cluster moves to a state where GIC context is lost at least you had the GIC CPU IF state saved. If we do not save it, well, there is no way to do that anymore since the last CPU cannot
access other CPUs GIC CPU IF registers (or better, banked GIC distributor registers).
If you force hotplug on CPUs other than 0 (that's the way it is done on OMAP4
in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU IF
state as per cold boot, so yes, it is useless there.

Actually, on OMAP there is no need to save any CPU interface registers.

For my OMAP4 PM rebasing, for time-being I will go with exported GIC functions so that I don't have too many redundancies with GIC save/restore code.

Quoted 63 lines · Santosh Shilimkar

Quoted 46 lines · Lorenzo Pieralisi

Quoted 31 lines · Santosh Shilimkar

Lorenzo, Colin,

arch/arm/common/gic.c

Quoted 22 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
   arch/arm/common/gic.c |  212
+++++++++++++++++++++++++++++++++++++++++++++++++
   1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

I missed one more comment in the last review.

Quoted 12 lines

arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 519

Quoted 9 lines

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

+static int gic_notifier(struct notifier_block *self, unsigned long cmd,
        void *v)
+{
+       int i;
+
+       for (i = 0; i<     MAX_GIC_NR; i++) {
+               switch (cmd) {
+               case CPU_PM_ENTER:
+                       gic_cpu_save(i);

On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down.

Quoted 16 lines

Yes, it's true, but that's the only chance we have to save the GIC CPU IF state if the GIC context is lost, right ?
It is a private memory map per processor; I agree, it might be useless if just one CPU is shutdown, but at that point in time you do not know the state of other CPUs. If the cluster moves to a state where GIC context is lost at least you had the GIC CPU IF state saved. If we do not save it, well, there is no way to do that anymore since the last CPU cannot
access other CPUs GIC CPU IF registers (or better, banked GIC distributor registers).
If you force hotplug on CPUs other than 0 (that's the way it is done on OMAP4
in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU IF
state as per cold boot, so yes, it is useless there.

Quoted 5 lines

Actually, on OMAP there is no need to save any CPU interface registers.

For my OMAP4 PM rebasing, for time-being I will go with exported GIC functions so that I don't have too many redundancies with GIC save/restore code.

I think you should try to balance cpu idle latency with reuse of common code. In this case, you are avoiding restoring 7 registers by reimplementing the bare minimum that is necessary for OMAP4, which is unlikely to make a measurable impact on wakeup latency. Can you try starting with reusing all the common code, and add some timestamps during wakeup to measure where the longest delays are, to determine where you should diverge from the common code and use omap-optimized code?

Quoted 77 lines · Colin Cross

Quoted 68 lines · On Thu, Jul 21, 2011 at 3:46 AM, Santosh Shilimkar <[email protected]>…

Quoted 57 lines · Lorenzo Pieralisi

Quoted 31 lines · Santosh Shilimkar

Lorenzo, Colin,

arch/arm/common/gic.c

Quoted 22 lines · Lorenzo Pieralisi

From: Colin Cross<[email protected]>

When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers.

Signed-off-by Colin Cross<[email protected]>

---
? arch/arm/common/gic.c | ?212
+++++++++++++++++++++++++++++++++++++++++++++++++
? 1 files changed, 212 insertions(+), 0 deletions(-)

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

index 4ddd0a6..8d62e07 100644

[...]

I missed one more comment in the last review.

Quoted 13 lines

arch/arm/common/gic.c

Quoted 10 lines

+static int gic_notifier(struct notifier_block *self, unsigned long cmd,
? ? ? ?void *v)

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

+{
+ ? ? ? int i;
+
+ ? ? ? for (i = 0; i< ? ? MAX_GIC_NR; i++) {
+ ? ? ? ? ? ? ? switch (cmd) {
+ ? ? ? ? ? ? ? case CPU_PM_ENTER:
+ ? ? ? ? ? ? ? ? ? ? ? gic_cpu_save(i);

On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down.

Yes, it's true, but that's the only chance we have to save the GIC CPU IF
state if the GIC context is lost, right ?
It is a private memory map per processor; I agree, it might be useless if just one CPU is shutdown, but at that point in time you do not know the state of other CPUs. If the cluster moves to a state where GIC context
is lost at least you had the GIC CPU IF state saved. If we do not save it, well, there is no way to do that anymore since the last CPU cannot

Quoted 9 lines

access other CPUs GIC CPU IF registers (or better, banked GIC distributor
registers).
If you force hotplug on CPUs other than 0 (that's the way it is done on OMAP4
in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU IF
state as per cold boot, so yes, it is useless there.

Quoted 5 lines

Actually, on OMAP there is no need to save any CPU interface registers.

For my OMAP4 PM rebasing, for time-being I will go with exported GIC functions so that I don't have too many redundancies with GIC save/restore code.

Quoted 8 lines

I think you should try to balance cpu idle latency with reuse of common code. ?In this case, you are avoiding restoring 7 registers by reimplementing the bare minimum that is necessary for OMAP4, which is unlikely to make a measurable impact on wakeup latency. ?Can you try starting with reusing all the common code, and add some timestamps during wakeup to measure where the longest delays are, to determine where you should diverge from the common code and use omap-optimized code?

I am going to use all the common code but having them exported functions gives more flexibility to call them in right and needed places. As discussed earlier, I plan to use the common GIC code wherever it's needed on OMAP.

My main point was we are saving and restoring GIC CPU interface registers for a case where they are actually not lost.

On Thu, Jul 21, 2011 at 10:10 PM, Santosh Shilimkar

Quoted 18 lines

Quoted 4 lines

Quoted 3 lines

For my OMAP4 PM rebasing, for time-being I will go with exported GIC functions so that I don't have too many redundancies with GIC save/restore code.

Quoted 8 lines

I think you should try to balance cpu idle latency with reuse of common code. In this case, you are avoiding restoring 7 registers by reimplementing the bare minimum that is necessary for OMAP4, which is unlikely to make a measurable impact on wakeup latency. Can you try starting with reusing all the common code, and add some timestamps during wakeup to measure where the longest delays are, to determine where you should diverge from the common code and use omap-optimized code?

I am going to use all the common code but having them exported functions gives more flexibility to call them in right and needed places. As discussed earlier, I plan to use the common GIC code wherever it's needed on OMAP.

Quoted 2 lines

My main point was we are saving and restoring GIC CPU interface registers for a case where they are actually not lost.

Yes, but you're still avoiding 7 registers, which is unlikely to be worth the complexity of calling these functions differently from every other platform.

From: Colin Cross <[email protected]>

When the cpu is powered down in a low power mode, the vfp registers may be reset.

This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the cpu's vfp registers.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/vfp/vfpmodule.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

arch/arm/vfp/vfpmodule.c +8 -0

index f25e7ec..6f08dbe 100644
@@ -21,6 +21,7 @@
 #include <asm/cputype.h>
 #include <asm/thread_notify.h>
 #include <asm/vfp.h>
+#include <asm/cpu_pm.h>
 
 #include "vfpinstr.h"
 #include "vfp.h"

@@ -169,6 +170,44 @@ static struct notifier_block vfp_notifier_block = {
 	.notifier_call	= vfp_notifier,
 };
 
+#ifdef CONFIG_CPU_PM
+static int vfp_cpu_pm_notifier(struct notifier_block *self, unsigned long cmd,
+	void *v)
+{
+	u32 fpexc = fmrx(FPEXC);
+	unsigned int cpu = smp_processor_id();
+

We need to sort this out so we have a _sane_ approach to this, rather than inventing more and more creative ways to save VFP state and restore it later.

In order to define a common idle interface for the kernel to enter low power modes, this patch provides include files and code that manages OS calls for low power entry and exit.

arch/arm/kernel/sr_entry.S new file +21 -0

new file mode 100644
index 0000000..4fa9bef
@@ -0,0 +1,213 @@
+/*
+ * Copyright (c) 2008-2011 ARM Ltd
+ *
+ * Author(s):  Jon Callan, Lorenzo Pieralisi
+ *
+ * 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<generated/asm-offsets.h>
+#include<asm/thread_info.h>
+#include<asm/memory.h>
+#include<asm/ptrace.h>
+#include<asm/glue-proc.h>
+#include<asm/assembler.h>
+#include<asm-generic/errno-base.h>
+#include<mach/entry-macro.S>
+

Quoted 4 lines · Lorenzo Pieralisi

In order to define a common idle interface for the kernel to enter low power modes, this patch provides include files and code that manages OS calls for low power entry and exit.

[....]

Quoted 27 lines

arch/arm/kernel/sr_entry.S new file +21 -0

new file mode 100644
index 0000000..4fa9bef
@@ -0,0 +1,213 @@
+/*
+ * Copyright (c) 2008-2011 ARM Ltd
+ *
+ * Author(s):  Jon Callan, Lorenzo Pieralisi
+ *
+ * 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<generated/asm-offsets.h>
+#include<asm/thread_info.h>
+#include<asm/memory.h>
+#include<asm/ptrace.h>
+#include<asm/glue-proc.h>
+#include<asm/assembler.h>
+#include<asm-generic/errno-base.h>
+#include<mach/entry-macro.S>
+

arch/arm/kernel/sr_entry.S line 31

Quoted 10 lines

arch/arm/kernel/sr_entry.S new file +10 -0

+	.text
+
+ENTRY(default_sleep)
+	b out		@ BTAC allocates branch and enters loop mode
+idle:			@ power down is entered with GIC CPU IF still on which
+	dsb		@ might get wfi instruction to complete before the
+	wfi		@ CPU is shut down -- infinite loop
+out:
+    	b idle
+ENDPROC(default_sleep)

Q: What happens for some reason CPU didn't hit targeted state in IDLE. Does CPU keep looping here forever.

On OMAP4, we need to issue additional interconnect barrier before WFI. How can we make provision for the same

In order to define a common idle interface for the kernel to enter low power modes, this patch provides include files and code that manages OS calls for low power entry and exit.

In ARM world processor HW is categorized as CPU and Cluster.

Corresponding states defined by this common IF are:

C-state [CPU state]:

0 - RUN MODE
1 - STANDBY
2 - DORMANT (not supported by this patch)
3 - SHUTDOWN

R-state [CLUSTER state]

0 - RUN
1 - STANDBY (not supported by this patch)
2 - L2 RAM retention
3 - SHUTDOWN

idle modes are entered through

cpu_enter_idle(cstate, rstate, flags) [sr_entry.S]

which could replace the current processor.idle entry in proc info, since it just executes wfi for shallow C-states.

Cluster low-power states are reached if and only if all the CPUs in the cluster are in low-power mode.

Only one cluster is supported at present, and the kernel infrastructure should be improved to allow multiple clusters to be defined and enumerated.

Current page table dir and stack pointers are saved using a per-cpu variable; this scheme breaks as soon as clusters are added to the kernel.

The code keeps a cpumask of alive CPUs and manages the state transitions accordingly.

Most of the variables needed when the CPU is powered down (MMU off) are allocated through a platform hook:

platform_context_pointer(unsigned int size)

that returns memory flat-mapped by this patchset as strongly ordered to avoid toying with L2 cleaning when a single CPU enters lowpower.

Fully tested on dual-core A9 cluster.

Signed-off-by Lorenzo Pieralisi <[email protected]>

---

arch/arm/include/asm/sr_platform_api.h |   28 ++++
arch/arm/kernel/sr_api.c               |  197 +++++++++++++++++++++++++++++
arch/arm/kernel/sr_entry.S             |  213 ++++++++++++++++++++++++++++++++
3 files changed, 438 insertions(+), 0 deletions(-)

create mode 100644 arch/arm/include/asm/sr_platform_api.h create mode 100644 arch/arm/kernel/sr_api.c create mode 100644 arch/arm/kernel/sr_entry.S

arch/arm/include/asm/sr_platform_api.h new file +28 -0

new file mode 100644
index 0000000..32367be
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2008-2011 ARM Limited
+ *
+ * Author(s): Jon Callan, Lorenzo Pieralisi
+ *
+ * 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 ASMARM_SR_PLATFORM_API_H
+#define ASMARM_SR_PLATFORM_API_H
+
+#define SR_SAVE_L2	(1 << 31)
+#define SR_SAVE_SCU	(1 << 30)
+#define SR_SAVE_ALL	(SR_SAVE_L2 | SR_SAVE_SCU)
+
+struct lp_state {
+	u16 cpu;
+	u16 cluster;
+};
+
+extern void (*sr_sleep)(void);
+extern void (*arch_reset_handler(void))(void);
+extern int cpu_enter_idle(unsigned cstate, unsigned rstate, unsigned flags);
+extern void *platform_context_pointer(unsigned int);
+#endif

arch/arm/kernel/sr_api.c new file +197 -0

new file mode 100644
index 0000000..4e48f60
@@ -0,0 +1,197 @@
+/*
+ * Copyright (C) 2008-2011 ARM Limited
+ *
+ * Author(s): Jon Callan, Lorenzo Pieralisi
+ *
+ * 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/slab.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/sched.h>
+#include <linux/cache.h>
+#include <linux/cpu.h>
+
+#include <asm/cacheflush.h>
+#include <asm/tlbflush.h>
+#include <asm/system.h>
+#include <asm/cpu_pm.h>
+#include <asm/lb_lock.h>
+#include <asm/sr_platform_api.h>
+
+#include "sr_helpers.h"
+#include "sr.h"
+
+
+struct ____cacheline_aligned sr_main_table main_table = {
+	.num_clusters	= SR_NR_CLUSTERS,
+	.cpu_idle_mask	= { { CPU_BITS_NONE }, },
+};
+
+static int late_init(void);
+
+int sr_runtime_init(void)
+{
+	int ret;
+
+	context_memory_uncached =
+		platform_context_pointer(CONTEXT_SPACE_UNCACHED);
+
+	if (!context_memory_uncached)
+		return -ENOMEM;
+
+	ret = linux_sr_setup_translation_tables();
+
+	if (ret < 0)
+		return ret;
+
+	ret = sr_context_init();
+
+	return ret;
+}
+
+/* return the warm-boot entry point virtual address */
+void (*arch_reset_handler(void))(void)
+{
+	return (void (*)(void)) arch->reset;
+}
+
+static int late_init(void)
+{
+	int rc;
+	struct sr_cluster *cluster;
+	int cluster_index, cpu_index = sr_platform_get_cpu_index();
+
+	cluster_index = sr_platform_get_cluster_index();
+	cluster = main_table.cluster_table + cluster_index;
+	main_table.os_mmu_context[cluster_index][cpu_index] =
+				current->active_mm->pgd;
+	cpu_switch_mm(main_table.fw_mmu_context, current->active_mm);
+	rc = sr_platform_init();
+	cpu_switch_mm(main_table.os_mmu_context[cluster_index][cpu_index],
+				current->active_mm);
+	return rc;
+}
+
+void (*sr_sleep)(void) = default_sleep;
+
+void enter_idle(unsigned cstate, unsigned rstate, unsigned flags)
+{
+	struct sr_cpu *cpu;
+	struct sr_cluster *cluster;
+	cpumask_t *cpuidle_mask;
+	int cpu_index, cluster_index;
+
+	cluster_index = sr_platform_get_cluster_index();
+	cpu_index = sr_platform_get_cpu_index();
+	cpuidle_mask = &main_table.cpu_idle_mask[cluster_index];
+	/*
+	 * WARNING: cluster support will break if multiple clusters are
+	 * instantiated within the kernel. The current version works
+	 * with just one cluster and cpu_index is the hardware processor
+	 * id in cluster index 0.
+	 */
+	main_table.os_mmu_context[cluster_index][cpu_index] =
+				current->active_mm->pgd;
+	cpu_switch_mm(main_table.fw_mmu_context, current->active_mm);
+	local_flush_tlb_all();
+
+	cluster = main_table.cluster_table + cluster_index;
+	cpu = cluster->cpu_table + cpu_index;
+
+	get_spinlock(cpu_index, cluster->lock);
+
+	__cpu_set(cpu_index, cpuidle_mask);
+
+	if (cpumask_weight(cpuidle_mask) == num_online_cpus())
+		cluster->power_state = rstate;
+
+	cluster->cluster_down = (cluster->power_state >= 2);
+
+	cpu->power_state = cstate;
+
+	cpu_pm_enter();
+
+	if (cluster->cluster_down)
+		cpu_complex_pm_enter();
+
+	sr_platform_enter_cstate(cpu_index, cpu, cluster);
+
+	sr_save_context(cluster, cpu, flags);
+
+	release_spinlock(cpu_index, cluster->lock);
+
+	/* Point of no return */
+	(*sr_sleep)();
+
+	/*
+	 * In case we wanted sr_sleep to return
+	 * here is code to turn MMU off and go
+	 * the whole hog on the resume path
+	 */
+
+	cpu_reset((virt_to_phys((void *) arch->reset)));
+}
+
+void exit_idle(struct sr_main_table *mt)
+{
+	struct sr_cpu *cpu;
+	struct sr_cluster *cluster;
+	int cpu_index, cluster_index;
+
+	cpu_index = sr_platform_get_cpu_index();
+
+	cluster_index = sr_platform_get_cluster_index();
+
+	cluster = mt->cluster_table + cluster_index;
+	cpu = cluster->cpu_table + cpu_index;
+
+	PA(get_spinlock)(cpu_index, cluster->lock);
+
+	PA(sr_restore_context)(cluster, cpu);
+
+	sr_platform_leave_cstate(cpu_index, cpu, cluster);
+
+	if (cluster->cluster_down) {
+		cpu_complex_pm_exit();
+		cluster->cluster_down = 0;
+	}
+
+	cpu_pm_exit();
+
+	cpu_clear(cpu_index, main_table.cpu_idle_mask[cluster_index]);
+
+	cpu->power_state = 0;
+	cluster->power_state = 0;
+
+	release_spinlock(cpu_index, cluster->lock);
+	cpu_switch_mm(main_table.os_mmu_context[cluster_index][cpu_index],
+					current->active_mm);
+	local_flush_tlb_all();
+}
+
+
+int sr_init(void)
+{
+	if (lookup_arch()) {
+		printk(KERN_EMERG "SR INIT: Undetected architecture id\n");
+		BUG();
+	}
+
+	if (sr_runtime_init()) {
+		printk(KERN_EMERG "SR INIT: runtime init error\n");
+		BUG();
+	}
+
+	if (late_init()) {
+		printk(KERN_EMERG "SR INIT: late init error\n");
+		BUG();
+	}
+
+	return 0;
+}
+arch_initcall(sr_init);

arch/arm/kernel/sr_entry.S new file +213 -0

new file mode 100644
index 0000000..4fa9bef
@@ -0,0 +1,213 @@
+/*
+ * Copyright (c) 2008-2011 ARM Ltd
+ *
+ * Author(s):  Jon Callan, Lorenzo Pieralisi
+ *
+ * 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 <generated/asm-offsets.h>
+#include <asm/thread_info.h>
+#include <asm/memory.h>
+#include <asm/ptrace.h>
+#include <asm/glue-proc.h>
+#include <asm/assembler.h>
+#include <asm-generic/errno-base.h>
+#include <mach/entry-macro.S>
+
+	.text
+
+ENTRY(default_sleep)
+	b out		@ BTAC allocates branch and enters loop mode
+idle:			@ power down is entered with GIC CPU IF still on which
+	dsb		@ might get wfi instruction to complete before the
+	wfi		@ CPU is shut down -- infinite loop
+out:
+    	b idle
+ENDPROC(default_sleep)
+
+
+ENTRY(sr_suspend)
+	b cpu_do_suspend
+ENDPROC(sr_suspend)
+
+ENTRY(sr_resume)
+	add	lr, lr, #(PAGE_OFFSET - PLAT_PHYS_OFFSET)
+	stmfd	sp!, {r4 - r11, lr}
+	ldr	lr, =mmu_on
+	b 	cpu_do_resume
+mmu_on:
+	ldmfd	sp!, {r4 - r11, pc}
+ENDPROC(sr_resume)
+
+/*
+ * This code is in the .data section to retrieve stack pointers stored in
+ * platform_cpu_stacks and platform_cpu_nc_stacks with a pc relative load.
+ * It cannot live in .text since that section can be treated as read-only
+ * and would break the code, which requires stack pointers to be saved on
+ * idle entry.
+ */
+	.data
+	.align
+	.global idle_save_context
+	.global idle_restore_context
+	.global idle_mt
+	.global platform_cpu_stacks
+	.global platform_cpu_nc_stacks
+
+/*
+ * idle entry point
+ * Must be called with IRQ disabled
+ * Idle states are differentiated between CPU and Cluster states
+ *
+ * r0 = cstate defines the CPU power state
+ * r1 = rstate defines the Cluster power state
+ * r2 = flags define what has to be saved
+ *
+ * C-STATE mapping
+ * 0 - run
+ * 1 - wfi (aka standby)
+ * 2 - dormant (not supported)
+ * 3 - shutdown
+ *
+ * R-STATE mapping
+ * 0 - run
+ * 1 - not supported
+ * 2 - L2 retention
+ * 3 - Off mode (every platform defines it, e.g. GIC power domain)
+ *
+ * Cluster low-power states might be hit if and only if all the CPUs making up
+ * the clusters are in some deep C-STATE
+ *
+ */
+
+ENTRY(cpu_enter_idle)
+	cmp	r0, #2		@ this function can replace the idle function
+	wfilt			@ in the processor struct. If targeted power
+	movlt	r0, #0		@ states are shallow ones it just executes wfi
+	movlt	pc, lr		@ and returns
+	cmp	r0, #3
+	cmpls	r1, #3
+	mvnhi	r0, #EINVAL
+	movhi	pc, lr
+	stmfd	sp!, {r4 - r12, lr}
+	stmfd	sp, {r0, r1}
+#ifdef CONFIG_SMP
+	adr	r0, platform_cpu_stacks
+	ALT_SMP(mrc p15, 0, r1, c0, c0, 5)
+	ALT_UP(mov r1, #0)
+	and	r1, r1, #15
+	str	sp, [r0, r1, lsl #2]	@ stack phys addr - save it for resume
+#else
+	str	sp, platform_cpu_stacks
+#endif
+	sub	sp, sp, #8
+	ldmfd	sp!,{r0, r1}
+	bl 	enter_idle
+	mov	r0, #0
+	ldmfd   sp!, {r4 - r12, pc}
+ENDPROC(cpu_enter_idle)
+
+/*
+ * This hook, though not strictly necessary, provides an entry point where, if
+ * needed, stack pointers can be switched in case it is needed to improve L2
+ * retention management (uncached stack).
+ */
+ENTRY(sr_save_context)
+	adr	r12, idle_save_context
+	ldr	r12, [r12]
+	bx	r12
+ENDPROC(sr_save_context)
+
+ENTRY(sr_reset_entry_point)
+	@ This is the entry point from the platform warm start code
+	@ It runs with MMU off straight from reset
+	setmode	PSR_I_BIT | PSR_F_BIT | SVC_MODE, r0 @ set SVC, irqs off
+#ifdef CONFIG_SMP
+	adr	r0, platform_cpu_nc_stacks
+	ALT_SMP(mrc p15, 0, r1, c0, c0, 5)
+	ALT_UP(mov r1, #0)
+	and	r1, r1, #15
+	ldr	r0, [r0, r1, lsl #2]		@ stack phys addr
+#else
+	ldr	r0, platform_cpu_nc_stacks	@ stack phys addr
+#endif
+	mov	sp, r0
+	adr	r0, idle_mt 	@ get phys address of main table and pass it on
+	ldr	r0, [r0]
+	ldr	lr, =return_from_idle
+	adr	r1, resume
+	ldr	r1, [r1]
+	bx	r1
+return_from_idle:
+	@ return to enter_idle caller, with success
+	mov	r0, #0
+	ldmfd	sp!, {r4 - r12, pc}	@ return from idle - registers saved in
+ENDPROC(sr_reset_entry_point)		@ cpu_enter_idle() are still there
+
+
+ENTRY(sr_restore_context)
+	add	lr, lr, #(PAGE_OFFSET - PLAT_PHYS_OFFSET)
+	stmfd	sp!, {r4, lr}
+	adr	r12, idle_restore_context
+	ldr	r12, [r12]
+	ldr	lr, =switch_stack
+	bx	r12
+switch_stack:
+	@ CPU context restored, time to switch to Linux stack and pop out
+#ifdef CONFIG_SMP
+	adr	r0, platform_cpu_stacks
+	ALT_SMP(mrc p15, 0, r1, c0, c0, 5)
+	ALT_UP(mov r1, #0)
+	and	r1, r1, #15
+	ldr	r0, [r0, r1, lsl #2]	@ top stack addr
+#else
+	ldr	r0, platform_cpu_stacks	@ top stack addr
+#endif
+	mov	r3, r0
+#ifdef CONFIG_SMP
+	adr	r0, platform_cpu_nc_stacks
+	ALT_SMP(mrc p15, 0, r1, c0, c0, 5)
+	ALT_UP(mov r1, #0)
+	and	r1, r1, #15
+	ldr	r0, [r0, r1, lsl #2]		@ non-cacheable stack phys addr
+#else
+	ldr	r0, platform_cpu_nc_stacks	@ non-cacheable stack phys addr
+#endif
+	sub	r2, r0, sp
+	sub	r0, r3, r2
+	mov	r1, sp
+	mov	r4, r0
+	bl	memcpy		@ copy stack used in resume to current stack
+	mov	sp, r4
+	bl 	cpu_init	@ init banked registers
+	ldmfd	sp!, {r4, pc}
+ENDPROC(sr_restore_context)
+
+idle_save_context:
+	.long 0
+idle_restore_context:
+	.long 0
+
+idle_mt:
+	.long main_table - PAGE_OFFSET + PLAT_PHYS_OFFSET
+
+resume:
+	.long exit_idle - PAGE_OFFSET + PLAT_PHYS_OFFSET
+
+platform_cpu_stacks:
+	.rept	CONFIG_NR_CPUS
+	.long	0				@ preserve stack phys ptr here
+	.endr
+
+platform_cpu_nc_stacks:
+	.rept	CONFIG_NR_CPUS
+	.long	0				@ preserve uncached
+						@ stack phys ptr here
+	.endr
+
+	.end

Signature · 1 line

1.7.4.4

Hi Lorenzo,

only a few comments at this stage.

The sr_entry.S code is both exclusively .arm (using conditionals and long-distance adr, i.e. not Thumb2-clean), and it uses post-armv5 instructions (like wfi). Same for the other *.S code in the patch series. It's non-generic assembly within arch/arch/kernel/, wouldn't one better place this into arch/arm/mm/...-v[67].S ?

Then, sr_suspend/sr_resume; these functions are "C-exported" and are directly calling cpu_do_suspend/do_resume to pass a supplied buffer; I've done that for one iteration of the hibernation patch, yes, but that was a bit sneaky and Russell stated then the interface is cpu_suspend/cpu_resume not the proc funcs directly. Unless _those_ have been changed they're also unsafe to call from C funcs (clobber all regs). Couldn't you simply use cpu_suspend/resume directly ?

How much memory do all the pagedirs require that are being kept around ? Why does each core need a separate one, what would happen to just use a single "identity table" for all ?
I understand you can't use swapper_pg_dir for idle, so a separate one has to be allocated, yet the question remains why per-cpu required ?

I'm currently transitioning between jobs; will re-subscribe to arm-kernel under a different email address soon, this one is likely to stop working in August. Sorry the inconvenience and high-latency responses till then :(

FrankH.

Quoted 10 lines · Frank Hofmann

Hi Lorenzo,

only a few comments at this stage.

The sr_entry.S code is both exclusively .arm (using conditionals and long-distance adr, i.e. not Thumb2-clean), and it uses post-armv5 instructions (like wfi). Same for the other *.S code in the patch series. It's non-generic assembly within arch/arch/kernel/, wouldn't one better place this into arch/arm/mm/...-v[67].S ?

Yes, it is certainly something I should improve to make it more generic.

Quoted 9 lines

Then, sr_suspend/sr_resume; these functions are "C-exported" and are directly calling cpu_do_suspend/do_resume to pass a supplied buffer; I've done that for one iteration of the hibernation patch, yes, but that was a bit sneaky and Russell stated then the interface is cpu_suspend/cpu_resume not the proc funcs directly. Unless _those_ have been changed they're also unsafe to call from C funcs (clobber all regs). Couldn't you simply use cpu_suspend/resume directly ?

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should. That's why I use cpu_do_{suspend,resume}, so that I can choose the memory used to save registers (uncached), but that's an abuse of API.

I agree this is sneaky, but I wanted to avoid duplicating code that just saves registers. Maybe moving to an uncached stack might solve this problem if we want to reuse cpu_suspend from cpu idle, which is still an open point.

Quoted 8 lines

How much memory do all the pagedirs require that are being kept around ? Why does each core need a separate one, what would happen to just use a single "identity table" for all ?
I understand you can't use swapper_pg_dir for idle, so a separate one has to be allocated, yet the question remains why per-cpu required ?

I just allocate a 1:1 mapping once cloned from init_mm, reused for all CPUs, with an additional mapping.

The array of pointers is there to save pgdir on idle entry, one per-cpu.

Quoted 7 lines

I'm currently transitioning between jobs; will re-subscribe to arm-kernel under a different email address soon, this one is likely to stop working in August. Sorry the inconvenience and high-latency responses till then :(

FrankH.

May I thank you very much for the review in the interim,

Lorenzo

Quoted 5 lines · Lorenzo Pieralisi

Quoted 4 lines · Frank Hofmann

Hi Lorenzo,

only a few comments at this stage.

Quoted 10 lines

Quoted 7 lines

How much memory do all the pagedirs require that are being kept around ? Why does each core need a separate one, what would happen to just use a single "identity table" for all ?
I understand you can't use swapper_pg_dir for idle, so a separate one has to be allocated, yet the question remains why per-cpu required ?

I just allocate a 1:1 mapping once cloned from init_mm, reused for all CPUs, with an additional mapping.

Have started to wonder whether this facility (a 1:1-mapped pagedir for kernel text/data, or maybe even "non-user text/data") could/should be made available on a global scale; after all, both kexec, reset, hibernate, some forms of idle/suspend do require "some sort of that" to go through MMU reinitialization. I'm unfortunately not deep enough in the VM subsystem to say how exactly this best would have to look like.

Merely mentioning this because it looks while everyone creates 1:1 mappings, there's ever so slight differences between how the 1:1 mapping(s) are created; we've seen:

(quoted diff) +0 -0

 	* (re)using current->active_mm->pgd
 	* (re)using swapper_pg_dir (different lengths for the 1:1 section)
 	* using a separately-allocated/initialized pgdir

Such proliferation usually means there's a justified need to do that kind of thing. Just the gritty details haven't been sorted how everyone with that need could do _the same_ thing instead of reinventing various square wheels.

The main reason why I've used swapper_pg_dir in the hibernation patch is because it's the only static one in the system (the hibernation hooks are in irqs-off codepaths and pgd_alloc isn't a good idea then), and happens to have "clear" lower sections (in the user area) so that one's not actually substituting anything when creating 1:1 mappings (and getting rid of them restores the "pristine" state). But this assumption only holds as long as swapper_pg_dir's use isn't changed. So a little creepy feeling remains.

A generic "reset_mmu_pg_dir", wouldn't that be a good thing to have ?

Quoted 1 line

The array of pointers is there to save pgdir on idle entry, one per-cpu.

If you're going through cpu_{do_}suspend/resume, the TTBRs are saved/restored anyway, what do you need to keep the virtual addresses around for ?

Quoted 7 lines

Quoted 7 lines

I'm currently transitioning between jobs; will re-subscribe to arm-kernel under a different email address soon, this one is likely to stop working in August. Sorry the inconvenience and high-latency responses till then :(

Quoted 3 lines

Quoted 1 line

FrankH.

May I thank you very much for the review in the interim,

You're welcome.
FrankH.

Quoted 6 lines

Quoted 1 line

The array of pointers is there to save pgdir on idle entry, one per-cpu.

If you're going through cpu_{do_}suspend/resume, the TTBRs are saved/restored anyway, what do you need to keep the virtual addresses around for ?

Because I switch mm before calling suspend, which is called with a cloned pgdir. I am not sure I can avoid that.

On Mon, 11 Jul 2011, Lorenzo Pieralisi wrote:

[ ... ]

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

The array of pointers is there to save pgdir on idle entry, one per-cpu.

If you're going through cpu_{do_}suspend/resume, the TTBRs are saved/restored anyway, what do you need to keep the virtual addresses around for ?

Quoted 2 lines

Because I switch mm before calling suspend, which is called with a cloned pgdir. I am not sure I can avoid that.

On resume, you'll be restoring the same thread as was previously running, right ? If so, all you do there is copying current->active_mm->pgd to some other place ?

Also, if you'd be using cpu_suspend(), would there still be a need for cpu_switch_mm() before ? It'd rather be a case of possibly calling that before the MMU-off sequence / cpu_resume() ?

Or is it that you use the new pgdir to make a memory region uncacheable ?

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. ?Please explain something to me here. ?You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. ?Why is this the case?

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

However, lets recap. ?What we do in cpu_suspend() in order is:

- Save the ABI registers onto the stack, and some private state
- Save the CPU specific state onto the stack
- Flush the L1 cache
- Call the platform specific suspend finisher

On resume, with the MMU and caches off:

- Platform defined entry point is called, which _may_ be cpu_resume ?directly.
- Platform initial code is executed to do whatever that requires
- cpu_resume will be called
- cpu_resume loads the previously saved private state
- The CPU specific state is restored
- Page table is modified to permit 1:1 mapping for MMU enable
- MMU is enabled with caches disabled
- Page table modification is undone
- Caches are enabled in the main control register
- CPU exception mode stacks are reinitialized
- CPU specific init function is called
- ABI registers are popped off and 'cpu_suspend' function returns

So, as far as L2 goes, in the suspend finisher:

- If L2 state is lost, the finisher needs to clean dirty data from L2 ?to ensure that it is preserved in RAM.
?Note: There is no need to disable or even invalidate the L2 cache as ?we should not be writing any data in the finisher function which we ?later need after resume.

- If L2 state is not lost, the finisher needs to clean the saved state ?as a minimum, to sure that this is visible when the main control register ?C bit is clear. ?The easiest way to do that is to find the top of stack ?via current_thread_info() - we have a macro for that, and then add ?THREAD_SIZE to find the top of stack. ?'sp' will be the current bottom ?of stack.

In the resume initial code:

- If L2 state was lost, the L2 configuration needs to be restored. ?This generally needs to happen before cpu_resume is called:
?- there are CPUs which need L2 setup before the MMU is enabled. ?- OMAP3 currently does this in its assembly, which is convenient to ? ?allow it to make the SMI calls to the secure world. ?The same will ? ?be true of any CPU running in non-secure mode.

- If L2 state was not lost, and the platform choses not to clean and ?invalidate the ABI registers from the stack, and the platform restores ?the L2 configuration before calling cpu_resume, then the ABI registers ?will be read out of L2 on return if that's where they are - at that ?point everything will be setup correctly.

?This will give the greatest performance, which is important for CPU ?idle use of these code paths.

Now, considering SMP, there's an issue here: do we know at the point where one CPU goes down whether L2 state will be lost?

If the answer is that state will not be lost, we can do the minimum. If all L2 state is lost, we need to do as above. ?If we don't know the answer, then we have to assume that L2 state will be lost.

But wait - L2 cache (or more accurately, the outer cache) is common between CPUs in a SMP system. ?So, if we're _not_ the last CPU to go down, then we assume that L2 state will not be lost. ?It is the last CPUs responsibility to deal with L2 state when it goes into a PM mode that results in L2 state being lost.

Lastly, should generic code deal with flushing L2 and setting it back up on resume? ?A couple of points there:

1. Will generic code know whether L2 state will be lost, or should it ? assume that L2 state is always lost and do a full L2 flush. ?That ? seems wasteful if we have another CPU still running (which would ? also have to flush L2.)

2. L2 configuration registers are not accessible to CPUs operating in ? non-secure mode like OMAPs. ?Generic code on these CPUs has _no_ ? way to restore and re-enable the L2 cache. ?It needs to make ? implementation specific SMI calls to achieve that.

So, I believe the answer to that is no. ?However, I think we can still do a change to improve the situation:

1. Pass in r2 and r3 to the suspend finisher the bottom and top of the ? stack which needs to be cleaned from L2. ?This covers the saved ? state but not the ABI registers.

2. Mandate that L2 configuration is to be restored by platforms in their ? pre-cpu_resume code so L2 is available when the C bit is set.

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. ?Please explain something to me here. ?You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. ?Why is this the case?

Quoted 2 lines

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

OMAP is very different, because it doesn't use cpu_suspend. It saves it's state to SAR ram, which is mapped uncached, which avoids L2 problems.

The sleep_save_sp location also needs to be cleaned.

Quoted 18 lines

In the resume initial code:

- If L2 state was lost, the L2 configuration needs to be restored. ?This generally needs to happen before cpu_resume is called:
?- there are CPUs which need L2 setup before the MMU is enabled. ?- OMAP3 currently does this in its assembly, which is convenient to ? ?allow it to make the SMI calls to the secure world. ?The same will ? ?be true of any CPU running in non-secure mode.

- If L2 state was not lost, and the platform choses not to clean and ?invalidate the ABI registers from the stack, and the platform restores ?the L2 configuration before calling cpu_resume, then the ABI registers ?will be read out of L2 on return if that's where they are - at that ?point everything will be setup correctly.

?This will give the greatest performance, which is important for CPU ?idle use of these code paths.

Quoted 2 lines

Now, considering SMP, there's an issue here: do we know at the point where one CPU goes down whether L2 state will be lost?

CPU idle will need a voting mechanism between the two CPUs, and the second CPU to go down needs to determine the minimum state supported by both CPUs. Any time a CPU goes down, we should know for sure that either the L2 will not be lost, or the L2 might be lost if the other CPU goes idle.

In practice, I don't think many SoCs will support low power modes that lose the L2 during idle, only during suspend. Tegra and OMAP4 both don't support the modes where the L2 is powered down in idle.

Quoted 13 lines · Colin Cross

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. ?Please explain something to me here. ?You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. ?Why is this the case?

Quoted 6 lines

Quoted 2 lines

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

OMAP is very different, because it doesn't use cpu_suspend. ?It saves it's state to SAR ram, which is mapped uncached, which avoids L2 problems.

I'm afraid your information is out of date. ?See:

http://ftp.arm.linux.org.uk/git/?p=linux-2.6-arm.git;a=commitdiff;h=076f2cc449188b7d3d4866730afa3ac7be3e6640;hp=2637ce30e145557bf89ebcf35b2d78e729e16e5a

?arch/arm/mach-omap2/pm34xx.c ? ?| ? 47 +++---------- ?arch/arm/mach-omap2/sleep34xx.S | ?143 +-------------------------------------- ?2 files changed, 13 insertions(+), 177 deletions(-)

for the trivial conversion, and:

http://ftp.arm.linux.org.uk/git/?p=linux-2.6-arm.git;a=commit;h=46e130d298a384b677426e19faec311749ff2677

?arch/arm/mach-omap2/pm.h ? ? ? ?| ? 20 ++-
?arch/arm/mach-omap2/pm34xx.c ? ?| ? 20 ++--
?arch/arm/mach-omap2/sleep34xx.S | ?303 ++++++++++++++++++++++----------------
?arch/arm/plat-omap/sram.c ? ? ? | ? 15 +--

?4 files changed, 206 insertions(+), 152 deletions(-)

for the cleanup of the SRAM code, most of which was found not to need being in SRAM. ?That's all tested as working (see the tested-by's) on a range of OMAP3 platforms.

The whole series is at:

http://ftp.arm.linux.org.uk/git/?p=linux-2.6-arm.git;a=shortlog;h=refs/heads/suspend

There is only one currently merged SoC which hasn't been converted:
shmobile. ?That's in progress, and there may already be patches for it. There's no blockers on that as far as I know other than availability of time.

Quoted 1 line

The sleep_save_sp location also needs to be cleaned.

Right.

Quoted 18 lines

Quoted 18 lines

In the resume initial code:

- If L2 state was lost, the L2 configuration needs to be restored. ?This generally needs to happen before cpu_resume is called:
?- there are CPUs which need L2 setup before the MMU is enabled. ?- OMAP3 currently does this in its assembly, which is convenient to ? ?allow it to make the SMI calls to the secure world. ?The same will ? ?be true of any CPU running in non-secure mode.

- If L2 state was not lost, and the platform choses not to clean and ?invalidate the ABI registers from the stack, and the platform restores ?the L2 configuration before calling cpu_resume, then the ABI registers ?will be read out of L2 on return if that's where they are - at that ?point everything will be setup correctly.

?This will give the greatest performance, which is important for CPU ?idle use of these code paths.

Quoted 8 lines

Quoted 2 lines

Now, considering SMP, there's an issue here: do we know at the point where one CPU goes down whether L2 state will be lost?

CPU idle will need a voting mechanism between the two CPUs, and the second CPU to go down needs to determine the minimum state supported by both CPUs. ?Any time a CPU goes down, we should know for sure that either the L2 will not be lost, or the L2 might be lost if the other CPU goes idle.

No it doesn't. ?As I've said: L2 is shared between the two CPUs.

If L2 state is preserved, then the only thing that needs cleaning from L2 is the state which must be accessible to bring the system back up, which is the CPU specific state, the cpu_suspend state and the sleep_save_sp store.

If L2 state is lost, the _first_ CPU to go down in a two-CPU system must do as above. ?If nothing else happens L2 state will be preserved. So there's absolutely no point what so ever the first CPU cleaning the entire L2 state - the second CPU will still be scribbling into L2 at that point.

However, when the second CPU goes down, that's when the L2 state would need to be preserved if the L2.

So, let's recap. ?In a two CPU system, when precisely one CPU goes into sleep mode, only the minimum L2 state needs cleaning. ?Nothing else. Only when the second CPU goes down does it matter whether L2 state needs to be preserved by cleaning or not because that is when L2 data loss may occur.

Quoted 3 lines

In practice, I don't think many SoCs will support low power modes that lose the L2 during idle, only during suspend. ?Tegra and OMAP4 both don't support the modes where the L2 is powered down in idle.

That's good, that means less to worry about. ?For idle, all we need to care about is the CPU specific state, the cpu_suspend state and the sleep_save_sp store. ?Great, that means things can be even simpler - cpu_suspend() just needs to know whether we're calling it because of idle, or whether we're calling it because of system suspend.

Quoted 13 lines · Colin Cross

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case?

Quoted 6 lines

Quoted 2 lines

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

OMAP is very different, because it doesn't use cpu_suspend. It saves it's state to SAR ram, which is mapped uncached, which avoids L2 problems.

I'm afraid your information is out of date. See:

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case?

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

However, lets recap. What we do in cpu_suspend() in order is:

- Save the ABI registers onto the stack, and some private state
- Save the CPU specific state onto the stack

- Flush the L1 cache
- Call the platform specific suspend finisher

On resume, with the MMU and caches off:

- Platform defined entry point is called, which _may_ be cpu_resume directly.
- Platform initial code is executed to do whatever that requires
- cpu_resume will be called
- cpu_resume loads the previously saved private state
- The CPU specific state is restored
- Page table is modified to permit 1:1 mapping for MMU enable

- MMU is enabled with caches disabled
- Page table modification is undone
- Caches are enabled in the main control register
- CPU exception mode stacks are reinitialized
- CPU specific init function is called
- ABI registers are popped off and 'cpu_suspend' function returns

So, as far as L2 goes, in the suspend finisher:

- If L2 state is lost, the finisher needs to clean dirty data from L2 to ensure that it is preserved in RAM.
Note: There is no need to disable or even invalidate the L2 cache as we should not be writing any data in the finisher function which we later need after resume.

- If L2 state is not lost, the finisher needs to clean the saved state as a minimum, to sure that this is visible when the main control register C bit is clear. The easiest way to do that is to find the top of stack via current_thread_info() - we have a macro for that, and then add THREAD_SIZE to find the top of stack. 'sp' will be the current bottom of stack.

In the resume initial code:

- If L2 state was lost, the L2 configuration needs to be restored. This generally needs to happen before cpu_resume is called:
- there are CPUs which need L2 setup before the MMU is enabled.
- OMAP3 currently does this in its assembly, which is convenient to allow it to make the SMI calls to the secure world. The same will be true of any CPU running in non-secure mode.

- If L2 state was not lost, and the platform choses not to clean and invalidate the ABI registers from the stack, and the platform restores the L2 configuration before calling cpu_resume, then the ABI registers will be read out of L2 on return if that's where they are - at that point everything will be setup correctly.

This will give the greatest performance, which is important for CPU idle use of these code paths.

Now, considering SMP, there's an issue here: do we know at the point where one CPU goes down whether L2 state will be lost?

If the answer is that state will not be lost, we can do the minimum. If all L2 state is lost, we need to do as above. If we don't know the answer, then we have to assume that L2 state will be lost.

But wait - L2 cache (or more accurately, the outer cache) is common between CPUs in a SMP system. So, if we're _not_ the last CPU to go down, then we assume that L2 state will not be lost. It is the last CPUs responsibility to deal with L2 state when it goes into a PM mode that results in L2 state being lost.

Lastly, should generic code deal with flushing L2 and setting it back up on resume? A couple of points there:

1. Will generic code know whether L2 state will be lost, or should it assume that L2 state is always lost and do a full L2 flush. That seems wasteful if we have another CPU still running (which would also have to flush L2.)

2. L2 configuration registers are not accessible to CPUs operating in non-secure mode like OMAPs. Generic code on these CPUs has _no_ way to restore and re-enable the L2 cache. It needs to make implementation specific SMI calls to achieve that.

So, I believe the answer to that is no. However, I think we can still do a change to improve the situation:

1. Pass in r2 and r3 to the suspend finisher the bottom and top of the stack which needs to be cleaned from L2. This covers the saved state but not the ABI registers.

2. Mandate that L2 configuration is to be restored by platforms in their pre-cpu_resume code so L2 is available when the C bit is set.

(Just to add few more points on top of what Colin already commented)

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case?

Quoted 3 lines

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

This part is not done yet Russell. Infact the cpu_resume() function need an update to work with L2 enable configuration.

Quoted 4 lines

However, lets recap. What we do in cpu_suspend() in order is:

- Save the ABI registers onto the stack, and some private state
- Save the CPU specific state onto the stack

We need to disable C bit here to avoid any speculative artifacts during further operations before WFI.

Quoted 2 lines

- Flush the L1 cache
- Call the platform specific suspend finisher

Also finisher function should issue the WFI and just in case for some reason CPU doesn't hit the targeted low power state, finisher function takes care of things like enabling C bit, SMP bit etc.

Quoted 9 lines

On resume, with the MMU and caches off:

- Platform defined entry point is called, which _may_ be cpu_resume directly.
- Platform initial code is executed to do whatever that requires
- cpu_resume will be called
- cpu_resume loads the previously saved private state
- The CPU specific state is restored
- Page table is modified to permit 1:1 mapping for MMU enable

1:1 idmap used here should be mapped as non-cached to avoid L2 related issues. I faced similar issue in OMAP sleep code earlier and later thanks to Colin, we got the fixed my making use of non-cached idmap.

Quoted 14 lines · Santosh Shilimkar

(Just to add few more points on top of what Colin already commented)

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case?

Quoted 5 lines

Quoted 3 lines

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

This part is not done yet Russell. Infact the cpu_resume() function need an update to work with L2 enable configuration.

The code does deal with L2 cache enable in the resume path...

Quoted 6 lines

Quoted 4 lines

However, lets recap. What we do in cpu_suspend() in order is:

- Save the ABI registers onto the stack, and some private state
- Save the CPU specific state onto the stack

We need to disable C bit here to avoid any speculative artifacts during further operations before WFI.

Which you are doing.

Quoted 2 lines

Quoted 2 lines

- Flush the L1 cache
- Call the platform specific suspend finisher

Quoted 4 lines

Also finisher function should issue the WFI and just in case for some reason CPU doesn't hit the targeted low power state, finisher function takes care of things like enabling C bit, SMP bit etc.

You're restoring the C bit in the non-off paths already which follow a failed WFI. You're not touching the SMP bit there though - was it ever reset? I don't think so.

Quoted 9 lines

Quoted 9 lines

On resume, with the MMU and caches off:

- Platform defined entry point is called, which _may_ be cpu_resume directly.
- Platform initial code is executed to do whatever that requires
- cpu_resume will be called
- cpu_resume loads the previously saved private state
- The CPU specific state is restored
- Page table is modified to permit 1:1 mapping for MMU enable

Quoted 3 lines

1:1 idmap used here should be mapped as non-cached to avoid L2 related issues. I faced similar issue in OMAP sleep code earlier and later thanks to Colin, we got the fixed my making use of non-cached idmap.

It is specified that if the main control register C bit is clear, accesses will be uncached.

Whether you get cache hits or not is probably implementation dependent, and provided that the state information is cleaned from the L2 cache, it doesn't matter if we hit the L2 cached copy or the RAM copy. It's the same data.

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case?

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

However, lets recap. What we do in cpu_suspend() in order is:

- Save the ABI registers onto the stack, and some private state
- Save the CPU specific state onto the stack

- Flush the L1 cache
- Call the platform specific suspend finisher

On resume, with the MMU and caches off:

- Platform defined entry point is called, which _may_ be cpu_resume directly.
- Platform initial code is executed to do whatever that requires
- cpu_resume will be called
- cpu_resume loads the previously saved private state
- The CPU specific state is restored
- Page table is modified to permit 1:1 mapping for MMU enable
- MMU is enabled with caches disabled
- Page table modification is undone
- Caches are enabled in the main control register
- CPU exception mode stacks are reinitialized
- CPU specific init function is called
- ABI registers are popped off and 'cpu_suspend' function returns

So, as far as L2 goes, in the suspend finisher:

- If L2 state is lost, the finisher needs to clean dirty data from L2 to ensure that it is preserved in RAM.
Note: There is no need to disable or even invalidate the L2 cache as we should not be writing any data in the finisher function which we later need after resume.

- If L2 state is not lost, the finisher needs to clean the saved state as a minimum, to sure that this is visible when the main control register C bit is clear. The easiest way to do that is to find the top of stack via current_thread_info() - we have a macro for that, and then add THREAD_SIZE to find the top of stack. 'sp' will be the current bottom of stack.

In the resume initial code:

- If L2 state was lost, the L2 configuration needs to be restored. This generally needs to happen before cpu_resume is called:
- there are CPUs which need L2 setup before the MMU is enabled.
- OMAP3 currently does this in its assembly, which is convenient to allow it to make the SMI calls to the secure world. The same will be true of any CPU running in non-secure mode.

- If L2 state was not lost, and the platform choses not to clean and invalidate the ABI registers from the stack, and the platform restores the L2 configuration before calling cpu_resume, then the ABI registers will be read out of L2 on return if that's where they are - at that point everything will be setup correctly.

This will give the greatest performance, which is important for CPU idle use of these code paths.

Now, considering SMP, there's an issue here: do we know at the point where one CPU goes down whether L2 state will be lost?

If the answer is that state will not be lost, we can do the minimum. If all L2 state is lost, we need to do as above. If we don't know the answer, then we have to assume that L2 state will be lost.

But wait - L2 cache (or more accurately, the outer cache) is common between CPUs in a SMP system. So, if we're _not_ the last CPU to go down, then we assume that L2 state will not be lost. It is the last CPUs responsibility to deal with L2 state when it goes into a PM mode that results in L2 state being lost.

Lastly, should generic code deal with flushing L2 and setting it back up on resume? A couple of points there:

1. Will generic code know whether L2 state will be lost, or should it assume that L2 state is always lost and do a full L2 flush. That seems wasteful if we have another CPU still running (which would also have to flush L2.)

2. L2 configuration registers are not accessible to CPUs operating in non-secure mode like OMAPs. Generic code on these CPUs has _no_ way to restore and re-enable the L2 cache. It needs to make implementation specific SMI calls to achieve that.

So, I believe the answer to that is no. However, I think we can still do a change to improve the situation:

1. Pass in r2 and r3 to the suspend finisher the bottom and top of the stack which needs to be cleaned from L2. This covers the saved state but not the ABI registers.

2. Mandate that L2 configuration is to be restored by platforms in their pre-cpu_resume code so L2 is available when the C bit is set.

Thank you very much Russell for this recap.

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Lorenzo Pieralisi

Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should.

Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case?

Quoted 3 lines

OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues.

OMAP4, it is SMP configs I am talking about.

arch/arm/kernel/sr_api.c line 67

Quoted 5 lines · Lorenzo Pieralisi

arch/arm/kernel/sr_api.c new file +5 -0

+static int late_init(void)
+{
+	int rc;
+	struct sr_cluster *cluster;
+	int cluster_index, cpu_index = sr_platform_get_cpu_index();

Stop this madness, and use the standard linux APIs like smp_processor_id here. It might actually help you find bugs, like the fact that you're in a preemptible context here, and so could be rescheduled onto any other CPU in the system _after_ you've read the MPIDR register.

arch/arm/kernel/sr_api.c line 76

Quoted 9 lines

arch/arm/kernel/sr_api.c new file +9 -0

+
+	cluster_index = sr_platform_get_cluster_index();
+	cluster = main_table.cluster_table + cluster_index;
+	main_table.os_mmu_context[cluster_index][cpu_index] =
+				current->active_mm->pgd;
+	cpu_switch_mm(main_table.fw_mmu_context, current->active_mm);
+	rc = sr_platform_init();
+	cpu_switch_mm(main_table.os_mmu_context[cluster_index][cpu_index],
+				current->active_mm);

CPU numbers are unique in the system, why do you need a 'cluster_index' to save this? In fact why do you even need to save it in a structure at all?

Plus, "cluster" is not used, please get rid of it.

Quoted 12 lines · Russell King - ARM Linux

arch/arm/kernel/sr_api.c line 67

Quoted 5 lines · Lorenzo Pieralisi

arch/arm/kernel/sr_api.c new file +5 -0

+static int late_init(void)
+{
+	int rc;
+	struct sr_cluster *cluster;
+	int cluster_index, cpu_index = sr_platform_get_cpu_index();

Stop this madness, and use the standard linux APIs like smp_processor_id here. It might actually help you find bugs, like the fact that you're in a preemptible context here, and so could be rescheduled onto any other CPU in the system _after_ you've read the MPIDR register.

Quoted 14 lines

arch/arm/kernel/sr_api.c line 76

Quoted 9 lines

arch/arm/kernel/sr_api.c new file +9 -0

+
+	cluster_index = sr_platform_get_cluster_index();
+	cluster = main_table.cluster_table + cluster_index;
+	main_table.os_mmu_context[cluster_index][cpu_index] =
+				current->active_mm->pgd;
+	cpu_switch_mm(main_table.fw_mmu_context, current->active_mm);
+	rc = sr_platform_init();
+	cpu_switch_mm(main_table.os_mmu_context[cluster_index][cpu_index],
+				current->active_mm);

CPU numbers are unique in the system, why do you need a 'cluster_index' to save this? In fact why do you even need to save it in a structure at all?

Quoted 1 line

Plus, "cluster" is not used, please get rid of it.

Oh, that's another thing. This thread is about introducing idle support not cluster support. Cluster support is surely something different (esp. as it seems from the above code that you're trying to support several clusters of MPCore CPUs, each with physical 0-N CPU numbers.)

Cluster support should be an entirely separate patch series.

In any case, smp_processor_id() will be (and must be) unique in any given running kernel across all CPUs, even if you have clusters of N CPUs all physically numbered 0-N.

This patch provides the code infrastructure needed to maintain a generic per-cpu architecture implementation of idle code.

sr_platform.c :
- code manages patchset initialization and memory management

sr_context.c:
- code initializes run-time context save/restore generic support

sr_power.c:
- provides the generic infrastructure to enter exit low power modes and communicate with Power Control Unit (PCU)

v7 support hinges on the basic infrastructure to provide per-cpu arch implementation basically through standard function pointers signatures.

Preprocessor defines include size of data needed to save/restore L2 state. This define value should be moved to the respective subsystem (PL310) once the patchset IF to that subsystem is settled.

Signed-off-by Lorenzo Pieralisi<[email protected]>

---

arch/arm/kernel/sr_helpers.h new file +34 -0

new file mode 100644
index 0000000..1ae3a9a
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2008-2011 ARM Limited
+ * Author(s): Jon Callan, Lorenzo Pieralisi
+ *
+ * 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.
+ *
+ */
+
+static inline int sr_platform_get_cpu_index(void)
+{
+	unsigned int cpu;
+	__asm__ __volatile__(
+			"mrc	p15, 0, %0, c0, c0, 5\n\t"
+			: "=r" (cpu));
+	return cpu&  0xf;
+}
+
+/*
+ * Placeholder for further extensions
+ */
+static inline int sr_platform_get_cluster_index(void)
+{
+	return 0;
+}
+
+static inline void __iomem *sr_platform_cbar(void)
+{
+	void __iomem *base;
+	__asm__ __volatile__(
+			"mrc	p15, 4, %0, c15, c0, 0\n\t"
+			: "=r" (base));
+	return base;

Quoted 24 lines · Lorenzo Pieralisi

This patch provides the code infrastructure needed to maintain a generic per-cpu architecture implementation of idle code.

sr_platform.c :
- code manages patchset initialization and memory management

sr_context.c:
- code initializes run-time context save/restore generic support

sr_power.c:
- provides the generic infrastructure to enter exit low power modes and communicate with Power Control Unit (PCU)

v7 support hinges on the basic infrastructure to provide per-cpu arch implementation basically through standard function pointers signatures.

Preprocessor defines include size of data needed to save/restore L2 state. This define value should be moved to the respective subsystem (PL310) once the patchset IF to that subsystem is settled.

Signed-off-by Lorenzo Pieralisi<[email protected]>

---

[...]

Quoted 43 lines

arch/arm/kernel/sr_helpers.h new file +37 -0

new file mode 100644
index 0000000..1ae3a9a
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2008-2011 ARM Limited
+ * Author(s): Jon Callan, Lorenzo Pieralisi
+ *
+ * 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.
+ *
+ */
+
+static inline int sr_platform_get_cpu_index(void)
+{
+	unsigned int cpu;
+	__asm__ __volatile__(
+			"mrc	p15, 0, %0, c0, c0, 5\n\t"
+			: "=r" (cpu));
+	return cpu&  0xf;
+}
+
+/*
+ * Placeholder for further extensions
+ */
+static inline int sr_platform_get_cluster_index(void)
+{
+	return 0;
+}
+
+static inline void __iomem *sr_platform_cbar(void)
+{
+	void __iomem *base;
+	__asm__ __volatile__(
+			"mrc	p15, 4, %0, c15, c0, 0\n\t"
+			: "=r" (base));
+	return base;
+}
+
+#ifdef CONFIG_SMP

arch/arm/kernel/sr_helpers.h line 44

Quoted 7 lines

arch/arm/kernel/sr_helpers.h new file +7 -0

+static inline void exit_coherency(void)
+{
+	unsigned int v;
+	asm volatile (
+		"mrc	p15, 0, %0, c1, c0, 1\n"
+		"bic	%0, %0, %1\n"
+		"mcr	p15, 0, %0, c1, c0, 1\n"

You should have a isb here.

arch/arm/kernel/sr_helpers.h line 48

Quoted 4 lines

arch/arm/kernel/sr_helpers.h new file +4 -0

+		 : "=&r" (v)
+		 : "Ir" (0x40)
+		 : );
+}

To avoid aborts on platform which doesn't provide
access to SMP bit, NSACR bit 18 should be read.
Something like....

   mrc     p15, 0, r0, c1, c1, 2
   tst     r0, #(1 << 18)
   mrcne   p15, 0, r0, c1, c0, 1
   bicne   r0, r0, #(1 << 6)
   mcrne   p15, 0, r0, c1, c0, 1