[PATCH v3 0/7] arm64: hibernate: Support DEBUG_PAGEALLOC and hibernate on non-boot cpu

[PATCH v3 0/7] arm64: hibernate: Support DEBUG_PAGEALLOC and hibernate on non-boot cpu

[James Morse]

Hi all,

These patches improve arm64's hibernate support:
* Patches 1-3 fix the hibernate problem with DEBUG_PAGEALLOC reported by Will.
  This also addresses the outstanding comment from Catalin [0] regarding
  cleaning of the whole kernel to the PoC. Now we clean two new sections
  .mmuoff.{data,text}.

* Patch 4 detects the 'resume on the wrong CPU' condition that we may
  experience with kexec, and prints an error message. (data still gets lost)

* Patches 5 & 6 improve on this error message by causing resume to occur on the
  same CPU that we hibernated on.

* Patch 7 reverts the code that tries to prevent hibernate with no CPU0,
  which isn't safe against kexec shuffling the CPU order.

The patch to core hibernate code now uses a macro and a kconfig symbol
(in the same way as config_arch_hibernate_header) to avoid the header
include problem. It should be possible to use this approach with
Chen Yu's hlt/mwait problem too [1].

With this series applied I can hotplug CPU0, kexec, hibernate and then
resume[3], the implementation is s discussed on the list[2].

Patch 4 will cause a trivial merge conflict with mainline, due to a fix
for hibernate which added an include for asm/smp.h. The fix is to keep
both hunks.

This series is based on arm64/for-next/core, and can be retrieved from:
git://linux-arm.org/linux-jm.git -b hibernate/update/v3

Changes since v2:
 * Split wrong-CPU logic into an earlier patch that just returns an error.
 * Moved secondary_holding_pen and friends into the .mmuoff.text section.
 * Added a mmuoff.data section for secondary_holding_pen_release etc.
 * Changed core code patch to use macros instead of a weak function.
   CONFIG_ARCH_HIBERNATION_HEADER now implies an asm/suspend.h header.
 * Wording in error messages 'hibernate' not 'suspend'.

Changes since v1:
 * Fixed 'Brining' typo.
 * Added .mmuoff.text section and gathered functions together.
 * Put sections.h in alphabetical order.


[v1] http://www.spinics.net/lists/arm-kernel/msg507805.html
[v2] http://permalink.gmane.org/gmane.linux.power-management.general/77467

[0] http://www.spinics.net/lists/arm-kernel/msg499305.html
[1] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1176775.html
[2] http://www.spinics.net/lists/arm-kernel/msg499036.html
[3] hotplug cpu0, kexec, hibernate, resume
-------------------------%<-------------------------
root@ubuntu:~# echo disk > /sys/power/state
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.007 seconds) done.
PM: Preallocating image memory... done (allocated 10112 pages)
PM: Allocated 647168 kbytes in 2.69 seconds (240.58 MB/s)
Freezing remaining freezable tasks ... (elapsed 0.005 seconds) done.
PM: freeze of devices complete after 27.013 msecs
PM: late freeze of devices complete after 20.624 msecs
PM: noirq freeze of devices complete after 22.211 msecs
Disabling non-boot CPUs ...
PM: Creating hibernation image:
PM: Need to copy 10103 pages
PM: Hibernation image created (10103 pages copied)
PM: noirq thaw of devices complete after 22.350 msecs
PM: early thaw of devices complete after 23.680 msecs
PM: thaw of devices complete after 98.331 msecs
hibernate: Suspending on cpu 0 [mpidr:0x103]
PM: Using 1 thread(s) for compression.
PM: Compressing and saving image data (10105 pages)...
PM: Image saving progress:   0%
atkbd serio0: keyboard reset failed on 1c060000.kmi
atkbd serio1: keyboard reset failed on 1c070000.kmi
PM: Image saving progress:  10%
PM: Image saving progress:  20%
PM: Image saving progress:  30%
PM: Image saving progress:  40%
PM: Image saving progress:  50%
PM: Image saving progress:  60%
PM: Image saving progress:  70%
PM: Image saving progress:  80%
PM: Image saving progress:  90%
PM: Image saving progress: 100%
PM: Image saving done.
PM: Wrote 646720 kbytes in 93.08 seconds (6.94 MB/s)
PM: S|
reboot: Power down

[ ... ]

