[U-Boot] [PATCH 1/1] bootefi: allow return without EFI_BOOT_SERVICES.Exit

[Alexander Graf]

On 06/24/2017 05:39 PM, Heinrich Schuchardt wrote:
> The Unified Extensible Firmware Interface Specification, version 2.7,
> defines in chapter 2.1.2 - UEFI Application that an EFI application may
> either directly return or call EFI_BOOT_SERVICES.Exit().
>
> Unfortunately U-Boot makes the incorrect assumption that
> EFI_BOOT_SERVICES.Exit() is always called.
>
> So the following application leads to a memory exception on the aarch64
> architecture when returning:
>
> EFI_STATUS efi_main(
>    EFI_HANDLE handle,
>    EFI_SYSTEM_TABlE systable) {
> 	return EFI_SUCCESS;
> }
>
> With this patch the entry point is stored in the image handle.
>
> The new wrapper function do_enter is used to call the EFI entry point.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>   cmd/bootefi.c     | 18 ++++++++++++++++--
>   include/efi_api.h |  4 ++++
>   2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index a0a5434967..fcb223e999 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -9,6 +9,7 @@
>   #include <common.h>
>   #include <command.h>
>   #include <dm.h>
> +#include <efi_api.h>
>   #include <efi_loader.h>
>   #include <errno.h>
>   #include <libfdt.h>
> @@ -141,6 +142,18 @@ static void *copy_fdt(void *fdt)
>   	return new_fdt;
>   }
>   
> +static asmlinkage ulong do_enter(void *image_handle,

Please call it efi_do_enter instead :). Otherwise we might get into 
naming conflicts sooner or later.

> +				 struct efi_system_table *st)
> +{
> +	struct efi_loaded_image *handle = image_handle;
> +	efi_status_t ret = EFI_LOAD_ERROR;
> +
> +	if (handle->entry)
> +		ret = handle->entry(image_handle, st);
> +	st->boottime->exit(image_handle, ret, 0 , NULL);
> +	return ret;
> +}
> +
>   #ifdef CONFIG_ARM64
>   static unsigned long efi_run_in_el2(ulong (*entry)(void *image_handle,
>   		struct efi_system_table *st), void *image_handle,
> @@ -149,7 +162,7 @@ static unsigned long efi_run_in_el2(ulong (*entry)(void *image_handle,
>   	/* Enable caches again */
>   	dcache_enable();
>   
> -	return entry(image_handle, st);
> +	return do_enter(image_handle, st);
>   }
>   #endif
>   
> @@ -237,6 +250,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>   		efi_status_t status = loaded_image_info.exit_status;
>   		return status == EFI_SUCCESS ? 0 : -EINVAL;
>   	}
> +	loaded_image_info.entry = entry;
>   
>   #ifdef CONFIG_ARM64
>   	/* On AArch64 we need to make sure we call our payload in < EL3 */
> @@ -254,7 +268,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>   	}
>   #endif
>   
> -	return entry(&loaded_image_info, &systab);
> +	return do_enter(&loaded_image_info, &systab);
>   }
>   
>   
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 5c3836a51b..611c35c422 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -253,6 +253,10 @@ struct efi_loaded_image {
>   	efi_status_t exit_status;
>   	struct jmp_buf_data exit_jmp;
>   #endif
> +#ifdef CONFIG_CMD_BOOTEFI
> +	ulong (*entry)(void *image_handle, struct efi_system_table *st)
> +		asmlinkage;

I don't understand why we need another field in the loaded image struct 
here. Can't we just pass entry as a parameter to do_enter()?


Alex

> +#endif
>   };
>   
>   #define DEVICE_PATH_GUID \