arm64: kernel: Add support for hibernate/suspend-to-disk

06a71a24bae5 ("arm64: KVM: unregister notifiers in hyp mode teardown path").

Some architectures require code written to memory as if it were data to be 'cleaned' from any data caches before the processor can fetch them as new instructions.

During resume from hibernate, the snapshot code copies some pages directly, meaning these architectures do not get a chance to perform their cache maintenance. Modify the read and decompress code to call flush_icache_range() on all pages that are restored, so that the restored in-place pages are guaranteed to be executable on these architectures.

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

Acked-by Pavel Machek <[email protected]>

Acked-by Rafael J. Wysocki <[email protected]>

Acked-by Catalin Marinas <[email protected]>

---
 kernel/power/swap.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

kernel/power/swap.c +0 -0

index 12cd989dadf6..a30645d2e93f 100644

Quoted 10 lines · James Morse

 kernel/power/swap.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

kernel/power/swap.c +0 -0

index 12cd989dadf6..a30645d2e93f 100644
@@ -37,6 +37,14 @@
 #define HIBERNATE_SIG	"S1SUSPEND"
 

kernel/power/swap.c line 45

Quoted 7 lines

kernel/power/swap.c +6 -0

 /*
+ * When reading an {un,}compressed image, we may restore pages in place,
+ * in which case some architectures need these pages cleaning before they
+ * can be executed. We don't know which pages these may be, so clean the lot.
+ */
+bool clean_pages_on_read = false;
+bool clean_pages_on_decompress = false;

Should these be static?

Quoted 11 lines · Will Deacon

Quoted 10 lines · James Morse

 kernel/power/swap.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

kernel/power/swap.c +0 -0

index 12cd989dadf6..a30645d2e93f 100644
@@ -37,6 +37,14 @@
 #define HIBERNATE_SIG	"S1SUSPEND"
 

Quoted 9 lines

kernel/power/swap.c line 45

Quoted 7 lines

kernel/power/swap.c +6 -0

 /*
+ * When reading an {un,}compressed image, we may restore pages in place,
+ * in which case some architectures need these pages cleaning before they
+ * can be executed. We don't know which pages these may be, so clean the lot.
+ */
+bool clean_pages_on_read = false;
+bool clean_pages_on_decompress = false;

Should these be static?

Bother, yes.

