[RFC] ARM hibernation / suspend-to-disk support code

Commit Message

Frank Hofmann May 22, 2011, 7:01 a.m. UTC

> [ ... ]
> > > r8_FIQ..r12_FIQ can store arbitrary state used by the FIQ handler,
> > > if FIQ is in use.  Can we expect any driver using FIQ to save/restore
> > > this state itself, rather than doing it globally?  This may be
> > > reasonable.
> > 
> > We don't need to save/restore those, because at the time the snapshot is 
> > taken interrupts are off and we cannot be in any trap handler either. On 
> > resume, the kernel that has been loaded has initialized them properly 
> > already.
> 
> I'm not sure if this is a safe assumption in general.  We may decide to
> switch to loading hibernate images from boot loaders, for example, and
> it may not hold any more.  Generally, please don't assume _anything_ has
> been properly initialized during resume, before the image is loaded.
> This has already beaten us a couple of times.
> 
> Thanks,
> Rafael

Hi Rafael,

regarding "cannot rely on any state", if that is so then it seems quite unnecessary to save/restore _any_ ARM non-#SYSTEM_MODE state - it'd better be reinitialized by calling cpu_init() after the "core restore" callback. The archives were quite a bit helpful in this context:

http://www.mail-archive.com/[e-mail]/msg12671.html

that's the same situation isn't it ?

Regarding FIQ: I agree with Russell and others who previously stated that a driver using FIQ is responsible for implementing suspend/resume hooks itself. But what could be done is to disable/enable it for the snapshot, just in case.

I've also found out that the vmlinux.lds changes don't seem to be necessary - they've been the last remnant of the "old" ARM hibernation patch, but just leaving those out looks to make no difference (the nosave data still ends up in the same place, with the same elf section attributes).

With all this in mind, the core part of the patch becomes somewhat simpler. See attached.


I'll test this on Monday.
FrankH.

Comments

Rafael Wysocki May 22, 2011, 9:54 a.m. UTC | #1

On Sunday, May 22, 2011, Frank Hofmann wrote:
> > [ ... ]
> > > > r8_FIQ..r12_FIQ can store arbitrary state used by the FIQ handler,
> > > > if FIQ is in use.  Can we expect any driver using FIQ to save/restore
> > > > this state itself, rather than doing it globally?  This may be
> > > > reasonable.
> > > 
> > > We don't need to save/restore those, because at the time the snapshot is 
> > > taken interrupts are off and we cannot be in any trap handler either. On 
> > > resume, the kernel that has been loaded has initialized them properly 
> > > already.
> > 
> > I'm not sure if this is a safe assumption in general.  We may decide to
> > switch to loading hibernate images from boot loaders, for example, and
> > it may not hold any more.  Generally, please don't assume _anything_ has
> > been properly initialized during resume, before the image is loaded.
> > This has already beaten us a couple of times.
> > 
> > Thanks,
> > Rafael
> 
> Hi Rafael,
> 
> regarding "cannot rely on any state", if that is so then it seems quite
> unnecessary to save/restore _any_ ARM non-#SYSTEM_MODE state - it'd better
> be reinitialized by calling cpu_init() after the "core restore" callback.

If they are always initialized in the same way and if you can do the CPU
initialization at this point, I agree.

> The archives were quite a bit helpful in this context:
> 
> http://www.mail-archive.com/[e-mail]/msg12671.html
> 
> that's the same situation isn't it ?

I'm not sure, I'm not an ARM expert, but it seems so.

> Regarding FIQ: I agree with Russell and others who previously stated that
> a driver using FIQ is responsible for implementing suspend/resume hooks itself.

I agree with that too.

> But what could be done is to disable/enable it for the snapshot, just in case.

OK

> I've also found out that the vmlinux.lds changes don't seem to be necessary
> - they've been the last remnant of the "old" ARM hibernation patch, but just
> leaving those out looks to make no difference (the nosave data still ends up
> in the same place, with the same elf section attributes).
> 
> With all this in mind, the core part of the patch becomes somewhat simpler.
> See attached.

It looks reasonable, but it slightly concerns me that you seem to assume
that swapper_pg_dir will be the same in both the boot and the target kernel.
We used to make this assumption on x86 too, but it started to cause
problems to happen at one point.  To address those problems we had to create
temporary page tables using page frames that are guaranteed not to be
overwritten in the last phase of restore (that would be the for () loop in
your __swsusp_arch_restore_image()).

Thanks,
Rafael

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 44c16f0..f282bac 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -207,6 +207,9 @@  config ARM_PATCH_PHYS_VIRT_16BIT
 	def_bool y
 	depends on ARM_PATCH_PHYS_VIRT && ARCH_MSM
 
+config ARCH_HIBERNATION_POSSIBLE
+	def_bool n
+
 source "init/Kconfig"
 
 source "kernel/Kconfig.freezer"
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 431077c..c7ef454 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -250,6 +250,7 @@  static inline void *phys_to_virt(phys_addr_t x)
  */
 #define __pa(x)			__virt_to_phys((unsigned long)(x))
 #define __va(x)			((void *)__phys_to_virt((unsigned long)(x)))
