[PATCH v6 0/2] hibernation support on ARM

[PATCH v6 0/2] hibernation support on ARM

[Sebastian Capella]

Patches adding support for hibernation on ARM
 - ARM hibernation / suspend-to-disk
 - Change soft_restart to use non-tracing raw_local_irq_disable

Patches based on v3.13 tag, verified hibernation on beaglebone black on a
branch based on 3.13 merged with initial omap support from Russ Dill which
can be found here (includes v1 patchset):
http://git.linaro.org/git-ro/people/sebastian.capella/linux.git hibernation_3.13_russMerge

[PATCH v6 1/2] ARM: avoid tracers in soft_restart

 arch/arm/kernel/process.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

 Use raw_local_irq_disable in place of local_irq_disable to avoid
 infinite abort recursion while tracing. (unchanged since v3)

[PATCH v6 2/2] ARM hibernation / suspend-to-disk
 arch/arm/include/asm/memory.h |    1 +
 arch/arm/kernel/Makefile      |    1 +
 arch/arm/kernel/hibernate.c   |  113 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/Kconfig           |    5 ++
 include/linux/suspend.h       |    2 +
 5 files changed, 122 insertions(+)

 Adds support for ARM based hibernation


Additional notes:
-----------------

There are two checkpatch warnings added by this patch.  These follow
behavior in existing hibernation implementations on other platforms.

    WARNING: externs should be avoided in .c files
    #116: FILE: arch/arm/kernel/hibernate.c:26:
    +extern const void __nosave_begin, __nosave_end;

  This extern is picking up the linker nosave region definitions, only
  used in hibernate.  Follows same extern line used mips, powerpc, s390,
  sh, sparc, x86 & unicore32

    WARNING: externs should be avoided in .c files
    #199: FILE: arch/arm/kernel/hibernate.c:109:
    +extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);

  This extern is used in the arch/arm/ in hibernate, process and bL_switcher


Changes in v6:
--------------
* Simplify static variable names

Changes in v5:
--------------
* Fixed checkpatch warning on trailing whitespace

Changes in v4:
--------------
* updated comment for soft_restart with review feedback
* dropped freeze_processes patch which was queued separately 
  to 3.14 by Rafael Wysocki:
  https://lkml.org/lkml/2014/2/25/683

Changes in v3:
--------------
* added comment to use of soft_restart
* drop irq disable soft_restart patch
* add patch to avoid tracers in soft_restart by using raw_local_irq_*

Changes in v2:
--------------
* Removed unneeded flush_thread, use of __naked and cpu_init.
* dropped Cyril Chemparathy <cyril@ti.com> from Cc: list as 
  emails are bouncing.

Thanks,

Sebastian Capella

[PATCH v6 0/2] hibernation support on ARM

[Sebastian Capella]

Patches adding support for hibernation on ARM
 - ARM hibernation / suspend-to-disk
 - Change soft_restart to use non-tracing raw_local_irq_disable

Patches based on v3.13 tag, verified hibernation on beaglebone black on a
branch based on 3.13 merged with initial omap support from Russ Dill which
can be found here (includes v1 patchset):
http://git.linaro.org/git-ro/people/sebastian.capella/linux.git hibernation_3.13_russMerge

[PATCH v6 1/2] ARM: avoid tracers in soft_restart

 arch/arm/kernel/process.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

 Use raw_local_irq_disable in place of local_irq_disable to avoid
 infinite abort recursion while tracing. (unchanged since v3)

[PATCH v6 2/2] ARM hibernation / suspend-to-disk
 arch/arm/include/asm/memory.h |    1 +
 arch/arm/kernel/Makefile      |    1 +
 arch/arm/kernel/hibernate.c   |  113 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/Kconfig           |    5 ++
 include/linux/suspend.h       |    2 +
 5 files changed, 122 insertions(+)

 Adds support for ARM based hibernation


Additional notes:
-----------------

There are two checkpatch warnings added by this patch.  These follow
behavior in existing hibernation implementations on other platforms.

    WARNING: externs should be avoided in .c files
    #116: FILE: arch/arm/kernel/hibernate.c:26:
    +extern const void __nosave_begin, __nosave_end;

  This extern is picking up the linker nosave region definitions, only
  used in hibernate.  Follows same extern line used mips, powerpc, s390,
  sh, sparc, x86 & unicore32

    WARNING: externs should be avoided in .c files
    #199: FILE: arch/arm/kernel/hibernate.c:109:
    +extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);

  This extern is used in the arch/arm/ in hibernate, process and bL_switcher


Changes in v6:
--------------
* Simplify static variable names

Changes in v5:
--------------
* Fixed checkpatch warning on trailing whitespace

Changes in v4:
--------------
* updated comment for soft_restart with review feedback
* dropped freeze_processes patch which was queued separately 
  to 3.14 by Rafael Wysocki:
  https://lkml.org/lkml/2014/2/25/683

Changes in v3:
--------------
* added comment to use of soft_restart
* drop irq disable soft_restart patch
* add patch to avoid tracers in soft_restart by using raw_local_irq_*

Changes in v2:
--------------
* Removed unneeded flush_thread, use of __naked and cpu_init.
* dropped Cyril Chemparathy <cyril@ti.com> from Cc: list as 
  emails are bouncing.

Thanks,

Sebastian Capella

[PATCH v6 1/2] ARM: avoid tracers in soft_restart

[Sebastian Capella]

Use of tracers in local_irq_disable is causes recursive aborts when
called with irqs disabled and using a temporary stack (hibernation).
Replace local_irq_disable with raw_local_irq_disable instead to
avoid tracers.

Signed-off-by: Sebastian Capella <sebastian.capella@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Holt <robin.m.holt@gmail.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/kernel/process.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 92f7b15..f58b723 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -100,7 +100,7 @@ void soft_restart(unsigned long addr)
 	u64 *stack = soft_restart_stack + ARRAY_SIZE(soft_restart_stack);
 
 	/* Disable interrupts first */
-	local_irq_disable();
+	raw_local_irq_disable();
 	local_fiq_disable();
 
 	/* Disable the L2 if we're the last man standing. */
-- 
1.7.9.5

[PATCH v6 1/2] ARM: avoid tracers in soft_restart

[Sebastian Capella]

Use of tracers in local_irq_disable is causes recursive aborts when
called with irqs disabled and using a temporary stack (hibernation).
Replace local_irq_disable with raw_local_irq_disable instead to
avoid tracers.

Signed-off-by: Sebastian Capella <sebastian.capella@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Holt <robin.m.holt@gmail.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/kernel/process.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 92f7b15..f58b723 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -100,7 +100,7 @@ void soft_restart(unsigned long addr)
 	u64 *stack = soft_restart_stack + ARRAY_SIZE(soft_restart_stack);
 
 	/* Disable interrupts first */
-	local_irq_disable();
+	raw_local_irq_disable();
 	local_fiq_disable();
 
 	/* Disable the L2 if we're the last man standing. */
-- 
1.7.9.5

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

From: Russ Dill <Russ.Dill@ti.com>

Enable hibernation for ARM architectures and provide ARM
architecture specific calls used during hibernation.

The swsusp hibernation framework depends on the
platform first having functional suspend/resume.

Then, in order to enable hibernation on a given platform, a
platform_hibernation_ops structure may need to be registered with
the system in order to save/restore any SoC-specific / cpu specific
state needing (re)init over a suspend-to-disk/resume-from-disk cycle.

For example:

     - "secure" SoCs that have different sets of control registers
       and/or different CR reg access patterns.

     - SoCs with L2 caches as the activation sequence there is
       SoC-dependent; a full off-on cycle for L2 is not done
       by the hibernation support code.

     - SoCs requiring steps on wakeup _before_ the "generic" parts
       done by cpu_suspend / cpu_resume can work correctly.

     - SoCs having persistent state which is maintained during suspend
       and resume, but will be lost during the power off cycle after
       suspend-to-disk.

This is a rebase/rework of Frank Hofmann's v5 hibernation patchset.

Acked-by: Russ Dill <Russ.Dill@ti.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sebastian Capella <sebastian.capella@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Len Brown <len.brown@intel.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Jonathan Austin <jonathan.austin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/include/asm/memory.h |    1 +
 arch/arm/kernel/Makefile      |    1 +
 arch/arm/kernel/hibernate.c   |  113 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/Kconfig           |    5 ++
 include/linux/suspend.h       |    2 +
 5 files changed, 122 insertions(+)
 create mode 100644 arch/arm/kernel/hibernate.c

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 8756e4b..1079ea8 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -291,6 +291,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((phys_addr_t)(x)))
+#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x), 0))
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 
 extern phys_addr_t (*arch_virt_to_idmap)(unsigned long x);
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index a30fc9b..8afa848 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_ARTHUR)		+= arthur.o
 obj-$(CONFIG_ISA_DMA)		+= dma-isa.o
 obj-$(CONFIG_PCI)		+= bios32.o isa.o
 obj-$(CONFIG_ARM_CPU_SUSPEND)	+= sleep.o suspend.o
