diff mbox series

[v5,01/20] x86/efistub: Branch straight to kernel entry point from C code

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

Commit Message

Ard Biesheuvel June 7, 2023, 7:23 a.m. UTC 
  Instead of returning to the calling code in assembler that does nothing
more than perform an indirect call with the boot_params pointer in
register ESI/RSI, perform the jump directly from the EFI stub C code.
This will allow the asm entrypoint code to be dropped entirely in
subsequent patches.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/x86-stub.c | 21 ++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

Borislav Petkov June 7, 2023, 6:53 p.m. UTC | #1 
On Wed, Jun 07, 2023 at 09:23:23AM +0200, Ard Biesheuvel wrote:
> -	return bzimage_addr;
> +	if (IS_ENABLED(CONFIG_X86_64))
> +		/* add offset of startup_64() */
> +		bzimage_addr += 0x200;

Uh, magic.

Well, there's this:

arch/x86/boot/compressed/head_64.S:

        .code64
        .org 0x200
SYM_CODE_START(startup_64)
        /*
         * 64bit entry is 0x200 and it is ABI so immutable!
         * We come here either from startup_32 or directly from a
         * 64bit bootloader.


Looking at Documentation/arch/x86/boot.rst, we actually say in the
xloadflags section:

  Bit 0 (read): XLF_KERNEL_64

        - If 1, this kernel has the legacy 64-bit entry point at 0x200.

and header.S sets that:

xloadflags:
#ifdef CONFIG_X86_64
# define XLF0 XLF_KERNEL_64                     /* 64-bit kernel */

so you checking CONFIG_X86_64 is probably ok.

It might be cleaner, though, if you test XLF_KERNEL_64 directly and act
accordingly, although, do I understand it correctly, that the EFI
libstub goes together with the kernel it was built for so the checks
would be doing the same thing...? I.e., the libstub cannot be somehow
"glued" with another kernel or so, which doesn't set CONFIG_X86_64.

Thx.
Ard Biesheuvel June 7, 2023, 7:39 p.m. UTC | #2 
On Wed, 7 Jun 2023 at 20:53, Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Jun 07, 2023 at 09:23:23AM +0200, Ard Biesheuvel wrote:
> > -     return bzimage_addr;
> > +     if (IS_ENABLED(CONFIG_X86_64))
> > +             /* add offset of startup_64() */
> > +             bzimage_addr += 0x200;
>
> Uh, magic.
>
> Well, there's this:
>
> arch/x86/boot/compressed/head_64.S:
>
>         .code64
>         .org 0x200
> SYM_CODE_START(startup_64)
>         /*
>          * 64bit entry is 0x200 and it is ABI so immutable!
>          * We come here either from startup_32 or directly from a
>          * 64bit bootloader.
>
>
> Looking at Documentation/arch/x86/boot.rst, we actually say in the
> xloadflags section:
>
>   Bit 0 (read): XLF_KERNEL_64
>
>         - If 1, this kernel has the legacy 64-bit entry point at 0x200.
>
> and header.S sets that:
>
> xloadflags:
> #ifdef CONFIG_X86_64
> # define XLF0 XLF_KERNEL_64                     /* 64-bit kernel */
>
> so you checking CONFIG_X86_64 is probably ok.
>
> It might be cleaner, though, if you test XLF_KERNEL_64 directly and act
> accordingly, although, do I understand it correctly, that the EFI
> libstub goes together with the kernel it was built for so the checks
> would be doing the same thing...? I.e., the libstub cannot be somehow
> "glued" with another kernel or so, which doesn't set CONFIG_X86_64.
>

This code gets removed again later in the series, so i didn't bother
with any of this.

I think checking those XLF flags from code is something to avoid in
any case - it is part of the file representation, and consumed by
loaders. I am not sure we can assume that they are always accessible
from the running code (and EFI is not guaranteed to load the first
sector as this is the file header not the payload)
diff mbox series

Patch

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 220be75a5cdc1f4c..d4a730f053bdcbfa 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -803,10 +803,19 @@  static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
 	return EFI_SUCCESS;
 }
 
+static void __noreturn enter_kernel(unsigned long kernel_addr,
+				    struct boot_params *boot_params)
+{
+	/* enter decompressed kernel with boot_params pointer in RSI/ESI */
+	asm("jmp *%0"::"r"(kernel_addr), "S"(boot_params));
+
+	unreachable();
+}
+
 /*
- * On success, we return the address of startup_32, which has potentially been
- * relocated by efi_relocate_kernel.
- * On failure, we exit to the firmware via efi_exit instead of returning.
+ * On success, this routine will jump to the relocated image directly and never
+ * return.  On failure, it will exit to the firmware via efi_exit() instead of
+ * returning.
  */
 asmlinkage unsigned long efi_main(efi_handle_t handle,
 				  efi_system_table_t *sys_table_arg,
@@ -950,7 +959,11 @@  asmlinkage unsigned long efi_main(efi_handle_t handle,
 		goto fail;
 	}
 
-	return bzimage_addr;
+	if (IS_ENABLED(CONFIG_X86_64))
+		/* add offset of startup_64() */
+		bzimage_addr += 0x200;
+
+	enter_kernel(bzimage_addr, boot_params);
 fail:
 	efi_err("efi_main() failed!\n");