arm64: hibernate: Support DEBUG_PAGEALLOC and hibernate on non-boot cpu

Resume from hibernate needs to clean any text executed by the kernel with the MMU off to the PoC. Collect these functions together into a new .mmuoff.text section. __boot_cpu_mode and secondary_holding_pen_release are data that is read or written with the MMU off. Add these to a new .mmuoff.data section.

This covers booting of secondary cores and the cpu_suspend() path used by cpu-idle and suspend-to-ram.

The bulk of head.S is not included, as the primary boot code is only ever executed once, the kernel never needs to ensure it is cleaned to a particular point in the cache.

Signed-off-by James Morse <[email protected]>

---
Changes since v2:
* mmuoff.data section added
* secondary_holding_pen_release initialisation moved to head.S
* secondary_holding_pen and set_cpu_boot_mode_flag moved into mmuoff.text

 arch/arm64/include/asm/sections.h  |  2 ++
 arch/arm64/kernel/head.S           | 10 +++++++++-
 arch/arm64/kernel/sleep.S          |  2 ++
 arch/arm64/kernel/smp_spin_table.c |  2 +-
 arch/arm64/kernel/vmlinux.lds.S    |  8 ++++++++
 arch/arm64/mm/proc.S               |  4 ++++
 6 files changed, 26 insertions(+), 2 deletions(-)

arch/arm64/include/asm/sections.h +2 -0

index cb68eb348566..7eecfa110330 100644
@@ -24,5 +24,7 @@ extern char __hibernate_exit_text_start[], __hibernate_exit_text_end[];
 extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 extern char __hyp_text_start[], __hyp_text_end[];
 extern char __idmap_text_start[], __idmap_text_end[];
+extern char __mmuoff_text_start[], __mmuoff_text_end[];
+extern char __mmuoff_data_start[], __mmuoff_data_end[];
 
 #endif /* __ASM_SECTIONS_H */

arch/arm64/kernel/head.S +0 -0

index 2c6e598a94dc..6e56cd136d27 100644

arch/arm64/kernel/head.S +2 -0

+
 /*
  * Kernel startup entry point.
  * ---------------------------

@@ -477,6 +480,7 @@ ENTRY(kimage_vaddr)
  * Returns either BOOT_CPU_MODE_EL1 or BOOT_CPU_MODE_EL2 in x20 if
  * booted in EL1 or EL2 respectively.
  */
+	.pushsection ".mmuoff.text", "ax"
 ENTRY(el2_setup)
 	mrs	x0, CurrentEL
 	cmp	x0, #CurrentEL_EL2

Architecture code may need to do extra work when secondary CPUs are disabled during hibernate and resume. This may include pushing sleeping CPUs into a deeper power-saving state, or influencing which CPU resume occurs on.

Define a macro arch_hibernation_disable_cpus(), which defaults to calling disable_nonboot_cpus() if undefined. Architectures that need to do extra work around these calls can use this to influence disable_nonboot_cpus() behaviour. The macro should be defined in asm/suspend.h, and ARCH_HIBERNATION_CPUHP should be added to Kconfig.

Signed-off-by James Morse <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Pavel Machek <[email protected]>

---
Changes since v2:
* Added CONFIG_ARCH_HIBERNATION_CPUHP include guard allowing

* Switch to macro approach.

 kernel/power/hibernate.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

kernel/power/hibernate.c +0 -0

index fca9254280ee..338745e78f7e 100644
@@ -31,8 +31,16 @@
 #include <linux/ktime.h>
 #include <trace/events/power.h>

kernel/power/hibernate.c +3 -2

+#endif
 
 static int nocompress;
 static int noresume;

@@ -279,7 +287,7 @@ static int create_image(int platform_mode)
 	if (error || hibernation_test(TEST_PLATFORM))
 		goto Platform_finish;
 
-	error = disable_nonboot_cpus();
+	error = arch_hibernation_disable_cpus(true);
 	if (error || hibernation_test(TEST_CPUS))
 		goto Enable_cpus;
 