(They probably shouldn't be explicitly initialised either...)

Add support for hibernate/suspend-to-disk.

Suspend borrows code from cpu_suspend() to write cpu state onto the stack, before calling swsusp_save() to save the memory image.

Restore creates a set of temporary page tables, covering only the linear map, copies the restore code to a 'safe' page, then uses the copy to restore the memory image. The copied code executes in the lower half of the address space, and once complete, restores the original kernel's page tables. It then calls into cpu_resume(), and follows the normal cpu_suspend() path back into the suspend code.

To restore a kernel using KASLR, the address of the page tables, and cpu_resume() are stored in the hibernate arch-header and the el2 vectors are pivotted via the 'safe' page in low memory.

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

Tested-by Kevin Hilman <[email protected]> # Tested on Juno R2

Cc Catalin Marinas <[email protected]>

Hi all,

This is the today's version of hibernate for arm64, based on arm64's for-next/core with latest commit 6a1f54711447 ("arm64: acpi: add acpi=on cmdline option to prefer ACPI boot over DT").

Quoted 3 lines · James Morse

This is the today's version of hibernate for arm64, based on arm64's for-next/core with latest commit 6a1f54711447 ("arm64: acpi: add acpi=on cmdline option to prefer ACPI boot over DT").

I've queued this locally, but it breaks an allmodconfig build:

kernel/built-in.o: In function `saveable_page':
memremap.c:(.text+0x100f90): undefined reference to kernel_page_present' kernel/built-in.o: In function swsusp_save':
memremap.c:(.text+0x1026f0): undefined reference to `kernel_page_present'
make: *** [vmlinux] Error 1

Can you take a look, please?

depends on !HIBERNATION || ARCH_SUPPORTS_DEBUG_PAGEALLOC && !PPC && !SPARC

Quoted 6 lines · Will Deacon

Quoted 3 lines · James Morse

This is the today's version of hibernate for arm64, based on arm64's for-next/core with latest commit 6a1f54711447 ("arm64: acpi: add acpi=on cmdline option to prefer ACPI boot over DT").

I've queued this locally, but it breaks an allmodconfig build:

... I should have tested that ...

Quoted 7 lines

kernel/built-in.o: In function `saveable_page':
memremap.c:(.text+0x100f90): undefined reference to kernel_page_present' kernel/built-in.o: In function swsusp_save':
memremap.c:(.text+0x1026f0): undefined reference to `kernel_page_present'
make: *** [vmlinux] Error 1

Can you take a look, please?

This is caused by DEBUG_PAGEALLOC, which clears the PTE_VALID bit from 'free' pages. Hibernate uses it as a hint that it shouldn't save/access that page. This function is used to test whether the PTE_VALID bit has been cleared by kernel_map_pages(), hibernate is the only user.

Fixing this exposes a bigger problem with that configuration though: if the resume kernel has cut free pages out of the linear map, we copy this swiss-cheese view of memory, and try to use it to restore...

We can fixup the copy of the linear map, but it then explodes in my lazy 'clean the whole kernel to PoC' after resume, as now both the kernel and linear map have holes in them.

For now I think forbidding this configuration is best. ARCH_HIBERNATION_POSSIBLE depends on !DEBUG_PAGEALLOC would be best, but we can't do that, as DEBUG_PAGEALLOC has:

Quoted 1 line

depends on !HIBERNATION || ARCH_SUPPORTS_DEBUG_PAGEALLOC && !PPC && !SPARC

which causes kconfig to winge about loops in its dependencies.

-----------------------%<-----------------------

arch/arm64/Kconfig +1 -1

index b87f303765d4..0a7ff709a851 100644
@@ -579,6 +579,7 @@ source kernel/Kconfig.hz
 
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
        def_bool y
+       depends on !HIBERNATION
 
 config ARCH_HAS_HOLES_MEMORYMODEL
        def_bool y if SPARSEMEM
-----------------------%<-----------------------

Thanks,

James

If memory is located above 1<<VA_BITS, kvm adds an extra level to its page tables, merging the runtime tables and boot tables that contain the idmap. This lets us avoid the trampoline dance during initialisation.

This also means there is no trampoline page mapped, so __cpu_reset_hyp_mode() can't call __kvm_hyp_reset() in this page. The good news is the idmap is still mapped, so we don't need the trampoline page. The bad news is we can't call it directly as the idmap is above HYP_PAGE_OFFSET, so its address is masked by kvm_call_hyp.

Add a function __extended_idmap_trampoline which will branch into __kvm_hyp_reset in the idmap, change kvm_hyp_reset_entry() to return this address if __kvm_cpu_uses_extended_idmap(). In this case __kvm_hyp_reset() will still switch to the boot tables (which are the merged tables that were already in use), and branch into the idmap (where it already was).

This fixes boot failures on these systems, where we fail to execute the missing trampoline page when tearing down kvm in init_subsystems():

[    2.508922] kvm [1]: 8-bit VMID
[    2.512057] kvm [1]: Hyp mode initialized successfully
[    2.517242] kvm [1]: interrupt-controller at e1140000 IRQ13
[    2.522622] kvm [1]: timer IRQ3
[    2.525783] Kernel panic - not syncing: HYP panic:
[    2.525783] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    2.525783] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    2.525783] VCPU:          (null)
[    2.525783]
[    2.547667] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.6.0-rc5+ #1
[    2.555137] Hardware name: Default string Default string/Default string, BIOS ROD0084E 09/03/2015
[    2.563994] Call trace:
[    2.566432] [<ffffff80080888d0>] dump_backtrace+0x0/0x240
[    2.571818] [<ffffff8008088b24>] show_stack+0x14/0x20
[    2.576858] [<ffffff80083423ac>] dump_stack+0x94/0xb8
[    2.581899] [<ffffff8008152130>] panic+0x10c/0x250
[    2.586677] [<ffffff8008152024>] panic+0x0/0x250
[    2.591281] SMP: stopping secondary CPUs
[    3.649692] SMP: failed to stop secondary CPUs 0-2,4-7
[    3.654818] Kernel Offset: disabled
[    3.658293] Memory Limit: none
[    3.661337] ---[ end Kernel panic - not syncing: HYP panic:
[    3.661337] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    3.661337] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    3.661337] VCPU:          (null)
[    3.661337]

