diff mbox series

[v5,06/20] x86/decompressor: Store boot_params pointer in callee save register

Message ID 20230607072342.4054036-7-ardb@kernel.org
State Superseded
Headers show
Series efi/x86: Avoid bare metal decompressor during EFI boot | expand

Commit Message

Ard Biesheuvel June 7, 2023, 7:23 a.m. UTC 
Instead of pushing and popping %RSI several times to preserve the struct
boot_params pointer across the execution of the startup code, move it
into a callee save register before the first call into C, and copy it
back when needed.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/head_64.S | 34 +++++++-------------
 1 file changed, 11 insertions(+), 23 deletions(-)

Comments

Borislav Petkov July 10, 2023, 9:06 a.m. UTC | #1 
On Wed, Jun 07, 2023 at 09:23:28AM +0200, Ard Biesheuvel wrote:
> Instead of pushing and popping %RSI several times to preserve the struct
> boot_params pointer across the execution of the startup code, move it
> into a callee save register before the first call into C, and copy it
> back when needed.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/boot/compressed/head_64.S | 34 +++++++-------------
>  1 file changed, 11 insertions(+), 23 deletions(-)

I like that.

We do a similar dance in arch/x86/kernel/head_64.S. Care to fix that
too, in a separate patch?

Thx.
Ard Biesheuvel July 10, 2023, 9:55 p.m. UTC | #2 
On Mon, 10 Jul 2023 at 11:07, Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Jun 07, 2023 at 09:23:28AM +0200, Ard Biesheuvel wrote:
> > Instead of pushing and popping %RSI several times to preserve the struct
> > boot_params pointer across the execution of the startup code, move it
> > into a callee save register before the first call into C, and copy it
> > back when needed.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/x86/boot/compressed/head_64.S | 34 +++++++-------------
> >  1 file changed, 11 insertions(+), 23 deletions(-)
>
> I like that.
>
> We do a similar dance in arch/x86/kernel/head_64.S. Care to fix that
> too, in a separate patch?
>

I already did, actually, but I dropped it from this series because it
was getting too long, and not essential for the overall goal of the
changes.

https://lore.kernel.org/all/20230602101313.3557775-19-ardb@kernel.org/
Borislav Petkov July 11, 2023, 7:57 a.m. UTC | #3 
On Mon, Jul 10, 2023 at 11:55:49PM +0200, Ard Biesheuvel wrote:
> I already did, actually, but I dropped it from this series because it
> was getting too long, and not essential for the overall goal of the
> changes.

Yeah, might wanna add it, though, for the simple reason that
compressed/head_64.S and kernel/head_64.S will otherwise differ in the
%rsi handling unnecessarily and people might wonder why.

Thx.
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 9f90661744741210..2d1b0ee94929f7ec 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -405,10 +405,14 @@  SYM_CODE_START(startup_64)
 	lretq
 
 .Lon_kernel_cs:
+	/*
+	 * RSI holds a pointer to a boot_params structure provided by the
+	 * loader, and this needs to be preserved across C function calls. So
+	 * move it into a callee saved register.
+	 */
+	movq	%rsi, %r15
 
-	pushq	%rsi
 	call	load_stage1_idt
-	popq	%rsi
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 	/*
@@ -421,10 +425,8 @@  SYM_CODE_START(startup_64)
 	 * detection/setup to ensure that has been done in advance of any dependent
 	 * code.
 	 */
-	pushq	%rsi
-	movq	%rsi, %rdi		/* real mode address */
+	movq	%r15, %rdi		/* pass struct boot_params pointer */
 	call	sev_enable
-	popq	%rsi
 #endif
 
 	/*
@@ -437,13 +439,9 @@  SYM_CODE_START(startup_64)
 	 *   - Non zero RDX means trampoline needs to enable 5-level
 	 *     paging.
 	 *
-	 * RSI holds real mode data and needs to be preserved across
-	 * this function call.
 	 */
-	pushq	%rsi
-	movq	%rsi, %rdi		/* real mode address */
+	movq	%r15, %rdi		/* pass struct boot_params pointer */
 	call	paging_prepare
-	popq	%rsi
 
 	/* Save the trampoline address in RCX */
 	movq	%rax, %rcx
@@ -468,14 +466,9 @@  trampoline_return:
 	 *
 	 * RDI is address of the page table to use instead of page table
 	 * in trampoline memory (if required).
-	 *
-	 * RSI holds real mode data and needs to be preserved across
-	 * this function call.
 	 */
-	pushq	%rsi
 	leaq	rva(top_pgtable)(%rbx), %rdi
 	call	cleanup_trampoline
-	popq	%rsi
 
 	/* Zero EFLAGS */
 	pushq	$0
@@ -485,7 +478,6 @@  trampoline_return:
  * Copy the compressed kernel to the end of our buffer
  * where decompression in place becomes safe.
  */
-	pushq	%rsi
 	leaq	(_bss-8)(%rip), %rsi
 	leaq	rva(_bss-8)(%rbx), %rdi
 	movl	$(_bss - startup_32), %ecx
@@ -493,7 +485,6 @@  trampoline_return:
 	std
 	rep	movsq
 	cld
-	popq	%rsi
 
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
@@ -525,30 +516,27 @@  SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	shrq	$3, %rcx
 	rep	stosq
 
-	pushq	%rsi
 	call	load_stage2_idt
 
 	/* Pass boot_params to initialize_identity_maps() */
-	movq	(%rsp), %rdi
+	movq	%r15, %rdi		/* pass struct boot_params pointer */
 	call	initialize_identity_maps
-	popq	%rsi
 
 /*
  * Do the extraction, and jump to the new kernel..
  */
-	pushq	%rsi			/* Save the real mode argument */
-	movq	%rsi, %rdi		/* real mode address */
+	movq	%r15, %rdi		/* pass struct boot_params pointer */
 	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
 	leaq	input_data(%rip), %rdx  /* input_data */
 	movl	input_len(%rip), %ecx	/* input_len */
 	movq	%rbp, %r8		/* output target address */
 	movl	output_len(%rip), %r9d	/* decompressed length, end of relocs */
 	call	extract_kernel		/* returns kernel entry point in %rax */
-	popq	%rsi
 
 /*
  * Jump to the decompressed kernel.
  */
+	movq	%r15, %rsi
 	jmp	*%rax
 SYM_FUNC_END(.Lrelocated)