+obj-$(CONFIG_HIBERNATION)	+= hibernate.o
 obj-$(CONFIG_SMP)		+= smp.o
 ifdef CONFIG_MMU
 obj-$(CONFIG_SMP)		+= smp_tlb.o
diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
new file mode 100644
index 0000000..a41e0e3
--- /dev/null
+++ b/arch/arm/kernel/hibernate.c
@@ -0,0 +1,113 @@
+/*
+ * 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 <rjw@sisk.pl>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/mm.h>
+#include <linux/suspend.h>
+#include <asm/tlbflush.h>
+#include <asm/cacheflush.h>
+#include <asm/system_misc.h>
+#include <asm/idmap.h>
+#include <asm/suspend.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 notrace save_processor_state(void)
+{
+	WARN_ON(num_online_cpus() != 1);
+	local_fiq_disable();
+}
+
+void notrace restore_processor_state(void)
+{
+	local_fiq_enable();
+}
+
+/*
+ * Snapshot kernel memory and reset the system.
+ *
+ * swsusp_save() is executed in the suspend finisher so that the CPU
+ * context pointer and memory are part of the saved image, which is
+ * required by the resume kernel image to restart execution from
+ * swsusp_arch_suspend().
+ *
+ * soft_restart is not technically needed, but is used to get success
+ * returned from cpu_suspend.
+ *
+ * When soft reboot completes, the hibernation snapshot is written out.
+ */
+static int notrace arch_save_image(unsigned long unused)
+{
+	int ret;
+
+	ret = swsusp_save();
+	if (ret == 0)
+		soft_restart(virt_to_phys(cpu_resume));
+	return ret;
+}
+
+/*
+ * Save the current CPU state before suspend / poweroff.
+ */
+int notrace swsusp_arch_suspend(void)
+{
+	return cpu_suspend(0, arch_save_image);
+}
+
+/*
+ * The framework loads the hibernation image into a linked list anchored
+ * at restore_pblist, 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.
+ */
+static void notrace arch_restore_image(void *unused)
+{
+	struct pbe *pbe;
+
+	cpu_switch_mm(idmap_pgd, &init_mm);
+	for (pbe = restore_pblist; pbe; pbe = pbe->next)
+		copy_page(pbe->orig_address, pbe->address);
+
+	soft_restart(virt_to_phys(cpu_resume));
+}
+
+static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
+
+/*
+ * Resume from the hibernation image.
+ * Due to the kernel heap / data restore, stack contents change underneath
+ * and that would make function calls impossible; switch to a temporary
+ * stack within the nosave region to avoid that problem.
+ */
+int swsusp_arch_resume(void)
+{
+	extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
+	call_with_stack(arch_restore_image, 0,
+		resume_stack + sizeof(resume_stack));
+	return 0;
+}
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 1f8fed9..83707702 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -611,6 +611,11 @@ config CPU_USE_DOMAINS
 config IO_36
 	bool
 
+config ARCH_HIBERNATION_POSSIBLE
+	bool
+	depends on MMU
+	default y if CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || CPU_XSCALE || CPU_XSC3 || CPU_V6 || CPU_V6K || CPU_V7
+
 comment "Processor Features"
 
 config ARM_LPAE
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index f73cabf..38bbf95 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -320,6 +320,8 @@ extern unsigned long get_safe_page(gfp_t gfp_mask);
 extern void hibernation_set_ops(const struct platform_hibernation_ops *ops);
 extern int hibernate(void);
 extern bool system_entering_hibernation(void);
+asmlinkage int swsusp_save(void);
+extern struct pbe *restore_pblist;
 #else /* CONFIG_HIBERNATION */
 static inline void register_nosave_region(unsigned long b, unsigned long e) {}
 static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
-- 
1.7.9.5

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

From: Russ Dill <Russ.Dill@ti.com>

Enable hibernation for ARM architectures and provide ARM
architecture specific calls used during hibernation.

The swsusp hibernation framework depends on the
platform first having functional suspend/resume.

Then, in order to enable hibernation on a given platform, a
platform_hibernation_ops structure may need to be registered with
the system in order to save/restore any SoC-specific / cpu specific
state needing (re)init over a suspend-to-disk/resume-from-disk cycle.

For example:

     - "secure" SoCs that have different sets of control registers
       and/or different CR reg access patterns.

     - SoCs with L2 caches as the activation sequence there is
       SoC-dependent; a full off-on cycle for L2 is not done
       by the hibernation support code.

     - SoCs requiring steps on wakeup _before_ the "generic" parts
       done by cpu_suspend / cpu_resume can work correctly.

     - SoCs having persistent state which is maintained during suspend
       and resume, but will be lost during the power off cycle after
       suspend-to-disk.

This is a rebase/rework of Frank Hofmann's v5 hibernation patchset.

Acked-by: Russ Dill <Russ.Dill@ti.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Sebastian Capella <sebastian.capella@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Len Brown <len.brown@intel.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Jonathan Austin <jonathan.austin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: "Uwe Kleine-K?nig" <u.kleine-koenig@pengutronix.de>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/include/asm/memory.h |    1 +
 arch/arm/kernel/Makefile      |    1 +
 arch/arm/kernel/hibernate.c   |  113 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/Kconfig           |    5 ++
 include/linux/suspend.h       |    2 +
 5 files changed, 122 insertions(+)
 create mode 100644 arch/arm/kernel/hibernate.c

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 8756e4b..1079ea8 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -291,6 +291,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((phys_addr_t)(x)))
+#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x), 0))
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 
 extern phys_addr_t (*arch_virt_to_idmap)(unsigned long x);
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index a30fc9b..8afa848 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_ARTHUR)		+= arthur.o
 obj-$(CONFIG_ISA_DMA)		+= dma-isa.o
 obj-$(CONFIG_PCI)		+= bios32.o isa.o
 obj-$(CONFIG_ARM_CPU_SUSPEND)	+= sleep.o suspend.o
+obj-$(CONFIG_HIBERNATION)	+= hibernate.o
 obj-$(CONFIG_SMP)		+= smp.o
 ifdef CONFIG_MMU
 obj-$(CONFIG_SMP)		+= smp_tlb.o
diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
new file mode 100644
index 0000000..a41e0e3
--- /dev/null
+++ b/arch/arm/kernel/hibernate.c
@@ -0,0 +1,113 @@
+/*
+ * 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 <rjw@sisk.pl>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/mm.h>
+#include <linux/suspend.h>
+#include <asm/tlbflush.h>
+#include <asm/cacheflush.h>
+#include <asm/system_misc.h>
+#include <asm/idmap.h>
+#include <asm/suspend.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 notrace save_processor_state(void)
+{
+	WARN_ON(num_online_cpus() != 1);
+	local_fiq_disable();
+}
+
+void notrace restore_processor_state(void)
+{
+	local_fiq_enable();
+}
+
+/*
+ * Snapshot kernel memory and reset the system.
+ *
+ * swsusp_save() is executed in the suspend finisher so that the CPU
+ * context pointer and memory are part of the saved image, which is
+ * required by the resume kernel image to restart execution from
+ * swsusp_arch_suspend().
+ *
+ * soft_restart is not technically needed, but is used to get success
+ * returned from cpu_suspend.
+ *
+ * When soft reboot completes, the hibernation snapshot is written out.
+ */
+static int notrace arch_save_image(unsigned long unused)
+{
+	int ret;
+
+	ret = swsusp_save();
+	if (ret == 0)
+		soft_restart(virt_to_phys(cpu_resume));
+	return ret;
+}
+
+/*
+ * Save the current CPU state before suspend / poweroff.
+ */
+int notrace swsusp_arch_suspend(void)
+{
+	return cpu_suspend(0, arch_save_image);
+}
+
+/*
+ * The framework loads the hibernation image into a linked list anchored
+ *@restore_pblist, 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.
+ */
+static void notrace arch_restore_image(void *unused)
+{
+	struct pbe *pbe;
+
+	cpu_switch_mm(idmap_pgd, &init_mm);
+	for (pbe = restore_pblist; pbe; pbe = pbe->next)
+		copy_page(pbe->orig_address, pbe->address);
+
+	soft_restart(virt_to_phys(cpu_resume));
+}
+
+static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
+
+/*
+ * Resume from the hibernation image.
+ * Due to the kernel heap / data restore, stack contents change underneath
+ * and that would make function calls impossible; switch to a temporary
+ * stack within the nosave region to avoid that problem.
+ */
+int swsusp_arch_resume(void)
+{
+	extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
+	call_with_stack(arch_restore_image, 0,
+		resume_stack + sizeof(resume_stack));
+	return 0;
+}
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 1f8fed9..83707702 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -611,6 +611,11 @@ config CPU_USE_DOMAINS
 config IO_36
 	bool
 