Freezing user space processes ... (elapsed 0.000 seconds) done.
PM: Using 1 thread(s) for decompression.
PM: Loading and decompressing image data (10105 pages)...
hibernate: Suspended on cpu 5 [mpidr:0x103]
hibernate: Suspended on a CPU that is offline! Brining CPU up.
Detected VIPT I-cache on CPU5
CPU5: Booted secondary processor [410fd033]
random: nonblocking pool is initialized
PM: Image loading progress:   0%
PM: Image loading progress:  10%
PM: Image loading progress:  20%
PM: Image loading progress:  30%
PM: Image loading progress:  40%
PM: Image loading progress:  50%
PM: Image loading progress:  60%
PM: Image loading progress:  70%
PM: Image loading progress:  80%
PM: Image loading progress:  90%
PM: Image loading progress: 100%
PM: Image loading done.
PM: Read 646720 kbytes in 30.47 seconds (21.22 MB/s)
PM: quiesce of devices complete after 32.958 msecs
PM: late quiesce of devices complete after 11.574 msecs
PM: noirq quiesce of devices complete after 24.918 msecs
hibernate: Disabling secondary CPUs ...
IRQ1 no longer affine to CPU0
IRQ6 no longer affine to CPU0
IRQ28 no longer affine to CPU0
IRQ29 no longer affine to CPU0
IRQ32 no longer affine to CPU0
IRQ34 no longer affine to CPU0
IRQ35 no longer affine to CPU0
IRQ37 no longer affine to CPU0
IRQ41 no longer affine to CPU0
IRQ48 no longer affine to CPU0
CPU0: shutdown

psci: CPU0 killed.
PM: noirq restore of devices complete after 27.419 msecs
PM: early restore of devices complete after 23.554 msecs
PM: restore of devices complete after 113.188 msecs
Restarting tasks ... done.
root at ubuntu:~# atkbd serio0: keyboard reset failed on 1c060000.kmi
atkbd serio1: keyboard reset failed on 1c070000.kmi
root at ubuntu:~# cat /proc/cpuinfo
processor       : 0
BogoMIPS        : 100.00
Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd03
CPU revision    : 3

root at ubuntu:~#
-------------------------%<-------------------------


James Morse (7):
  arm64: Create sections.h
  arm64: vmlinux.ld: Add .mmuoff.{text,data} sections
  arm64: hibernate: Support DEBUG_PAGEALLOC
  arm64: hibernate: Detect hibernate image created on non-boot CPU
  PM / Hibernate: Allow arch code to influence CPU hotplug during
    hibernate
  arm64: hibernate: Resume on the CPU that created the hibernate image
  Revert "arm64: hibernate: Refuse to hibernate if the boot cpu is
    offline"

 arch/arm64/Kconfig                 |   5 +-
 arch/arm64/include/asm/Kbuild      |   1 -
 arch/arm64/include/asm/sections.h  |  30 ++++++++
 arch/arm64/include/asm/suspend.h   |   4 ++
 arch/arm64/include/asm/traps.h     |   3 +-
 arch/arm64/include/asm/virt.h      |   5 +-
 arch/arm64/kernel/alternative.c    |   7 +-
 arch/arm64/kernel/head.S           |  10 ++-
 arch/arm64/kernel/hibernate.c      | 137 +++++++++++++++++++++++++++----------
 arch/arm64/kernel/sleep.S          |   2 +
 arch/arm64/kernel/smp_spin_table.c |   2 +-
 arch/arm64/kernel/vmlinux.lds.S    |   8 +++
 arch/arm64/kvm/reset.c             |   3 +-
 arch/arm64/mm/pageattr.c           |  40 ++++++++++-
 arch/arm64/mm/proc.S               |   4 ++
 kernel/power/hibernate.c           |  14 +++-
 16 files changed, 219 insertions(+), 56 deletions(-)
 create mode 100644 arch/arm64/include/asm/sections.h

-- 
2.8.0.rc3

[PATCH v3 1/7] arm64: Create sections.h

[James Morse]

Each time new section markers are added, kernel/vmlinux.ld.S is updated,
and new extern char __start_foo[] definitions are scattered through the
tree.

Create asm/include/sections.h to collect these definitions (and include
the existing asm-generic version).

Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/Kbuild     |  1 -
 arch/arm64/include/asm/sections.h | 28 ++++++++++++++++++++++++++++
 arch/arm64/include/asm/traps.h    |  3 +--
 arch/arm64/include/asm/virt.h     |  5 +----
 arch/arm64/kernel/alternative.c   |  7 +++----
 arch/arm64/kernel/hibernate.c     |  6 ------
 arch/arm64/kvm/reset.c            |  3 +--
 7 files changed, 34 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm64/include/asm/sections.h

diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
index f43d2c44c765..2b3d2d24acba 100644
--- a/arch/arm64/include/asm/Kbuild
+++ b/arch/arm64/include/asm/Kbuild
@@ -32,7 +32,6 @@ generic-y += poll.h
 generic-y += preempt.h
 generic-y += resource.h
 generic-y += rwsem.h
-generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
 generic-y += serial.h
diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
new file mode 100644
index 000000000000..cb68eb348566
--- /dev/null
+++ b/arch/arm64/include/asm/sections.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2016 ARM Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ASM_SECTIONS_H
+#define __ASM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char __alt_instructions[], __alt_instructions_end[];
+extern char __exception_text_start[], __exception_text_end[];
+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[];
+
+#endif /* __ASM_SECTIONS_H */
diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 0cc2f29bf9da..90156c803f11 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -19,6 +19,7 @@
 #define __ASM_TRAP_H
 
 #include <linux/list.h>
