[U-Boot] [PATCH v2 0/3] Handling of EFI application exit
EFI applications may proceed to loading and starting an operating system. In this case they do not return to OpenWrt.
On the other hand they may return to OpenWrt. In this case they may either call EFI_BOOT_SERVICES.Exit of return directly with a status code.
The first patch ensures correct handling of applications returning without calling EFI_BOOT_SERVICES.Exit.
The second patch is necessary to output the actual status code of the EFI application.
The third patch was split of the first version of the second. It contains the definition of all return codes of the UEFI standard.
--- v2 put the updated patches into a patch series
Heinrich Schuchardt (3): bootefi: allow return without EFI_BOOT_SERVICES.Exit efi_loader: provide meaningful status code efi_loader: define all known status codes
cmd/bootefi.c | 38 +++++++++++++++++++++++++------------- include/efi.h | 47 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 59 insertions(+), 26 deletions(-)
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) +{ + 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); }
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
Currenty any EFI status other than EFI_SUCCESS is reported as Application terminated, r = -22
With the patch the status code returned by the EFI application is printed.
Cc: Alexander Graf agraf@suse.de Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: Do not output status code mnemonic. Alexander suggested this to reduce code size. We may add a CONFIG option for mnemonic output later. --- cmd/bootefi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f52da205c9..7ddeead671 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -252,8 +252,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
if (setjmp(&loaded_image_info.exit_jmp)) { - efi_status_t status = loaded_image_info.exit_status; - return status == EFI_SUCCESS ? 0 : -EINVAL; + return loaded_image_info.exit_status; }
#ifdef CONFIG_ARM64 @@ -281,7 +280,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { char *saddr, *sfdt; unsigned long addr, fdt_addr = 0; - int r = 0; + unsigned long r;
if (argc < 2) return CMD_RET_USAGE; @@ -306,12 +305,13 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("## Starting EFI application at %08lx ...\n", addr); r = do_bootefi_exec((void *)addr, (void*)fdt_addr); - printf("## Application terminated, r = %d\n", r); + printf("## Application terminated, r = %lu\n", + r & ~(1UL << (EFI_BITS_PER_LONG - 1)));
- if (r != 0) - r = 1; - - return r; + if (r != EFI_SUCCESS) + return 1; + else + return 0; }
#ifdef CONFIG_SYS_LONGHELP
On 03.07.17 22:41, Heinrich Schuchardt wrote:
Currenty any EFI status other than EFI_SUCCESS is reported as Application terminated, r = -22
With the patch the status code returned by the EFI application is printed.
Cc: Alexander Graf agraf@suse.de Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Reviewed-by: Alexander Graf agraf@suse.de
This too will have to wait for the next merge window :).
Alex
efi.h held only a few EFI status codes.
The patch adds the missing definitions for later usage.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 This patch was split of version 1 of efi_loader: provide meaningful status code --- include/efi.h | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-)
diff --git a/include/efi.h b/include/efi.h index 3d587807e8..242da3c3be 100644 --- a/include/efi.h +++ b/include/efi.h @@ -39,19 +39,40 @@ struct efi_device_path; #define EFI_BITS_PER_LONG 64 #endif
-#define EFI_SUCCESS 0 -#define EFI_LOAD_ERROR (1 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_INVALID_PARAMETER (2 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_UNSUPPORTED (3 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_BAD_BUFFER_SIZE (4 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_BUFFER_TOO_SMALL (5 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_NOT_READY (6 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_DEVICE_ERROR (7 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_WRITE_PROTECTED (8 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_OUT_OF_RESOURCES (9 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_NOT_FOUND (14 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_ACCESS_DENIED (15 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_SECURITY_VIOLATION (26 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_SUCCESS 0 +#define EFI_LOAD_ERROR (1 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_INVALID_PARAMETER (2 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_UNSUPPORTED (3 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_BAD_BUFFER_SIZE (4 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_BUFFER_TOO_SMALL (5 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_NOT_READY (6 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_DEVICE_ERROR (7 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_WRITE_PROTECTED (8 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_OUT_OF_RESOURCES (9 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_VOLUME_CORRUPTED (10 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_VOLUME_FULL (11 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_NO_MEDIA (12 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_MEDIA_CHANGED (13 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_NOT_FOUND (14 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_ACCESS_DENIED (15 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_NO_RESPONSE (16 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_NO_MAPPING (17 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_TIMEOUT (18 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_NOT_STARTED (19 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_ALREADY_STARTED (20 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_ABORTED (21 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_ICMP_ERROR (22 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_TFTP_ERROR (23 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_PROTOCOL_ERROR (24 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_INCOMPATIBLE_VERSION (25 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_SECURITY_VIOLATION (26 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_CRC_ERROR (27 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_END_OF_MEDIA (28 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_END_OF_FILE (31 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_INVALID_LANGUAGE (32 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_COMPROMISED_DATA (33 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_IP_ADDRESS_CONFLICT (34 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_HTTP_ERROR (35 | (1UL << (EFI_BITS_PER_LONG - 1)))
typedef unsigned long efi_status_t; typedef u64 efi_physical_addr_t;
On 03.07.17 22:41, Heinrich Schuchardt wrote:
efi.h held only a few EFI status codes.
The patch adds the missing definitions for later usage.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2 This patch was split of version 1 of efi_loader: provide meaningful status code
include/efi.h | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-)
diff --git a/include/efi.h b/include/efi.h index 3d587807e8..242da3c3be 100644 --- a/include/efi.h +++ b/include/efi.h @@ -39,19 +39,40 @@ struct efi_device_path; #define EFI_BITS_PER_LONG 64 #endif
-#define EFI_SUCCESS 0 -#define EFI_LOAD_ERROR (1 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_INVALID_PARAMETER (2 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_UNSUPPORTED (3 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_BAD_BUFFER_SIZE (4 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_BUFFER_TOO_SMALL (5 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_NOT_READY (6 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_DEVICE_ERROR (7 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_WRITE_PROTECTED (8 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_OUT_OF_RESOURCES (9 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_NOT_FOUND (14 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_ACCESS_DENIED (15 | (1UL << (EFI_BITS_PER_LONG - 1))) -#define EFI_SECURITY_VIOLATION (26 | (1UL << (EFI_BITS_PER_LONG - 1))) +#define EFI_SUCCESS 0 +#define EFI_LOAD_ERROR (1 | (1UL << (EFI_BITS_PER_LONG - 1)))
While you're touching these anyway, would you mind to convert the error bit into something more readable?
#define EFI_ERROR_MASK (1UL << (EFI_BITS_PER_LONG - 1)) #define EFI_LOAD_ERROR (EFI_ERROR_MASK | 1)
We can then reuse the same constant in patch 2/3 to mask the bit out of the return value ;).
Alex
participants (2)
-
Alexander Graf -
Heinrich Schuchardt