+config ARCH_HIBERNATION_POSSIBLE
+	bool
+	depends on MMU
+	default y if CPU_ARM920T || CPU_ARM926T || CPU_SA1100 || CPU_XSCALE || CPU_XSC3 || CPU_V6 || CPU_V6K || CPU_V7
+
 comment "Processor Features"
 
 config ARM_LPAE
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index f73cabf..38bbf95 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -320,6 +320,8 @@ extern unsigned long get_safe_page(gfp_t gfp_mask);
 extern void hibernation_set_ops(const struct platform_hibernation_ops *ops);
 extern int hibernate(void);
 extern bool system_entering_hibernation(void);
+asmlinkage int swsusp_save(void);
+extern struct pbe *restore_pblist;
 #else /* CONFIG_HIBERNATION */
 static inline void register_nosave_region(unsigned long b, unsigned long e) {}
 static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
-- 
1.7.9.5

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Stephen Boyd]

On 02/27/14 15:57, Sebastian Capella wrote:
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 8756e4b..1079ea8 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -291,6 +291,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((phys_addr_t)(x)))
> +#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x), 0))

Just curious, is there a reason for the RELOC_HIDE() here? Or
__pa_symbol() for that matter? It looks like only x86 uses this on the
__nosave_{begin,end} symbol. Maybe it's copy-pasta?

I also wonder if anyone has thought about making a __weak
pfn_is_nosave() function so that architectures don't need to implement
the same thing every time. Consolidating those shouldn't be part of this
patch though.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Stephen Boyd]

On 02/27/14 15:57, Sebastian Capella wrote:
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 8756e4b..1079ea8 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -291,6 +291,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((phys_addr_t)(x)))
> +#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x), 0))

Just curious, is there a reason for the RELOC_HIDE() here? Or
__pa_symbol() for that matter? It looks like only x86 uses this on the
__nosave_{begin,end} symbol. Maybe it's copy-pasta?

I also wonder if anyone has thought about making a __weak
pfn_is_nosave() function so that architectures don't need to implement
the same thing every time. Consolidating those shouldn't be part of this
patch though.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Russ Dill]

On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> On 02/27/14 15:57, Sebastian Capella wrote:
>> diff --git a/arch/arm/include/asm/memory.h
>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
>> a/arch/arm/include/asm/memory.h +++
>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> 
> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> __pa_symbol() for that matter? It looks like only x86 uses this on
> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?

>From my understanding this needs to stick around so long as gcc 3.x is
supported (did it get dropped yet?) on ARM Linux since it doesn't
support -fno-strict-overflow.

> I also wonder if anyone has thought about making a __weak 
> pfn_is_nosave() function so that architectures don't need to
> implement the same thing every time. Consolidating those shouldn't
> be part of this patch though.
> 

Yes, I think just a couple of the architectures do anything besides
checking if the address falls within the nosave section.

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Russ Dill]

On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> On 02/27/14 15:57, Sebastian Capella wrote:
>> diff --git a/arch/arm/include/asm/memory.h
>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
>> a/arch/arm/include/asm/memory.h +++
>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> 
> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> __pa_symbol() for that matter? It looks like only x86 uses this on
> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?

>From my understanding this needs to stick around so long as gcc 3.x is
supported (did it get dropped yet?) on ARM Linux since it doesn't
support -fno-strict-overflow.

> I also wonder if anyone has thought about making a __weak 
> pfn_is_nosave() function so that architectures don't need to
> implement the same thing every time. Consolidating those shouldn't
> be part of this patch though.
> 

Yes, I think just a couple of the architectures do anything besides
checking if the address falls within the nosave section.

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Russ Dill]

On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> On 02/27/14 15:57, Sebastian Capella wrote:
>> diff --git a/arch/arm/include/asm/memory.h
>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
>> a/arch/arm/include/asm/memory.h +++
>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> 
> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> __pa_symbol() for that matter? It looks like only x86 uses this on
> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?

>From my understanding this needs to stick around so long as gcc 3.x is
supported (did it get dropped yet?) on ARM Linux since it doesn't
support -fno-strict-overflow.

> I also wonder if anyone has thought about making a __weak 
> pfn_is_nosave() function so that architectures don't need to
> implement the same thing every time. Consolidating those shouldn't
> be part of this patch though.
> 

Yes, I think just a couple of the architectures do anything besides
checking if the address falls within the nosave section.

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Stephen Boyd]

On 02/27/14 17:47, Russ Dill wrote:
> On 02/27/2014 04:09 PM, Stephen Boyd wrote:
>> On 02/27/14 15:57, Sebastian Capella wrote:
>>> diff --git a/arch/arm/include/asm/memory.h
>>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
>>> a/arch/arm/include/asm/memory.h +++
>>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
>>> __pa(RELOC_HIDE((unsigned long)(x), 0))
>> Just curious, is there a reason for the RELOC_HIDE() here? Or 
>> __pa_symbol() for that matter? It looks like only x86 uses this on
>> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> From my understanding this needs to stick around so long as gcc 3.x is
> supported (did it get dropped yet?) on ARM Linux since it doesn't
> support -fno-strict-overflow.

I don't think it's been dropped yet but I wonder if anyone has tried
recent kernels with such a compiler?

Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
same treatment? Or the tagtable loop in atags_parse.c? Do the other
architectures also need to be fixed? That link Sebastian points to says
that ppc originally needed it but pfn_is_nosave() on ppc doesn't use
RELOC_HIDE anywhere in their __pa() macro from what I can tell.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Stephen Boyd]

On 02/27/14 17:47, Russ Dill wrote:
> On 02/27/2014 04:09 PM, Stephen Boyd wrote:
>> On 02/27/14 15:57, Sebastian Capella wrote:
>>> diff --git a/arch/arm/include/asm/memory.h
>>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
>>> a/arch/arm/include/asm/memory.h +++
>>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
>>> __pa(RELOC_HIDE((unsigned long)(x), 0))
>> Just curious, is there a reason for the RELOC_HIDE() here? Or 
>> __pa_symbol() for that matter? It looks like only x86 uses this on
>> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> From my understanding this needs to stick around so long as gcc 3.x is
> supported (did it get dropped yet?) on ARM Linux since it doesn't
> support -fno-strict-overflow.

I don't think it's been dropped yet but I wonder if anyone has tried
recent kernels with such a compiler?

Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
same treatment? Or the tagtable loop in atags_parse.c? Do the other
architectures also need to be fixed? That link Sebastian points to says
that ppc originally needed it but pfn_is_nosave() on ppc doesn't use
RELOC_HIDE anywhere in their __pa() macro from what I can tell.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Thu, Feb 27, 2014 at 11:57:58PM +0000, Sebastian Capella wrote:

[...]

> diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
> new file mode 100644
> index 0000000..a41e0e3
> --- /dev/null
> +++ b/arch/arm/kernel/hibernate.c
> @@ -0,0 +1,113 @@
> +/*
> + * 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 <rjw@sisk.pl>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/suspend.h>
> +#include <asm/tlbflush.h>
> +#include <asm/cacheflush.h>

You can drop tlbflush.h and cacheflush.h, they do not seem to be needed.

> +#include <asm/system_misc.h>
> +#include <asm/idmap.h>
> +#include <asm/suspend.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 notrace save_processor_state(void)
> +{
> +	WARN_ON(num_online_cpus() != 1);
> +	local_fiq_disable();
> +}
> +
> +void notrace restore_processor_state(void)
> +{
> +	local_fiq_enable();
> +}
> +
> +/*
> + * Snapshot kernel memory and reset the system.
> + *
> + * swsusp_save() is executed in the suspend finisher so that the CPU
> + * context pointer and memory are part of the saved image, which is
> + * required by the resume kernel image to restart execution from
> + * swsusp_arch_suspend().
> + *
> + * soft_restart is not technically needed, but is used to get success
> + * returned from cpu_suspend.
> + *
> + * When soft reboot completes, the hibernation snapshot is written out.
> + */
> +static int notrace arch_save_image(unsigned long unused)
> +{
> +	int ret;
> +
> +	ret = swsusp_save();
> +	if (ret == 0)
> +		soft_restart(virt_to_phys(cpu_resume));
> +	return ret;
> +}
> +
> +/*
> + * Save the current CPU state before suspend / poweroff.
> + */
> +int notrace swsusp_arch_suspend(void)
> +{
> +	return cpu_suspend(0, arch_save_image);
> +}
> +
> +/*
> + * The framework loads the hibernation image into a linked list anchored
> + * at restore_pblist, 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.
> + */

