Skip to contentSkip to conversation

ARM: simplify handover from UEFI to decompressor

PATCH Superseded Ard Biesheuvel 2 participants 10 messages Mar 27, 2020

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Ard Biesheuvel

author maintainer
#0 lore raw

cover ARM: simplify handover from UEFI to decompressor

The EFI stub in the ARM kernel runs in the context of the firmware, which means it runs with the caches and MMU on. Currently, we relocate the zImage so it appears in the first 128 MiB, disable the MMU and caches and invoke the decompressor via its ordinary entry point. However, since we can pass the base of DRAM directly, there is no need to relocate the zImage, which also means there is no need to disable and re-enable the caches and create new page tables etc.

This simplification is implemented by patch #5. Patches #1 - #4 are prerequisite changes to permit the decompressor to execute from the offset chosen by the UEFI firmware.

Note that this applies onto linux-next with Geert's patch 'ARM: boot: Obtain start of physical memory from DTB' applied [0]

Cc Geert Uytterhoeven <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Russell King <[email protected]>

[0] https://lore.kernel.org/linux-arm-kernel/[email protected]/

Ard Biesheuvel (5):
ARM: decompressor: move headroom variable out of LC0
ARM: decompressor: split off _edata and stack base into separate
object
ARM: decompressor: defer loading of the contents of the LC0 structure
ARM: decompressor: move GOT into .data for EFI enabled builds
ARM: decompressor: run decompressor in place if loaded via UEFI

 arch/arm/boot/compressed/head.S           | 84 +++++++-------------
 arch/arm/boot/compressed/vmlinux.lds.S    |  5 ++
 drivers/firmware/efi/libstub/arm32-stub.c | 41 ++--------
 3 files changed, 38 insertions(+), 92 deletions(-)
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH 1/5 ARM: decompressor: move headroom variable out of LC0

Before breaking up LC0 into different pieces, move out the variable that is already place relative (given that it subtracts 'restart' in the expression) and so its value does not need to be added to the runtime address of the LC0 symbol itself.

---
 arch/arm/boot/compressed/head.S | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/boot/compressed/head.S +4 -2
index 2d2a42865b39..bb674febf640 100644
@@ -339,7 +339,7 @@ not_angel:
 		 */
 		mov	r0, pc
 		cmp	r0, r4
-		ldrcc	r0, LC0+28
+		ldrcc	r0, .Lheadroom
 		addcc	r0, r0, pc
 		cmpcc	r4, r0
 		orrcc	r4, r4, #1		@ remember we skipped cache_on
@@ -716,9 +716,11 @@ LC0:		.word	LC0			@ r1
 		.word	_got_start		@ r11
 		.word	_got_end		@ ip
 		.word	.L_user_stack_end	@ sp
-		.word	_end - restart + 16384 + 1024*1024
 		.size	LC0, . - LC0
 
+.Lheadroom:
+		.word	_end - restart + 16384 + 1024*1024
+
 .Linflated_image_size_offset:
 		.long	(input_data_end - 4) - .
 
Signature · 1 line
2.17.1
Reviewed-by on v1 1/5
#2 lore raw

Re: [PATCH 1/5] ARM: decompressor: move headroom variable out of LC0

Hi Ard,

Thanks for your patch!

Quoted 2 lines · Ard Biesheuvel

Before breaking up LC0 into different pieces, move out the variable that is already place relative (given that it subtracts 'restart' in

placed

Quoted 4 lines

the expression) and so its value does not need to be added to the runtime address of the LC0 symbol itself.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#3 lore raw

Re: [PATCH 1/5] ARM: decompressor: move headroom variable out of LC0

Hallo Geert,

Quoted 11 lines · Geert Uytterhoeven

Hi Ard,

Thanks for your patch!

Quoted 2 lines · Ard Biesheuvel

