diff mbox series

[v2,03/16] x86/compressed: efi-mixed: move bootargs parsing out of 32-bit startup code

Message ID 20220921145422.437618-4-ardb@kernel.org
State Superseded
Headers show
Series x86: head_64.S spring cleaning | expand

Commit Message

Ard Biesheuvel Sept. 21, 2022, 2:54 p.m. UTC
Move the logic that chooses between the different EFI entrypoints out of
the 32-bit boot path, and into a 64-bit helper that can perform the same
task much more cleanly. While at it, document the mixed mode boot flow
in a code comment.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/efi_mixed.S | 43 ++++++++++++++++++++
 arch/x86/boot/compressed/head_64.S   | 24 ++---------
 2 files changed, 47 insertions(+), 20 deletions(-)

Comments

Borislav Petkov Oct. 6, 2022, 11:03 a.m. UTC | #1
On Wed, Sep 21, 2022 at 04:54:09PM +0200, Ard Biesheuvel wrote:
> Move the logic that chooses between the different EFI entrypoints out of
> the 32-bit boot path, and into a 64-bit helper that can perform the same
> task much more cleanly. While at it, document the mixed mode boot flow
> in a code comment.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/boot/compressed/efi_mixed.S | 43 ++++++++++++++++++++
>  arch/x86/boot/compressed/head_64.S   | 24 ++---------
>  2 files changed, 47 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/efi_mixed.S b/arch/x86/boot/compressed/efi_mixed.S
> index 67e7edcdfea8..77e77c3ea393 100644
> --- a/arch/x86/boot/compressed/efi_mixed.S
> +++ b/arch/x86/boot/compressed/efi_mixed.S
> @@ -22,6 +22,49 @@
>  
>  	.code64
>  	.text
> +/*
> + * When booting in 64-bit mode on 32-bit EFI firmware, startup_64_mixedmode()
> + * is the first thing that runs after switching to long mode. Depending on
> + * whether the EFI handover protocol or the compat entry point was used to
> + * enter the kernel, it will either branch to the 64-bit EFI handover
> + * entrypoint at offset 0x390 in the image, or to the 64-bit EFI PE/COFF
> + * entrypoint efi_pe_entry(). In the former case, the bootloader must provide a
> + * struct bootparams pointer as the third argument, so the presence of such a
> + * pointer is used to disambiguate.
> + *
> + *                                                             +--------------+
> + *  +------------------+     +------------+            +------>| efi_pe_entry |
> + *  | efi32_pe_entry   |---->|            |            |       +-----------+--+
> + *  +------------------+     |            |     +------+---------------+   |
> + *                           | startup_32 |---->| startup_64_mixedmode |   |
> + *  +------------------+     |            |     +------+---------------+   V
> + *  | efi32_stub_entry |---->|            |            |     +------------------+
> + *  +------------------+     +------------+            +---->| efi64_stub_entry |
> + *                                                           +-------------+----+
> + *                           +------------+     +----------+               |
> + *                           | startup_64 |<----| efi_main |<--------------+
> + *                           +------------+     +----------+
> + */

That is much appreciated.

Questions:

- is this whole handover ABI documented somewhere?

- efi32_pe_entry() is the 32-bit PE/COFF entry point? I.e., that is
called by a 32-bit EFI fw when the kernel is a PE/COFF executable?

But then Documentation/admin-guide/efi-stub.rst talks about the EFI stub
and exactly that. Hmm, so what is efi32_pe_entry() then?

> +SYM_FUNC_START(startup_64_mixedmode)

	... mixed_mode