Comment above needs updating. We are switching page tables to a set of
page tables that are certain to live at the same location in the older
kernel, that's the only reason, as we discussed. soft_restart will make
sure (again) to switch to 1:1 page tables so that we can call cpu_resume
with the MMU off.

> +static void notrace arch_restore_image(void *unused)
> +{
> +	struct pbe *pbe;
> +
> +	cpu_switch_mm(idmap_pgd, &init_mm);
> +	for (pbe = restore_pblist; pbe; pbe = pbe->next)
> +		copy_page(pbe->orig_address, pbe->address);
> +
> +	soft_restart(virt_to_phys(cpu_resume));
> +}
> +
> +static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> +
> +/*
> + * Resume from the hibernation image.
> + * Due to the kernel heap / data restore, stack contents change underneath
> + * and that would make function calls impossible; switch to a temporary
> + * stack within the nosave region to avoid that problem.
> + */
> +int swsusp_arch_resume(void)
> +{
> +	extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> +	call_with_stack(arch_restore_image, 0,
> +		resume_stack + sizeof(resume_stack));

This does not guarantee your stack is 8-byte aligned, that's not AAPCS
compliant and might buy you trouble.

Either you align the stack or you align the pointer you are passing.

Please have a look at kernel/process.c

Thanks,
Lorenzo

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Thu, Feb 27, 2014 at 11:57:58PM +0000, Sebastian Capella wrote:

[...]

> diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
> new file mode 100644
> index 0000000..a41e0e3
> --- /dev/null
> +++ b/arch/arm/kernel/hibernate.c
> @@ -0,0 +1,113 @@
> +/*
> + * 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 <rjw@sisk.pl>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/suspend.h>
> +#include <asm/tlbflush.h>
> +#include <asm/cacheflush.h>

You can drop tlbflush.h and cacheflush.h, they do not seem to be needed.

> +#include <asm/system_misc.h>
> +#include <asm/idmap.h>
> +#include <asm/suspend.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 notrace save_processor_state(void)
> +{
> +	WARN_ON(num_online_cpus() != 1);
> +	local_fiq_disable();
> +}
> +
> +void notrace restore_processor_state(void)
> +{
> +	local_fiq_enable();
> +}
> +
> +/*
> + * Snapshot kernel memory and reset the system.
> + *
> + * swsusp_save() is executed in the suspend finisher so that the CPU
> + * context pointer and memory are part of the saved image, which is
> + * required by the resume kernel image to restart execution from
> + * swsusp_arch_suspend().
> + *
> + * soft_restart is not technically needed, but is used to get success
> + * returned from cpu_suspend.
> + *
> + * When soft reboot completes, the hibernation snapshot is written out.
> + */
> +static int notrace arch_save_image(unsigned long unused)
> +{
> +	int ret;
> +
> +	ret = swsusp_save();
> +	if (ret == 0)
> +		soft_restart(virt_to_phys(cpu_resume));
> +	return ret;
> +}
> +
> +/*
> + * Save the current CPU state before suspend / poweroff.
> + */
> +int notrace swsusp_arch_suspend(void)
> +{
> +	return cpu_suspend(0, arch_save_image);
> +}
> +
> +/*
> + * The framework loads the hibernation image into a linked list anchored
> + * at restore_pblist, 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.
> + */

Comment above needs updating. We are switching page tables to a set of
page tables that are certain to live at the same location in the older
kernel, that's the only reason, as we discussed. soft_restart will make
sure (again) to switch to 1:1 page tables so that we can call cpu_resume
with the MMU off.

> +static void notrace arch_restore_image(void *unused)
> +{
> +	struct pbe *pbe;
> +
> +	cpu_switch_mm(idmap_pgd, &init_mm);
> +	for (pbe = restore_pblist; pbe; pbe = pbe->next)
> +		copy_page(pbe->orig_address, pbe->address);
> +
> +	soft_restart(virt_to_phys(cpu_resume));
> +}
> +
> +static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> +
> +/*
> + * Resume from the hibernation image.
> + * Due to the kernel heap / data restore, stack contents change underneath
> + * and that would make function calls impossible; switch to a temporary
> + * stack within the nosave region to avoid that problem.
> + */
> +int swsusp_arch_resume(void)
> +{
> +	extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> +	call_with_stack(arch_restore_image, 0,
> +		resume_stack + sizeof(resume_stack));

This does not guarantee your stack is 8-byte aligned, that's not AAPCS
compliant and might buy you trouble.

Either you align the stack or you align the pointer you are passing.

Please have a look at kernel/process.c

Thanks,
Lorenzo

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Thu, Feb 27, 2014 at 11:57:58PM +0000, Sebastian Capella wrote:

[...]

> diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
> new file mode 100644
> index 0000000..a41e0e3
> --- /dev/null
> +++ b/arch/arm/kernel/hibernate.c
> @@ -0,0 +1,113 @@
> +/*
> + * 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 <rjw@sisk.pl>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/suspend.h>
> +#include <asm/tlbflush.h>
> +#include <asm/cacheflush.h>

You can drop tlbflush.h and cacheflush.h, they do not seem to be needed.

> +#include <asm/system_misc.h>
> +#include <asm/idmap.h>
> +#include <asm/suspend.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 notrace save_processor_state(void)
> +{
> +	WARN_ON(num_online_cpus() != 1);
> +	local_fiq_disable();
> +}
> +
> +void notrace restore_processor_state(void)
> +{
> +	local_fiq_enable();
> +}
> +
> +/*
> + * Snapshot kernel memory and reset the system.
> + *
> + * swsusp_save() is executed in the suspend finisher so that the CPU
> + * context pointer and memory are part of the saved image, which is
> + * required by the resume kernel image to restart execution from
> + * swsusp_arch_suspend().
> + *
> + * soft_restart is not technically needed, but is used to get success
> + * returned from cpu_suspend.
> + *
> + * When soft reboot completes, the hibernation snapshot is written out.
> + */
> +static int notrace arch_save_image(unsigned long unused)
> +{
> +	int ret;
> +
> +	ret = swsusp_save();
> +	if (ret == 0)
> +		soft_restart(virt_to_phys(cpu_resume));
> +	return ret;
> +}
> +
> +/*
> + * Save the current CPU state before suspend / poweroff.
> + */
> +int notrace swsusp_arch_suspend(void)
> +{
> +	return cpu_suspend(0, arch_save_image);
> +}
> +
> +/*
> + * The framework loads the hibernation image into a linked list anchored
> + * at restore_pblist, 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.
> + */

Comment above needs updating. We are switching page tables to a set of
page tables that are certain to live at the same location in the older
kernel, that's the only reason, as we discussed. soft_restart will make
sure (again) to switch to 1:1 page tables so that we can call cpu_resume
with the MMU off.

> +static void notrace arch_restore_image(void *unused)
> +{
> +	struct pbe *pbe;
> +
> +	cpu_switch_mm(idmap_pgd, &init_mm);
> +	for (pbe = restore_pblist; pbe; pbe = pbe->next)
> +		copy_page(pbe->orig_address, pbe->address);
> +
> +	soft_restart(virt_to_phys(cpu_resume));
> +}
> +
> +static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> +
> +/*
> + * Resume from the hibernation image.
> + * Due to the kernel heap / data restore, stack contents change underneath
> + * and that would make function calls impossible; switch to a temporary
> + * stack within the nosave region to avoid that problem.
> + */
> +int swsusp_arch_resume(void)
> +{
> +	extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> +	call_with_stack(arch_restore_image, 0,
> +		resume_stack + sizeof(resume_stack));

This does not guarantee your stack is 8-byte aligned, that's not AAPCS
compliant and might buy you trouble.

Either you align the stack or you align the pointer you are passing.

Please have a look at kernel/process.c

Thanks,
Lorenzo

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Russell King - ARM Linux]

On Thu, Feb 27, 2014 at 06:19:49PM -0800, Stephen Boyd wrote:
> On 02/27/14 17:47, Russ Dill wrote:
> > On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> >> On 02/27/14 15:57, Sebastian Capella wrote:
> >>> diff --git a/arch/arm/include/asm/memory.h
> >>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
> >>> a/arch/arm/include/asm/memory.h +++
> >>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
> >>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> >> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> >> __pa_symbol() for that matter? It looks like only x86 uses this on
> >> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> > From my understanding this needs to stick around so long as gcc 3.x is
> > supported (did it get dropped yet?) on ARM Linux since it doesn't
> > support -fno-strict-overflow.
> 
> I don't think it's been dropped yet but I wonder if anyone has tried
> recent kernels with such a compiler?
> 
> Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
> same treatment?

We've never had to play these kinds of games on ARM irrespective of
compiler version.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Russell King - ARM Linux]

On Thu, Feb 27, 2014 at 06:19:49PM -0800, Stephen Boyd wrote:
> On 02/27/14 17:47, Russ Dill wrote:
> > On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> >> On 02/27/14 15:57, Sebastian Capella wrote:
> >>> diff --git a/arch/arm/include/asm/memory.h
> >>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
> >>> a/arch/arm/include/asm/memory.h +++
> >>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
> >>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> >> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> >> __pa_symbol() for that matter? It looks like only x86 uses this on
> >> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> > From my understanding this needs to stick around so long as gcc 3.x is
> > supported (did it get dropped yet?) on ARM Linux since it doesn't
> > support -fno-strict-overflow.
> 
> I don't think it's been dropped yet but I wonder if anyone has tried
> recent kernels with such a compiler?
> 
> Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
> same treatment?

We've never had to play these kinds of games on ARM irrespective of
compiler version.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Sorry, I appear to be having problems with my emails where list emails are
bouncing back to me.  I'm working on correcting this now.

Quoting Lorenzo Pieralisi (2014-02-28 01:50:22)
> On Thu, Feb 27, 2014 at 11:57:58PM +0000, Sebastian Capella wrote:
> 
> [...]
> 
> > diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
> > new file mode 100644
> > index 0000000..a41e0e3
> > --- /dev/null
> > +++ b/arch/arm/kernel/hibernate.c
...
> > +#include <linux/suspend.h>
> > +#include <asm/tlbflush.h>
> > +#include <asm/cacheflush.h>
> 
> You can drop tlbflush.h and cacheflush.h, they do not seem to be needed.

Done! Thanks!

> > +
> > +/*
> > + * The framework loads the hibernation image into a linked list anchored
> > + * at restore_pblist, 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.
> > + */
> 
> Comment above needs updating. We are switching page tables to a set of
> page tables that are certain to live at the same location in the older
> kernel, that's the only reason, as we discussed. soft_restart will make
> sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> with the MMU off.

How does this look?

The framework loads as much of the hibernation image to final physical
pages as possible.  Any pages that were in use, will need to be restored
prior to the soft_restart.  The pages to restore are maintained in
the list anchored at restore_pblist.  At this point, we can swap the
pages to their final location.  We must switch the mapping to 1:1 to
ensure that when we overwrite the page table physical pages we're using
a known physical location (idmap_pgd) with known contents.

> > +/*
> > + * Resume from the hibernation image.
> > + * Due to the kernel heap / data restore, stack contents change underneath
> > + * and that would make function calls impossible; switch to a temporary
> > + * stack within the nosave region to avoid that problem.
> > + */
> > +int swsusp_arch_resume(void)
> > +{
> > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > +     call_with_stack(arch_restore_image, 0,
> > +             resume_stack + sizeof(resume_stack));
> 
> This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> compliant and might buy you trouble.
> 
> Either you align the stack or you align the pointer you are passing.
> 
> Please have a look at kernel/process.c

I've added this for now, do you see any issues?

-static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
+static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
-               resume_stack + sizeof(resume_stack));
+               resume_stack + ARRAY_SIZE(resume_stack));