Reported-by Will Deacon <[email protected]>

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

---
 arch/arm64/include/asm/kvm_host.h |  3 ++-
 arch/arm64/kvm/hyp-init.S         |  5 +++++
 arch/arm64/kvm/hyp/entry.S        | 19 +++++++++++++++++++
 arch/arm64/kvm/reset.c            | 30 +++++++++++++++++++++++-------
 4 files changed, 49 insertions(+), 8 deletions(-)

arch/arm64/include/asm/kvm_host.h +2 -1

index b6f194d9f6b2..88a34670ddef 100644
@@ -46,7 +46,8 @@
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
 int kvm_arch_dev_ioctl_check_extension(long ext);
-phys_addr_t kvm_hyp_reset_entry(void);
+unsigned long kvm_hyp_reset_entry(void);
+void __extended_idmap_trampoline(phys_addr_t boot_pgd, phys_addr_t idmap_start);
 
 struct kvm_arch {
 	/* The VMID generation used for the virt. memory system */

arch/arm64/kvm/hyp-init.S +5 -0

index 44ec4cb23ae7..a873a6d8be90 100644
@@ -140,6 +140,11 @@ merged:
 ENDPROC(__kvm_hyp_init)
 
 	/*
+	 * Reset kvm back to the hyp stub. This is the trampoline dance in
+	 * reverse. If kvm used an extended idmap, __extended_idmap_trampoline
+	 * calls this code directly in the idmap. In this case switching to the
+	 * boot tables is a no-op.
+	 *
 	 * x0: HYP boot pgd
 	 * x1: HYP phys_idmap_start
 	 */

arch/arm64/kvm/hyp/entry.S +19 -0

index ce9e5e5f28cf..70254a65bd5b 100644
@@ -164,3 +164,22 @@ alternative_endif
 
 	eret
 ENDPROC(__fpsimd_guest_restore)
+
+/*
+ * When using the extended idmap, we don't have a trampoline page we can use
+ * while we switch pages tables during __kvm_hyp_reset. Accessing the idmap
+ * directly would be ideal, but if we're using the extended idmap then the
+ * idmap is located above HYP_PAGE_OFFSET, and the address will be masked by
+ * kvm_call_hyp using kern_hyp_va.
+ *
+ * x0: HYP boot pgd
+ * x1: HYP phys_idmap_start
+ */
+ENTRY(__extended_idmap_trampoline)
+	mov	x4, x1
+	adr_l	x3, __kvm_hyp_reset
+
+	/* insert __kvm_hyp_reset()s offset into phys_idmap_start */
+	bfi	x4, x3, #0, #PAGE_SHIFT
+	br	x4
+ENDPROC(__extended_idmap_trampoline)

arch/arm64/kvm/reset.c +14 -7

index 4062e6dd4cc1..b1ad730e1567 100644
@@ -135,12 +135,28 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 
 extern char __hyp_idmap_text_start[];
 
-phys_addr_t kvm_hyp_reset_entry(void)
+unsigned long kvm_hyp_reset_entry(void)
 {
-	unsigned long offset;
-
-	offset = (unsigned long)__kvm_hyp_reset
-		 - ((unsigned long)__hyp_idmap_text_start & PAGE_MASK);
-
-	return TRAMPOLINE_VA + offset;
+	if (!__kvm_cpu_uses_extended_idmap()) {
+		unsigned long offset;
+
+		/*
+		 * Find the address of __kvm_hyp_reset() in the trampoline page.
+		 * This is present in the running page tables, and the boot page
+		 * tables, so we call the code here to start the trampoline
+		 * dance in reverse.
+		 */
+		offset = (unsigned long)__kvm_hyp_reset
+			 - ((unsigned long)__hyp_idmap_text_start & PAGE_MASK);
+
+		return TRAMPOLINE_VA + offset;

On Fri, 29 Apr 2016 18:27:03 +0100
James Morse <[email protected]> wrote:

Hi James,

Quoted 143 lines

If memory is located above 1<<VA_BITS, kvm adds an extra level to its page tables, merging the runtime tables and boot tables that contain the idmap. This lets us avoid the trampoline dance during initialisation.

This also means there is no trampoline page mapped, so __cpu_reset_hyp_mode() can't call __kvm_hyp_reset() in this page. The good news is the idmap is still mapped, so we don't need the trampoline page. The bad news is we can't call it directly as the idmap is above HYP_PAGE_OFFSET, so its address is masked by kvm_call_hyp.

Add a function __extended_idmap_trampoline which will branch into __kvm_hyp_reset in the idmap, change kvm_hyp_reset_entry() to return this address if __kvm_cpu_uses_extended_idmap(). In this case __kvm_hyp_reset() will still switch to the boot tables (which are the merged tables that were already in use), and branch into the idmap (where it already was).

This fixes boot failures on these systems, where we fail to execute the missing trampoline page when tearing down kvm in init_subsystems():

[    2.508922] kvm [1]: 8-bit VMID
[    2.512057] kvm [1]: Hyp mode initialized successfully
[    2.517242] kvm [1]: interrupt-controller at e1140000 IRQ13
[    2.522622] kvm [1]: timer IRQ3
[    2.525783] Kernel panic - not syncing: HYP panic:
[    2.525783] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    2.525783] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    2.525783] VCPU:          (null)
[    2.525783]
[    2.547667] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.6.0-rc5+ #1
[    2.555137] Hardware name: Default string Default string/Default string, BIOS ROD0084E 09/03/2015
[    2.563994] Call trace:
[    2.566432] [<ffffff80080888d0>] dump_backtrace+0x0/0x240
[    2.571818] [<ffffff8008088b24>] show_stack+0x14/0x20
[    2.576858] [<ffffff80083423ac>] dump_stack+0x94/0xb8
[    2.581899] [<ffffff8008152130>] panic+0x10c/0x250
[    2.586677] [<ffffff8008152024>] panic+0x0/0x250
[    2.591281] SMP: stopping secondary CPUs
[    3.649692] SMP: failed to stop secondary CPUs 0-2,4-7
[    3.654818] Kernel Offset: disabled
[    3.658293] Memory Limit: none
[    3.661337] ---[ end Kernel panic - not syncing: HYP panic:
[    3.661337] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    3.661337] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    3.661337] VCPU:          (null)
[    3.661337]

