| Message ID | 20170703204126.11992-2-xypron.glpk@gmx.de |
|---|---|
| State | Superseded |
| Delegated to: | Alexander Graf |
| Headers | show |
On 03.07.17 22:41, 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. > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > v2: > do not store entry in loaded_image_info but use additonal > function parameter as suggested by Alexander > --- > cmd/bootefi.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 771300ee94..f52da205c9 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -147,15 +147,27 @@ static void *copy_fdt(void *fdt) > return new_fdt; > } > > +static asmlinkage ulong efi_do_enter(asmlinkage ulong (*entry)( > + void *image_handle, struct efi_system_table *st), > + void *image_handle, struct efi_system_table *st) Please put entry as the last argument. That way we don't waste any cycles on shuffling around registers between this parameter ordering and the eventual call to entry(). If you're interested in what difference it makes, compile both versions - one with entry as first and one with entry as last argument and compare the objdump -d output. Otherwise looks good to me. I suppose this is not a critical bug fix, so it can wait for U-Boot 2017.09? Alex
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 771300ee94..f52da205c9 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -147,15 +147,27 @@ static void *copy_fdt(void *fdt) return new_fdt; } +static asmlinkage ulong efi_do_enter(asmlinkage ulong (*entry)( + void *image_handle, struct efi_system_table *st), + void *image_handle, struct efi_system_table *st) +{ + efi_status_t ret = EFI_LOAD_ERROR; + + if (entry) + ret = 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, - struct efi_system_table *st) +static unsigned long efi_run_in_el2(asmlinkage ulong (*entry)( + void *image_handle, struct efi_system_table *st), + void *image_handle, struct efi_system_table *st) { /* Enable caches again */ dcache_enable(); - return entry(image_handle, st); + return efi_do_enter(entry, image_handle, st); } #endif @@ -260,7 +272,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) } #endif - return entry(&loaded_image_info, &systab); + return efi_do_enter(entry, &loaded_image_info, &systab); }
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. Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- v2: do not store entry in loaded_image_info but use additonal function parameter as suggested by Alexander --- cmd/bootefi.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)