Thanks Lorenzo!

Sebastian

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Sorry, I appear to be having problems with my emails where list emails are
bouncing back to me.  I'm working on correcting this now.

Quoting Lorenzo Pieralisi (2014-02-28 01:50:22)
> On Thu, Feb 27, 2014 at 11:57:58PM +0000, Sebastian Capella wrote:
> 
> [...]
> 
> > diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
> > new file mode 100644
> > index 0000000..a41e0e3
> > --- /dev/null
> > +++ b/arch/arm/kernel/hibernate.c
...
> > +#include <linux/suspend.h>
> > +#include <asm/tlbflush.h>
> > +#include <asm/cacheflush.h>
> 
> You can drop tlbflush.h and cacheflush.h, they do not seem to be needed.

Done! Thanks!

> > +
> > +/*
> > + * The framework loads the hibernation image into a linked list anchored
> > + * at restore_pblist, 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.
> > + */
> 
> Comment above needs updating. We are switching page tables to a set of
> page tables that are certain to live at the same location in the older
> kernel, that's the only reason, as we discussed. soft_restart will make
> sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> with the MMU off.

How does this look?

The framework loads as much of the hibernation image to final physical
pages as possible.  Any pages that were in use, will need to be restored
prior to the soft_restart.  The pages to restore are maintained in
the list anchored at restore_pblist.  At this point, we can swap the
pages to their final location.  We must switch the mapping to 1:1 to
ensure that when we overwrite the page table physical pages we're using
a known physical location (idmap_pgd) with known contents.

> > +/*
> > + * Resume from the hibernation image.
> > + * Due to the kernel heap / data restore, stack contents change underneath
> > + * and that would make function calls impossible; switch to a temporary
> > + * stack within the nosave region to avoid that problem.
> > + */
> > +int swsusp_arch_resume(void)
> > +{
> > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > +     call_with_stack(arch_restore_image, 0,
> > +             resume_stack + sizeof(resume_stack));
> 
> This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> compliant and might buy you trouble.
> 
> Either you align the stack or you align the pointer you are passing.
> 
> Please have a look at kernel/process.c

I've added this for now, do you see any issues?

-static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
+static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
-               resume_stack + sizeof(resume_stack));
+               resume_stack + ARRAY_SIZE(resume_stack));

Thanks Lorenzo!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:

[...]

> > > +
> > > +/*
> > > + * The framework loads the hibernation image into a linked list anchored
> > > + * at restore_pblist, 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.
> > > + */
> > 
> > Comment above needs updating. We are switching page tables to a set of
> > page tables that are certain to live at the same location in the older
> > kernel, that's the only reason, as we discussed. soft_restart will make
> > sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> > with the MMU off.
> 
> How does this look?
> 
> The framework loads as much of the hibernation image to final physical
> pages as possible.  Any pages that were in use, will need to be restored
> prior to the soft_restart.  The pages to restore are maintained in
> the list anchored at restore_pblist.  At this point, we can swap the
> pages to their final location.  We must switch the mapping to 1:1 to
> ensure that when we overwrite the page table physical pages we're using
> a known physical location (idmap_pgd) with known contents.

It is ok, a tad too verbose. All I care about is a comment describing
what's really needed, the existing one was confusing and wrong.

> > > +/*
> > > + * Resume from the hibernation image.
> > > + * Due to the kernel heap / data restore, stack contents change underneath
> > > + * and that would make function calls impossible; switch to a temporary
> > > + * stack within the nosave region to avoid that problem.
> > > + */
> > > +int swsusp_arch_resume(void)
> > > +{
> > > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > > +     call_with_stack(arch_restore_image, 0,
> > > +             resume_stack + sizeof(resume_stack));
> > 
> > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > compliant and might buy you trouble.
> > 
> > Either you align the stack or you align the pointer you are passing.
> > 
> > Please have a look at kernel/process.c
> 
> I've added this for now, do you see any issues?
> 
> -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> -               resume_stack + sizeof(resume_stack));
> +               resume_stack + ARRAY_SIZE(resume_stack));

I do not see why the stack depends on the PAGE_SIZE. I would be surprised
if you need more than a few bytes (given that soft_restart switches stack
again...), go through it with a debugger, it is easy to check the stack
usage and allow for some extra buffer (but half a page is not needed).

My main concern was alignment, and now that's fixed.

Thanks !
Lorenzo

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:

[...]