+#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x),0))
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 
 /*
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 8d95446..a1f54cd 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -44,6 +44,7 @@  obj-$(CONFIG_ARM_THUMBEE)	+= thumbee.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_ARM_UNWIND)	+= unwind.o
 obj-$(CONFIG_HAVE_TCM)		+= tcm.o
+obj-$(CONFIG_HIBERNATION)	+= cpu.o swsusp.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 obj-$(CONFIG_SWP_EMULATE)	+= swp_emulate.o
 CFLAGS_swp_emulate.o		:= -Wa,-march=armv7-a
diff --git a/arch/arm/kernel/cpu.c b/arch/arm/kernel/cpu.c
new file mode 100644
index 0000000..0f1c31f
--- /dev/null
+++ b/arch/arm/kernel/cpu.c
@@ -0,0 +1,64 @@ 
+/*
+ * Hibernation support specific for ARM
+ *
+ * Derived from work on ARM hibernation support by:
+ *
+ * Ubuntu project, hibernation support for mach-dove
+ * Copyright (C) 2010 Nokia Corporation (Hiroshi Doyu)
+ * Copyright (C) 2010 Texas Instruments, Inc. (Teerth Reddy et al.)
+ *	https://lkml.org/lkml/2010/6/18/4
+ *	https://lists.linux-foundation.org/pipermail/linux-pm/2010-June/027422.html
+ *	https://patchwork.kernel.org/patch/96442/
+ *
+ * Copyright (C) 2006 Rafael J. Wysocki <[e-mail]>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/mm.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
+#include <asm/tlbflush.h>
+
+extern const void __nosave_begin, __nosave_end;
+
+int pfn_is_nosave(unsigned long pfn)
+{
+	unsigned long nosave_begin_pfn = __pa_symbol(&__nosave_begin) >> PAGE_SHIFT;
+	unsigned long nosave_end_pfn = PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT;
+
+	return (pfn >= nosave_begin_pfn) && (pfn < nosave_end_pfn);
+}
+
+void save_processor_state(void)
+{
+	flush_thread();
+	local_fiq_disable();
+}
+
+void restore_processor_state(void)
+{
+	local_flush_tlb_all();
+	local_fiq_enable();
+}
+
+u8 __swsusp_arch_ctx[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
+u8 __swsusp_resume_stk[PAGE_SIZE/2] __nosavedata;
+
+/*
+ * The framework loads the hibernation image into this linked list,
+ * for swsusp_arch_resume() to copy back to the proper destinations.
+ *
+ * To make this work if resume is triggered from initramfs, the
+ * pagetables need to be switched to allow writes to kernel mem.
+ */
+void notrace __swsusp_arch_restore_image(void)
+{
+	extern struct pbe *restore_pblist;
+	struct pbe *pbe;
+
+	cpu_switch_mm(__virt_to_phys(swapper_pg_dir), &init_mm);
+
+	for (pbe = restore_pblist; pbe; pbe = pbe->next)
+		copy_page(pbe->orig_address, pbe->address);
+}
diff --git a/arch/arm/kernel/swsusp.S b/arch/arm/kernel/swsusp.S
new file mode 100644
index 0000000..d7ebbe9
--- /dev/null
+++ b/arch/arm/kernel/swsusp.S
@@ -0,0 +1,68 @@ 
+/*
+ * Hibernation support specific for ARM
+ *
+ * Based on work by:
+ *
+ * Ubuntu project, hibernation support for mach-dove,
+ * Copyright (C) 2010 Nokia Corporation (Hiroshi Doyu)
+ * Copyright (C) 2010 Texas Instruments, Inc. (Teerth Reddy et al.)
+ *	https://lkml.org/lkml/2010/6/18/4
+ *	https://lists.linux-foundation.org/pipermail/linux-pm/2010-June/027422.html
+ *	https://patchwork.kernel.org/patch/96442/
+ *
+ * Copyright (C) 2006 Rafael J. Wysocki <[e-mail]>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/linkage.h>
+#include <asm/memory.h>
+#include <asm/page.h>
+#include <asm/cache.h>
+#include <asm/ptrace.h>
+
+/*
+ * Save the current CPU state before suspend / poweroff.
+ */
+ENTRY(swsusp_arch_suspend)
+	ldr	r0, =__swsusp_arch_ctx
+	mrs	r1, cpsr
+	stm	r0!, {r1-r12,lr}	/* CPSR, nonvolatile regs */
+	stmfd	sp!, {lr}
+	bl	__save_processor_state
+	ldmfd	sp!, {lr}
+	b	swsusp_save
+ENDPROC(swsusp_arch_suspend)
+
+/*
+ * Restore the memory image from the pagelists, and load the CPU registers
+ * from saved state.
+ */
+ENTRY(swsusp_arch_resume)
+	/*
+	 * Switch stack to a nosavedata region to make sure image restore
+	 * doesn't clobber it underneath itself.
+	 */
+	ldr	sp, =(__swsusp_resume_stk + PAGE_SIZE / 2)
+	bl	__swsusp_arch_restore_image
+
+	/*
+	 * Restore the CPU registers.
+	 */
+	ldr	r0, =__swsusp_arch_ctx
+	ldr	r1, [r0], #4
+	msr	cpsr, r1
+	ldm	r0!, {r2-r12,lr}
+	/*
+	 * From here on we have a valid stack again. Core state is
+	 * not restored yet, redirect to the machine-specific
+	 * implementation to get that done.
+	 * Resume has succeeded at this point; if the machine-specific
+	 * code wants to fail it needs to panic.
+	 */
+	mov	r1, #0
+	push	{r1,lr}
+	bl	__restore_processor_state	/* restore core state */
+	bl	cpu_init
+	pop	{r0,pc}
+ENDPROC(swsusp_arch_resume)