Before breaking up LC0 into different pieces, move out the variable that is already place relative (given that it subtracts 'restart' in

placed

Actually, what I meant here was 'place-relative', i.e., the expression subtracts two symbol offsets, whereas the others are absolute, link time values that need to be fixed up according to the actual runtime location of the code.

Quoted 7 lines
Quoted 4 lines

the expression) and so its value does not need to be added to the runtime address of the LC0 symbol itself.

Thanks,
Ard.

author maintainer
#4 lore raw

PATCH 2/5 ARM: decompressor: split off _edata and stack base into separate object

In reply to Ard Biesheuvel

In preparation of moving the handling of the LC0 object to a later stage in the decompressor startup code, move out _edata and the initial value of the stack pointer, which are now used in two places, and are needed earlier than the remaining contents of LC0.

---
 arch/arm/boot/compressed/head.S | 33 +++++++++-----------
 1 file changed, 15 insertions(+), 18 deletions(-)
arch/arm/boot/compressed/head.S +15 -18
index bb674febf640..a6cf2f8115e5 100644
@@ -259,19 +259,16 @@ not_angel:
 		 * Find the start of physical memory.
 		 * Try the DTB first, if available.
 		 */
-		adr	r0, LC0
-		ldr	r1, [r0]	@ get absolute LC0
-		ldr	sp, [r0, #24]	@ get stack location
-		sub	r1, r0, r1	@ compute relocation offset
-		add	sp, sp, r1	@ apply relocation
+		adr	r0, LC1
+		ldmia	r0, {r1, r6}
+		add	sp, r1, r0	@ get stack location
+		add	r6, r6, r0	@ get &_edata
 
 #ifdef CONFIG_ARM_APPENDED_DTB
 		/*
 		 * Look for an appended DTB. If found, use it and
 		 * move stack away from it.
 		 */
-		ldr	r6, [r0, #12]	@ get &_edata
-		add	r6, r6, r1	@ relocate it
 		ldmia	r6, {r0, r5}	@ get DTB signature and size
 #ifndef __ARMEB__
 		ldr	r1, =0xedfe0dd0	@ sig is 0xd00dfeed big endian
@@ -345,22 +342,19 @@ not_angel:
 		orrcc	r4, r4, #1		@ remember we skipped cache_on
 		blcs	cache_on
 
-restart:	adr	r0, LC0
-		ldmia	r0, {r1, r2, r3, r6, r11, r12}
-		ldr	sp, [r0, #24]
+restart:	adr	r0, LC1
+		ldmia	r0, {r1, r6}
+		add	sp, r1, r0
+		add	r6, r6, r0
 
-		/*
-		 * We might be running at a different address.  We need
-		 * to fix up various pointers.
-		 */
+		adr	r0, LC0
+		ldmia	r0, {r1, r2, r3, r11, r12}
 		sub	r0, r0, r1		@ calculate the delta offset
-		add	r6, r6, r0		@ _edata
 
 		get_inflated_image_size	r9, r10, lr
 
 #ifndef CONFIG_ZBOOT_ROM
 		/* malloc space is above the relocated stack (64k max) */
-		add	sp, sp, r0
 		add	r10, sp, #0x10000
 #else
 		/*
@@ -712,12 +706,15 @@ not_relocated:	mov	r0, #0
 LC0:		.word	LC0			@ r1
 		.word	__bss_start		@ r2
 		.word	_end			@ r3
-		.word	_edata			@ r6
 		.word	_got_start		@ r11
 		.word	_got_end		@ ip
-		.word	.L_user_stack_end	@ sp
 		.size	LC0, . - LC0
 
+		.type	LC1, #object
+LC1:		.word	.L_user_stack_end - LC1	@ sp
+		.word	_edata - LC1		@ r6
+		.size	LC1, . - LC1
+
 .Lheadroom:
 		.word	_end - restart + 16384 + 1024*1024
 
Signature · 1 line
2.17.1
Reviewed-by on v1 2/5
#5 lore raw

Re: [PATCH 2/5] ARM: decompressor: split off _edata and stack base into separate object

Quoted 6 lines · Ard Biesheuvel

In preparation of moving the handling of the LC0 object to a later stage in the decompressor startup code, move out _edata and the initial value of the stack pointer, which are now used in two places, and are needed earlier than the remaining contents of LC0.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#6 lore raw

PATCH 3/5 ARM: decompressor: defer loading of the contents of the LC0 structure

In reply to Ard Biesheuvel

The remaining contents of LC0 are only used after the point in the decompressor startup code where we enter via 'wont_overwrite'. So move the loading of the LC0 structure after it. This will allow us to jump to wont_overwrite directly from the EFI stub, and execute the decompressor in place at the offset it was loaded by the UEFI firmware.

---
 arch/arm/boot/compressed/head.S | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)
arch/arm/boot/compressed/head.S +4 -11
index a6cf2f8115e5..35d4db00eaf9 100644
@@ -347,10 +347,6 @@ restart:	adr	r0, LC1
 		add	sp, r1, r0
 		add	r6, r6, r0
 
-		adr	r0, LC0
-		ldmia	r0, {r1, r2, r3, r11, r12}
-		sub	r0, r0, r1		@ calculate the delta offset
-
 		get_inflated_image_size	r9, r10, lr
 
 #ifndef CONFIG_ZBOOT_ROM
@@ -368,9 +364,6 @@ restart:	adr	r0, LC1
 		mov	r5, #0			@ init dtb size to 0
 #ifdef CONFIG_ARM_APPENDED_DTB
 /*
- *   r0  = delta
- *   r2  = BSS start
- *   r3  = BSS end
  *   r4  = final kernel address (possibly with LSB set)
  *   r5  = appended dtb size (still unknown)
  *   r6  = _edata
@@ -378,8 +371,6 @@ restart:	adr	r0, LC1
  *   r8  = atags/device tree pointer
  *   r9  = size of decompressed image
  *   r10 = end of this image, including  bss/stack/malloc space if non XIP
- *   r11 = GOT start
- *   r12 = GOT end
  *   sp  = stack pointer
  *
  * if there are device trees (dtb) appended to zImage, advance r10 so that the
@@ -427,7 +418,6 @@ restart:	adr	r0, LC1
 		/* temporarily relocate the stack past the DTB work space */
 		add	sp, sp, r5
 
-		stmfd	sp!, {r0-r3, ip, lr}
 		mov	r0, r8
 		mov	r1, r6
 		mov	r2, r5
@@ -446,7 +436,6 @@ restart:	adr	r0, LC1
 		mov	r2, r5
 		bleq	atags_to_fdt
 
-		ldmfd	sp!, {r0-r3, ip, lr}
 		sub	sp, sp, r5
 #endif
 
@@ -583,6 +572,10 @@ dtb_check_done:
 		mov	pc, r0
 
 wont_overwrite:
+		adr	r0, LC0
+		ldmia	r0, {r1, r2, r3, r11, r12}
+		sub	r0, r0, r1		@ calculate the delta offset
+
 /*
  * If delta is zero, we are running at the address we were linked at.
  *   r0  = delta
Signature · 1 line
2.17.1
Reviewed-by on v1 3/5
#7 lore raw

Re: [PATCH 3/5] ARM: decompressor: defer loading of the contents of the LC0 structure

Quoted 8 lines · Ard Biesheuvel

The remaining contents of LC0 are only used after the point in the decompressor startup code where we enter via 'wont_overwrite'. So move the loading of the LC0 structure after it. This will allow us to jump to wont_overwrite directly from the EFI stub, and execute the decompressor in place at the offset it was loaded by the UEFI firmware.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#8 lore raw

PATCH 4/5 ARM: decompressor: move GOT into .data for EFI enabled builds

In reply to Ard Biesheuvel

We will be running the decompressor in place after a future patch, instead of copying it around first. This means we no longer have to disable and re-enable the MMU and caches either. However, this means we will be loaded with the restricted permissions set by the UEFI firmware, which means that we have to move the GOT table into the data section in order for the contents to be writable by the code itself.

---
 arch/arm/boot/compressed/vmlinux.lds.S | 5 +++++
 1 file changed, 5 insertions(+)
arch/arm/boot/compressed/vmlinux.lds.S +5 -0
index b247f399de71..d0619ec05705 100644
@@ -63,9 +63,11 @@ SECTIONS
   _etext = .;
 
   .got.plt		: { *(.got.plt) }
+#ifndef CONFIG_EFI_STUB
   _got_start = .;
   .got			: { *(.got) }
   _got_end = .;
+#endif
 
   /* ensure the zImage file size is always a multiple of 64 bits */
   /* (without a dummy byte, ld just ignores the empty section) */
@@ -74,6 +76,9 @@ SECTIONS
 #ifdef CONFIG_EFI_STUB
   .data : ALIGN(4096) {
     __pecoff_data_start = .;
+    _got_start = .;
+    *(.got)
+    _got_end = .;
     /*
      * The EFI stub always executes from RAM, and runs strictly before the
      * decompressor, so we can make an exception for its r/w data, and keep it
Signature · 1 line
2.17.1
author maintainer
#9 lore raw

PATCH 5/5 ARM: decompressor: run decompressor in place if loaded via UEFI

In reply to Ard Biesheuvel

The decompressor can load from anywhere in memory, and the only reason the EFI stub code relocates it is to ensure it appears within the first 128 MiB of memory, so that the uncompressed kernel appears at the right offset in memory.

We can short circuit this, and simply jump into the decompressor startup code at the point where it knows where the base of memory lives. This also means there is no need to disable the MMU and caches, create new page tables and re-enable them.

---
 arch/arm/boot/compressed/head.S           | 34 ++++------------
 drivers/firmware/efi/libstub/arm32-stub.c | 41 +++-----------------
 2 files changed, 12 insertions(+), 63 deletions(-)
arch/arm/boot/compressed/head.S +7 -27
index 35d4db00eaf9..5558b25677f0 100644
@@ -1478,37 +1478,17 @@ reloc_code_end:
 
 #ifdef CONFIG_EFI_STUB
 ENTRY(efi_enter_kernel)
-		mov	r7, r0				@ preserve image base
-		mov	r4, r1				@ preserve DT pointer
+		mov	r4, r0				@ preserve image base
+		mov	r8, r1				@ preserve DT pointer
 
-		mov	r0, r4				@ DT start
-		add	r1, r4, r2			@ DT end
+		mov	r0, r8				@ DT start
+		add	r1, r8, r2			@ DT end
 		bl	cache_clean_flush
 
-		mov	r0, r7				@ relocated zImage
-		ldr	r1, =_edata			@ size of zImage
-		add	r1, r1, r0			@ end of zImage
-		bl	cache_clean_flush
-
-		@ The PE/COFF loader might not have cleaned the code we are
-		@ running beyond the PoU, and so calling cache_off below from
-		@ inside the PE/COFF loader allocated region is unsafe unless
-		@ we explicitly clean it to the PoC.
-		adr	r0, call_cache_fn		@ region of code we will
-		adr	r1, 0f				@ run with MMU off
-		bl	cache_clean_flush
-		bl	cache_off
-
-		@ Set parameters for booting zImage according to boot protocol
-		@ put FDT address in r2, it was returned by efi_entry()
-		@ r1 is the machine type, and r0 needs to be 0
-		mov	r0, #0
-		mov	r1, #0xFFFFFFFF
-		mov	r2, r4
-		add	r7, r7, #(__efi_start - start)
-		mov	pc, r7				@ no mode switch
+		mov	r5, #0				@ appended DTB size
+		mov	r7, #0xFFFFFFFF			@ machine ID
+		b	wont_overwrite
 ENDPROC(efi_enter_kernel)
-0:
 #endif
 
 		.align
drivers/firmware/efi/libstub/arm32-stub.c +5 -36

52 lines not on this page · raw message

Signature · 1 line
2.17.1