> > > +
> > > +/*
> > > + * The framework loads the hibernation image into a linked list anchored
> > > + * at restore_pblist, 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.
> > > + */
> > 
> > Comment above needs updating. We are switching page tables to a set of
> > page tables that are certain to live at the same location in the older
> > kernel, that's the only reason, as we discussed. soft_restart will make
> > sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> > with the MMU off.
> 
> How does this look?
> 
> The framework loads as much of the hibernation image to final physical
> pages as possible.  Any pages that were in use, will need to be restored
> prior to the soft_restart.  The pages to restore are maintained in
> the list anchored at restore_pblist.  At this point, we can swap the
> pages to their final location.  We must switch the mapping to 1:1 to
> ensure that when we overwrite the page table physical pages we're using
> a known physical location (idmap_pgd) with known contents.

It is ok, a tad too verbose. All I care about is a comment describing
what's really needed, the existing one was confusing and wrong.

> > > +/*
> > > + * Resume from the hibernation image.
> > > + * Due to the kernel heap / data restore, stack contents change underneath
> > > + * and that would make function calls impossible; switch to a temporary
> > > + * stack within the nosave region to avoid that problem.
> > > + */
> > > +int swsusp_arch_resume(void)
> > > +{
> > > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > > +     call_with_stack(arch_restore_image, 0,
> > > +             resume_stack + sizeof(resume_stack));
> > 
> > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > compliant and might buy you trouble.
> > 
> > Either you align the stack or you align the pointer you are passing.
> > 
> > Please have a look at kernel/process.c
> 
> I've added this for now, do you see any issues?
> 
> -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> -               resume_stack + sizeof(resume_stack));
> +               resume_stack + ARRAY_SIZE(resume_stack));

I do not see why the stack depends on the PAGE_SIZE. I would be surprised
if you need more than a few bytes (given that soft_restart switches stack
again...), go through it with a debugger, it is easy to check the stack
usage and allow for some extra buffer (but half a page is not needed).

My main concern was alignment, and now that's fixed.

Thanks !
Lorenzo

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:

[...]

> > > +
> > > +/*
> > > + * The framework loads the hibernation image into a linked list anchored
> > > + * at restore_pblist, 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.
> > > + */
> > 
> > Comment above needs updating. We are switching page tables to a set of
> > page tables that are certain to live at the same location in the older
> > kernel, that's the only reason, as we discussed. soft_restart will make
> > sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> > with the MMU off.
> 
> How does this look?
> 
> The framework loads as much of the hibernation image to final physical
> pages as possible.  Any pages that were in use, will need to be restored
> prior to the soft_restart.  The pages to restore are maintained in
> the list anchored at restore_pblist.  At this point, we can swap the
> pages to their final location.  We must switch the mapping to 1:1 to
> ensure that when we overwrite the page table physical pages we're using
> a known physical location (idmap_pgd) with known contents.

It is ok, a tad too verbose. All I care about is a comment describing
what's really needed, the existing one was confusing and wrong.

> > > +/*
> > > + * Resume from the hibernation image.
> > > + * Due to the kernel heap / data restore, stack contents change underneath
> > > + * and that would make function calls impossible; switch to a temporary
> > > + * stack within the nosave region to avoid that problem.
> > > + */
> > > +int swsusp_arch_resume(void)
> > > +{
> > > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > > +     call_with_stack(arch_restore_image, 0,
> > > +             resume_stack + sizeof(resume_stack));
> > 
> > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > compliant and might buy you trouble.
> > 
> > Either you align the stack or you align the pointer you are passing.
> > 
> > Please have a look at kernel/process.c
> 
> I've added this for now, do you see any issues?
> 
> -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> -               resume_stack + sizeof(resume_stack));
> +               resume_stack + ARRAY_SIZE(resume_stack));

I do not see why the stack depends on the PAGE_SIZE. I would be surprised
if you need more than a few bytes (given that soft_restart switches stack
again...), go through it with a debugger, it is easy to check the stack
usage and allow for some extra buffer (but half a page is not needed).

My main concern was alignment, and now that's fixed.

Thanks !
Lorenzo

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> 
> [...]
> 
> > > > +
> > > > +/*
> > > > + * The framework loads the hibernation image into a linked list anchored
> > > > + * at restore_pblist, 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.
> > > > + */
> > > 
> > > Comment above needs updating. We are switching page tables to a set of
> > > page tables that are certain to live at the same location in the older
> > > kernel, that's the only reason, as we discussed. soft_restart will make
> > > sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> > > with the MMU off.
> > 
> > How does this look?
> > 
> > The framework loads as much of the hibernation image to final physical
> > pages as possible.  Any pages that were in use, will need to be restored
> > prior to the soft_restart.  The pages to restore are maintained in
> > the list anchored at restore_pblist.  At this point, we can swap the
> > pages to their final location.  We must switch the mapping to 1:1 to
> > ensure that when we overwrite the page table physical pages we're using
> > a known physical location (idmap_pgd) with known contents.
> 
> It is ok, a tad too verbose. All I care about is a comment describing
> what's really needed, the existing one was confusing and wrong.

Maybe more like: 

Restore physical pages that were in use while loading hibernation image.
Use idmap_pgd so our page tables use the same physical address as the
hibernation image.  Will be overwriten with the same contents.

> > > > +/*
> > > > + * Resume from the hibernation image.
> > > > + * Due to the kernel heap / data restore, stack contents change underneath
> > > > + * and that would make function calls impossible; switch to a temporary
> > > > + * stack within the nosave region to avoid that problem.
> > > > + */
> > > > +int swsusp_arch_resume(void)
> > > > +{
> > > > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > > > +     call_with_stack(arch_restore_image, 0,
> > > > +             resume_stack + sizeof(resume_stack));
> > > 
> > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > compliant and might buy you trouble.
> > > 
> > > Either you align the stack or you align the pointer you are passing.
> > > 
> > > Please have a look at kernel/process.c
> > 
> > I've added this for now, do you see any issues?
> > 
> > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > -               resume_stack + sizeof(resume_stack));
> > +               resume_stack + ARRAY_SIZE(resume_stack));
> 
> I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> if you need more than a few bytes (given that soft_restart switches stack
> again...), go through it with a debugger, it is easy to check the stack
> usage and allow for some extra buffer (but half a page is not needed).

I assuming this is becase the no-save region is one page anyway (we skip
restoring the no-save region physical page).  So maybe 1/2 is a way to
leave some room for whatever else may need to be here, but in any case
the 4k is used for nosave.  I think you're right that it can be much less.

I'll check into this...

Thanks!

Sebastian

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> 
> [...]
> 
> > > > +
> > > > +/*
> > > > + * The framework loads the hibernation image into a linked list anchored
> > > > + * at restore_pblist, 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.
> > > > + */
> > > 
> > > Comment above needs updating. We are switching page tables to a set of
> > > page tables that are certain to live at the same location in the older
> > > kernel, that's the only reason, as we discussed. soft_restart will make
> > > sure (again) to switch to 1:1 page tables so that we can call cpu_resume
> > > with the MMU off.
> > 
> > How does this look?
> > 
> > The framework loads as much of the hibernation image to final physical
> > pages as possible.  Any pages that were in use, will need to be restored
> > prior to the soft_restart.  The pages to restore are maintained in
> > the list anchored at restore_pblist.  At this point, we can swap the
> > pages to their final location.  We must switch the mapping to 1:1 to
> > ensure that when we overwrite the page table physical pages we're using
> > a known physical location (idmap_pgd) with known contents.
> 
> It is ok, a tad too verbose. All I care about is a comment describing
> what's really needed, the existing one was confusing and wrong.

Maybe more like: 

Restore physical pages that were in use while loading hibernation image.
Use idmap_pgd so our page tables use the same physical address as the
hibernation image.  Will be overwriten with the same contents.

> > > > +/*
> > > > + * Resume from the hibernation image.
> > > > + * Due to the kernel heap / data restore, stack contents change underneath
> > > > + * and that would make function calls impossible; switch to a temporary
> > > > + * stack within the nosave region to avoid that problem.
> > > > + */
> > > > +int swsusp_arch_resume(void)
> > > > +{
> > > > +     extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
> > > > +     call_with_stack(arch_restore_image, 0,
> > > > +             resume_stack + sizeof(resume_stack));
> > > 
> > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > compliant and might buy you trouble.
> > > 
> > > Either you align the stack or you align the pointer you are passing.
> > > 
> > > Please have a look at kernel/process.c
> > 
> > I've added this for now, do you see any issues?
> > 
> > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > -               resume_stack + sizeof(resume_stack));
> > +               resume_stack + ARRAY_SIZE(resume_stack));
> 
> I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> if you need more than a few bytes (given that soft_restart switches stack
> again...), go through it with a debugger, it is easy to check the stack
> usage and allow for some extra buffer (but half a page is not needed).