+#include <asm/sections.h>
 
 struct pt_regs;
 
@@ -52,8 +53,6 @@ static inline int __in_irqentry_text(unsigned long ptr)
 
 static inline int in_exception_text(unsigned long ptr)
 {
-	extern char __exception_text_start[];
-	extern char __exception_text_end[];
 	int in;
 
 	in = ptr >= (unsigned long)&__exception_text_start &&
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index bbc6a8cf83f1..db5739413677 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -45,6 +45,7 @@
 #ifndef __ASSEMBLY__
 
 #include <asm/ptrace.h>
+#include <asm/sections.h>
 
 /*
  * __boot_cpu_mode records what mode CPUs were booted in.
@@ -87,10 +88,6 @@ extern void verify_cpu_run_el(void);
 static inline void verify_cpu_run_el(void) {}
 #endif
 
-/* The section containing the hypervisor text */
-extern char __hyp_text_start[];
-extern char __hyp_text_end[];
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* ! __ASM__VIRT_H */
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index d2ee1b21a10d..4434dabde898 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -25,14 +25,13 @@
 #include <asm/alternative.h>
 #include <asm/cpufeature.h>
 #include <asm/insn.h>
+#include <asm/sections.h>
 #include <linux/stop_machine.h>
 
 #define __ALT_PTR(a,f)		(u32 *)((void *)&(a)->f + (a)->f)
 #define ALT_ORIG_PTR(a)		__ALT_PTR(a, orig_offset)
 #define ALT_REPL_PTR(a)		__ALT_PTR(a, alt_offset)
 
-extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
-
 struct alt_region {
 	struct alt_instr *begin;
 	struct alt_instr *end;
@@ -124,8 +123,8 @@ static int __apply_alternatives_multi_stop(void *unused)
 {
 	static int patched = 0;
 	struct alt_region region = {
-		.begin	= __alt_instructions,
-		.end	= __alt_instructions_end,
+		.begin	= (struct alt_instr *)__alt_instructions,
+		.end	= (struct alt_instr *)__alt_instructions_end,
 	};
 
 	/* We always have a CPU 0 at this point (__init) */
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index f8df75d740f4..56e548fe0386 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -52,12 +52,6 @@ extern int in_suspend;
 /* Do we need to reset el2? */
 #define el2_reset_needed() (is_hyp_mode_available() && !is_kernel_in_hyp_mode())
 
-/*
- * Start/end of the hibernate exit code, this must be copied to a 'safe'
- * location in memory, and executed from there.
- */
-extern char __hibernate_exit_text_start[], __hibernate_exit_text_end[];
-
 /* temporary el2 vectors in the __hibernate_exit_text section. */
 extern char hibernate_el2_vectors[];
 
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index b1ad730e1567..c2d3594dd546 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -32,6 +32,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_coproc.h>
 #include <asm/kvm_mmu.h>
+#include <asm/sections.h>
 
 /*
  * ARMv8 Reset Values
@@ -133,8 +134,6 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 	return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
 }
 
-extern char __hyp_idmap_text_start[];
-
 unsigned long kvm_hyp_reset_entry(void)
 {
 	if (!__kvm_cpu_uses_extended_idmap()) {
-- 
2.8.0.rc3

[PATCH v3 2/7] arm64: vmlinux.ld: Add .mmuoff.{text,data} sections

[James Morse]

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 <james.morse@arm.com>
---
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(-)

diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
index cb68eb348566..7eecfa110330 100644
--- a/arch/arm64/include/asm/sections.h
+++ b/arch/arm64/include/asm/sections.h
@@ -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 */
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 2c6e598a94dc..6e56cd136d27 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -52,6 +52,9 @@
 #error TEXT_OFFSET must be less than 2MB
 #endif
 
+/* INVALID_HWID is defined as ULONG_MAX, but we can't include linux/kernel.h */
+#define ULONG_MAX ~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
@@ -627,11 +631,14 @@ ENDPROC(set_cpu_boot_mode_flag)
  * This is not in .bss, because we set it sufficiently early that the boot-time
  * zeroing of .bss would clobber it.
  */
-	.pushsection	.data..cacheline_aligned
+	.pushsection ".mmuoff.data", "aw"
 	.align	L1_CACHE_SHIFT
 ENTRY(__boot_cpu_mode)
 	.long	BOOT_CPU_MODE_EL2
 	.long	BOOT_CPU_MODE_EL1
+
+ENTRY(secondary_holding_pen_release)
+	.quad INVALID_HWID
 	.popsection
 
 	/*
@@ -687,6 +694,7 @@ __secondary_switched:
 	mov	x29, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
+	.popsection
 
 /*
  * The booting CPU updates the failed status @__early_cpu_boot_status,
diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
index 9a3aec97ac09..e66ce9b7bbde 100644
--- a/arch/arm64/kernel/sleep.S
+++ b/arch/arm64/kernel/sleep.S
@@ -97,6 +97,7 @@ ENTRY(__cpu_suspend_enter)
 ENDPROC(__cpu_suspend_enter)
 	.ltorg
 
+	.pushsection ".mmuoff.text", "ax"
 ENTRY(cpu_resume)
 	bl	el2_setup		// if in EL2 drop to EL1 cleanly
 	/* enable the MMU early - so we can access sleep_save_stash by va */
@@ -106,6 +107,7 @@ ENTRY(cpu_resume)
 	adrp	x26, swapper_pg_dir
 	b	__cpu_setup
 ENDPROC(cpu_resume)
+	.popsection
 
 ENTRY(_cpu_resume)
 	mrs	x1, mpidr_el1
diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c
index 18a71bcd26ee..04b465c72538 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -29,7 +29,7 @@
 #include <asm/smp_plat.h>
 
 extern void secondary_holding_pen(void);
-volatile unsigned long secondary_holding_pen_release = INVALID_HWID;
+extern volatile unsigned long secondary_holding_pen_release;
 
 static phys_addr_t cpu_release_addr[NR_CPUS];
 
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 0de7be4f1a9d..7db9c94213a9 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -118,6 +118,9 @@ SECTIONS
 			__exception_text_end = .;
 			IRQENTRY_TEXT
 			SOFTIRQENTRY_TEXT
+			__mmuoff_text_start = .;
+			*(.mmuoff.text)
+			__mmuoff_text_end = .;
 			TEXT_TEXT
 			SCHED_TEXT
 			LOCK_TEXT
@@ -193,6 +196,11 @@ SECTIONS
 	_sdata = .;
 	RW_DATA_SECTION(L1_CACHE_BYTES, PAGE_SIZE, THREAD_SIZE)
 	PECOFF_EDATA_PADDING
+	.mmuoff.data : {
+		__mmuoff_data_start = .;
+		*(.mmuoff.data)
+		__mmuoff_data_end = .;
+	}
 	_edata = .;
 
 	BSS_SECTION(0, 0, 0)
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index c4317879b938..655ff3ec90f2 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -83,6 +83,7 @@ ENDPROC(cpu_do_suspend)
  *
  * x0: Address of context pointer
  */
+	.pushsection ".mmuoff.text", "ax"
 ENTRY(cpu_do_resume)
 	ldp	x2, x3, [x0]
 	ldp	x4, x5, [x0, #16]
@@ -111,6 +112,7 @@ ENTRY(cpu_do_resume)
 	isb
 	ret
 ENDPROC(cpu_do_resume)
+	.popsection
 #endif
 
 /*
@@ -172,6 +174,7 @@ ENDPROC(idmap_cpu_replace_ttbr1)
  *	Initialise the processor for turning the MMU on.  Return in x0 the
  *	value of the SCTLR_EL1 register.
  */
+	.pushsection ".mmuoff.text", "ax"
 ENTRY(__cpu_setup)
 	tlbi	vmalle1				// Invalidate local TLB
 	dsb	nsh
@@ -255,3 +258,4 @@ ENDPROC(__cpu_setup)
 crval:
 	.word	0xfcffffff			// clear
 	.word	0x34d5d91d			// set
+	.popsection
-- 
2.8.0.rc3

[PATCH v3 3/7] arm64: hibernate: Support DEBUG_PAGEALLOC

[James Morse]

DEBUG_PAGEALLOC removes the valid bit of page table entries to prevent
any access to unallocated memory. Hibernate uses this as a hint that those
pages don't need to be saved/restored. This patch adds the
kernel_page_present() function it uses.

hibernate.c copies the resume kernel's linear map for use during restore.
Add _copy_pte() to fill-in the holes made by DEBUG_PAGEALLOC in the resume
kernel, so we can restore data the original kernel had at these addresses.

Finally, DEBUG_PAGEALLOC means the linear-map alias of KERNEL_START to
KERNEL_END may have holes in it, so we can't lazily clean this whole
area to the PoC. Only clean the new mmuoff regions, and the kernel/kvm
idmaps.

This reverts commit da24eb1f3f9e2c7b75c5f8c40d8e48e2c4789596.

Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v2:
 * Added cleaning of mmuoff.data section.
 * Added dcache_clean_range() macro, more readable?

 arch/arm64/Kconfig            |  1 -
 arch/arm64/kernel/hibernate.c | 45 ++++++++++++++++++++++++++++++++++---------
 arch/arm64/mm/pageattr.c      | 40 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1b196bf99320..79341f6d1b6a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -599,7 +599,6 @@ source kernel/Kconfig.preempt
 source kernel/Kconfig.hz
 
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
-	depends on !HIBERNATION
 	def_bool y
 
 config ARCH_HAS_HOLES_MEMORYMODEL
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 56e548fe0386..1c1278d5ec50 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -223,6 +223,7 @@ out:
 	return rc;
 }
 
+#define dcache_clean_range(start, end)	__flush_dcache_area(start, (end - start))
 
 int swsusp_arch_suspend(void)
 {
@@ -235,8 +236,14 @@ int swsusp_arch_suspend(void)
 	if (__cpu_suspend_enter(&state)) {
 		ret = swsusp_save();
 	} else {
-		/* Clean kernel to PoC for secondary core startup */
-		__flush_dcache_area(LMADDR(KERNEL_START), KERNEL_END - KERNEL_START);
+		/* Clean kernel core startup/idle code to PoC*/
+		dcache_clean_range(__mmuoff_text_start, __mmuoff_text_end);
+		dcache_clean_range(__mmuoff_data_start, __mmuoff_data_end);
+		dcache_clean_range(__idmap_text_start, __idmap_text_end);
+
+		/* Clean kvm setup code to PoC? */
+		if (el2_reset_needed())
+			dcache_clean_range(__hyp_idmap_text_start, __hyp_idmap_text_end);
 
 		/*
 		 * Tell the hibernation core that we've just restored
@@ -252,6 +259,32 @@ int swsusp_arch_suspend(void)
 	return ret;
 }
 
+static void _copy_pte(pte_t *dst_pte, pte_t *src_pte, unsigned long addr)
+{
+	unsigned long pfn = virt_to_pfn(addr);
+
+	if (pte_valid(*src_pte)) {
+		/*
+		 * Resume will overwrite areas that may be marked
+		 * read only (code, rodata). Clear the RDONLY bit from
+		 * the temporary mappings we use during restore.
+		 */
+		set_pte(dst_pte, __pte(pte_val(*src_pte) & ~PTE_RDONLY));
+	} else if (debug_pagealloc_enabled()) {
+		/*
+		 * debug_pagealloc may have removed the PTE_VALID bit if
+		 * the page isn't in use by the resume kernel. It may have
+		 * been in use by the original kernel, in which case we need
+		 * to put it back in our copy to do the restore.
+		 *
+		 * Check for mappable memory that gives us a translation
+		 * like part of the linear map.
+		 */
+		if (pfn_valid(pfn) && pte_pfn(*src_pte) == pfn)
+			set_pte(dst_pte, __pte((pte_val(*src_pte) & ~PTE_RDONLY) | PTE_VALID));
+	}
+}
+
 static int copy_pte(pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long start,
 		    unsigned long end)
 {
@@ -267,13 +300,7 @@ static int copy_pte(pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long start,
 
 	src_pte = pte_offset_kernel(src_pmd, start);
 	do {
-		if (!pte_none(*src_pte))
-			/*
-			 * Resume will overwrite areas that may be marked
-			 * read only (code, rodata). Clear the RDONLY bit from
-			 * the temporary mappings we use during restore.
-			 */
-			set_pte(dst_pte, __pte(pte_val(*src_pte) & ~PTE_RDONLY));
+		_copy_pte(dst_pte, src_pte, addr);
 	} while (dst_pte++, src_pte++, addr += PAGE_SIZE, addr != end);
 
 	return 0;
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index ca6d268e3313..b6c0da84258c 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -139,4 +139,42 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
 					__pgprot(0),
 					__pgprot(PTE_VALID));
 }
-#endif
+#ifdef CONFIG_HIBERNATION
+/*
+ * When built with CONFIG_DEBUG_PAGEALLOC and CONFIG_HIBERNATION, this function
+ * is used to determine if a linear map page has been marked as not-present by
+ * CONFIG_DEBUG_PAGEALLOC. Walk the page table and check the PTE_VALID bit.
+ * This is based on kern_addr_valid(), which almost does what we need.
+ */
+bool kernel_page_present(struct page *page)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *pte;
+	unsigned long addr = (unsigned long)page_address(page);
+
+	pgd = pgd_offset_k(addr);
+	if (pgd_none(*pgd))
+		return false;
+
+	pud = pud_offset(pgd, addr);
+	if (pud_none(*pud))
+		return false;
+	if (pud_sect(*pud))
+		return true;
+
+	pmd = pmd_offset(pud, addr);
+	if (pmd_none(*pmd))
+		return false;
+	if (pmd_sect(*pmd))
+		return true;
+
+	pte = pte_offset_kernel(pmd, addr);
+	if (pte_none(*pte))
+		return false;
+
+	return pte_valid(*pte);
+}
+#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_DEBUG_PAGEALLOC */
-- 
2.8.0.rc3

[PATCH v3 4/7] arm64: hibernate: Detect hibernate image created on non-boot CPU

[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.

Save the MPIDR of the CPU we hibernated on in the hibernate arch-header,
and fail to resume if it doesn't match CPU0's when we come to read the
header.

This will still lead to data loss (hibernate image is discarded), but
we at least get the opportunity to print an error message. Later patches
will allow hibernate to disable all but the CPU we hibernated on.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
Change since v2:
 * This patch was split out of v2:patch4. Without any core-code changes,
   printing an error like this is the best we can do.

This patch will cause a trivial merge conflict with mainline, due to a fix
for hibernate which added an include for asm/smp.h. The fix is to keep
both hunks.

 arch/arm64/kernel/hibernate.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 1c1278d5ec50..8c7c6d7d4cd4 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -15,6 +15,7 @@
  * License terms: GNU General Public License (GPL) version 2
  */
 #define pr_fmt(x) "hibernate: " x
+#include <linux/cpu.h>
 #include <linux/kvm_host.h>
 #include <linux/mm.h>
 #include <linux/notifier.h>
@@ -26,6 +27,7 @@
 
 #include <asm/barrier.h>
 #include <asm/cacheflush.h>
+#include <asm/cputype.h>
 #include <asm/irqflags.h>
 #include <asm/memory.h>
 #include <asm/mmu_context.h>
@@ -33,6 +35,7 @@
 #include <asm/pgtable.h>
 #include <asm/pgtable-hwdef.h>
 #include <asm/sections.h>
+#include <asm/smp_plat.h>
 #include <asm/suspend.h>
 #include <asm/virt.h>
 
@@ -59,6 +62,12 @@ extern char hibernate_el2_vectors[];
 extern char __hyp_stub_vectors[];
 
 /*
+ * The logical cpu number we should resume on, initialised to a non-cpu
+ * number.
+ */
+static int sleep_cpu = -EINVAL;
+
+/*
  * Values that may not change over hibernate/resume. We put the build number
  * and date in here so that we guarantee not to resume with a different
  * kernel.
@@ -80,6 +89,8 @@ static struct arch_hibernate_hdr {
 	 * re-configure el2.
 	 */
 	phys_addr_t	__hyp_stub_vectors;
+
+	u64		sleep_cpu_mpidr;
 } resume_hdr;
 
 static inline void arch_hdr_invariants(struct arch_hibernate_hdr_invariants *i)
@@ -122,6 +133,11 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)
 	else
 		hdr->__hyp_stub_vectors = 0;
 
+	/* Save the mpidr of the cpu we called cpu_suspend() on... */
+	hdr->sleep_cpu_mpidr = cpu_logical_map(sleep_cpu);
+	pr_info("Hibernating on CPU %d [mpidr:0x%llx]\n", sleep_cpu,
+		hdr->sleep_cpu_mpidr);
+
 	return 0;
 }
 EXPORT_SYMBOL(arch_hibernation_header_save);