I guess.
Ard Biesheuvel Oct. 6, 2022, 11:29 a.m. UTC | #2
On Thu, 6 Oct 2022 at 13:03, Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Sep 21, 2022 at 04:54:09PM +0200, Ard Biesheuvel wrote:
> > Move the logic that chooses between the different EFI entrypoints out of
> > the 32-bit boot path, and into a 64-bit helper that can perform the same
> > task much more cleanly. While at it, document the mixed mode boot flow
> > in a code comment.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/x86/boot/compressed/efi_mixed.S | 43 ++++++++++++++++++++
> >  arch/x86/boot/compressed/head_64.S   | 24 ++---------
> >  2 files changed, 47 insertions(+), 20 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/efi_mixed.S b/arch/x86/boot/compressed/efi_mixed.S
> > index 67e7edcdfea8..77e77c3ea393 100644
> > --- a/arch/x86/boot/compressed/efi_mixed.S
> > +++ b/arch/x86/boot/compressed/efi_mixed.S
> > @@ -22,6 +22,49 @@
> >
> >       .code64
> >       .text
> > +/*
> > + * When booting in 64-bit mode on 32-bit EFI firmware, startup_64_mixedmode()
> > + * is the first thing that runs after switching to long mode. Depending on
> > + * whether the EFI handover protocol or the compat entry point was used to
> > + * enter the kernel, it will either branch to the 64-bit EFI handover
> > + * entrypoint at offset 0x390 in the image, or to the 64-bit EFI PE/COFF
> > + * entrypoint efi_pe_entry(). In the former case, the bootloader must provide a
> > + * struct bootparams pointer as the third argument, so the presence of such a
> > + * pointer is used to disambiguate.
> > + *
> > + *                                                             +--------------+
> > + *  +------------------+     +------------+            +------>| efi_pe_entry |
> > + *  | efi32_pe_entry   |---->|            |            |       +-----------+--+
> > + *  +------------------+     |            |     +------+---------------+   |
> > + *                           | startup_32 |---->| startup_64_mixedmode |   |
> > + *  +------------------+     |            |     +------+---------------+   V
> > + *  | efi32_stub_entry |---->|            |            |     +------------------+
> > + *  +------------------+     +------------+            +---->| efi64_stub_entry |
> > + *                                                           +-------------+----+
> > + *                           +------------+     +----------+               |
> > + *                           | startup_64 |<----| efi_main |<--------------+
> > + *                           +------------+     +----------+
> > + */
>
> That is much appreciated.
>
> Questions:
>
> - is this whole handover ABI documented somewhere?
>

Documentation/x86/boot.rst has a section on this (at the end), but we
should really stop using it. It is only implemented by out-of-tree
GRUB at the moment (last time I checked) and leaking all those struct
bootparams specific details into every bootloader is not great,
especially the ones that intend to be generic and boot any EFI OS on
any EFI arch.


> - efi32_pe_entry() is the 32-bit PE/COFF entry point? I.e., that is
> called by a 32-bit EFI fw when the kernel is a PE/COFF executable?
>

Yes. But I should note that this is actually something that goes
outside of the EFI spec as well: 32-bit firmware can /load/ 64-bit
PE/COFF binaries but not *start* them.

Commit 97aa276579b28b86f4a3e235b50762c0191c2ac3 has some more
background. This is currently implement by 32-bit OVMF, and
systemd-boot.

> But then Documentation/admin-guide/efi-stub.rst talks about the EFI stub
> and exactly that. Hmm, so what is efi32_pe_entry() then?
>

That is the same thing. The EFI stub is what enables the kernel (or
decompressor) to masquerade as a PE/COFF executable.

In short, every EFI stub kernel on every architecture has a native
PE/COFF entry point that calls the EFI stub, and the EFi stub does the
arch-specific bootloader work and boots it.

In addition, the x86_64 EFI stub kernel has an extra, non-native
PE/COFF entry point, which is exposed in a way that is not covered by
the EFI spec, but which allows Linux specific loaders such as
systemd-boot to boot such kernels on 32-bit firmware without having to
do the whole struct bootparams dance in the bootloader.
Borislav Petkov Oct. 7, 2022, 9:30 a.m. UTC | #3
On Thu, Oct 06, 2022 at 01:29:55PM +0200, Ard Biesheuvel wrote:
> Documentation/x86/boot.rst has a section on this (at the end),

Ah, and I really like that NOTE at the end.

> but we should really stop using it. It is only implemented by
> out-of-tree GRUB at the moment (last time I checked) and leaking all
> those struct bootparams specific details into every bootloader is not
> great, especially the ones that intend to be generic and boot any EFI
> OS on any EFI arch.

I'm all for making early asm code simpler so yes, can we start removing
it?

Dunno, maybe ifdef around it with a Kconfig option which is default off
and see who complains...

> That is the same thing. The EFI stub is what enables the kernel (or
> decompressor) to masquerade as a PE/COFF executable.
> 
> In short, every EFI stub kernel on every architecture has a native
> PE/COFF entry point that calls the EFI stub, and the EFi stub does the
> arch-specific bootloader work and boots it.

Right.

> In addition, the x86_64 EFI stub kernel has an extra, non-native
> PE/COFF entry point, which is exposed in a way that is not covered by
> the EFI spec, but which allows Linux specific loaders such as
> systemd-boot to boot such kernels on 32-bit firmware without having to
> do the whole struct bootparams dance in the bootloader.

Ok, thanks for explaining.

