ARM: mm: add L2 suspend/resume support

As a follow up to the last L2 resume patch on alkml:

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

please have a look at this patchset and merge the required bits if needed, in particular MMU off for L2 retention use case.

Description:

In order to support the deepest C-states on all ARM platforms, code to resume L2 state could be added to the kernel to avoid reliance on bootloaders.

If L2 RAM is retained on power down, it is not cleaned on shutdown to preserve its content and improve performance. Hence, L2 state should be restored before the MMU is turned on, so that CPU resume code can search the data saved on power down in the L2 cache.

This patch addresses this issue and extends outercache and l2x0 driver to support L2 resume in a platform independent way, security issues notwithstanding that must be tackled separately.

Since the L2 should be enabled when the MMU is possibly off, L2 physical address should be passed from platform code or retrieved from the device tree so that the code can be made generic and platform independent. For that purpose, the signature of L2 init function has to be upgraded.

The resume code should not use the stack and it must fetch data using program counter relative loads so that it does not matter if it is run in physical or virtual address space.

This patch depends on three patches:

ARM: 7090/1: CACHE-L2X0: filter start address can be 0 and is often 0
http://www.spinics.net/lists/arm-kernel/msg140126.html
available in rmk/for-next commit 2afda86d91b5f15a744182d7ddacf68f6a6054c9

ARM: 7080/1: l2x0: make sure I&D are not locked down on init
http://www.spinics.net/lists/arm-kernel/msg139119.html
available in rmk/for-next commit bac7e6ecf60933b68af910eb4c83a775a8b20b19

ARM: 7009/1: l2x0: Add OF based initialization
http://www.spinics.net/lists/arm-kernel/msg131123.htm
available in rmk/for-next commit 41c86ff5be44e26978282f86c20598181b999142

Platform code initializing L2 has not been patched (l2x0_init requires a new parameter which is the physical base address) yet, waiting to see if there are platforms that can take advantage of this code.

Tested on Samsung Origen within suspend and cpuidle code paths. DT support compile tested.

Lorenzo Pieralisi (3):
ARM: mm: add outercache resume hook
ARM: mm: add l2x0 physical address parameter to init
ARM: mm: add l2x0 suspend/resume support

 arch/arm/include/asm/hardware/cache-l2x0.h |   24 +++++-
 arch/arm/include/asm/outercache.h          |    7 ++
 arch/arm/kernel/asm-offsets.c              |   12 +++
 arch/arm/mm/Makefile                       |    3 +
 arch/arm/mm/cache-l2x0.c                   |   17 +++-
 arch/arm/mm/l2x0-sleep.S                   |  136 ++++++++++++++++++++++++++++
 6 files changed, 193 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/mm/l2x0-sleep.S

1.7.4.4

In order to support L2 resume functions in a generic way in the kernel, this patch adds a resume hook to the outercache layer and a simple wrapper to simplify the function pointer check and the resume function call.

---
 arch/arm/include/asm/outercache.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

index d838743..1b70d86 100644
@@ -34,6 +34,7 @@ struct outer_cache_fns {
 	void (*sync)(void);
 #endif
 	void (*set_debug)(unsigned long);
+	void (*resume)(void);
 };
 
 #ifdef CONFIG_OUTER_CACHE

@@ -74,6 +75,11 @@ static inline void outer_disable(void)
 		outer_cache.disable();
 }
 
+static inline void outer_resume(void)
+{
+	if (outer_cache.resume)
+		outer_cache.resume();
+}
 #else
 
 static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)

@@ -86,6 +92,7 @@ static inline void outer_flush_all(void) { }
 static inline void outer_inv_all(void) { }
 static inline void outer_disable(void) { }
 
+static inline void outer_resume(void) { }
 #endif
 
 #ifdef CONFIG_OUTER_CACHE_SYNC

1.7.4.4

This patch redefines the l2x0_init function, and it adds a parameter defining the L2 physical base address in preparation for L2 resume support. The device tree init code retrieves the physical address from the "reg" array and converts it to a physical address pointer.

---
 arch/arm/include/asm/hardware/cache-l2x0.h |    3 ++-
 arch/arm/mm/cache-l2x0.c                   |   12 ++++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

