On 03.07.17 18:36, Heinrich Schuchardt wrote:

On 07/03/2017 02:01 PM, Alexander Graf wrote:

...

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@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.

Sure.

...

...

•             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()?

I could not see how to pass 'entry' to do_enter() as parameter without changing assembly code routine armv8_switch_to_el2.

armv8_switch_to_el2 simply takes x0-x3 and passes it into the payload function (efi_run_in_el2 in our case). So we would have 4 parameters to pass.

However, efi_run_in_el2() already gets the entry pointer today:

static unsigned long efi_run_in_el2(ulong (*entry)(void *image_handle, struct efi_system_table *st), void *image_handle, struct efi_system_table *st)

So instead of calling

return entry(image_handle, st);

you would just change to that

return efi_do_enter(image_handle, st, entry);

and have the entry function pointer available in your function.

Alex