@@ -433,7 +441,7 @@ static int resume_target_kernel(bool platform_mode)
 	if (error)
 		goto Cleanup;
 
-	error = disable_nonboot_cpus();
+	error = arch_hibernation_disable_cpus(false);
 	if (error)
 		goto Enable_cpus;
 

Quoted 11 lines · James Morse

Architecture code may need to do extra work when secondary CPUs are disabled during hibernate and resume. This may include pushing sleeping CPUs into a deeper power-saving state, or influencing which CPU resume occurs on.

Define a macro arch_hibernation_disable_cpus(), which defaults to calling disable_nonboot_cpus() if undefined. Architectures that need to do extra work around these calls can use this to influence disable_nonboot_cpus() behaviour. The macro should be defined in asm/suspend.h, and ARCH_HIBERNATION_CPUHP should be added to Kconfig.

Quoted 3 lines

Signed-off-by James Morse <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Pavel Machek <[email protected]>

As you noted, this could be used to address the x86 issue that Yu is working on, so I'd like it to go in as the first patch in the series and through the PM tree.

For the x86 case we'll also need the complementary "enable", so why don't you add it here and then use it instead of the enable_nonboot_cpus()?

Quoted 5 lines · James Morse

Architecture code may need to do extra work when secondary CPUs are disabled during hibernate and resume. This may include pushing sleeping CPUs into a deeper power-saving state, or influencing which CPU resume occurs on.

Quoted 5 lines

Define a macro arch_hibernation_disable_cpus(), which defaults to calling disable_nonboot_cpus() if undefined. Architectures that need to do extra work around these calls can use this to influence disable_nonboot_cpus() behaviour. The macro should be defined in asm/suspend.h, and ARCH_HIBERNATION_CPUHP should be added to Kconfig.

As you noted, this could be used to address the x86 issue that Yu is working on, so I'd like it to go in as the first patch in the series and through the PM tree.

Quoted 4 lines

---
Changes since v2:
* Added CONFIG_ARCH_HIBERNATION_CPUHP include guard allowing

What about calling it CONFIG_ARCH_HIBERNATION_CPU_OFFLINE? It's not hotplug really.

Quoted 15 lines

kernel/power/hibernate.c +5 -0

index fca9254280ee..338745e78f7e 100644
@@ -31,8 +31,16 @@
 #include <linux/ktime.h>
 #include <trace/events/power.h>
 
+#ifdef CONFIG_ARCH_HIBERNATION_CPUHP
+/* Arch definition of the arch_hibernation_disable_cpus() macro? */
+#include <asm/suspend.h>
+#endif
+
 #include "power.h"
 

kernel/power/hibernate.c line 42

Quoted 2 lines

kernel/power/hibernate.c +2 -0

+#ifndef arch_hibernation_disable_cpus
+#define arch_hibernation_disable_cpus(x) disable_nonboot_cpus()

For the x86 case we'll also need the complementary "enable", so why don't you add it here and then use it instead of the enable_nonboot_cpus()?

Quoted 5 lines

kernel/power/hibernate.c +1 -0

+#endif
 
 static int nocompress;
 static int noresume;

kernel/power/hibernate.c int hibernation_platform_enter(void) · line 562

Quoted 6 lines

kernel/power/hibernate.c +1 -1

@@ -551,7 +559,7 @@ int hibernation_platform_enter(void)
 	if (error)
 		goto Platform_finish;
 
-	error = disable_nonboot_cpus();
+	error = arch_hibernation_disable_cpus(true);

Does this one here actually matter?

Hi Rafael,

Quoted 6 lines · Rafael J. Wysocki

Quoted 5 lines · James Morse

Architecture code may need to do extra work when secondary CPUs are disabled during hibernate and resume. This may include pushing sleeping CPUs into a deeper power-saving state, or influencing which CPU resume occurs on.