Reported-by Will Deacon <[email protected]>

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

---
 arch/arm64/include/asm/kvm_host.h |  3 ++-
 arch/arm64/kvm/hyp-init.S         |  5 +++++
 arch/arm64/kvm/hyp/entry.S        | 19 +++++++++++++++++++
 arch/arm64/kvm/reset.c            | 30 +++++++++++++++++++++++-------
 4 files changed, 49 insertions(+), 8 deletions(-)

arch/arm64/include/asm/kvm_host.h +2 -1

index b6f194d9f6b2..88a34670ddef 100644
@@ -46,7 +46,8 @@
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
 int kvm_arch_dev_ioctl_check_extension(long ext);
-phys_addr_t kvm_hyp_reset_entry(void);
+unsigned long kvm_hyp_reset_entry(void);
+void __extended_idmap_trampoline(phys_addr_t boot_pgd, phys_addr_t idmap_start);
 
 struct kvm_arch {
 	/* The VMID generation used for the virt. memory system */

arch/arm64/kvm/hyp-init.S +5 -0

index 44ec4cb23ae7..a873a6d8be90 100644
@@ -140,6 +140,11 @@ merged:
 ENDPROC(__kvm_hyp_init)
 
 	/*
+	 * Reset kvm back to the hyp stub. This is the trampoline dance in
+	 * reverse. If kvm used an extended idmap, __extended_idmap_trampoline
+	 * calls this code directly in the idmap. In this case switching to the
+	 * boot tables is a no-op.
+	 *
 	 * x0: HYP boot pgd
 	 * x1: HYP phys_idmap_start
 	 */

arch/arm64/kvm/hyp/entry.S +19 -0

index ce9e5e5f28cf..70254a65bd5b 100644
@@ -164,3 +164,22 @@ alternative_endif
 
 	eret
 ENDPROC(__fpsimd_guest_restore)
+
+/*
+ * When using the extended idmap, we don't have a trampoline page we can use
+ * while we switch pages tables during __kvm_hyp_reset. Accessing the idmap
+ * directly would be ideal, but if we're using the extended idmap then the
+ * idmap is located above HYP_PAGE_OFFSET, and the address will be masked by
+ * kvm_call_hyp using kern_hyp_va.
+ *
+ * x0: HYP boot pgd
+ * x1: HYP phys_idmap_start
+ */
+ENTRY(__extended_idmap_trampoline)
+	mov	x4, x1
+	adr_l	x3, __kvm_hyp_reset
+
+	/* insert __kvm_hyp_reset()s offset into phys_idmap_start */
+	bfi	x4, x3, #0, #PAGE_SHIFT
+	br	x4
+ENDPROC(__extended_idmap_trampoline)

arch/arm64/kvm/reset.c +14 -7

index 4062e6dd4cc1..b1ad730e1567 100644
@@ -135,12 +135,28 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 
 extern char __hyp_idmap_text_start[];
 
-phys_addr_t kvm_hyp_reset_entry(void)
+unsigned long kvm_hyp_reset_entry(void)
 {
-	unsigned long offset;
-
-	offset = (unsigned long)__kvm_hyp_reset
-		 - ((unsigned long)__hyp_idmap_text_start & PAGE_MASK);
-
-	return TRAMPOLINE_VA + offset;
+	if (!__kvm_cpu_uses_extended_idmap()) {
+		unsigned long offset;
+
+		/*
+		 * Find the address of __kvm_hyp_reset() in the trampoline page.
+		 * This is present in the running page tables, and the boot page
+		 * tables, so we call the code here to start the trampoline
+		 * dance in reverse.
+		 */
+		offset = (unsigned long)__kvm_hyp_reset
+			 - ((unsigned long)__hyp_idmap_text_start & PAGE_MASK);
+
+		return TRAMPOLINE_VA + offset;

arch/arm64/kvm/reset.c int kvm_reset_vcpu(struct kvm_vcpu *vcpu) · line 162

Quoted 10 lines

arch/arm64/kvm/reset.c +9 -0

+	} else {
+		/*
+		 * KVM is running with merged page tables, which don't have the
+		 * trampoline page mapped. We know the idmap is still mapped,
+		 * but can't be called into directly. Use
+		 * __extended_idmap_trampoline to do the call.
+		 */
+		return (unsigned long)kvm_ksym_ref(__extended_idmap_trampoline);
+	}
 }

This looks correct, and gives me yet another incentive to revisit this code with a view of keeping the idmap at all times in the EL2 page tables. This should ensure that (1) the merged page tables become the only configuration, and (2) that we get rid off of the potential TLB conflict when swapping TTBR0_EL2 at boot time (intermediate level caching). In the meantime:

Reviewed-by Marc Zyngier <[email protected]>

Quoted 166 lines · Marc Zyngier

On Fri, 29 Apr 2016 18:27:03 +0100
James Morse <[email protected]> wrote:

Hi James,

Quoted 143 lines

If memory is located above 1<<VA_BITS, kvm adds an extra level to its page tables, merging the runtime tables and boot tables that contain the idmap. This lets us avoid the trampoline dance during initialisation.

This also means there is no trampoline page mapped, so __cpu_reset_hyp_mode() can't call __kvm_hyp_reset() in this page. The good news is the idmap is still mapped, so we don't need the trampoline page. The bad news is we can't call it directly as the idmap is above HYP_PAGE_OFFSET, so its address is masked by kvm_call_hyp.

Add a function __extended_idmap_trampoline which will branch into __kvm_hyp_reset in the idmap, change kvm_hyp_reset_entry() to return this address if __kvm_cpu_uses_extended_idmap(). In this case __kvm_hyp_reset() will still switch to the boot tables (which are the merged tables that were already in use), and branch into the idmap (where it already was).

This fixes boot failures on these systems, where we fail to execute the missing trampoline page when tearing down kvm in init_subsystems():

[    2.508922] kvm [1]: 8-bit VMID
[    2.512057] kvm [1]: Hyp mode initialized successfully
[    2.517242] kvm [1]: interrupt-controller at e1140000 IRQ13
[    2.522622] kvm [1]: timer IRQ3
[    2.525783] Kernel panic - not syncing: HYP panic:
[    2.525783] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    2.525783] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    2.525783] VCPU:          (null)
[    2.525783]
[    2.547667] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.6.0-rc5+ #1
[    2.555137] Hardware name: Default string Default string/Default string, BIOS ROD0084E 09/03/2015
[    2.563994] Call trace:
[    2.566432] [<ffffff80080888d0>] dump_backtrace+0x0/0x240
[    2.571818] [<ffffff8008088b24>] show_stack+0x14/0x20
[    2.576858] [<ffffff80083423ac>] dump_stack+0x94/0xb8
[    2.581899] [<ffffff8008152130>] panic+0x10c/0x250
[    2.586677] [<ffffff8008152024>] panic+0x0/0x250
[    2.591281] SMP: stopping secondary CPUs
[    3.649692] SMP: failed to stop secondary CPUs 0-2,4-7
[    3.654818] Kernel Offset: disabled
[    3.658293] Memory Limit: none
[    3.661337] ---[ end Kernel panic - not syncing: HYP panic:
[    3.661337] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    3.661337] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    3.661337] VCPU:          (null)
[    3.661337]

