[U-Boot] [PATCH v3 1/1] efi_loader: refactor efi_setup_loaded_image()

[Alexander Graf]

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 at 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