Quoted 8 lines

Quoted 5 lines

Define a macro arch_hibernation_disable_cpus(), which defaults to calling disable_nonboot_cpus() if undefined. Architectures that need to do extra work around these calls can use this to influence disable_nonboot_cpus() behaviour. The macro should be defined in asm/suspend.h, and ARCH_HIBERNATION_CPUHP should be added to Kconfig.

As you noted, this could be used to address the x86 issue that Yu is working on, so I'd like it to go in as the first patch in the series and through the PM tree.

Sure, I will split the series here, and group the later patches so there are no dependencies.

Quoted 1 line

Quoted 6 lines

Quoted 3 lines

---
Changes since v2:
* Added CONFIG_ARCH_HIBERNATION_CPUHP include guard allowing

What about calling it CONFIG_ARCH_HIBERNATION_CPU_OFFLINE? It's not hotplug really.

Even with the enable calls added? CONFIG_ARCH_HIBERNATION_CPU_HOOKS?

Quoted 15 lines

Quoted 15 lines

kernel/power/hibernate.c +5 -0

index fca9254280ee..338745e78f7e 100644
@@ -31,8 +31,16 @@
 #include <linux/ktime.h>
 #include <trace/events/power.h>
 
+#ifdef CONFIG_ARCH_HIBERNATION_CPUHP
+/* Arch definition of the arch_hibernation_disable_cpus() macro? */
+#include <asm/suspend.h>
+#endif
+
 #include "power.h"
 

Quoted 5 lines

kernel/power/hibernate.c line 42

Quoted 2 lines

kernel/power/hibernate.c +2 -0

+#ifndef arch_hibernation_disable_cpus
+#define arch_hibernation_disable_cpus(x) disable_nonboot_cpus()

For the x86 case we'll also need the complementary "enable", so why don't you add it here and then use it instead of the enable_nonboot_cpus()?

Done. I've added the 'in_suspend' argument so that it's symmetrical, this may be slightly different to the behaviour of your 'in_resume_hibernate' flag.

On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a user hotplugs this CPU out, then uses kexec to boot a new kernel, the new kernel will assign logical id 0 to a different physical CPU. This breaks hibernate as hibernate and resume will be attempted on different CPUs. A previous patch detects this situation when we come to resume, and returns an error. (data stored in the hibernate image is lost)

We currently forbid hibernate if CPU0 has been hotplugged out to avoid this situation without kexec.

Use arch_hibernation_disable_cpus() to direct which CPU we should resume on based on the MPIDR of the CPU we hibernated on. This allows us to hibernate/resume on any CPU, even if the logical numbers have been shuffled by kexec.

Signed-off-by James Morse <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

---
Changes since v2:
* Storing/reading/checking sleep_cpu moved into an earlier patch
* Moved to macro approach.
* Added hidden ARCH_HIBERNATION_CPUHP config option.

 arch/arm64/Kconfig               |  4 ++++
 arch/arm64/include/asm/suspend.h |  4 ++++
 arch/arm64/kernel/hibernate.c    | 48 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +4 -0

index 79341f6d1b6a..14ef59a90cfd 100644
@@ -1017,6 +1017,10 @@ config ARCH_HIBERNATION_HEADER
 	def_bool y
 	depends on HIBERNATION
 
+config ARCH_HIBERNATION_CPUHP
+	def_bool y
+	depends on HIBERNATION
+
 config ARCH_SUSPEND_POSSIBLE
 	def_bool y
 

arch/arm64/include/asm/suspend.h +4 -0

index 024d623f662e..9b3e8d9bfc8c 100644
@@ -47,4 +47,8 @@ int swsusp_arch_resume(void);
 int arch_hibernation_header_save(void *addr, unsigned int max_size);
 int arch_hibernation_header_restore(void *addr);
 
+/* Used to resume on the CPU we hibernated on */
+int _arch_hibernation_disable_cpus(bool suspend);
+#define arch_hibernation_disable_cpus(x) _arch_hibernation_disable_cpus(x)
+
 #endif