index c48cb1e..54bf625 100644
@@ -94,7 +94,8 @@
 #define L2X0_ADDR_FILTER_EN		1
 
 #ifndef __ASSEMBLY__
-extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
+extern void __init l2x0_init(void __iomem *base, void __iomem *pbase,
+		__u32 aux_val, __u32 aux_mask);
 extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
 #endif
 

index 0d85d22..46a507a 100644
@@ -299,7 +299,8 @@ static void __init l2x0_unlock(__u32 cache_id)
 	}
 }
 
-void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
+void __init l2x0_init(void __iomem *base, void __iomem *pbase,
+			__u32 aux_val, __u32 aux_mask)
 {
 	__u32 aux;
 	__u32 cache_id;

@@ -455,6 +456,9 @@ static const struct of_device_id l2x0_ids[] __initconst = {
 int __init l2x0_of_init(__u32 aux_val, __u32 aux_mask)
 {
 	struct device_node *np;
+	u32 reg[2];
+	int ret;
+
 	void (*l2_setup)(const struct device_node *np,
 		__u32 *aux_val, __u32 *aux_mask);
 

@@ -465,13 +469,17 @@ int __init l2x0_of_init(__u32 aux_val, __u32 aux_mask)
 	if (!l2x0_base)
 		return -ENOMEM;
 
+	ret = of_property_read_u32_array(np, "reg", reg, ARRAY_SIZE(reg));
+	if (ret < 0)
+		return ret;
+
 	/* L2 configuration can only be changed if the cache is disabled */
 	if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
 		l2_setup = of_match_node(l2x0_ids, np)->data;
 		if (l2_setup)
 			l2_setup(np, &aux_val, &aux_mask);
 	}
-	l2x0_init(l2x0_base, aux_val, aux_mask);
+	l2x0_init(l2x0_base, (void __iomem *)reg[0], aux_val, aux_mask);
 	return 0;
 }
 #endif

1.7.4.4

I've no idea why many people have a really dire time understanding the following basic fact. I keep seeing the same things: virtual addresses as integers and physical addresses as pointers.

Virtual addresses are pointers.
Physical addresses are NOT pointers but integers.

Why? You CAN'T dereference a physical address when running in the virtual address space (which is the space which the kernel runs in.)

Stop doing it.

Hi Russell,

thanks for looking at this series.

I should not have converted it to a pointer, you are right, sorry; I am not using it as such unless the MMU is off.

Well, to be precise the kernel runs in it when the MMU is on.

By no means, the purpose is NOT to dereference it when the MMU is on and I am not doing that.

But I agree the physical address should be an integer parameter and be passed as so, lest someone might want for whatever reason to dereference it when the MMU is on and unleash hell.

Thank you,
Lorenzo

When suspend to RAM or cpuidle require the system to enter the deepest C states, L2 cache logic can be lost.
This patch adds assembly hooks that allow to restore the context for l2x0 series of L2 controllers upon system resume.

Context is saved once for all at boot time, along with the L2 physical address and cache type.
The resume code can be called with MMU either on or off and it executes specific code accordingly, by checking the MMU status in the SCTRL system register.

Code is in place to check if L2 is already enabled on resume to avoid writing L2 registers that would cause faults.

The resume hook avoids using the stack since it might be called before the C environment is up and running and fetches data using program counter relative addressing so that it can be run both with MMU on or off to simplify its adoption.

---
 arch/arm/include/asm/hardware/cache-l2x0.h |   21 ++++-
 arch/arm/kernel/asm-offsets.c              |   12 +++
 arch/arm/mm/Makefile                       |    3 +
 arch/arm/mm/cache-l2x0.c                   |    5 +-
 arch/arm/mm/l2x0-sleep.S                   |  136 ++++++++++++++++++++++++++++
 5 files changed, 174 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mm/l2x0-sleep.S

index 54bf625..05312eb 100644
@@ -97,6 +97,25 @@
 extern void __init l2x0_init(void __iomem *base, void __iomem *pbase,
 		__u32 aux_val, __u32 aux_mask);
 extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
-#endif
 
+#ifndef CONFIG_PM_SLEEP
+static inline void l2x0_resume(void) {}
+static inline void l2x0_save_context(void __iomem *pbase, __u32 cache_id) {}
+#else
+struct l2x0_regs {
+	__u32 aux_ctrl;
+	__u32 tag_latency;
+	__u32 data_latency;
+	__u32 afilter_start;
+	__u32 afilter_end;
+	__u32 debug_ctrl;
+	__u32 prefetch_ctrl;
+	__u32 power_ctrl;
+} __packed;
+
+extern struct l2x0_regs l2x0_data;
+extern void l2x0_resume(void);
+extern void l2x0_save_context(void __iomem *pbase, __u32 cache_id);
+#endif /* CONFIG_PM_SLEEP */
+#endif /* __ASSEMBLY__ */
 #endif

index 16baba2..91d7b7b 100644
@@ -20,6 +20,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
 #include <linux/kbuild.h>
 
 /*

@@ -128,6 +129,17 @@ int main(void)
 #ifdef MULTI_CACHE
   DEFINE(CACHE_FLUSH_KERN_ALL,	offsetof(struct cpu_cache_fns, flush_kern_all));
 #endif
+#ifdef CONFIG_CACHE_L2X0
+  DEFINE(L2X0_REGS_SZ,		sizeof(struct l2x0_regs));
+  DEFINE(L2X0_R_AUX_CTRL,	offsetof(struct l2x0_regs, aux_ctrl));
+  DEFINE(L2X0_R_TAG_LATENCY,	offsetof(struct l2x0_regs, tag_latency));
+  DEFINE(L2X0_R_DATA_LATENCY,	offsetof(struct l2x0_regs, data_latency));
+  DEFINE(L2X0_R_AFILTER_START,	offsetof(struct l2x0_regs, afilter_start));
+  DEFINE(L2X0_R_AFILTER_END,	offsetof(struct l2x0_regs, afilter_end));
+  DEFINE(L2X0_R_DEBUG_CTRL,	offsetof(struct l2x0_regs, debug_ctrl));
+  DEFINE(L2X0_R_PREFETCH_CTRL,	offsetof(struct l2x0_regs, prefetch_ctrl));
+  DEFINE(L2X0_R_POWER_CTRL,	offsetof(struct l2x0_regs, power_ctrl));
+#endif
   BLANK();
   DEFINE(DMA_BIDIRECTIONAL,	DMA_BIDIRECTIONAL);
   DEFINE(DMA_TO_DEVICE,		DMA_TO_DEVICE);

index bca7e61..5936d6b 100644
@@ -98,5 +98,8 @@ AFLAGS_proc-v7.o	:=-Wa,-march=armv7-a
 
 obj-$(CONFIG_CACHE_FEROCEON_L2)	+= cache-feroceon-l2.o
 obj-$(CONFIG_CACHE_L2X0)	+= cache-l2x0.o
+ifeq ($(CONFIG_PM_SLEEP),y)
+obj-$(CONFIG_CACHE_L2X0)	+= l2x0-sleep.o
+endif
 obj-$(CONFIG_CACHE_XSC3L2)	+= cache-xsc3l2.o
 obj-$(CONFIG_CACHE_TAUROS2)	+= cache-tauros2.o

index 46a507a..9b9d619 100644
@@ -28,7 +28,7 @@
 
 #define CACHE_LINE_SIZE		32
 
-static void __iomem *l2x0_base;
+void __iomem *l2x0_base;
 static DEFINE_SPINLOCK(l2x0_lock);
 static uint32_t l2x0_way_mask;	/* Bitmask of active ways */
 static uint32_t l2x0_size;

@@ -358,7 +358,7 @@ void __init l2x0_init(void __iomem *base, void __iomem *pbase,
 		writel_relaxed(aux, l2x0_base + L2X0_AUX_CTRL);
 
 		l2x0_inv_all();
-
+		l2x0_save_context(pbase, cache_id & L2X0_CACHE_ID_PART_MASK);
 		/* enable L2X0 */
 		writel_relaxed(1, l2x0_base + L2X0_CTRL);
 	}

@@ -371,6 +371,7 @@ void __init l2x0_init(void __iomem *base, void __iomem *pbase,
 	outer_cache.inv_all = l2x0_inv_all;
 	outer_cache.disable = l2x0_disable;
 	outer_cache.set_debug = l2x0_set_debug;
+	outer_cache.resume = l2x0_resume;
 
 	printk(KERN_INFO "%s cache controller enabled\n", type);
 	printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",

1.7.4.4

...

...

Yes, that's easily fixed, I can move the code in the same section. That is valid also for callers of e.g. cpu_resume as well though, or the jump must be an absolute one, because it is the same code pattern, but from text to data.

Lorenzo

Why is this assembly code? As Barry's previous patch shows, it's entirely possible to save all the state required at init time from the existing C code without needing any additional functions.

What about parts where the security gets in the way of reinitializing the L2 cache?

	mov	r3, #0
	add	r12, r1, #L2X0_LOCKDOWN_WAY_D_BASE
	str	r3, [r12, #4]				@ I lock
	str	r3, [r12], #L2X0_LOCKDOWN_STRIDE	@ D lock and increment
	bne	enable_l2
resume_l310:
	mov	r2, #7
unlock:
	str	r3, [r12, #4]				@ I lock
	str	r3, [r12], #L2X0_LOCKDOWN_STRIDE	@ D lock and increment
	subs	r2, r2, #1
	bne	unlock

It need not be, but variables that are needed when the MMU is off (L2 phys address and cache type) must be made visible to C code and initialized there then.

That's a fair point. I think L2 must be initialized by secure code, and we should not reprogramme it at all.

Having said that, this code only adds support for L2 resume when the MMU is off to Barry's patch. The security "problem" exists anyway. I check the control register to see if it is already enabled; all in all it is just an assembly stub that can be called before or after cpu_resume, that's it, to support suspend and idle also when L2 is retained and the boot firmware just jumps back to Linux on power up.

Whether we need an L2 resume function in the kernel or not, I think it is still a moot point. Let's keep the discussion going.

I added a comment to the function entry for the flags, but it is agreed, it can be improved; and yes a branch is the way to go.

Definitely, I reshuffled the instructions to adapt the latest changes and I missed out on this optimization you spotted straight away. Thanks for this.

Consider it done.

Thanks,
Lorenzo

Lorenzo,

I too didn't like this approach.

The platforms which needs to take care of security needs to take care of the L2 on their own and this code doesn't help them.

Other platform which wants to use the L2 code, looks like are fine to use the C-version which saves registers in init and restores it in resume path whenever needed. The patch which was posted on that was good enough. Regarding turning OFF MMU is really not necessary unless and until some hardware's are buggy. In that case, instead of adding that un-necessary code in generic code, it's better to patch only that buggy SOC code.

Regards
Santosh

Santosh,

I summed up the reasons for this patch to exist in the cover letter and I mentioned that some bits can be merged IF needed:

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

Having said that:

- This patch does not switch the MMU off
- This patch does not cover security, and I mentioned that
- Russell referred to saving registers not restoring them, for a reason
- If the resume hook is written in C you cannot call it when the MMU is off, which we need if L2 is retained. Unless you think OMAP is the only platform supporting L2 RAM retention, but there are people who might disagree.

And if you think it is not worth merging since most SoCs will run in non-secure mode, then that's a very valid point, but it is true for both patch series, not just for this one.

Lorenzo

I read that.

I noticed that. That comment was miss-placed.

I never said it is covering security.
"The platforms which needs to take care of security needs to take care of the L2 on their own and this code doesn't help them."

I raised this point on very first patch for L2. My point here was all the L2 registers cab be saved in init function and C-file.

Again I never said that. I guess MX support it as well. But I don't know whether they have secure limitations as well. If not then probably this asm resume function can help.

I agree. From various discussion so far, apart MX, nobody complained about C-version of the code. Hence the comment. If there are more platforms which needs it, then the asm version would be useful and should be preferred over C-version.

Regards
Santosh

Santosh,

Can you also look at Barry Song's version of this infrastructure as well? He posted it a couple of days ago:

Subject: [PATCH v2] ARM: cache-l2x0: add resume entry for l2 in secure mode
Message-ID: <[email protected]>

with this diffstat:

 4 files changed, 101 insertions(+), 9 deletions(-)

Just finished reviewing it. Apart from missing POR register, rest of the patch looks fine and should address the asm code need. I have just requested Shawn to try that out on IMX.

Regards
Santosh