@@ -137,6 +153,14 @@ int arch_hibernation_header_restore(void *addr)
 		return -EINVAL;
 	}
 
+	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");
+		sleep_cpu = -EINVAL;
+		return -EINVAL;
+	}
 	resume_hdr = *hdr;
 
 	return 0;
@@ -234,6 +258,7 @@ int swsusp_arch_suspend(void)
 	local_dbg_save(flags);
 
 	if (__cpu_suspend_enter(&state)) {
+		sleep_cpu = smp_processor_id();
 		ret = swsusp_save();
 	} else {
 		/* Clean kernel core startup/idle code to PoC*/
@@ -251,6 +276,7 @@ int swsusp_arch_suspend(void)
 		 */
 		in_suspend = 0;
 
+		sleep_cpu = -EINVAL;
 		__cpu_suspend_exit();
 	}
 
-- 
2.8.0.rc3

[PATCH v3 5/7] PM / Hibernate: Allow arch code to influence CPU hotplug during hibernate

[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.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
---
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(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index fca9254280ee..338745e78f7e 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -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"
 
+#ifndef arch_hibernation_disable_cpus
+#define arch_hibernation_disable_cpus(x) disable_nonboot_cpus()
+#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;
 
@@ -551,7 +559,7 @@ int hibernation_platform_enter(void)
 	if (error)
 		goto Platform_finish;
 
-	error = disable_nonboot_cpus();
+	error = arch_hibernation_disable_cpus(true);
 	if (error)
 		goto Enable_cpus;
 
-- 
2.8.0.rc3

[PATCH v3 6/7] arm64: hibernate: Resume on the CPU that created the hibernate image

[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.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
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(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 79341f6d1b6a..14ef59a90cfd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -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
 
diff --git a/arch/arm64/include/asm/suspend.h b/arch/arm64/include/asm/suspend.h
index 024d623f662e..9b3e8d9bfc8c 100644
--- a/arch/arm64/include/asm/suspend.h
+++ b/arch/arm64/include/asm/suspend.h
@@ -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
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 8c7c6d7d4cd4..cbcc8243575e 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -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;
+
+	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.
+		 */
+		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);
+			if (ret) {
+				pr_err("Secondary CPUs are not disabled\n");
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
-- 
2.8.0.rc3

[PATCH v3 7/7] Revert "arm64: hibernate: Refuse to hibernate if the boot cpu is offline"

[James Morse]

Now that we use the MPIDR to resume on the same CPU that we hibernated on,
we no longer need to refuse to hibernate if the boot cpu is offline. (Which
we can't possibly know if kexec causes logical CPUs to be renumbered).

This reverts commit 1fe492ce6482b77807b25d29690a48c46456beee.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/kernel/hibernate.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index cbcc8243575e..c1f1b509b177 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -18,7 +18,6 @@
 #include <linux/cpu.h>
 #include <linux/kvm_host.h>
 #include <linux/mm.h>
-#include <linux/notifier.h>
 #include <linux/pm.h>
 #include <linux/sched.h>
 #include <linux/suspend.h>
@@ -519,31 +518,6 @@ out:
 	return rc;
 }
 
-static int check_boot_cpu_online_pm_callback(struct notifier_block *nb,
-					     unsigned long action, void *ptr)
-{
-	if (action == PM_HIBERNATION_PREPARE &&
-	     cpumask_first(cpu_online_mask) != 0) {
-		pr_warn("CPU0 is offline.\n");
-		return notifier_from_errno(-ENODEV);
-	}
-
-	return NOTIFY_OK;
-}
-
-static int __init check_boot_cpu_online_init(void)
-{
-	/*
-	 * Set this pm_notifier callback with a lower priority than
-	 * cpu_hotplug_pm_callback, so that cpu_hotplug_pm_callback will be
-	 * called earlier to disable cpu hotplug before the cpu online check.
-	 */
-	pm_notifier(check_boot_cpu_online_pm_callback, -INT_MAX);
-
-	return 0;
-}
-core_initcall(check_boot_cpu_online_init);
-
 int _arch_hibernation_disable_cpus(bool suspend)
 {
 	int cpu, ret;
-- 
2.8.0.rc3

[PATCH v3 2/7] arm64: vmlinux.ld: Add .mmuoff.{text,data} sections

[Mark Rutland]

Hi James,

This looks mostly fine.

On Tue, Jun 28, 2016 at 03:51:45PM +0100, James Morse wrote:
> 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 <james.morse@arm.com>
> ---
> 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(-)
> 
> diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
> index cb68eb348566..7eecfa110330 100644
> --- a/arch/arm64/include/asm/sections.h
> +++ b/arch/arm64/include/asm/sections.h
> @@ -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 */
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 2c6e598a94dc..6e56cd136d27 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -52,6 +52,9 @@
>  #error TEXT_OFFSET must be less than 2MB
>  #endif
>  
> +/* INVALID_HWID is defined as ULONG_MAX, but we can't include linux/kernel.h */
> +#define ULONG_MAX ~0

We could avoid the duplication and simplify the change using __section()
in smp_spin_table.c, e.g.

volatile unsigned long __section(".mmuoff.data) 
secondary_holding_pen_release = INVALID_HWID

> +
>  /*
>   * 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
> @@ -627,11 +631,14 @@ ENDPROC(set_cpu_boot_mode_flag)
>   * This is not in .bss, because we set it sufficiently early that the boot-time
>   * zeroing of .bss would clobber it.
>   */
> -	.pushsection	.data..cacheline_aligned
> +	.pushsection ".mmuoff.data", "aw"
>  	.align	L1_CACHE_SHIFT
>  ENTRY(__boot_cpu_mode)
>  	.long	BOOT_CPU_MODE_EL2
>  	.long	BOOT_CPU_MODE_EL1

We might need to pad things in .mmuoff.data to the CWG.

__boot_cpu_mode is written with the MMU off, and read with the MMU on,
while secondary_holding_pen_release is written with the MMU on and read
with the MMU off.

Maintenance for either could corrupt one or the other, if they fall in
the same CWG.

Other than that, this looks good to me.

Thanks,
Mark.

[PATCH v3 5/7] PM / Hibernate: Allow arch code to influence CPU hotplug during hibernate

[Rafael J. Wysocki]

On Tuesday, June 28, 2016 03:51:48 PM James Morse wrote:
> 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 <james.morse@arm.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>

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.

A couple of nits below. 

> ---
> 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.

>  * Switch to macro approach.
> 
>  kernel/power/hibernate.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index fca9254280ee..338745e78f7e 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -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"
>  
> +#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()?

> +#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;
>  
> @@ -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?

>  	if (error)
>  		goto Enable_cpus;
>  
> 

Thanks,
Rafael

[PATCH v3 5/7] PM / Hibernate: Allow arch code to influence CPU hotplug during hibernate

[Chen Yu]

On 2016?06?29? 08:10, Rafael J. Wysocki wrote:
> On Tuesday, June 28, 2016 03:51:48 PM James Morse wrote:
>> 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 <james.morse@arm.com>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Pavel Machek <pavel@ucw.cz>
> 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.
>
This patch looks friendly for that fix :), thanks.
> 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()?
>
Agreed.

Yu

[PATCH v3 5/7] PM / Hibernate: Allow arch code to influence CPU hotplug during hibernate

[James Morse]

Hi Rafael,

On 29/06/16 01:10, Rafael J. Wysocki wrote:
> On Tuesday, June 28, 2016 03:51:48 PM James Morse wrote:
>> 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.

> 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.


> 
>> ---
>> 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?


>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> index fca9254280ee..338745e78f7e 100644
>> --- a/kernel/power/hibernate.c
>> +++ b/kernel/power/hibernate.c
>> @@ -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"
>>  
>> +#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.


> 
>> +#endif
>>  
>>  static int nocompress;
>>  static int noresume;


>> @@ -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?

I added it for completeness/symmetry (but not the enable call because it
wouldn't have any users). I assume its something to do with acpi...



Thanks,

James

[PATCH v3 5/7] PM / Hibernate: Allow arch code to influence CPU hotplug during hibernate

[Rafael J. Wysocki]

On Monday, July 04, 2016 10:04:26 AM James Morse wrote:
> Hi Rafael,
> 
> On 29/06/16 01:10, Rafael J. Wysocki wrote:
> > On Tuesday, June 28, 2016 03:51:48 PM James Morse wrote:
> >> 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.
> 
> > 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.
> 
> 
> > 
> >> ---
> >> 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?

Fine by me. :-)

> >> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> >> index fca9254280ee..338745e78f7e 100644
> >> --- a/kernel/power/hibernate.c
> >> +++ b/kernel/power/hibernate.c
> >> @@ -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"
> >>  
> >> +#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.

Well, it turns out that I was wrong, we don't need the "enable" thing, so
I guess it can stay the way it was.

Thanks,
Rafael

[PATCH v3 6/7] arm64: hibernate: Resume on the CPU that created the hibernate image

[Lorenzo Pieralisi]

On Tue, Jun 28, 2016 at 03:51:49PM +0100, James Morse wrote:
> 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 <james.morse@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> 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(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 79341f6d1b6a..14ef59a90cfd 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -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
>  
> diff --git a/arch/arm64/include/asm/suspend.h b/arch/arm64/include/asm/suspend.h
> index 024d623f662e..9b3e8d9bfc8c 100644
> --- a/arch/arm64/include/asm/suspend.h
> +++ b/arch/arm64/include/asm/suspend.h
> @@ -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
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 8c7c6d7d4cd4..cbcc8243575e 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -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;
> +
> +	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...

> +		 */
> +		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.

Thanks,
Lorenzo

> +			if (ret) {
> +				pr_err("Secondary CPUs are not disabled\n");
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> -- 
> 2.8.0.rc3
> 

[PATCH v3 6/7] arm64: hibernate: Resume on the CPU that created the hibernate image

[James Morse]

Hi Lorenzo,

On 05/07/16 15:55, Lorenzo Pieralisi wrote:
> On Tue, Jun 28, 2016 at 03:51:49PM +0100, James Morse wrote:
>> 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.

>> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
>> index 8c7c6d7d4cd4..cbcc8243575e 100644
>> --- a/arch/arm64/kernel/hibernate.c
>> +++ b/arch/arm64/kernel/hibernate.c

>> +int _arch_hibernation_disable_cpus(bool suspend)
>> +{
>> +	int cpu, ret;
>> +
>> +	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...
> 
>> +		 */
>> +		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.

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.


Thanks,

James

[PATCH v3 6/7] arm64: hibernate: Resume on the CPU that created the hibernate image

[Lorenzo Pieralisi]

On Thu, Jul 07, 2016 at 05:58:42PM +0100, James Morse wrote:
> Hi Lorenzo,
> 
> On 05/07/16 15:55, Lorenzo Pieralisi wrote:
> > On Tue, Jun 28, 2016 at 03:51:49PM +0100, James Morse wrote:
> >> 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.
> 
> >> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> >> index 8c7c6d7d4cd4..cbcc8243575e 100644
> >> --- a/arch/arm64/kernel/hibernate.c
> >> +++ b/arch/arm64/kernel/hibernate.c
> 
> >> +int _arch_hibernation_disable_cpus(bool suspend)
> >> +{
> >> +	int cpu, ret;
> >> +
> >> +	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...
> > 
> >> +		 */
> >> +		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.
> 
> 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.

Thanks for that. I do not think there is any problem, the only thing
that nags me is that we are using the cpu_down() interface
inconsistently with the rest of the kernel, agreed the issues are quite
theoretical but still the code is inconsistent.

I wonder what's the best way to clean this up, possibly adding code
that allows us to define what logical index the boot cpu is to
disable_nonboot_cpus() (ie with disable_nonboot_cpus() falling back
to first online cpu to keep current behaviour).

Just tossing around ideas, something has to be changed in core code
anyway if we wanted to keep things consistent.

Other solution would be setting cpuhp_tasks_frozen in
_arch_hibernation_disable_cpus() but I think that should be done
within a cpu_hotplug_begin() protected region of code.

Lorenzo