I assuming this is becase the no-save region is one page anyway (we skip
restoring the no-save region physical page).  So maybe 1/2 is a way to
leave some room for whatever else may need to be here, but in any case
the 4k is used for nosave.  I think you're right that it can be much less.

I'll check into this...

Thanks!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Sebastian Capella (2014-02-28 15:38:54)
> Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > 
> > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > compliant and might buy you trouble.
> > > > 
> > > > Either you align the stack or you align the pointer you are passing.
> > > > 
> > > > Please have a look at kernel/process.c
> > > 
> > > I've added this for now, do you see any issues?
> > > 
> > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > -               resume_stack + sizeof(resume_stack));
> > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > 
> > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > if you need more than a few bytes (given that soft_restart switches stack
> > again...), go through it with a debugger, it is easy to check the stack
> > usage and allow for some extra buffer (but half a page is not needed).
> 
> I assuming this is becase the no-save region is one page anyway (we skip
> restoring the no-save region physical page).  So maybe 1/2 is a way to
> leave some room for whatever else may need to be here, but in any case
> the 4k is used for nosave.  I think you're right that it can be much less.

Hi Lorenzo,

Are you ok with this just being half a page?  Or do you want me to try
to reduce the stack size?  I am at Connect without my debugger, so in
that case it would have to wait until next week.

The change for alignment is in as discussed.

Thanks!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Sebastian Capella (2014-02-28 15:38:54)
> Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > 
> > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > compliant and might buy you trouble.
> > > > 
> > > > Either you align the stack or you align the pointer you are passing.
> > > > 
> > > > Please have a look at kernel/process.c
> > > 
> > > I've added this for now, do you see any issues?
> > > 
> > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > -               resume_stack + sizeof(resume_stack));
> > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > 
> > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > if you need more than a few bytes (given that soft_restart switches stack
> > again...), go through it with a debugger, it is easy to check the stack
> > usage and allow for some extra buffer (but half a page is not needed).
> 
> I assuming this is becase the no-save region is one page anyway (we skip
> restoring the no-save region physical page).  So maybe 1/2 is a way to
> leave some room for whatever else may need to be here, but in any case
> the 4k is used for nosave.  I think you're right that it can be much less.

Hi Lorenzo,

Are you ok with this just being half a page?  Or do you want me to try
to reduce the stack size?  I am at Connect without my debugger, so in
that case it would have to wait until next week.

The change for alignment is in as discussed.

Thanks!

Sebastian

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Sebastian Capella (2014-02-28 15:38:54)
> Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > 
> > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > compliant and might buy you trouble.
> > > > 
> > > > Either you align the stack or you align the pointer you are passing.
> > > > 
> > > > Please have a look at kernel/process.c
> > > 
> > > I've added this for now, do you see any issues?
> > > 
> > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > -               resume_stack + sizeof(resume_stack));
> > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > 
> > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > if you need more than a few bytes (given that soft_restart switches stack
> > again...), go through it with a debugger, it is easy to check the stack
> > usage and allow for some extra buffer (but half a page is not needed).
> 
> I assuming this is becase the no-save region is one page anyway (we skip
> restoring the no-save region physical page).  So maybe 1/2 is a way to
> leave some room for whatever else may need to be here, but in any case
> the 4k is used for nosave.  I think you're right that it can be much less.

Hi Lorenzo,

Are you ok with this just being half a page?  Or do you want me to try
to reduce the stack size?  I am at Connect without my debugger, so in
that case it would have to wait until next week.

The change for alignment is in as discussed.

Thanks!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Tue, Mar 04, 2014 at 09:55:31AM +0000, Sebastian Capella wrote:
> Quoting Sebastian Capella (2014-02-28 15:38:54)
> > Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > > 
> > > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > > compliant and might buy you trouble.
> > > > > 
> > > > > Either you align the stack or you align the pointer you are passing.
> > > > > 
> > > > > Please have a look at kernel/process.c
> > > > 
> > > > I've added this for now, do you see any issues?
> > > > 
> > > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > > -               resume_stack + sizeof(resume_stack));
> > > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > > 
> > > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > > if you need more than a few bytes (given that soft_restart switches stack
> > > again...), go through it with a debugger, it is easy to check the stack
> > > usage and allow for some extra buffer (but half a page is not needed).
> > 
> > I assuming this is becase the no-save region is one page anyway (we skip
> > restoring the no-save region physical page).  So maybe 1/2 is a way to
> > leave some room for whatever else may need to be here, but in any case
> > the 4k is used for nosave.  I think you're right that it can be much less.
> 
> Hi Lorenzo,
> 
> Are you ok with this just being half a page?  Or do you want me to try
> to reduce the stack size?  I am at Connect without my debugger, so in
> that case it would have to wait until next week.

I am ok, either you leave that as it is (that multiple division looks
horrible but it is just nitpicking on my side) or define it as an u8 array,
stick __attribute__((aligned(8)) to the definition (and explain why) and be
done with it.

You can add my:

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Tue, Mar 04, 2014 at 09:55:31AM +0000, Sebastian Capella wrote:
> Quoting Sebastian Capella (2014-02-28 15:38:54)
> > Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > > 
> > > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > > compliant and might buy you trouble.
> > > > > 
> > > > > Either you align the stack or you align the pointer you are passing.
> > > > > 
> > > > > Please have a look at kernel/process.c
> > > > 
> > > > I've added this for now, do you see any issues?
> > > > 
> > > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > > -               resume_stack + sizeof(resume_stack));
> > > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > > 
> > > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > > if you need more than a few bytes (given that soft_restart switches stack
> > > again...), go through it with a debugger, it is easy to check the stack
> > > usage and allow for some extra buffer (but half a page is not needed).
> > 
> > I assuming this is becase the no-save region is one page anyway (we skip
> > restoring the no-save region physical page).  So maybe 1/2 is a way to
> > leave some room for whatever else may need to be here, but in any case
> > the 4k is used for nosave.  I think you're right that it can be much less.
> 
> Hi Lorenzo,
> 
> Are you ok with this just being half a page?  Or do you want me to try
> to reduce the stack size?  I am at Connect without my debugger, so in
> that case it would have to wait until next week.

I am ok, either you leave that as it is (that multiple division looks
horrible but it is just nitpicking on my side) or define it as an u8 array,
stick __attribute__((aligned(8)) to the definition (and explain why) and be
done with it.

You can add my:

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Lorenzo Pieralisi]

On Tue, Mar 04, 2014 at 09:55:31AM +0000, Sebastian Capella wrote:
> Quoting Sebastian Capella (2014-02-28 15:38:54)
> > Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > > 
> > > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > > compliant and might buy you trouble.
> > > > > 
> > > > > Either you align the stack or you align the pointer you are passing.
> > > > > 
> > > > > Please have a look at kernel/process.c
> > > > 
> > > > I've added this for now, do you see any issues?
> > > > 
> > > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > > -               resume_stack + sizeof(resume_stack));
> > > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > > 
> > > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > > if you need more than a few bytes (given that soft_restart switches stack
> > > again...), go through it with a debugger, it is easy to check the stack
> > > usage and allow for some extra buffer (but half a page is not needed).
> > 
> > I assuming this is becase the no-save region is one page anyway (we skip
> > restoring the no-save region physical page).  So maybe 1/2 is a way to
> > leave some room for whatever else may need to be here, but in any case
> > the 4k is used for nosave.  I think you're right that it can be much less.
> 
> Hi Lorenzo,
> 
> Are you ok with this just being half a page?  Or do you want me to try
> to reduce the stack size?  I am at Connect without my debugger, so in
> that case it would have to wait until next week.