arch/arm64/kernel/hibernate.c +17 -2

index 8c7c6d7d4cd4..cbcc8243575e 100644
@@ -144,6 +144,7 @@ EXPORT_SYMBOL(arch_hibernation_header_save);
 
 int arch_hibernation_header_restore(void *addr)
 {
+	int ret;
 	struct arch_hibernate_hdr_invariants invariants;
 	struct arch_hibernate_hdr *hdr = addr;
 

@@ -156,11 +157,21 @@ int arch_hibernation_header_restore(void *addr)
 	sleep_cpu = get_logical_index(hdr->sleep_cpu_mpidr);
 	pr_info("Hibernated on CPU %d [mpidr:0x%llx]\n", sleep_cpu,
 		hdr->sleep_cpu_mpidr);
-	if (sleep_cpu != 0) {
-		pr_crit("Didn't hibernate on the firmware boot CPU!\n");
+	if (sleep_cpu <= 0) {
+		pr_crit("Hibernated on a CPU not known to this kernel!\n");
 		sleep_cpu = -EINVAL;
 		return -EINVAL;
 	}
+	if (!cpu_online(sleep_cpu)) {
+		pr_info("Hibernated on a CPU that is offline! Bringing CPU up.\n");
+		ret = cpu_up(sleep_cpu);
+		if (ret) {
+			pr_err("Failed to bring hibernate-CPU up!\n");
+			sleep_cpu = -EINVAL;
+			return ret;
+		}
+	}
+
 	resume_hdr = *hdr;
 
 	return 0;

@@ -532,3 +543,36 @@ static int __init check_boot_cpu_online_init(void)
 	return 0;
 }
 core_initcall(check_boot_cpu_online_init);
+
+int _arch_hibernation_disable_cpus(bool suspend)
+{
+	int cpu, ret;

arch/arm64/kernel/hibernate.c +9 -0

+			if (ret) {
+				pr_err("Secondary CPUs are not disabled\n");
+				break;
+			}
+		}
+	}
+
+	return ret;
+}

Signature · 1 line

2.8.0.rc3

Quoted 23 lines · James Morse

On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a user hotplugs this CPU out, then uses kexec to boot a new kernel, the new kernel will assign logical id 0 to a different physical CPU. This breaks hibernate as hibernate and resume will be attempted on different CPUs. A previous patch detects this situation when we come to resume, and returns an error. (data stored in the hibernate image is lost)

We currently forbid hibernate if CPU0 has been hotplugged out to avoid this situation without kexec.

Use arch_hibernation_disable_cpus() to direct which CPU we should resume on based on the MPIDR of the CPU we hibernated on. This allows us to hibernate/resume on any CPU, even if the logical numbers have been shuffled by kexec.

arch/arm64/kernel/hibernate.c +3 -0

index 8c7c6d7d4cd4..cbcc8243575e 100644
 
