| Message ID | 20200217124904.16331-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | efi/libstub/arm: make efi_entry() an ordinary PE/COFF entrypoint | expand |
On Mon, 2020-02-17 at 13:49 +0100, Ard Biesheuvel wrote: > Expose efi_entry() as the PE/COFF entrypoint directly, instead of > jumping into a wrapper that fiddles with stack buffers and other > stuff that the compiler is much better at. The only reason this > code exists is to obtain a pointer to the base of the image, but > we can get the same value from the loaded_image protocol, which > we already need for other reasons anyway. > > Update the return type as well, to make it consistent with what > is required for a PE/COFF executable entrypoint. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > Apologies to Atish for making him track a moving target, but this is > another > bit of cleanup work that I would like to merge before taking the > RISC-V > changes on top. > And I thought I am done rebasing my patches ;). Jokes apart, this is actually much cleaner approach in retrieving image base and parsing DT compared to what I currently have for RISC-V. > On RISC-V, I would expect efi_enter_kernel() to be implemented as a C > routine > that finds the hart id in the DT (which is passed as an argument), > and pass > it to the startup code of the kernel proper. > Yes. I probably won't need efi-entry.S for RISC-V at all which simplifies the RISC-V implementation. Yay!! Is there a tree with this patch ? I tried to find it in your kernel.org repo but couldn't find it. As there are 3 efi related patch set in flight, I am not sure about the order in which they should be applied. > arch/arm/boot/compressed/efi-header.S | 2 +- > arch/arm/boot/compressed/head.S | 44 +++----------- > arch/arm64/kernel/efi-entry.S | 64 +++----------------- > arch/arm64/kernel/efi-header.S | 2 +- > drivers/firmware/efi/libstub/arm-stub.c | 37 +++++------ > drivers/firmware/efi/libstub/arm32-stub.c | 1 + > drivers/firmware/efi/libstub/arm64-stub.c | 3 +- > 7 files changed, 42 insertions(+), 111 deletions(-) > > diff --git a/arch/arm/boot/compressed/efi-header.S > b/arch/arm/boot/compressed/efi-header.S > index a5983588f96b..e38fbda02b93 100644 > --- a/arch/arm/boot/compressed/efi-header.S > +++ b/arch/arm/boot/compressed/efi-header.S > @@ -60,7 +60,7 @@ optional_header: > .long __pecoff_code_size @ SizeOfCode > .long __pecoff_data_size @ > SizeOfInitializedData > .long 0 @ > SizeOfUninitializedData > - .long efi_stub_entry - start @ > AddressOfEntryPoint > + .long efi_entry - start @ > AddressOfEntryPoint > .long start_offset @ BaseOfCode > .long __pecoff_data_start - start @ BaseOfData > > diff --git a/arch/arm/boot/compressed/head.S > b/arch/arm/boot/compressed/head.S > index 088b0a060876..941cd28a13c3 100644 > --- a/arch/arm/boot/compressed/head.S > +++ b/arch/arm/boot/compressed/head.S > @@ -1437,50 +1437,22 @@ __enter_kernel: > reloc_code_end: > > #ifdef CONFIG_EFI_STUB > - .align 2 > -_start: .long start - . > - > -ENTRY(efi_stub_entry) > - @ allocate space on stack for passing current zImage > address > - @ and for the EFI stub to return of new entry point of > - @ zImage, as EFI stub may copy the kernel. Pointer > address > - @ is passed in r2. r0 and r1 are passed through from > the > - @ EFI firmware to efi_entry > - adr ip, _start > - ldr r3, [ip] > - add r3, r3, ip > - stmfd sp!, {r3, lr} > - mov r2, sp @ pass zImage address > in r2 > - bl efi_entry > - > - @ Check for error return from EFI stub. r0 has FDT > address > - @ or error code. > - cmn r0, #1 > - beq efi_load_fail > - > - @ Preserve return value of efi_entry() in r4 > - mov r4, r0 > +ENTRY(efi_enter_kernel) > + mov r4, r0 @ preserve entrypoint > + mov r5, r1 @ preserve DT pointer > 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 > + mov r2, r5 > > - @ Branch to (possibly) relocated zImage that is in [sp] > - ldr lr, [sp] > + @ Branch to (possibly) relocated zImage @ [r4] > ldr ip, =start_offset > - add lr, lr, ip > - mov pc, lr @ no mode > switch > - > -efi_load_fail: > - @ Return EFI_LOAD_ERROR to EFI firmware on error. > - ldr r0, =0x80000001 > - ldmfd sp!, {ip, pc} > -ENDPROC(efi_stub_entry) > + add r4, r4, ip > + mov pc, r4 @ no mode switch > +ENDPROC(efi_enter_kernel) > #endif > > .align > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi- > entry.S > index 304d5b02ca67..49e3b1ad1b36 100644 > --- a/arch/arm64/kernel/efi-entry.S > +++ b/arch/arm64/kernel/efi-entry.S > @@ -10,55 +10,18 @@ > > #include <asm/assembler.h> > > -#define EFI_LOAD_ERROR 0x8000000000000001 > - > __INIT > > - /* > - * We arrive here from the EFI boot manager with: > - * > - * * CPU in little-endian mode > - * * MMU on with identity-mapped RAM > - * * Icache and Dcache on > - * > - * We will most likely be running from some place other than > where > - * we want to be. The kernel image wants to be placed at > TEXT_OFFSET > - * from start of RAM. > - */ > -ENTRY(entry) > - /* > - * Create a stack frame to save FP/LR with extra space > - * for image_addr variable passed to efi_entry(). > - */ > - stp x29, x30, [sp, #-32]! > - mov x29, sp > - > - /* > - * Call efi_entry to do the real work. > - * x0 and x1 are already set up by firmware. Current runtime > - * address of image is calculated and passed via *image_addr. > - * > - * unsigned long efi_entry(void *handle, > - * efi_system_table_t *sys_table, > - * unsigned long *image_addr) ; > - */ > - adr_l x8, _text > - add x2, sp, 16 > - str x8, [x2] > - bl efi_entry > - cmn x0, #1 > - b.eq efi_load_fail > - > +ENTRY(efi_enter_kernel) > /* > * efi_entry() will have copied the kernel image if necessary > and we > - * return here with device tree address in x0 and the kernel > entry > - * point stored at *image_addr. Save those values in registers > which > - * are callee preserved. > + * end up here with device tree address in x1 and the kernel > entry > + * point stored in x0. Save those values in registers which are > + * callee preserved. > */ > - mov x20, x0 // DTB address > - ldr x0, [sp, #16] // relocated _text address > + mov x20, x1 // DTB address > ldr w21, =stext_offset > - add x21, x0, x21 > + add x21, x0, x21 // relocated Image address > > /* > * Calculate size of the kernel Image (same for original and > copy). > @@ -80,9 +43,8 @@ ENTRY(entry) > * entries for the VA range of the current image, so no > maintenance is > * necessary. > */ > - adr x0, entry > - adr x1, entry_end > - sub x1, x1, x0 > + adr x0, efi_enter_kernel > + mov x1, efi_enter_kernel_size > bl __flush_dcache_area > > /* Turn off Dcache and MMU */ > @@ -110,11 +72,5 @@ ENTRY(entry) > mov x2, xzr > mov x3, xzr > br x21 > - > -efi_load_fail: > - mov x0, #EFI_LOAD_ERROR > - ldp x29, x30, [sp], #32 > - ret > - > -entry_end: > -ENDPROC(entry) > +ENDPROC(efi_enter_kernel) > + .set efi_enter_kernel_size, . - efi_enter_kernel > diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi- > header.S > index a7cfacce3e15..40c704c5d3a5 100644 > --- a/arch/arm64/kernel/efi-header.S > +++ b/arch/arm64/kernel/efi-header.S > @@ -27,7 +27,7 @@ optional_header: > .long __initdata_begin - efi_header_end // SizeOfCode > .long __pecoff_data_size // > SizeOfInitializedData > .long 0 // > SizeOfUninitializedData > - .long __efistub_entry - _head // > AddressOfEntryPoint > + .long __efistub_efi_entry - _head // > AddressOfEntryPoint > .long efi_header_end - _head // BaseOfCode > > extra_header_fields: > diff --git a/drivers/firmware/efi/libstub/arm-stub.c > b/drivers/firmware/efi/libstub/arm-stub.c > index 56e475c1aa55..439c094f7bf2 100644 > --- a/drivers/firmware/efi/libstub/arm-stub.c > +++ b/drivers/firmware/efi/libstub/arm-stub.c > @@ -133,17 +133,20 @@ efi_status_t handle_kernel_image(unsigned long > *image_addr, > unsigned long *reserve_size, > unsigned long dram_base, > efi_loaded_image_t *image); > + > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned > long fdt); > + > /* > * EFI entry point for the arm/arm64 EFI stubs. This is the > entrypoint > * that is described in the PE/COFF header. Most of the code is the > same > * for both archictectures, with the arch-specific code provided in > the > * handle_kernel_image() function. > */ > -unsigned long efi_entry(void *handle, efi_system_table_t > *sys_table_arg, > - unsigned long *image_addr) > +efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t > *sys_table_arg) > { > efi_loaded_image_t *image; > efi_status_t status; > + unsigned long image_addr; > unsigned long image_size = 0; > unsigned long dram_base; > /* addr/point and size pairs for memory management*/ > @@ -153,7 +156,6 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > unsigned long fdt_size = 0; > char *cmdline_ptr = NULL; > int cmdline_size = 0; > - unsigned long new_fdt_addr; > efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID; > unsigned long reserve_addr = 0; > unsigned long reserve_size = 0; > @@ -165,8 +167,10 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > sys_table = sys_table_arg; > > /* Check if we were booted by the EFI firmware */ > - if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) > + if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) { > + status = EFI_INVALID_PARAMETER; > goto fail; > + } > > status = check_platform_features(); > if (status != EFI_SUCCESS) > @@ -187,6 +191,7 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > dram_base = get_dram_base(); > if (dram_base == EFI_ERROR) { > pr_efi_err("Failed to find DRAM base\n"); > + status = EFI_LOAD_ERROR; > goto fail; > } > > @@ -198,6 +203,7 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, > ULONG_MAX); > if (!cmdline_ptr) { > pr_efi_err("getting command line via > LOADED_IMAGE_PROTOCOL\n"); > + status = EFI_OUT_OF_RESOURCES; > goto fail; > } > > @@ -213,7 +219,7 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > > si = setup_graphics(); > > - status = handle_kernel_image(image_addr, &image_size, > + status = handle_kernel_image(&image_addr, &image_size, > &reserve_addr, > &reserve_size, > dram_base, image); > @@ -260,7 +266,7 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > pr_efi("Generating empty DTB\n"); > > if (!noinitrd()) { > - max_addr = efi_get_max_initrd_addr(dram_base, > *image_addr); > + max_addr = efi_get_max_initrd_addr(dram_base, > image_addr); > status = efi_load_initrd_dev_path(&initrd_addr, > &initrd_size, > max_addr); > if (status == EFI_SUCCESS) { > @@ -311,33 +317,30 @@ unsigned long efi_entry(void *handle, > efi_system_table_t *sys_table_arg, > > install_memreserve_table(); > > - new_fdt_addr = fdt_addr; > status = allocate_new_fdt_and_exit_boot(handle, > - &new_fdt_addr, > efi_get_max_fdt_addr(dram_base), > + &fdt_addr, > efi_get_max_fdt_addr(dram_base), > initrd_addr, initrd_size, cmdline_ptr, > fdt_addr, fdt_size); > + if (status != EFI_SUCCESS) > + goto fail_free_initrd; > > - /* > - * If all went well, we need to return the FDT address to the > - * calling function so it can be passed to kernel as part of > - * the kernel boot protocol. > - */ > - if (status == EFI_SUCCESS) > - return new_fdt_addr; > + efi_enter_kernel(image_addr, fdt_addr); > + /* not reached */ > > +fail_free_initrd: > pr_efi_err("Failed to update FDT and exit boot services\n"); > > efi_free(initrd_size, initrd_addr); > efi_free(fdt_size, fdt_addr); > > fail_free_image: > - efi_free(image_size, *image_addr); > + efi_free(image_size, image_addr); > efi_free(reserve_size, reserve_addr); > fail_free_cmdline: > free_screen_info(si); > efi_free(cmdline_size, (unsigned long)cmdline_ptr); > fail: > - return EFI_ERROR; > + return status; > } > > /* > diff --git a/drivers/firmware/efi/libstub/arm32-stub.c > b/drivers/firmware/efi/libstub/arm32-stub.c > index 7b2a6382b647..113298b8ea65 100644 > --- a/drivers/firmware/efi/libstub/arm32-stub.c > +++ b/drivers/firmware/efi/libstub/arm32-stub.c > @@ -227,6 +227,7 @@ efi_status_t handle_kernel_image(unsigned long > *image_addr, > * Relocate the zImage, so that it appears in the lowest 128 MB > * memory window. > */ > + *image_addr = image->image_base; > *image_size = image->image_size; > status = efi_relocate_kernel(image_addr, *image_size, > *image_size, > kernel_base + > MAX_UNCOMP_KERNEL_SIZE, 0, 0); > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c > b/drivers/firmware/efi/libstub/arm64-stub.c > index 719d03a64329..9254cd8ab2d3 100644 > --- a/drivers/firmware/efi/libstub/arm64-stub.c > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > @@ -43,7 +43,6 @@ efi_status_t handle_kernel_image(unsigned long > *image_addr, > { > efi_status_t status; > unsigned long kernel_size, kernel_memsize = 0; > - void *old_image_addr = (void *)*image_addr; > unsigned long preferred_offset; > u64 phys_seed = 0; > > @@ -141,7 +140,7 @@ efi_status_t handle_kernel_image(unsigned long > *image_addr, > } > *image_addr = *reserve_addr + TEXT_OFFSET; > } > - memcpy((void *)*image_addr, old_image_addr, kernel_size); > + memcpy((void *)*image_addr, image->image_base, kernel_size); > > return EFI_SUCCESS; > }
On Tue, 18 Feb 2020 at 02:53, Atish Patra <Atish.Patra@wdc.com> wrote: > > On Mon, 2020-02-17 at 13:49 +0100, Ard Biesheuvel wrote: > > Expose efi_entry() as the PE/COFF entrypoint directly, instead of > > jumping into a wrapper that fiddles with stack buffers and other > > stuff that the compiler is much better at. The only reason this > > code exists is to obtain a pointer to the base of the image, but > > we can get the same value from the loaded_image protocol, which > > we already need for other reasons anyway. > > > > Update the return type as well, to make it consistent with what > > is required for a PE/COFF executable entrypoint. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > Apologies to Atish for making him track a moving target, but this is > > another > > bit of cleanup work that I would like to merge before taking the > > RISC-V > > changes on top. > > > > And I thought I am done rebasing my patches ;). Jokes apart, this is > actually much cleaner approach in retrieving image base and parsing DT > compared to what I currently have for RISC-V. > > > > On RISC-V, I would expect efi_enter_kernel() to be implemented as a C > > routine > > that finds the hart id in the DT (which is passed as an argument), > > and pass > > it to the startup code of the kernel proper. > > > > Yes. I probably won't need efi-entry.S for RISC-V at all which > simplifies the RISC-V implementation. Yay!! > Glad to hear that. > Is there a tree with this patch ? I tried to find it in your kernel.org > repo but couldn't find it. As there are 3 efi related patch set in > flight, I am not sure about the order in which they should be applied. > > Please rebase onto https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next > > arch/arm/boot/compressed/efi-header.S | 2 +- > > arch/arm/boot/compressed/head.S | 44 +++----------- > > arch/arm64/kernel/efi-entry.S | 64 +++----------------- > > arch/arm64/kernel/efi-header.S | 2 +- > > drivers/firmware/efi/libstub/arm-stub.c | 37 +++++------ > > drivers/firmware/efi/libstub/arm32-stub.c | 1 + > > drivers/firmware/efi/libstub/arm64-stub.c | 3 +- > > 7 files changed, 42 insertions(+), 111 deletions(-) > > > > diff --git a/arch/arm/boot/compressed/efi-header.S > > b/arch/arm/boot/compressed/efi-header.S > > index a5983588f96b..e38fbda02b93 100644 > > --- a/arch/arm/boot/compressed/efi-header.S > > +++ b/arch/arm/boot/compressed/efi-header.S > > @@ -60,7 +60,7 @@ optional_header: > > .long __pecoff_code_size @ SizeOfCode > > .long __pecoff_data_size @ > > SizeOfInitializedData > > .long 0 @ > > SizeOfUninitializedData > > - .long efi_stub_entry - start @ > > AddressOfEntryPoint > > + .long efi_entry - start @ > > AddressOfEntryPoint > > .long start_offset @ BaseOfCode > > .long __pecoff_data_start - start @ BaseOfData > > > > diff --git a/arch/arm/boot/compressed/head.S > > b/arch/arm/boot/compressed/head.S > > index 088b0a060876..941cd28a13c3 100644 > > --- a/arch/arm/boot/compressed/head.S > > +++ b/arch/arm/boot/compressed/head.S > > @@ -1437,50 +1437,22 @@ __enter_kernel: > > reloc_code_end: > > > > #ifdef CONFIG_EFI_STUB > > - .align 2 > > -_start: .long start - . > > - > > -ENTRY(efi_stub_entry) > > - @ allocate space on stack for passing current zImage > > address > > - @ and for the EFI stub to return of new entry point of > > - @ zImage, as EFI stub may copy the kernel. Pointer > > address > > - @ is passed in r2. r0 and r1 are passed through from > > the > > - @ EFI firmware to efi_entry > > - adr ip, _start > > - ldr r3, [ip] > > - add r3, r3, ip > > - stmfd sp!, {r3, lr} > > - mov r2, sp @ pass zImage address > > in r2 > > - bl efi_entry > > - > > - @ Check for error return from EFI stub. r0 has FDT > > address > > - @ or error code. > > - cmn r0, #1 > > - beq efi_load_fail > > - > > - @ Preserve return value of efi_entry() in r4 > > - mov r4, r0 > > +ENTRY(efi_enter_kernel) > > + mov r4, r0 @ preserve entrypoint > > + mov r5, r1 @ preserve DT pointer > > 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 > > + mov r2, r5 > > > > - @ Branch to (possibly) relocated zImage that is in [sp] > > - ldr lr, [sp] > > + @ Branch to (possibly) relocated zImage @ [r4] > > ldr ip, =start_offset > > - add lr, lr, ip > > - mov pc, lr @ no mode > > switch > > - > > -efi_load_fail: > > - @ Return EFI_LOAD_ERROR to EFI firmware on error. > > - ldr r0, =0x80000001 > > - ldmfd sp!, {ip, pc} > > -ENDPROC(efi_stub_entry) > > + add r4, r4, ip > > + mov pc, r4 @ no mode switch > > +ENDPROC(efi_enter_kernel) > > #endif > > > > .align > > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi- > > entry.S > > index 304d5b02ca67..49e3b1ad1b36 100644 > > --- a/arch/arm64/kernel/efi-entry.S > > +++ b/arch/arm64/kernel/efi-entry.S > > @@ -10,55 +10,18 @@ > > > > #include <asm/assembler.h> > > > > -#define EFI_LOAD_ERROR 0x8000000000000001 > > - > > __INIT > > > > - /* > > - * We arrive here from the EFI boot manager with: > > - * > > - * * CPU in little-endian mode > > - * * MMU on with identity-mapped RAM > > - * * Icache and Dcache on > > - * > > - * We will most likely be running from some place other than > > where > > - * we want to be. The kernel image wants to be placed at > > TEXT_OFFSET > > - * from start of RAM. > > - */ > > -ENTRY(entry) > > - /* > > - * Create a stack frame to save FP/LR with extra space > > - * for image_addr variable passed to efi_entry(). > > - */ > > - stp x29, x30, [sp, #-32]! > > - mov x29, sp > > - > > - /* > > - * Call efi_entry to do the real work. > > - * x0 and x1 are already set up by firmware. Current runtime > > - * address of image is calculated and passed via *image_addr. > > - * > > - * unsigned long efi_entry(void *handle, > > - * efi_system_table_t *sys_table, > > - * unsigned long *image_addr) ; > > - */ > > - adr_l x8, _text > > - add x2, sp, 16 > > - str x8, [x2] > > - bl efi_entry > > - cmn x0, #1 > > - b.eq efi_load_fail > > - > > +ENTRY(efi_enter_kernel) > > /* > > * efi_entry() will have copied the kernel image if necessary > > and we > > - * return here with device tree address in x0 and the kernel > > entry > > - * point stored at *image_addr. Save those values in registers > > which > > - * are callee preserved. > > + * end up here with device tree address in x1 and the kernel > > entry > > + * point stored in x0. Save those values in registers which are > > + * callee preserved. > > */ > > - mov x20, x0 // DTB address > > - ldr x0, [sp, #16] // relocated _text address > > + mov x20, x1 // DTB address > > ldr w21, =stext_offset > > - add x21, x0, x21 > > + add x21, x0, x21 // relocated Image address > > > > /* > > * Calculate size of the kernel Image (same for original and > > copy). > > @@ -80,9 +43,8 @@ ENTRY(entry) > > * entries for the VA range of the current image, so no > > maintenance is > > * necessary. > > */ > > - adr x0, entry > > - adr x1, entry_end > > - sub x1, x1, x0 > > + adr x0, efi_enter_kernel > > + mov x1, efi_enter_kernel_size > > bl __flush_dcache_area > > > > /* Turn off Dcache and MMU */ > > @@ -110,11 +72,5 @@ ENTRY(entry) > > mov x2, xzr > > mov x3, xzr > > br x21 > > - > > -efi_load_fail: > > - mov x0, #EFI_LOAD_ERROR > > - ldp x29, x30, [sp], #32 > > - ret > > - > > -entry_end: > > -ENDPROC(entry) > > +ENDPROC(efi_enter_kernel) > > + .set efi_enter_kernel_size, . - efi_enter_kernel > > diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi- > > header.S > > index a7cfacce3e15..40c704c5d3a5 100644 > > --- a/arch/arm64/kernel/efi-header.S > > +++ b/arch/arm64/kernel/efi-header.S > > @@ -27,7 +27,7 @@ optional_header: > > .long __initdata_begin - efi_header_end // SizeOfCode > > .long __pecoff_data_size // > > SizeOfInitializedData > > .long 0 // > > SizeOfUninitializedData > > - .long __efistub_entry - _head // > > AddressOfEntryPoint > > + .long __efistub_efi_entry - _head // > > AddressOfEntryPoint > > .long efi_header_end - _head // BaseOfCode > > > > extra_header_fields: > > diff --git a/drivers/firmware/efi/libstub/arm-stub.c > > b/drivers/firmware/efi/libstub/arm-stub.c > > index 56e475c1aa55..439c094f7bf2 100644 > > --- a/drivers/firmware/efi/libstub/arm-stub.c > > +++ b/drivers/firmware/efi/libstub/arm-stub.c > > @@ -133,17 +133,20 @@ efi_status_t handle_kernel_image(unsigned long > > *image_addr, > > unsigned long *reserve_size, > > unsigned long dram_base, > > efi_loaded_image_t *image); > > + > > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned > > long fdt); > > + > > /* > > * EFI entry point for the arm/arm64 EFI stubs. This is the > > entrypoint > > * that is described in the PE/COFF header. Most of the code is the > > same > > * for both archictectures, with the arch-specific code provided in > > the > > * handle_kernel_image() function. > > */ > > -unsigned long efi_entry(void *handle, efi_system_table_t > > *sys_table_arg, > > - unsigned long *image_addr) > > +efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t > > *sys_table_arg) > > { > > efi_loaded_image_t *image; > > efi_status_t status; > > + unsigned long image_addr; > > unsigned long image_size = 0; > > unsigned long dram_base; > > /* addr/point and size pairs for memory management*/ > > @@ -153,7 +156,6 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > unsigned long fdt_size = 0; > > char *cmdline_ptr = NULL; > > int cmdline_size = 0; > > - unsigned long new_fdt_addr; > > efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID; > > unsigned long reserve_addr = 0; > > unsigned long reserve_size = 0; > > @@ -165,8 +167,10 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > sys_table = sys_table_arg; > > > > /* Check if we were booted by the EFI firmware */ > > - if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) > > + if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) { > > + status = EFI_INVALID_PARAMETER; > > goto fail; > > + } > > > > status = check_platform_features(); > > if (status != EFI_SUCCESS) > > @@ -187,6 +191,7 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > dram_base = get_dram_base(); > > if (dram_base == EFI_ERROR) { > > pr_efi_err("Failed to find DRAM base\n"); > > + status = EFI_LOAD_ERROR; > > goto fail; > > } > > > > @@ -198,6 +203,7 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, > > ULONG_MAX); > > if (!cmdline_ptr) { > > pr_efi_err("getting command line via > > LOADED_IMAGE_PROTOCOL\n"); > > + status = EFI_OUT_OF_RESOURCES; > > goto fail; > > } > > > > @@ -213,7 +219,7 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > > > si = setup_graphics(); > > > > - status = handle_kernel_image(image_addr, &image_size, > > + status = handle_kernel_image(&image_addr, &image_size, > > &reserve_addr, > > &reserve_size, > > dram_base, image); > > @@ -260,7 +266,7 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > pr_efi("Generating empty DTB\n"); > > > > if (!noinitrd()) { > > - max_addr = efi_get_max_initrd_addr(dram_base, > > *image_addr); > > + max_addr = efi_get_max_initrd_addr(dram_base, > > image_addr); > > status = efi_load_initrd_dev_path(&initrd_addr, > > &initrd_size, > > max_addr); > > if (status == EFI_SUCCESS) { > > @@ -311,33 +317,30 @@ unsigned long efi_entry(void *handle, > > efi_system_table_t *sys_table_arg, > > > > install_memreserve_table(); > > > > - new_fdt_addr = fdt_addr; > > status = allocate_new_fdt_and_exit_boot(handle, > > - &new_fdt_addr, > > efi_get_max_fdt_addr(dram_base), > > + &fdt_addr, > > efi_get_max_fdt_addr(dram_base), > > initrd_addr, initrd_size, cmdline_ptr, > > fdt_addr, fdt_size); > > + if (status != EFI_SUCCESS) > > + goto fail_free_initrd; > > > > - /* > > - * If all went well, we need to return the FDT address to the > > - * calling function so it can be passed to kernel as part of > > - * the kernel boot protocol. > > - */ > > - if (status == EFI_SUCCESS) > > - return new_fdt_addr; > > + efi_enter_kernel(image_addr, fdt_addr); > > + /* not reached */ > > > > +fail_free_initrd: > > pr_efi_err("Failed to update FDT and exit boot services\n"); > > > > efi_free(initrd_size, initrd_addr); > > efi_free(fdt_size, fdt_addr); > > > > fail_free_image: > > - efi_free(image_size, *image_addr); > > + efi_free(image_size, image_addr); > > efi_free(reserve_size, reserve_addr); > > fail_free_cmdline: > > free_screen_info(si); > > efi_free(cmdline_size, (unsigned long)cmdline_ptr); > > fail: > > - return EFI_ERROR; > > + return status; > > } > > > > /* > > diff --git a/drivers/firmware/efi/libstub/arm32-stub.c > > b/drivers/firmware/efi/libstub/arm32-stub.c > > index 7b2a6382b647..113298b8ea65 100644 > > --- a/drivers/firmware/efi/libstub/arm32-stub.c > > +++ b/drivers/firmware/efi/libstub/arm32-stub.c > > @@ -227,6 +227,7 @@ efi_status_t handle_kernel_image(unsigned long > > *image_addr, > > * Relocate the zImage, so that it appears in the lowest 128 MB > > * memory window. > > */ > > + *image_addr = image->image_base; > > *image_size = image->image_size; > > status = efi_relocate_kernel(image_addr, *image_size, > > *image_size, > > kernel_base + > > MAX_UNCOMP_KERNEL_SIZE, 0, 0); > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c > > b/drivers/firmware/efi/libstub/arm64-stub.c > > index 719d03a64329..9254cd8ab2d3 100644 > > --- a/drivers/firmware/efi/libstub/arm64-stub.c > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > > @@ -43,7 +43,6 @@ efi_status_t handle_kernel_image(unsigned long > > *image_addr, > > { > > efi_status_t status; > > unsigned long kernel_size, kernel_memsize = 0; > > - void *old_image_addr = (void *)*image_addr; > > unsigned long preferred_offset; > > u64 phys_seed = 0; > > > > @@ -141,7 +140,7 @@ efi_status_t handle_kernel_image(unsigned long > > *image_addr, > > } > > *image_addr = *reserve_addr + TEXT_OFFSET; > > } > > - memcpy((void *)*image_addr, old_image_addr, kernel_size); > > + memcpy((void *)*image_addr, image->image_base, kernel_size); > > > > return EFI_SUCCESS; > > } > > -- > Regards, > Atish
diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S index a5983588f96b..e38fbda02b93 100644 --- a/arch/arm/boot/compressed/efi-header.S +++ b/arch/arm/boot/compressed/efi-header.S @@ -60,7 +60,7 @@ optional_header: .long __pecoff_code_size @ SizeOfCode .long __pecoff_data_size @ SizeOfInitializedData .long 0 @ SizeOfUninitializedData - .long efi_stub_entry - start @ AddressOfEntryPoint + .long efi_entry - start @ AddressOfEntryPoint .long start_offset @ BaseOfCode .long __pecoff_data_start - start @ BaseOfData diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S index 088b0a060876..941cd28a13c3 100644 --- a/arch/arm/boot/compressed/head.S +++ b/arch/arm/boot/compressed/head.S @@ -1437,50 +1437,22 @@ __enter_kernel: reloc_code_end: #ifdef CONFIG_EFI_STUB - .align 2 -_start: .long start - . - -ENTRY(efi_stub_entry) - @ allocate space on stack for passing current zImage address - @ and for the EFI stub to return of new entry point of - @ zImage, as EFI stub may copy the kernel. Pointer address - @ is passed in r2. r0 and r1 are passed through from the - @ EFI firmware to efi_entry - adr ip, _start - ldr r3, [ip] - add r3, r3, ip - stmfd sp!, {r3, lr} - mov r2, sp @ pass zImage address in r2 - bl efi_entry - - @ Check for error return from EFI stub. r0 has FDT address - @ or error code. - cmn r0, #1 - beq efi_load_fail - - @ Preserve return value of efi_entry() in r4 - mov r4, r0 +ENTRY(efi_enter_kernel) + mov r4, r0 @ preserve entrypoint + mov r5, r1 @ preserve DT pointer 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 + mov r2, r5 - @ Branch to (possibly) relocated zImage that is in [sp] - ldr lr, [sp] + @ Branch to (possibly) relocated zImage @ [r4] ldr ip, =start_offset - add lr, lr, ip - mov pc, lr @ no mode switch - -efi_load_fail: - @ Return EFI_LOAD_ERROR to EFI firmware on error. - ldr r0, =0x80000001 - ldmfd sp!, {ip, pc} -ENDPROC(efi_stub_entry) + add r4, r4, ip + mov pc, r4 @ no mode switch +ENDPROC(efi_enter_kernel) #endif .align diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S index 304d5b02ca67..49e3b1ad1b36 100644 --- a/arch/arm64/kernel/efi-entry.S +++ b/arch/arm64/kernel/efi-entry.S @@ -10,55 +10,18 @@ #include <asm/assembler.h> -#define EFI_LOAD_ERROR 0x8000000000000001 - __INIT - /* - * We arrive here from the EFI boot manager with: - * - * * CPU in little-endian mode - * * MMU on with identity-mapped RAM - * * Icache and Dcache on - * - * We will most likely be running from some place other than where - * we want to be. The kernel image wants to be placed at TEXT_OFFSET - * from start of RAM. - */ -ENTRY(entry) - /* - * Create a stack frame to save FP/LR with extra space - * for image_addr variable passed to efi_entry(). - */ - stp x29, x30, [sp, #-32]! - mov x29, sp - - /* - * Call efi_entry to do the real work. - * x0 and x1 are already set up by firmware. Current runtime - * address of image is calculated and passed via *image_addr. - * - * unsigned long efi_entry(void *handle, - * efi_system_table_t *sys_table, - * unsigned long *image_addr) ; - */ - adr_l x8, _text - add x2, sp, 16 - str x8, [x2] - bl efi_entry - cmn x0, #1 - b.eq efi_load_fail - +ENTRY(efi_enter_kernel) /* * efi_entry() will have copied the kernel image if necessary and we - * return here with device tree address in x0 and the kernel entry - * point stored at *image_addr. Save those values in registers which - * are callee preserved. + * end up here with device tree address in x1 and the kernel entry + * point stored in x0. Save those values in registers which are + * callee preserved. */ - mov x20, x0 // DTB address - ldr x0, [sp, #16] // relocated _text address + mov x20, x1 // DTB address ldr w21, =stext_offset - add x21, x0, x21 + add x21, x0, x21 // relocated Image address /* * Calculate size of the kernel Image (same for original and copy). @@ -80,9 +43,8 @@ ENTRY(entry) * entries for the VA range of the current image, so no maintenance is * necessary. */ - adr x0, entry - adr x1, entry_end - sub x1, x1, x0 + adr x0, efi_enter_kernel + mov x1, efi_enter_kernel_size bl __flush_dcache_area /* Turn off Dcache and MMU */ @@ -110,11 +72,5 @@ ENTRY(entry) mov x2, xzr mov x3, xzr br x21 - -efi_load_fail: - mov x0, #EFI_LOAD_ERROR - ldp x29, x30, [sp], #32 - ret - -entry_end: -ENDPROC(entry) +ENDPROC(efi_enter_kernel) + .set efi_enter_kernel_size, . - efi_enter_kernel diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S index a7cfacce3e15..40c704c5d3a5 100644 --- a/arch/arm64/kernel/efi-header.S +++ b/arch/arm64/kernel/efi-header.S @@ -27,7 +27,7 @@ optional_header: .long __initdata_begin - efi_header_end // SizeOfCode .long __pecoff_data_size // SizeOfInitializedData .long 0 // SizeOfUninitializedData - .long __efistub_entry - _head // AddressOfEntryPoint + .long __efistub_efi_entry - _head // AddressOfEntryPoint .long efi_header_end - _head // BaseOfCode extra_header_fields: diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c index 56e475c1aa55..439c094f7bf2 100644 --- a/drivers/firmware/efi/libstub/arm-stub.c +++ b/drivers/firmware/efi/libstub/arm-stub.c @@ -133,17 +133,20 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, unsigned long *reserve_size, unsigned long dram_base, efi_loaded_image_t *image); + +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt); + /* * EFI entry point for the arm/arm64 EFI stubs. This is the entrypoint * that is described in the PE/COFF header. Most of the code is the same * for both archictectures, with the arch-specific code provided in the * handle_kernel_image() function. */ -unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, - unsigned long *image_addr) +efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg) { efi_loaded_image_t *image; efi_status_t status; + unsigned long image_addr; unsigned long image_size = 0; unsigned long dram_base; /* addr/point and size pairs for memory management*/ @@ -153,7 +156,6 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, unsigned long fdt_size = 0; char *cmdline_ptr = NULL; int cmdline_size = 0; - unsigned long new_fdt_addr; efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID; unsigned long reserve_addr = 0; unsigned long reserve_size = 0; @@ -165,8 +167,10 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, sys_table = sys_table_arg; /* Check if we were booted by the EFI firmware */ - if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) + if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) { + status = EFI_INVALID_PARAMETER; goto fail; + } status = check_platform_features(); if (status != EFI_SUCCESS) @@ -187,6 +191,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, dram_base = get_dram_base(); if (dram_base == EFI_ERROR) { pr_efi_err("Failed to find DRAM base\n"); + status = EFI_LOAD_ERROR; goto fail; } @@ -198,6 +203,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, ULONG_MAX); if (!cmdline_ptr) { pr_efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n"); + status = EFI_OUT_OF_RESOURCES; goto fail; } @@ -213,7 +219,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, si = setup_graphics(); - status = handle_kernel_image(image_addr, &image_size, + status = handle_kernel_image(&image_addr, &image_size, &reserve_addr, &reserve_size, dram_base, image); @@ -260,7 +266,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, pr_efi("Generating empty DTB\n"); if (!noinitrd()) { - max_addr = efi_get_max_initrd_addr(dram_base, *image_addr); + max_addr = efi_get_max_initrd_addr(dram_base, image_addr); status = efi_load_initrd_dev_path(&initrd_addr, &initrd_size, max_addr); if (status == EFI_SUCCESS) { @@ -311,33 +317,30 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg, install_memreserve_table(); - new_fdt_addr = fdt_addr; status = allocate_new_fdt_and_exit_boot(handle, - &new_fdt_addr, efi_get_max_fdt_addr(dram_base), + &fdt_addr, efi_get_max_fdt_addr(dram_base), initrd_addr, initrd_size, cmdline_ptr, fdt_addr, fdt_size); + if (status != EFI_SUCCESS) + goto fail_free_initrd; - /* - * If all went well, we need to return the FDT address to the - * calling function so it can be passed to kernel as part of - * the kernel boot protocol. - */ - if (status == EFI_SUCCESS) - return new_fdt_addr; + efi_enter_kernel(image_addr, fdt_addr); + /* not reached */ +fail_free_initrd: pr_efi_err("Failed to update FDT and exit boot services\n"); efi_free(initrd_size, initrd_addr); efi_free(fdt_size, fdt_addr); fail_free_image: - efi_free(image_size, *image_addr); + efi_free(image_size, image_addr); efi_free(reserve_size, reserve_addr); fail_free_cmdline: free_screen_info(si); efi_free(cmdline_size, (unsigned long)cmdline_ptr); fail: - return EFI_ERROR; + return status; } /* diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c index 7b2a6382b647..113298b8ea65 100644 --- a/drivers/firmware/efi/libstub/arm32-stub.c +++ b/drivers/firmware/efi/libstub/arm32-stub.c @@ -227,6 +227,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, * Relocate the zImage, so that it appears in the lowest 128 MB * memory window. */ + *image_addr = image->image_base; *image_size = image->image_size; status = efi_relocate_kernel(image_addr, *image_size, *image_size, kernel_base + MAX_UNCOMP_KERNEL_SIZE, 0, 0); diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c index 719d03a64329..9254cd8ab2d3 100644 --- a/drivers/firmware/efi/libstub/arm64-stub.c +++ b/drivers/firmware/efi/libstub/arm64-stub.c @@ -43,7 +43,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, { efi_status_t status; unsigned long kernel_size, kernel_memsize = 0; - void *old_image_addr = (void *)*image_addr; unsigned long preferred_offset; u64 phys_seed = 0; @@ -141,7 +140,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, } *image_addr = *reserve_addr + TEXT_OFFSET; } - memcpy((void *)*image_addr, old_image_addr, kernel_size); + memcpy((void *)*image_addr, image->image_base, kernel_size); return EFI_SUCCESS; }
Expose efi_entry() as the PE/COFF entrypoint directly, instead of jumping into a wrapper that fiddles with stack buffers and other stuff that the compiler is much better at. The only reason this code exists is to obtain a pointer to the base of the image, but we can get the same value from the loaded_image protocol, which we already need for other reasons anyway. Update the return type as well, to make it consistent with what is required for a PE/COFF executable entrypoint. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- Apologies to Atish for making him track a moving target, but this is another bit of cleanup work that I would like to merge before taking the RISC-V changes on top. On RISC-V, I would expect efi_enter_kernel() to be implemented as a C routine that finds the hart id in the DT (which is passed as an argument), and pass it to the startup code of the kernel proper. arch/arm/boot/compressed/efi-header.S | 2 +- arch/arm/boot/compressed/head.S | 44 +++----------- arch/arm64/kernel/efi-entry.S | 64 +++----------------- arch/arm64/kernel/efi-header.S | 2 +- drivers/firmware/efi/libstub/arm-stub.c | 37 +++++------ drivers/firmware/efi/libstub/arm32-stub.c | 1 + drivers/firmware/efi/libstub/arm64-stub.c | 3 +- 7 files changed, 42 insertions(+), 111 deletions(-)