I am ok, either you leave that as it is (that multiple division looks
horrible but it is just nitpicking on my side) or define it as an u8 array,
stick __attribute__((aligned(8)) to the definition (and explain why) and be
done with it.

You can add my:

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Lorenzo Pieralisi (2014-03-04 03:17:46)
> On Tue, Mar 04, 2014 at 09:55:31AM +0000, Sebastian Capella wrote:
> > Quoting Sebastian Capella (2014-02-28 15:38:54)
> > > Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > > > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > > > 
> > > > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > > > compliant and might buy you trouble.
> > > > > > 
> > > > > > Either you align the stack or you align the pointer you are passing.
> > > > > > 
> > > > > > Please have a look at kernel/process.c
> > > > > 
> > > > > I've added this for now, do you see any issues?
> > > > > 
> > > > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > > > -               resume_stack + sizeof(resume_stack));
> > > > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > > > 
> > > > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > > > if you need more than a few bytes (given that soft_restart switches stack
> > > > again...), go through it with a debugger, it is easy to check the stack
> > > > usage and allow for some extra buffer (but half a page is not needed).
> > > 
> > > I assuming this is becase the no-save region is one page anyway (we skip
> > > restoring the no-save region physical page).  So maybe 1/2 is a way to
> > > leave some room for whatever else may need to be here, but in any case
> > > the 4k is used for nosave.  I think you're right that it can be much less.
> > 
> > Hi Lorenzo,
> > 
> > Are you ok with this just being half a page?  Or do you want me to try
> > to reduce the stack size?  I am at Connect without my debugger, so in
> > that case it would have to wait until next week.
> 
> I am ok, either you leave that as it is (that multiple division looks
> horrible but it is just nitpicking on my side) or define it as an u8 array,
> stick __attribute__((aligned(8)) to the definition (and explain why) and be
> done with it.
> 
> You can add my:
> 
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Thanks Lorenzo!

Sebastian

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Lorenzo Pieralisi (2014-03-04 03:17:46)
> On Tue, Mar 04, 2014 at 09:55:31AM +0000, Sebastian Capella wrote:
> > Quoting Sebastian Capella (2014-02-28 15:38:54)
> > > Quoting Lorenzo Pieralisi (2014-02-28 14:49:33)
> > > > On Fri, Feb 28, 2014 at 08:15:57PM +0000, Sebastian Capella wrote:
> > > > > > 
> > > > > > This does not guarantee your stack is 8-byte aligned, that's not AAPCS
> > > > > > compliant and might buy you trouble.
> > > > > > 
> > > > > > Either you align the stack or you align the pointer you are passing.
> > > > > > 
> > > > > > Please have a look at kernel/process.c
> > > > > 
> > > > > I've added this for now, do you see any issues?
> > > > > 
> > > > > -static u8 resume_stack[PAGE_SIZE/2] __nosavedata;
> > > > > +static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> > > > > -               resume_stack + sizeof(resume_stack));
> > > > > +               resume_stack + ARRAY_SIZE(resume_stack));
> > > > 
> > > > I do not see why the stack depends on the PAGE_SIZE. I would be surprised
> > > > if you need more than a few bytes (given that soft_restart switches stack
> > > > again...), go through it with a debugger, it is easy to check the stack
> > > > usage and allow for some extra buffer (but half a page is not needed).
> > > 
> > > I assuming this is becase the no-save region is one page anyway (we skip
> > > restoring the no-save region physical page).  So maybe 1/2 is a way to
> > > leave some room for whatever else may need to be here, but in any case
> > > the 4k is used for nosave.  I think you're right that it can be much less.
> > 
> > Hi Lorenzo,
> > 
> > Are you ok with this just being half a page?  Or do you want me to try
> > to reduce the stack size?  I am at Connect without my debugger, so in
> > that case it would have to wait until next week.
> 
> I am ok, either you leave that as it is (that multiple division looks
> horrible but it is just nitpicking on my side) or define it as an u8 array,
> stick __attribute__((aligned(8)) to the definition (and explain why) and be
> done with it.
> 
> You can add my:
> 
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Thanks Lorenzo!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Sebastian Capella (2014-02-28 10:17:31)
> Quoting Russell King - ARM Linux (2014-02-28 02:20:18)
> > On Thu, Feb 27, 2014 at 06:19:49PM -0800, Stephen Boyd wrote:
> > > On 02/27/14 17:47, Russ Dill wrote:
> > > > On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> > > >> On 02/27/14 15:57, Sebastian Capella wrote:
> > > >>> diff --git a/arch/arm/include/asm/memory.h
> > > >>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
> > > >>> a/arch/arm/include/asm/memory.h +++
> > > >>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
> > > >>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> > > >> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> > > >> __pa_symbol() for that matter? It looks like only x86 uses this on
> > > >> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> > > > From my understanding this needs to stick around so long as gcc 3.x is
> > > > supported (did it get dropped yet?) on ARM Linux since it doesn't
> > > > support -fno-strict-overflow.
> > > 
> > > I don't think it's been dropped yet but I wonder if anyone has tried
> > > recent kernels with such a compiler?
> > > 
> > > Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
> > > same treatment?
> > 
> > We've never had to play these kinds of games on ARM irrespective of
> > compiler version.
> 
> I am using gcc 4.6.3.  I can try removing it but I suspect it will just
> work without it.  Let me see if I can get an older compiler and try both
> ways.

Hi,

I've been struggling a bit to test 3.x compilers on this.

I'm running an armv7 board, but the 3.x compilers I'm trying
don't appear to suport armv7.

Anyone have any suggestions?  Is this a worthwhile effort?

Thanks!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Sebastian Capella (2014-02-28 10:17:31)
> Quoting Russell King - ARM Linux (2014-02-28 02:20:18)
> > On Thu, Feb 27, 2014 at 06:19:49PM -0800, Stephen Boyd wrote:
> > > On 02/27/14 17:47, Russ Dill wrote:
> > > > On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> > > >> On 02/27/14 15:57, Sebastian Capella wrote:
> > > >>> diff --git a/arch/arm/include/asm/memory.h
> > > >>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
> > > >>> a/arch/arm/include/asm/memory.h +++
> > > >>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
> > > >>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> > > >> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> > > >> __pa_symbol() for that matter? It looks like only x86 uses this on
> > > >> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> > > > From my understanding this needs to stick around so long as gcc 3.x is
> > > > supported (did it get dropped yet?) on ARM Linux since it doesn't
> > > > support -fno-strict-overflow.
> > > 
> > > I don't think it's been dropped yet but I wonder if anyone has tried
> > > recent kernels with such a compiler?
> > > 
> > > Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
> > > same treatment?
> > 
> > We've never had to play these kinds of games on ARM irrespective of
> > compiler version.
> 
> I am using gcc 4.6.3.  I can try removing it but I suspect it will just
> work without it.  Let me see if I can get an older compiler and try both
> ways.

Hi,

I've been struggling a bit to test 3.x compilers on this.

I'm running an armv7 board, but the 3.x compilers I'm trying
don't appear to suport armv7.

Anyone have any suggestions?  Is this a worthwhile effort?

Thanks!

Sebastian

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Quoting Sebastian Capella (2014-02-28 10:17:31)
> Quoting Russell King - ARM Linux (2014-02-28 02:20:18)
> > On Thu, Feb 27, 2014 at 06:19:49PM -0800, Stephen Boyd wrote:
> > > On 02/27/14 17:47, Russ Dill wrote:
> > > > On 02/27/2014 04:09 PM, Stephen Boyd wrote:
> > > >> On 02/27/14 15:57, Sebastian Capella wrote:
> > > >>> diff --git a/arch/arm/include/asm/memory.h
> > > >>> b/arch/arm/include/asm/memory.h index 8756e4b..1079ea8 100644 ---
> > > >>> a/arch/arm/include/asm/memory.h +++
> > > >>> b/arch/arm/include/asm/memory.h @@ -291,6 +291,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((phys_addr_t)(x))) +#define __pa_symbol(x)
> > > >>> __pa(RELOC_HIDE((unsigned long)(x), 0))
> > > >> Just curious, is there a reason for the RELOC_HIDE() here? Or 
> > > >> __pa_symbol() for that matter? It looks like only x86 uses this on
> > > >> the __nosave_{begin,end} symbol. Maybe it's copy-pasta?
> > > > From my understanding this needs to stick around so long as gcc 3.x is
> > > > supported (did it get dropped yet?) on ARM Linux since it doesn't
> > > > support -fno-strict-overflow.
> > > 
> > > I don't think it's been dropped yet but I wonder if anyone has tried
> > > recent kernels with such a compiler?
> > > 
> > > Would the usage of &__pv_table_begin in arch/arm/mm/mmu.c also need the
> > > same treatment?
> > 
> > We've never had to play these kinds of games on ARM irrespective of
> > compiler version.
> 
> I am using gcc 4.6.3.  I can try removing it but I suspect it will just
> work without it.  Let me see if I can get an older compiler and try both
> ways.

Hi,

I've been struggling a bit to test 3.x compilers on this.

I'm running an armv7 board, but the 3.x compilers I'm trying
don't appear to suport armv7.

Anyone have any suggestions?  Is this a worthwhile effort?

Thanks!

Sebastian

Re: [PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Want to log my new email with this thread in case any questions arise
later and people have trouble finding me.

sebcape@gmail.com

Thanks!

Sebastian Capella

[PATCH v6 2/2] ARM hibernation / suspend-to-disk

[Sebastian Capella]

Want to log my new email with this thread in case any questions arise
later and people have trouble finding me.

sebcape at gmail.com

Thanks!

Sebastian Capella