| Message ID | 20180923115601.11938-1-xypron.glpk@gmx.de |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [U-Boot,v3,1/1] efi_loader: refactor efi_setup_loaded_image() | expand |
On 23.09.18 13:56, Heinrich Schuchardt wrote: > Create the handle of loaded images and the EFI_LOADED_IMAGE_PROTOCOL > inside efi_setup_loaded_image(). Do not use local variables. > > Currently we expect the loaded image handle to point to the loaded image > protocol. Additionally we have appended private fields to the protocol. > > With the patch the handle points to a loaded image object and the private > fields are added here. This matches how we handle the net and the gop > object. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > v3 > squashed two patches > https://lists.denx.de/pipermail/u-boot/2018-September/341130.html > https://lists.denx.de/pipermail/u-boot/2018-September/341589.html > v2 > avoid unused variables if configured without CONFIG_EFI_SELFTEST > --- > cmd/bootefi.c | 61 +++++++++++--------- > include/efi_api.h | 8 --- > include/efi_loader.h | 27 ++++++--- > lib/efi_loader/efi_boottime.c | 92 +++++++++++++++---------------- > lib/efi_loader/efi_image_loader.c | 23 ++++---- > 5 files changed, 111 insertions(+), 100 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 8bc6fa49f5..f76f29a7b1 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -325,19 +325,26 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) > return ret; > } > > -/* > - * Load an EFI payload into a newly allocated piece of memory, register all > - * EFI objects it would want to access and jump to it. > +/** > + * do_bootefi_exec() - execute EFI binary > + * > + * @efi: address of the binary > + * @device_path: path of the device from which the binary was loaded > + * @image_path: device path of the binary > + * Return: status code > + * > + * Load the EFI binary into a newly assigned memory unwinding the relocation > + * information, install the loaded image protocol, and call the binary. > */ > static efi_status_t do_bootefi_exec(void *efi, > struct efi_device_path *device_path, > struct efi_device_path *image_path) > { > - struct efi_loaded_image loaded_image_info = {}; > - struct efi_object loaded_image_info_obj = {}; > efi_handle_t mem_handle = NULL; > struct efi_device_path *memdp = NULL; > efi_status_t ret; > + struct efi_loaded_image_obj *image_handle; > + struct efi_loaded_image *loaded_image_info; Better initialize those to NULL so we don't have to remember to do that in efi_setup_loaded_image(). > > EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, > struct efi_system_table *st); > @@ -367,8 +374,8 @@ static efi_status_t do_bootefi_exec(void *efi, > assert(device_path && image_path); > } > > - efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, > - device_path, image_path); > + efi_setup_loaded_image(device_path, image_path, &image_handle, > + &loaded_image_info); > > /* > * gd lives in a fixed register which may get clobbered while we execute > @@ -377,9 +384,9 @@ static efi_status_t do_bootefi_exec(void *efi, > efi_save_gd(); > > /* Transfer environment variable bootargs as load options */ > - set_load_options(&loaded_image_info, "bootargs"); > + set_load_options(loaded_image_info, "bootargs"); > /* Load the EFI payload */ > - entry = efi_load_pe(efi, &loaded_image_info); > + entry = efi_load_pe(image_handle, efi, loaded_image_info); > if (!entry) { > ret = EFI_LOAD_ERROR; > goto exit; > @@ -387,10 +394,10 @@ static efi_status_t do_bootefi_exec(void *efi, > > if (memdp) { > struct efi_device_path_memory *mdp = (void *)memdp; > - mdp->memory_type = loaded_image_info.image_code_type; > - mdp->start_address = (uintptr_t)loaded_image_info.image_base; > + mdp->memory_type = loaded_image_info->image_code_type; > + mdp->start_address = (uintptr_t)loaded_image_info->image_base; > mdp->end_address = mdp->start_address + > - loaded_image_info.image_size; > + loaded_image_info->image_size; > } > > /* we don't support much: */ > @@ -400,8 +407,8 @@ static efi_status_t do_bootefi_exec(void *efi, > /* Call our payload! */ > debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry); > > - if (setjmp(&loaded_image_info.exit_jmp)) { > - ret = loaded_image_info.exit_status; > + if (setjmp(&image_handle->exit_jmp)) { > + ret = image_handle->exit_status; > goto exit; > } > > @@ -413,7 +420,7 @@ static efi_status_t do_bootefi_exec(void *efi, > > /* Move into EL2 and keep running there */ > armv8_switch_to_el2((ulong)entry, > - (ulong)&loaded_image_info_obj.handle, > + (ulong)&image_handle, This needs to become only "image_handle" now, no? Alex
On 09/23/2018 02:34 PM, Alexander Graf wrote: > > > On 23.09.18 13:56, Heinrich Schuchardt wrote: >> Create the handle of loaded images and the EFI_LOADED_IMAGE_PROTOCOL >> inside efi_setup_loaded_image(). Do not use local variables. >> >> Currently we expect the loaded image handle to point to the loaded image >> protocol. Additionally we have appended private fields to the protocol. >> >> With the patch the handle points to a loaded image object and the private >> fields are added here. This matches how we handle the net and the gop >> object. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> v3 >> squashed two patches >> https://lists.denx.de/pipermail/u-boot/2018-September/341130.html >> https://lists.denx.de/pipermail/u-boot/2018-September/341589.html >> v2 >> avoid unused variables if configured without CONFIG_EFI_SELFTEST >> --- >> cmd/bootefi.c | 61 +++++++++++--------- >> include/efi_api.h | 8 --- >> include/efi_loader.h | 27 ++++++--- >> lib/efi_loader/efi_boottime.c | 92 +++++++++++++++---------------- >> lib/efi_loader/efi_image_loader.c | 23 ++++---- >> 5 files changed, 111 insertions(+), 100 deletions(-) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index 8bc6fa49f5..f76f29a7b1 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -325,19 +325,26 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) >> return ret; >> } >> >> -/* >> - * Load an EFI payload into a newly allocated piece of memory, register all >> - * EFI objects it would want to access and jump to it. >> +/** >> + * do_bootefi_exec() - execute EFI binary >> + * >> + * @efi: address of the binary >> + * @device_path: path of the device from which the binary was loaded >> + * @image_path: device path of the binary >> + * Return: status code >> + * >> + * Load the EFI binary into a newly assigned memory unwinding the relocation >> + * information, install the loaded image protocol, and call the binary. >> */ >> static efi_status_t do_bootefi_exec(void *efi, >> struct efi_device_path *device_path, >> struct efi_device_path *image_path) >> { >> - struct efi_loaded_image loaded_image_info = {}; >> - struct efi_object loaded_image_info_obj = {}; >> efi_handle_t mem_handle = NULL; >> struct efi_device_path *memdp = NULL; >> efi_status_t ret; >> + struct efi_loaded_image_obj *image_handle; >> + struct efi_loaded_image *loaded_image_info; > > Better initialize those to NULL so we don't have to remember to do that > in efi_setup_loaded_image(). > >> >> EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, >> struct efi_system_table *st); >> @@ -367,8 +374,8 @@ static efi_status_t do_bootefi_exec(void *efi, >> assert(device_path && image_path); >> } >> >> - efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, >> - device_path, image_path); >> + efi_setup_loaded_image(device_path, image_path, &image_handle, >> + &loaded_image_info); >> >> /* >> * gd lives in a fixed register which may get clobbered while we execute >> @@ -377,9 +384,9 @@ static efi_status_t do_bootefi_exec(void *efi, >> efi_save_gd(); >> >> /* Transfer environment variable bootargs as load options */ >> - set_load_options(&loaded_image_info, "bootargs"); >> + set_load_options(loaded_image_info, "bootargs"); >> /* Load the EFI payload */ >> - entry = efi_load_pe(efi, &loaded_image_info); >> + entry = efi_load_pe(image_handle, efi, loaded_image_info); >> if (!entry) { >> ret = EFI_LOAD_ERROR; >> goto exit; >> @@ -387,10 +394,10 @@ static efi_status_t do_bootefi_exec(void *efi, >> >> if (memdp) { >> struct efi_device_path_memory *mdp = (void *)memdp; >> - mdp->memory_type = loaded_image_info.image_code_type; >> - mdp->start_address = (uintptr_t)loaded_image_info.image_base; >> + mdp->memory_type = loaded_image_info->image_code_type; >> + mdp->start_address = (uintptr_t)loaded_image_info->image_base; >> mdp->end_address = mdp->start_address + >> - loaded_image_info.image_size; >> + loaded_image_info->image_size; >> } >> >> /* we don't support much: */ >> @@ -400,8 +407,8 @@ static efi_status_t do_bootefi_exec(void *efi, >> /* Call our payload! */ >> debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry); >> >> - if (setjmp(&loaded_image_info.exit_jmp)) { >> - ret = loaded_image_info.exit_status; >> + if (setjmp(&image_handle->exit_jmp)) { >> + ret = image_handle->exit_status; >> goto exit; >> } >> >> @@ -413,7 +420,7 @@ static efi_status_t do_bootefi_exec(void *efi, >> >> /* Move into EL2 and keep running there */ >> armv8_switch_to_el2((ulong)entry, >> - (ulong)&loaded_image_info_obj.handle, >> + (ulong)&image_handle, > > This needs to become only "image_handle" now, no? &loaded_image_info_obj.handle wass of type (void **), i.e. a pointer to a pointer to a anonymous structure. &image_handle is of type (struct efi_loaded_image_obj **), i.e also a pointer to a pointer to a structure Regards Heinrich
On 23.09.18 15:19, Heinrich Schuchardt wrote: > On 09/23/2018 02:34 PM, Alexander Graf wrote: >> >> >> On 23.09.18 13:56, Heinrich Schuchardt wrote: >>> Create the handle of loaded images and the EFI_LOADED_IMAGE_PROTOCOL >>> inside efi_setup_loaded_image(). Do not use local variables. >>> >>> Currently we expect the loaded image handle to point to the loaded image >>> protocol. Additionally we have appended private fields to the protocol. >>> >>> With the patch the handle points to a loaded image object and the private >>> fields are added here. This matches how we handle the net and the gop >>> object. >>> >>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >>> --- >>> v3 >>> squashed two patches >>> https://lists.denx.de/pipermail/u-boot/2018-September/341130.html >>> https://lists.denx.de/pipermail/u-boot/2018-September/341589.html >>> v2 >>> avoid unused variables if configured without CONFIG_EFI_SELFTEST >>> --- >>> cmd/bootefi.c | 61 +++++++++++--------- >>> include/efi_api.h | 8 --- >>> include/efi_loader.h | 27 ++++++--- >>> lib/efi_loader/efi_boottime.c | 92 +++++++++++++++---------------- >>> lib/efi_loader/efi_image_loader.c | 23 ++++---- >>> 5 files changed, 111 insertions(+), 100 deletions(-) >>> >>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >>> index 8bc6fa49f5..f76f29a7b1 100644 >>> --- a/cmd/bootefi.c >>> +++ b/cmd/bootefi.c >>> @@ -325,19 +325,26 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) >>> return ret; >>> } >>> >>> -/* >>> - * Load an EFI payload into a newly allocated piece of memory, register all >>> - * EFI objects it would want to access and jump to it. >>> +/** >>> + * do_bootefi_exec() - execute EFI binary >>> + * >>> + * @efi: address of the binary >>> + * @device_path: path of the device from which the binary was loaded >>> + * @image_path: device path of the binary >>> + * Return: status code >>> + * >>> + * Load the EFI binary into a newly assigned memory unwinding the relocation >>> + * information, install the loaded image protocol, and call the binary. >>> */ >>> static efi_status_t do_bootefi_exec(void *efi, >>> struct efi_device_path *device_path, >>> struct efi_device_path *image_path) >>> { >>> - struct efi_loaded_image loaded_image_info = {}; >>> - struct efi_object loaded_image_info_obj = {}; >>> efi_handle_t mem_handle = NULL; >>> struct efi_device_path *memdp = NULL; >>> efi_status_t ret; >>> + struct efi_loaded_image_obj *image_handle; >>> + struct efi_loaded_image *loaded_image_info; >> >> Better initialize those to NULL so we don't have to remember to do that >> in efi_setup_loaded_image(). No answer here? >> >>> >>> EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, >>> struct efi_system_table *st); >>> @@ -367,8 +374,8 @@ static efi_status_t do_bootefi_exec(void *efi, >>> assert(device_path && image_path); >>> } >>> >>> - efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, >>> - device_path, image_path); >>> + efi_setup_loaded_image(device_path, image_path, &image_handle, >>> + &loaded_image_info); >>> >>> /* >>> * gd lives in a fixed register which may get clobbered while we execute >>> @@ -377,9 +384,9 @@ static efi_status_t do_bootefi_exec(void *efi, >>> efi_save_gd(); >>> >>> /* Transfer environment variable bootargs as load options */ >>> - set_load_options(&loaded_image_info, "bootargs"); >>> + set_load_options(loaded_image_info, "bootargs"); >>> /* Load the EFI payload */ >>> - entry = efi_load_pe(efi, &loaded_image_info); >>> + entry = efi_load_pe(image_handle, efi, loaded_image_info); >>> if (!entry) { >>> ret = EFI_LOAD_ERROR; >>> goto exit; >>> @@ -387,10 +394,10 @@ static efi_status_t do_bootefi_exec(void *efi, >>> >>> if (memdp) { >>> struct efi_device_path_memory *mdp = (void *)memdp; >>> - mdp->memory_type = loaded_image_info.image_code_type; >>> - mdp->start_address = (uintptr_t)loaded_image_info.image_base; >>> + mdp->memory_type = loaded_image_info->image_code_type; >>> + mdp->start_address = (uintptr_t)loaded_image_info->image_base; >>> mdp->end_address = mdp->start_address + >>> - loaded_image_info.image_size; >>> + loaded_image_info->image_size; >>> } >>> >>> /* we don't support much: */ >>> @@ -400,8 +407,8 @@ static efi_status_t do_bootefi_exec(void *efi, >>> /* Call our payload! */ >>> debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry); >>> >>> - if (setjmp(&loaded_image_info.exit_jmp)) { >>> - ret = loaded_image_info.exit_status; >>> + if (setjmp(&image_handle->exit_jmp)) { >>> + ret = image_handle->exit_status; >>> goto exit; >>> } >>> >>> @@ -413,7 +420,7 @@ static efi_status_t do_bootefi_exec(void *efi, >>> >>> /* Move into EL2 and keep running there */ >>> armv8_switch_to_el2((ulong)entry, >>> - (ulong)&loaded_image_info_obj.handle, >>> + (ulong)&image_handle, >> >> This needs to become only "image_handle" now, no? > > &loaded_image_info_obj.handle wass of type (void **), i.e. a pointer to > a pointer to a anonymous structure. > > &image_handle is of type (struct efi_loaded_image_obj **), i.e also a > pointer to a pointer to a structure The arguments to entry(...) have to be the same across all 3 calling mechanisms: drop-to-el2, drop-to-hyp, call-native. Currently 2 out of 3 pass "image_handle", one passes "&image_handle". Alex
On 09/23/2018 02:34 PM, Alexander Graf wrote: >> + * Load the EFI binary into a newly assigned memory unwinding the relocation >> + * information, install the loaded image protocol, and call the binary. >> */ >> static efi_status_t do_bootefi_exec(void *efi, >> struct efi_device_path *device_path, >> struct efi_device_path *image_path) >> { >> - struct efi_loaded_image loaded_image_info = {}; >> - struct efi_object loaded_image_info_obj = {}; >> efi_handle_t mem_handle = NULL; >> struct efi_device_path *memdp = NULL; >> efi_status_t ret; >> + struct efi_loaded_image_obj *image_handle; >> + struct efi_loaded_image *loaded_image_info; > Better initialize those to NULL so we don't have to remember to do that > in efi_setup_loaded_image(). > What is missing is checking the return code of efi_setup_loaded_image. I will fix that. Regards Heinrich >> >> EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, >> struct efi_system_table *st); >> @@ -367,8 +374,8 @@ static efi_status_t do_bootefi_exec(void *efi, >> assert(device_path && image_path); >> } >> >> - efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, >> - device_path, image_path); >> + efi_setup_loaded_image(device_path, image_path, &image_handle, >> + &loaded_image_info); >> >> /*
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 8bc6fa49f5..f76f29a7b1 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -325,19 +325,26 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) return ret; } -/* - * Load an EFI payload into a newly allocated piece of memory, register all - * EFI objects it would want to access and jump to it. +/** + * do_bootefi_exec() - execute EFI binary + * + * @efi: address of the binary + * @device_path: path of the device from which the binary was loaded + * @image_path: device path of the binary + * Return: status code + * + * Load the EFI binary into a newly assigned memory unwinding the relocation + * information, install the loaded image protocol, and call the binary. */ static efi_status_t do_bootefi_exec(void *efi, struct efi_device_path *device_path, struct efi_device_path *image_path) { - struct efi_loaded_image loaded_image_info = {}; - struct efi_object loaded_image_info_obj = {}; efi_handle_t mem_handle = NULL; struct efi_device_path *memdp = NULL; efi_status_t ret; + struct efi_loaded_image_obj *image_handle; + struct efi_loaded_image *loaded_image_info; EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, struct efi_system_table *st); @@ -367,8 +374,8 @@ static efi_status_t do_bootefi_exec(void *efi, assert(device_path && image_path); } - efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, - device_path, image_path); + efi_setup_loaded_image(device_path, image_path, &image_handle, + &loaded_image_info); /* * gd lives in a fixed register which may get clobbered while we execute @@ -377,9 +384,9 @@ static efi_status_t do_bootefi_exec(void *efi, efi_save_gd(); /* Transfer environment variable bootargs as load options */ - set_load_options(&loaded_image_info, "bootargs"); + set_load_options(loaded_image_info, "bootargs"); /* Load the EFI payload */ - entry = efi_load_pe(efi, &loaded_image_info); + entry = efi_load_pe(image_handle, efi, loaded_image_info); if (!entry) { ret = EFI_LOAD_ERROR; goto exit; @@ -387,10 +394,10 @@ static efi_status_t do_bootefi_exec(void *efi, if (memdp) { struct efi_device_path_memory *mdp = (void *)memdp; - mdp->memory_type = loaded_image_info.image_code_type; - mdp->start_address = (uintptr_t)loaded_image_info.image_base; + mdp->memory_type = loaded_image_info->image_code_type; + mdp->start_address = (uintptr_t)loaded_image_info->image_base; mdp->end_address = mdp->start_address + - loaded_image_info.image_size; + loaded_image_info->image_size; } /* we don't support much: */ @@ -400,8 +407,8 @@ static efi_status_t do_bootefi_exec(void *efi, /* Call our payload! */ debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry); - if (setjmp(&loaded_image_info.exit_jmp)) { - ret = loaded_image_info.exit_status; + if (setjmp(&image_handle->exit_jmp)) { + ret = image_handle->exit_status; goto exit; } @@ -413,7 +420,7 @@ static efi_status_t do_bootefi_exec(void *efi, /* Move into EL2 and keep running there */ armv8_switch_to_el2((ulong)entry, - (ulong)&loaded_image_info_obj.handle, + (ulong)&image_handle, (ulong)&systab, 0, (ulong)efi_run_in_el2, ES_TO_AARCH64); @@ -430,7 +437,7 @@ static efi_status_t do_bootefi_exec(void *efi, secure_ram_addr(_do_nonsec_entry)( efi_run_in_hyp, (uintptr_t)entry, - (uintptr_t)loaded_image_info_obj.handle, + (uintptr_t)image_handle, (uintptr_t)&systab); /* Should never reach here, efi exits with longjmp */ @@ -438,11 +445,12 @@ static efi_status_t do_bootefi_exec(void *efi, } #endif - ret = efi_do_enter(loaded_image_info_obj.handle, &systab, entry); + ret = efi_do_enter(image_handle, &systab, entry); exit: /* image has returned, loaded-image obj goes *poof*: */ - list_del(&loaded_image_info_obj.link); + if (image_handle) + efi_delete_handle(&image_handle->parent); if (mem_handle) efi_delete_handle(mem_handle); @@ -527,8 +535,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #endif #ifdef CONFIG_CMD_BOOTEFI_SELFTEST if (!strcmp(argv[1], "selftest")) { - struct efi_loaded_image loaded_image_info = {}; - struct efi_object loaded_image_info_obj = {}; + struct efi_loaded_image_obj *image_handle; + struct efi_loaded_image *loaded_image_info; /* Construct a dummy device path. */ bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, @@ -536,9 +544,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) (uintptr_t)&efi_selftest); bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest"); - efi_setup_loaded_image(&loaded_image_info, - &loaded_image_info_obj, - bootefi_device_path, bootefi_image_path); + efi_setup_loaded_image(bootefi_device_path, bootefi_image_path, + &image_handle, &loaded_image_info); /* * gd lives in a fixed register which may get clobbered while we * execute the payload. So save it here and restore it on every @@ -546,12 +553,12 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) */ efi_save_gd(); /* Transfer environment variable efi_selftest as load options */ - set_load_options(&loaded_image_info, "efi_selftest"); + set_load_options(loaded_image_info, "efi_selftest"); /* Execute the test */ - r = efi_selftest(loaded_image_info_obj.handle, &systab); + r = efi_selftest(image_handle, &systab); efi_restore_gd(); - free(loaded_image_info.load_options); - list_del(&loaded_image_info_obj.link); + free(loaded_image_info->load_options); + efi_delete_handle(&image_handle->parent); return r != EFI_SUCCESS; } else #endif diff --git a/include/efi_api.h b/include/efi_api.h index 5004f520ff..c42df6900a 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -340,14 +340,6 @@ struct efi_loaded_image { unsigned int image_code_type; unsigned int image_data_type; unsigned long unload; - - /* Below are efi loader private fields */ -#ifdef CONFIG_EFI_LOADER - void *reloc_base; - aligned_u64 reloc_size; - efi_status_t exit_status; - struct jmp_buf_data exit_jmp; -#endif }; #define DEVICE_PATH_GUID \ diff --git a/include/efi_loader.h b/include/efi_loader.h index 7738391aa2..74df070316 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -189,6 +189,20 @@ struct efi_object { void *handle; }; +/** + * struct efi_loaded_image_obj - handle of a loaded image + */ +struct efi_loaded_image_obj { + /* Generic EFI object parent class data */ + struct efi_object parent; + void *reloc_base; + aligned_u64 reloc_size; + efi_status_t exit_status; + struct jmp_buf_data exit_jmp; + EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, + struct efi_system_table *st); +}; + /** * struct efi_event * @@ -270,7 +284,8 @@ efi_status_t efi_set_watchdog(unsigned long timeout); /* Called from places to check whether a timer expired */ void efi_timer_check(void); /* PE loader implementation */ -void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info); +void *efi_load_pe(struct efi_loaded_image_obj *handle, void *efi, + struct efi_loaded_image *loaded_image_info); /* Called once to store the pristine gd pointer */ void efi_save_gd(void); /* Special case handler for error/abort that just tries to dtrt to get @@ -351,14 +366,12 @@ int efi_memory_init(void); /* Adds new or overrides configuration table entry to the system table */ efi_status_t efi_install_configuration_table(const efi_guid_t *guid, void *table); /* Sets up a loaded image */ -efi_status_t efi_setup_loaded_image( - struct efi_loaded_image *info, struct efi_object *obj, - struct efi_device_path *device_path, - struct efi_device_path *file_path); +efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, + struct efi_device_path *file_path, + struct efi_loaded_image_obj **handle_ptr, + struct efi_loaded_image **info_ptr); efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, void **buffer); -/* Print information about a loaded image */ -efi_status_t efi_print_image_info(struct efi_loaded_image *image, void *pc); /* Print information about all loaded images */ void efi_print_image_infos(void *pc); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b4de9961b8..2496608981 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1478,17 +1478,31 @@ static efi_status_t EFIAPI efi_install_configuration_table_ext(efi_guid_t *guid, * * Return: status code */ -efi_status_t efi_setup_loaded_image( - struct efi_loaded_image *info, struct efi_object *obj, - struct efi_device_path *device_path, - struct efi_device_path *file_path) +efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, + struct efi_device_path *file_path, + struct efi_loaded_image_obj **handle_ptr, + struct efi_loaded_image **info_ptr) { efi_status_t ret; + struct efi_loaded_image *info; + struct efi_loaded_image_obj *obj; + + info = calloc(1, sizeof(*info)); + if (!info) + return EFI_OUT_OF_RESOURCES; + obj = calloc(1, sizeof(*obj)); + if (!obj) { + free(info); + return EFI_OUT_OF_RESOURCES; + } /* Add internal object to object list */ - efi_add_handle(obj); - /* efi_exit() assumes that the handle points to the info */ - obj->handle = info; + efi_add_handle(&obj->parent); + + if (info_ptr) + *info_ptr = info; + if (handle_ptr) + *handle_ptr = obj; info->revision = EFI_LOADED_IMAGE_PROTOCOL_REVISION; info->file_path = file_path; @@ -1500,8 +1514,8 @@ efi_status_t efi_setup_loaded_image( * When asking for the device path interface, return * bootefi_device_path */ - ret = efi_add_protocol(obj->handle, &efi_guid_device_path, - device_path); + ret = efi_add_protocol(obj->parent.handle, + &efi_guid_device_path, device_path); if (ret != EFI_SUCCESS) goto failure; } @@ -1510,7 +1524,8 @@ efi_status_t efi_setup_loaded_image( * When asking for the loaded_image interface, just * return handle which points to loaded_image_info */ - ret = efi_add_protocol(obj->handle, &efi_guid_loaded_image, info); + ret = efi_add_protocol(obj->parent.handle, + &efi_guid_loaded_image, info); if (ret != EFI_SUCCESS) goto failure; @@ -1593,7 +1608,8 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, efi_handle_t *image_handle) { struct efi_loaded_image *info; - struct efi_object *obj; + struct efi_loaded_image_obj **image_obj = + (struct efi_loaded_image_obj **)image_handle; efi_status_t ret; EFI_ENTRY("%d, %p, %pD, %p, %zd, %p", boot_policy, parent_image, @@ -1609,18 +1625,6 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, goto error; } - info = calloc(1, sizeof(*info)); - if (!info) { - ret = EFI_OUT_OF_RESOURCES; - goto error; - } - obj = calloc(1, sizeof(*obj)); - if (!obj) { - free(info); - ret = EFI_OUT_OF_RESOURCES; - goto error; - } - if (!source_buffer) { struct efi_device_path *dp, *fp; @@ -1632,29 +1636,29 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, * file parts: */ efi_dp_split_file_path(file_path, &dp, &fp); - ret = efi_setup_loaded_image(info, obj, dp, fp); + ret = efi_setup_loaded_image(dp, fp, image_obj, &info); if (ret != EFI_SUCCESS) goto failure; } else { /* In this case, file_path is the "device" path, i.e. * something like a HARDWARE_DEVICE:MEMORY_MAPPED */ - ret = efi_setup_loaded_image(info, obj, file_path, NULL); + ret = efi_setup_loaded_image(file_path, NULL, image_obj, &info); if (ret != EFI_SUCCESS) - goto failure; + goto error; } - info->reserved = efi_load_pe(source_buffer, info); - if (!info->reserved) { + (*image_obj)->entry = efi_load_pe(*image_obj, source_buffer, info); + if (!(*image_obj)->entry) { ret = EFI_UNSUPPORTED; goto failure; } info->system_table = &systab; info->parent_handle = parent_image; - *image_handle = obj->handle; return EFI_EXIT(EFI_SUCCESS); failure: + efi_delete_handle(*image_handle); + *image_handle = NULL; free(info); - efi_delete_handle(obj); error: return EFI_EXIT(ret); } @@ -1676,18 +1680,16 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, unsigned long *exit_data_size, s16 **exit_data) { - EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, - struct efi_system_table *st); - struct efi_loaded_image *info = image_handle; + struct efi_loaded_image_obj *image_obj = + (struct efi_loaded_image_obj *)image_handle; efi_status_t ret; EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); - entry = info->reserved; efi_is_direct_boot = false; /* call the image! */ - if (setjmp(&info->exit_jmp)) { + if (setjmp(&image_obj->exit_jmp)) { /* * We called the entry point of the child image with EFI_CALL * in the lines below. The child image called the Exit() boot @@ -1710,12 +1712,12 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, assert(__efi_entry_check()); debug("%sEFI: %lu returned by started image\n", __efi_nesting_dec(), - (unsigned long)((uintptr_t)info->exit_status & + (unsigned long)((uintptr_t)image_obj->exit_status & ~EFI_ERROR_MASK)); - return EFI_EXIT(info->exit_status); + return EFI_EXIT(image_obj->exit_status); } - ret = EFI_CALL(entry(image_handle, &systab)); + ret = EFI_CALL(image_obj->entry(image_handle, &systab)); /* * Usually UEFI applications call Exit() instead of returning. @@ -1746,17 +1748,11 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, int16_t *exit_data) { /* - * We require that the handle points to the original loaded - * image protocol interface. - * - * For getting the longjmp address this is safer than locating - * the protocol because the protocol may have been reinstalled - * pointing to another memory location. - * * TODO: We should call the unload procedure of the loaded * image protocol. */ - struct efi_loaded_image *loaded_image_info = (void *)image_handle; + struct efi_loaded_image_obj *image_obj = + (struct efi_loaded_image_obj *)image_handle; EFI_ENTRY("%p, %ld, %ld, %p", image_handle, exit_status, exit_data_size, exit_data); @@ -1770,8 +1766,8 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, */ efi_restore_gd(); - loaded_image_info->exit_status = exit_status; - longjmp(&loaded_image_info->exit_jmp, 1); + image_obj->exit_status = exit_status; + longjmp(&image_obj->exit_jmp, 1); panic("EFI application exited"); } diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index fdf40a62c8..a18ce0a570 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -48,20 +48,21 @@ static int machines[] = { * If the program counter is located within the image the offset to the base * address is shown. * + * @obj: EFI object * @image: loaded image * @pc: program counter (use NULL to suppress offset output) * @return: status code */ -efi_status_t efi_print_image_info(struct efi_loaded_image *image, void *pc) +static efi_status_t efi_print_image_info(struct efi_loaded_image_obj *obj, + struct efi_loaded_image *image, + void *pc) { - if (!image) - return EFI_INVALID_PARAMETER; printf("UEFI image"); printf(" [0x%p:0x%p]", - image->reloc_base, image->reloc_base + image->reloc_size - 1); - if (pc && pc >= image->reloc_base && - pc < image->reloc_base + image->reloc_size) - printf(" pc=0x%zx", pc - image->reloc_base); + obj->reloc_base, obj->reloc_base + obj->reloc_size - 1); + if (pc && pc >= obj->reloc_base && + pc < obj->reloc_base + obj->reloc_size) + printf(" pc=0x%zx", pc - obj->reloc_base); if (image->file_path) printf(" '%pD'", image->file_path); printf("\n"); @@ -82,6 +83,7 @@ void efi_print_image_infos(void *pc) list_for_each_entry(handler, &efiobj->protocols, link) { if (!guidcmp(handler->guid, &efi_guid_loaded_image)) { efi_print_image_info( + (struct efi_loaded_image_obj *)efiobj, handler->protocol_interface, pc); } } @@ -196,7 +198,8 @@ static void efi_set_code_and_data_type( * piece of memory. On successful load it then returns the entry point for * the binary. Otherwise NULL. */ -void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) +void *efi_load_pe(struct efi_loaded_image_obj *handle, void *efi, + struct efi_loaded_image *loaded_image_info) { IMAGE_NT_HEADERS32 *nt; IMAGE_DOS_HEADER *dos; @@ -314,8 +317,8 @@ void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info) /* Populate the loaded image interface bits */ loaded_image_info->image_base = efi; loaded_image_info->image_size = image_size; - loaded_image_info->reloc_base = efi_reloc; - loaded_image_info->reloc_size = virt_size; + handle->reloc_base = efi_reloc; + handle->reloc_size = virt_size; return entry; }
Create the handle of loaded images and the EFI_LOADED_IMAGE_PROTOCOL inside efi_setup_loaded_image(). Do not use local variables. Currently we expect the loaded image handle to point to the loaded image protocol. Additionally we have appended private fields to the protocol. With the patch the handle points to a loaded image object and the private fields are added here. This matches how we handle the net and the gop object. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- v3 squashed two patches https://lists.denx.de/pipermail/u-boot/2018-September/341130.html https://lists.denx.de/pipermail/u-boot/2018-September/341589.html v2 avoid unused variables if configured without CONFIG_EFI_SELFTEST --- cmd/bootefi.c | 61 +++++++++++--------- include/efi_api.h | 8 --- include/efi_loader.h | 27 ++++++--- lib/efi_loader/efi_boottime.c | 92 +++++++++++++++---------------- lib/efi_loader/efi_image_loader.c | 23 ++++---- 5 files changed, 111 insertions(+), 100 deletions(-)