+int _arch_hibernation_disable_cpus(bool suspend)
+{
+	int cpu, ret;

arch/arm64/kernel/hibernate.c static int __init check_boot_cpu_online_init(void) · line 559

Quoted 10 lines

arch/arm64/kernel/hibernate.c +10 -0

+
+	if (suspend) {
+		/*
+		 * During hibernate we need frozen_cpus to be updated and saved.
+		 */
+		ret = disable_nonboot_cpus();
+	} else {
+		/*
+		 * Resuming from hibernate. From here, we can't race with
+		 * userspace, and don't need to update frozen_cpus.

Yes, but...

arch/arm64/kernel/hibernate.c static int __init check_boot_cpu_online_init(void) · line 569

Quoted 10 lines

arch/arm64/kernel/hibernate.c +10 -0

+		 */
+		pr_info("Disabling secondary CPUs ...\n");
+
+		/* sleep_cpu must have been loaded from the arch header */
+		BUG_ON(sleep_cpu < 0);
+
+		for_each_online_cpu(cpu) {
+			if (cpu == sleep_cpu)
+				continue;
+			ret = cpu_down(cpu);

This has a side effect, in that tasks are frozen here but we are now calling _cpu_down() through:

	cpu_down() -> do_cpu_down()

and we are telling the kernel that tasks are _not_ frozen, which AFAIK changes the cpuhp_tasks_frozen variable and related actions (eg __cpu_notify()), I suspect this may confuse some notifiers state machines that depend on CPU_TASKS_FROZEN to be set, maybe not but it is worth a look.

Hi Lorenzo,

Quoted 51 lines · Lorenzo Pieralisi

Quoted 23 lines · James Morse

On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a user hotplugs this CPU out, then uses kexec to boot a new kernel, the new kernel will assign logical id 0 to a different physical CPU. This breaks hibernate as hibernate and resume will be attempted on different CPUs. A previous patch detects this situation when we come to resume, and returns an error. (data stored in the hibernate image is lost)

We currently forbid hibernate if CPU0 has been hotplugged out to avoid this situation without kexec.

Use arch_hibernation_disable_cpus() to direct which CPU we should resume on based on the MPIDR of the CPU we hibernated on. This allows us to hibernate/resume on any CPU, even if the logical numbers have been shuffled by kexec.

arch/arm64/kernel/hibernate.c +3 -0

index 8c7c6d7d4cd4..cbcc8243575e 100644
 
+int _arch_hibernation_disable_cpus(bool suspend)
+{
+	int cpu, ret;

arch/arm64/kernel/hibernate.c static int __init check_boot_cpu_online_init(void) · line 559

Quoted 10 lines

arch/arm64/kernel/hibernate.c +10 -0

+
+	if (suspend) {
+		/*
+		 * During hibernate we need frozen_cpus to be updated and saved.
+		 */
+		ret = disable_nonboot_cpus();
+	} else {
+		/*
+		 * Resuming from hibernate. From here, we can't race with
+		 * userspace, and don't need to update frozen_cpus.

Yes, but...

arch/arm64/kernel/hibernate.c static int __init check_boot_cpu_online_init(void) · line 569

Quoted 10 lines

arch/arm64/kernel/hibernate.c +10 -0

+		 */
+		pr_info("Disabling secondary CPUs ...\n");
+
+		/* sleep_cpu must have been loaded from the arch header */
+		BUG_ON(sleep_cpu < 0);
+
+		for_each_online_cpu(cpu) {
+			if (cpu == sleep_cpu)
+				continue;
+			ret = cpu_down(cpu);

This has a side effect, in that tasks are frozen here but we are now calling _cpu_down() through:

Quoted 7 lines

	cpu_down() -> do_cpu_down()

and we are telling the kernel that tasks are _not_ frozen, which AFAIK changes the cpuhp_tasks_frozen variable and related actions (eg __cpu_notify()), I suspect this may confuse some notifiers state machines that depend on CPU_TASKS_FROZEN to be set, maybe not but it is worth a look.

Thanks for this - I missed that implication.

I've been through all the cpu notifiers in the core code, almost all of them either mask out the frozen bits, or do something symmetric.

The exceptions are:
__zcomp_cpu_notifier() and __zswap_cpu_dstmem_notifier(), which will free memory in this scenario.
evtchn_fifo_cpu_notification() which could end up in generic_handle_irq(). nmi_timer_cpu_notifier() (part of oprofile), will disable a perf timer. bcm2836_arm_irqchip_cpu_notify() will mask IPIs, but will still unmask them on CPU_STARTING_FROZEN.

coretemp_cpu_callback(), via_cputemp_cpu_callback() and powerclamp_cpu_callback() would remove platform devices, or stop threads, but these three are x86 specific.
All the rest live under /arch.

I haven't found any that are a problem, but this doesn't feel like the right thing to do. I will look for a way to tidy this up.