I like the simplification and obviating the need for the bootloader to
do any dancing before loading the kernel.
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/efi_mixed.S b/arch/x86/boot/compressed/efi_mixed.S
index 67e7edcdfea8..77e77c3ea393 100644
--- a/arch/x86/boot/compressed/efi_mixed.S
+++ b/arch/x86/boot/compressed/efi_mixed.S
@@ -22,6 +22,49 @@ 
 
 	.code64
 	.text
+/*
+ * When booting in 64-bit mode on 32-bit EFI firmware, startup_64_mixedmode()
+ * is the first thing that runs after switching to long mode. Depending on
+ * whether the EFI handover protocol or the compat entry point was used to
+ * enter the kernel, it will either branch to the 64-bit EFI handover
+ * entrypoint at offset 0x390 in the image, or to the 64-bit EFI PE/COFF
+ * entrypoint efi_pe_entry(). In the former case, the bootloader must provide a
+ * struct bootparams pointer as the third argument, so the presence of such a
+ * pointer is used to disambiguate.
+ *
+ *                                                             +--------------+
+ *  +------------------+     +------------+            +------>| efi_pe_entry |
+ *  | efi32_pe_entry   |---->|            |            |       +-----------+--+
+ *  +------------------+     |            |     +------+---------------+   |
+ *                           | startup_32 |---->| startup_64_mixedmode |   |
+ *  +------------------+     |            |     +------+---------------+   V
+ *  | efi32_stub_entry |---->|            |            |     +------------------+
+ *  +------------------+     +------------+            +---->| efi64_stub_entry |
+ *                                                           +-------------+----+
+ *                           +------------+     +----------+               |
+ *                           | startup_64 |<----| efi_main |<--------------+
+ *                           +------------+     +----------+
+ */
+SYM_FUNC_START(startup_64_mixedmode)
+	lea	efi32_boot_args(%rip), %rdx
+	mov	0(%rdx), %edi
+	mov	4(%rdx), %esi
+	mov	8(%rdx), %edx		// saved bootparams pointer
+	test	%edx, %edx
+	jnz	efi64_stub_entry
+	/*
+	 * efi_pe_entry uses MS calling convention, which requires 32 bytes of
+	 * shadow space on the stack even if all arguments are passed in
+	 * registers. We also need an additional 8 bytes for the space that
+	 * would be occupied by the return address, and this also results in
+	 * the correct stack alignment for entry.
+	 */
+	sub	$40, %rsp
+	mov	%rdi, %rcx		// MS calling convention
+	mov	%rsi, %rdx
+	jmp	efi_pe_entry
+SYM_FUNC_END(startup_64_mixedmode)
+
 SYM_FUNC_START(__efi64_thunk)
 	push	%rbp
 	push	%rbx
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 1ba2fc2357e6..b51f0e107c2e 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -261,25 +261,9 @@  SYM_FUNC_START(startup_32)
 	 */
 	leal	rva(startup_64)(%ebp), %eax
 #ifdef CONFIG_EFI_MIXED
-	movl	rva(efi32_boot_args)(%ebp), %edi
-	testl	%edi, %edi
-	jz	1f
-	leal	rva(efi64_stub_entry)(%ebp), %eax
-	movl	rva(efi32_boot_args+4)(%ebp), %esi
-	movl	rva(efi32_boot_args+8)(%ebp), %edx	// saved bootparams pointer
-	testl	%edx, %edx
-	jnz	1f
-	/*
-	 * efi_pe_entry uses MS calling convention, which requires 32 bytes of
-	 * shadow space on the stack even if all arguments are passed in
-	 * registers. We also need an additional 8 bytes for the space that
-	 * would be occupied by the return address, and this also results in
-	 * the correct stack alignment for entry.
-	 */
-	subl	$40, %esp
-	leal	rva(efi_pe_entry)(%ebp), %eax
-	movl	%edi, %ecx			// MS calling convention
-	movl	%esi, %edx
+	cmpb	$1, rva(efi_is64)(%ebp)
+	je	1f
+	leal	rva(startup_64_mixedmode)(%ebp), %eax
 1:
 #endif
 	/* Check if the C-bit position is correct when SEV is active */
@@ -766,7 +750,7 @@  SYM_DATA_END_LABEL(boot32_idt, SYM_L_GLOBAL, boot32_idt_end)
 SYM_DATA(image_offset, .long 0)
 #endif
 #ifdef CONFIG_EFI_MIXED
-SYM_DATA_LOCAL(efi32_boot_args, .long 0, 0, 0)
+SYM_DATA(efi32_boot_args, .long 0, 0, 0)
 SYM_DATA(efi_is64, .byte 1)
 
 #define ST32_boottime		60 // offsetof(efi_system_table_32_t, boottime)