Reported-by Will Deacon <[email protected]>

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

---
 arch/arm64/include/asm/kvm_host.h |  3 ++-
 arch/arm64/kvm/hyp-init.S         |  5 +++++
 arch/arm64/kvm/hyp/entry.S        | 19 +++++++++++++++++++
 arch/arm64/kvm/reset.c            | 30 +++++++++++++++++++++++-------
 4 files changed, 49 insertions(+), 8 deletions(-)

arch/arm64/include/asm/kvm_host.h +2 -1

index b6f194d9f6b2..88a34670ddef 100644
@@ -46,7 +46,8 @@
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
 int kvm_arch_dev_ioctl_check_extension(long ext);
-phys_addr_t kvm_hyp_reset_entry(void);
+unsigned long kvm_hyp_reset_entry(void);
+void __extended_idmap_trampoline(phys_addr_t boot_pgd, phys_addr_t idmap_start);
 
 struct kvm_arch {
 	/* The VMID generation used for the virt. memory system */

arch/arm64/kvm/hyp-init.S +5 -0

index 44ec4cb23ae7..a873a6d8be90 100644
@@ -140,6 +140,11 @@ merged:
 ENDPROC(__kvm_hyp_init)
 
 	/*
+	 * Reset kvm back to the hyp stub. This is the trampoline dance in
+	 * reverse. If kvm used an extended idmap, __extended_idmap_trampoline
+	 * calls this code directly in the idmap. In this case switching to the
+	 * boot tables is a no-op.
+	 *
 	 * x0: HYP boot pgd
 	 * x1: HYP phys_idmap_start
 	 */

arch/arm64/kvm/hyp/entry.S +19 -0

index ce9e5e5f28cf..70254a65bd5b 100644
@@ -164,3 +164,22 @@ alternative_endif
 
 	eret
 ENDPROC(__fpsimd_guest_restore)
+
+/*
+ * When using the extended idmap, we don't have a trampoline page we can use
+ * while we switch pages tables during __kvm_hyp_reset. Accessing the idmap
+ * directly would be ideal, but if we're using the extended idmap then the
+ * idmap is located above HYP_PAGE_OFFSET, and the address will be masked by
+ * kvm_call_hyp using kern_hyp_va.
+ *
+ * x0: HYP boot pgd
+ * x1: HYP phys_idmap_start
+ */
+ENTRY(__extended_idmap_trampoline)
+	mov	x4, x1
+	adr_l	x3, __kvm_hyp_reset
+
+	/* insert __kvm_hyp_reset()s offset into phys_idmap_start */
+	bfi	x4, x3, #0, #PAGE_SHIFT
+	br	x4
+ENDPROC(__extended_idmap_trampoline)

arch/arm64/kvm/reset.c +14 -7

index 4062e6dd4cc1..b1ad730e1567 100644
@@ -135,12 +135,28 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 
 extern char __hyp_idmap_text_start[];
 
-phys_addr_t kvm_hyp_reset_entry(void)
+unsigned long kvm_hyp_reset_entry(void)
 {
-	unsigned long offset;
-
-	offset = (unsigned long)__kvm_hyp_reset
-		 - ((unsigned long)__hyp_idmap_text_start & PAGE_MASK);
-
-	return TRAMPOLINE_VA + offset;
+	if (!__kvm_cpu_uses_extended_idmap()) {
+		unsigned long offset;
+
+		/*
+		 * Find the address of __kvm_hyp_reset() in the trampoline page.
+		 * This is present in the running page tables, and the boot page
+		 * tables, so we call the code here to start the trampoline
+		 * dance in reverse.
+		 */
+		offset = (unsigned long)__kvm_hyp_reset
+			 - ((unsigned long)__hyp_idmap_text_start & PAGE_MASK);
+
+		return TRAMPOLINE_VA + offset;

arch/arm64/kvm/reset.c int kvm_reset_vcpu(struct kvm_vcpu *vcpu) · line 162

Quoted 10 lines

arch/arm64/kvm/reset.c +9 -0

+	} else {
+		/*
+		 * KVM is running with merged page tables, which don't have the
+		 * trampoline page mapped. We know the idmap is still mapped,
+		 * but can't be called into directly. Use
+		 * __extended_idmap_trampoline to do the call.
+		 */
+		return (unsigned long)kvm_ksym_ref(__extended_idmap_trampoline);
+	}
 }

This looks correct, and gives me yet another incentive to revisit this code with a view of keeping the idmap at all times in the EL2 page tables. This should ensure that (1) the merged page tables become the only configuration, and (2) that we get rid off of the potential TLB conflict when swapping TTBR0_EL2 at boot time (intermediate level caching). In the meantime:

Quoted 2 lines

Reviewed-by Marc Zyngier <[email protected]>

thanks! Will, should this go via the kvmarm tree or the arm64 tree?

Quoted 52 lines · Christoffer Dall

Quoted 51 lines · Marc Zyngier

Quoted 49 lines · On Fri, 29 Apr 2016 18:27:03 +0100 James Morse <[email protected]> wrote:

If memory is located above 1<<VA_BITS, kvm adds an extra level to its page tables, merging the runtime tables and boot tables that contain the idmap. This lets us avoid the trampoline dance during initialisation.

This also means there is no trampoline page mapped, so __cpu_reset_hyp_mode() can't call __kvm_hyp_reset() in this page. The good news is the idmap is still mapped, so we don't need the trampoline page. The bad news is we can't call it directly as the idmap is above HYP_PAGE_OFFSET, so its address is masked by kvm_call_hyp.

Add a function __extended_idmap_trampoline which will branch into __kvm_hyp_reset in the idmap, change kvm_hyp_reset_entry() to return this address if __kvm_cpu_uses_extended_idmap(). In this case __kvm_hyp_reset() will still switch to the boot tables (which are the merged tables that were already in use), and branch into the idmap (where it already was).

This fixes boot failures on these systems, where we fail to execute the missing trampoline page when tearing down kvm in init_subsystems():

[    2.508922] kvm [1]: 8-bit VMID
[    2.512057] kvm [1]: Hyp mode initialized successfully
[    2.517242] kvm [1]: interrupt-controller at e1140000 IRQ13
[    2.522622] kvm [1]: timer IRQ3
[    2.525783] Kernel panic - not syncing: HYP panic:
[    2.525783] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    2.525783] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    2.525783] VCPU:          (null)
[    2.525783]
[    2.547667] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.6.0-rc5+ #1
[    2.555137] Hardware name: Default string Default string/Default string, BIOS ROD0084E 09/03/2015
[    2.563994] Call trace:
[    2.566432] [<ffffff80080888d0>] dump_backtrace+0x0/0x240
[    2.571818] [<ffffff8008088b24>] show_stack+0x14/0x20
[    2.576858] [<ffffff80083423ac>] dump_stack+0x94/0xb8
[    2.581899] [<ffffff8008152130>] panic+0x10c/0x250
[    2.586677] [<ffffff8008152024>] panic+0x0/0x250
[    2.591281] SMP: stopping secondary CPUs
[    3.649692] SMP: failed to stop secondary CPUs 0-2,4-7
[    3.654818] Kernel Offset: disabled
[    3.658293] Memory Limit: none
[    3.661337] ---[ end Kernel panic - not syncing: HYP panic:
[    3.661337] PS:200003c9 PC:0000007ffffff820 ESR:86000005
[    3.661337] FAR:0000007ffffff820 HPFAR:00000000003ffff0 PAR:0000000000000000
[    3.661337] VCPU:          (null)
[    3.661337]

Reported-by Will Deacon <[email protected]>

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

Quoted 3 lines

Quoted 2 lines

Reviewed-by Marc Zyngier <[email protected]>

thanks! Will, should this go via the kvmarm tree or the arm64 tree?

I believe this issue only exists with the hibernation patches, so I suggest we let Will handle this though the arm64 tree together with the rest of this series.