| Message ID | 20190419032236.8242-9-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: rework bootefi/bootmgr | expand |
On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > + /* > + * FIXME: Who is responsible for > + * free(loaded_image_info->load_options); > + * Once efi_exit() is implemented correctly, > + * handle itself doesn't exist here. > + */ Load option can only be freed when the image is unloaded. For applications that will happen when the application call Exit(). For drivers there is not guarantee that they will be ever unloaded. In Unload() we should not free load options if they were not allocated by us but by an EFI binary. In your implementation you have allocated the load options via calloc() and not from the EFI pool. So this may allow us to determine whether we allocated the load options in the Unload() service. Best regards Heinrich
On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > In the current implementation, bootefi command and EFI boot manager > don't use load_image API, instead, use more primitive and internal > functions. This will introduce duplicated code and potentially > unknown bugs as well as inconsistent behaviours. > > With this patch, do_efibootmgr() and do_boot_efi() are completely > overhauled and re-implemented using load_image API. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > cmd/bootefi.c | 187 ++++++++++++++++++---------------- > include/efi_loader.h | 5 +- > lib/efi_loader/efi_bootmgr.c | 43 ++++---- > lib/efi_loader/efi_boottime.c | 2 + > 4 files changed, 127 insertions(+), 110 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index e2ca68c4dc12..8c84fff590a7 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -242,89 +242,36 @@ static efi_status_t efi_install_fdt(const char *fdt_opt) > /** > * 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 > + * @handle: handle of loaded image > * 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) > +static efi_status_t do_bootefi_exec(efi_handle_t handle) > { > - efi_handle_t mem_handle = NULL; > - struct efi_device_path *memdp = NULL; > efi_status_t ret; > - struct efi_loaded_image_obj *image_obj = NULL; > - struct efi_loaded_image *loaded_image_info = NULL; > - > - /* > - * Special case for efi payload not loaded from disk, such as > - * 'bootefi hello' or for example payload loaded directly into > - * memory via JTAG, etc: > - */ > - if (!device_path && !image_path) { > - printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); > - /* actual addresses filled in after efi_load_pe() */ > - memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); > - device_path = image_path = memdp; > - /* > - * Grub expects that the device path of the loaded image is > - * installed on a handle. > - */ > - ret = efi_create_handle(&mem_handle); > - if (ret != EFI_SUCCESS) > - return ret; /* TODO: leaks device_path */ > - ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > - device_path); > - if (ret != EFI_SUCCESS) > - goto err_add_protocol; > - } else { > - assert(device_path && image_path); > - } > - > - ret = efi_setup_loaded_image(device_path, image_path, &image_obj, > - &loaded_image_info); > - if (ret) > - goto err_prepare; > > /* Transfer environment variable as load options */ > - ret = set_load_options((efi_handle_t)image_obj, "bootargs"); > - if (ret != EFI_SUCCESS) > - goto err_prepare; > - > - /* Load the EFI payload */ > - ret = efi_load_pe(image_obj, efi, loaded_image_info); > + ret = set_load_options(handle, "bootargs"); > if (ret != EFI_SUCCESS) > - goto err_prepare; > - > - 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->end_address = mdp->start_address + > - loaded_image_info->image_size; > - } > + return ret; > > /* we don't support much: */ > env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", > "{ro,boot}(blob)0000000000000000"); > > /* Call our payload! */ > - debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry); > - ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL)); > + ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); > > -err_prepare: > - /* image has returned, loaded-image obj goes *poof*: */ > efi_restore_gd(); > - free(loaded_image_info->load_options); > - efi_delete_handle(&image_obj->header); > > -err_add_protocol: > - if (mem_handle) > - efi_delete_handle(mem_handle); > + /* > + * FIXME: Who is responsible for > + * free(loaded_image_info->load_options); > + * Once efi_exit() is implemented correctly, > + * handle itself doesn't exist here. > + */ > > return ret; > } > @@ -339,8 +286,7 @@ err_add_protocol: > */ > static int do_efibootmgr(const char *fdt_opt) > { > - struct efi_device_path *device_path, *file_path; > - void *addr; > + efi_handle_t handle; > efi_status_t ret; > > /* Allow unaligned memory access */ > @@ -362,19 +308,19 @@ static int do_efibootmgr(const char *fdt_opt) > else if (ret != EFI_SUCCESS) > return CMD_RET_FAILURE; > > - addr = efi_bootmgr_load(&device_path, &file_path); > - if (!addr) > - return 1; > + ret = efi_bootmgr_load(&handle); > + if (ret != EFI_SUCCESS) { > + printf("EFI boot manager: Cannot load any image\n"); > + return CMD_RET_FAILURE; > + } > > - printf("## Starting EFI application at %p ...\n", addr); > - ret = do_bootefi_exec(addr, device_path, file_path); > - printf("## Application terminated, r = %lu\n", > - ret & ~EFI_ERROR_MASK); > + ret = do_bootefi_exec(handle); > + printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); > > if (ret != EFI_SUCCESS) > - return 1; > + return CMD_RET_FAILURE; > > - return 0; > + return CMD_RET_SUCCESS; > } > > /* > @@ -389,7 +335,12 @@ static int do_efibootmgr(const char *fdt_opt) > */ > static int do_bootefi_image(const char *image_opt, const char *fdt_opt) > { > - unsigned long addr; > + void *image_buf; > + struct efi_device_path *device_path, *image_path; > + struct efi_device_path *memdp = NULL, *file_path = NULL; > + unsigned long addr, size; > + const char *size_str; > + efi_handle_t mem_handle = NULL, handle; > efi_status_t ret; > > /* Allow unaligned memory access */ > @@ -414,33 +365,90 @@ static int do_bootefi_image(const char *image_opt, const char *fdt_opt) > #ifdef CONFIG_CMD_BOOTEFI_HELLO > if (!strcmp(image_opt, "hello")) { > char *saddr; > - ulong size = __efi_helloworld_end - __efi_helloworld_begin; > > saddr = env_get("loadaddr"); > + size = __efi_helloworld_end - __efi_helloworld_begin; > + > if (saddr) > addr = simple_strtoul(saddr, NULL, 16); > else > addr = CONFIG_SYS_LOAD_ADDR; > - memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); > + > + image_buf = map_sysmem(addr, size); > + memcpy(image_buf, __efi_helloworld_begin, size); > + > + device_path = NULL; > + image_path = NULL; > } else > #endif > { > + size_str = env_get("filesize"); > + if (size_str) > + size = simple_strtoul(size_str, NULL, 16); > + else > + size = 0; > + > addr = simple_strtoul(image_opt, NULL, 16); > /* Check that a numeric value was passed */ > if (!addr && *image_opt != '0') > return CMD_RET_USAGE; > + > + image_buf = map_sysmem(addr, size); > + > + device_path = bootefi_device_path; > + image_path = bootefi_image_path; > } > > - printf("## Starting EFI application at %08lx ...\n", addr); > - ret = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path, > - bootefi_image_path); > - printf("## Application terminated, r = %lu\n", > - ret & ~EFI_ERROR_MASK); > + if (!device_path && !image_path) { > + /* > + * Special case for efi payload not loaded from disk, > + * such as 'bootefi hello' or for example payload > + * loaded directly into memory via JTAG, etc: > + */ > + memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, > + (uintptr_t)image_buf, size); > + file_path = memdp; /* append(memdp, NULL) */ > + > + /* > + * Make sure that device for device_path exist > + * in load_image(). Otherwise, shell and grub will fail. > + */ > + ret = efi_create_handle(&mem_handle); > + if (ret != EFI_SUCCESS) > + /* TODO: leaks device_path */ > + goto err_add_protocol; > + > + ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > + memdp); > + if (ret != EFI_SUCCESS) > + goto err_add_protocol; > + } else { > + assert(device_path && image_path); > + file_path = efi_dp_append(device_path, image_path); > + } > + > + ret = EFI_CALL(efi_load_image(false, efi_root, > + file_path, image_buf, size, &handle)); > + if (ret != EFI_SUCCESS) > + goto err_prepare; > + > + ret = do_bootefi_exec(handle); > + printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); > + > +err_prepare: > + if (device_path) > + efi_free_pool(file_path); > + > +err_add_protocol: > + if (mem_handle) > + efi_delete_handle(mem_handle); > + if (file_path) /* hence memdp */ > + efi_free_pool(file_path); > > if (ret != EFI_SUCCESS) > return CMD_RET_FAILURE; > - else > - return CMD_RET_SUCCESS; > + > + return CMD_RET_SUCCESS; > } > > #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > @@ -598,7 +606,7 @@ static char bootefi_help_text[] = > " Use environment variable efi_selftest to select a single test.\n" > " Use 'setenv efi_selftest list' to enumerate all tests.\n" > #endif > - "bootefi bootmgr [fdt addr]\n" > + "bootefi bootmgr [fdt address]\n" > " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" > "\n" > " If specified, the device tree located at <fdt address> gets\n" > @@ -623,6 +631,13 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path) > ret = efi_dp_from_name(dev, devnr, path, &device, &image); > if (ret == EFI_SUCCESS) { > bootefi_device_path = device; > + if (image) { > + /* FIXME: image should not contain device */ > + struct efi_device_path *image_tmp = image; > + > + efi_dp_split_file_path(image, &device, &image); > + efi_free_pool(image_tmp); > + } > bootefi_image_path = image; > } else { > bootefi_device_path = NULL; > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 93f7672aecbc..39ed8a6fa592 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -412,8 +412,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > struct efi_device_path *file_path, > struct efi_loaded_image_obj **handle_ptr, > struct efi_loaded_image **info_ptr); > -efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, > - void **buffer, efi_uintn_t *size); > /* Print information about all loaded images */ > void efi_print_image_infos(void *pc); > > @@ -567,8 +565,7 @@ struct efi_load_option { > > void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); > unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); > -void *efi_bootmgr_load(struct efi_device_path **device_path, > - struct efi_device_path **file_path); > +efi_status_t efi_bootmgr_load(efi_handle_t *handle); > > #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > index 4fccadc5483d..13ec79b2098b 100644 > --- a/lib/efi_loader/efi_bootmgr.c > +++ b/lib/efi_loader/efi_bootmgr.c > @@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t *vendor, > * if successful. This checks that the EFI_LOAD_OPTION is active (enabled) > * and that the specified file to boot exists. > */ > -static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > - struct efi_device_path **file_path) > +static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) > { > struct efi_load_option lo; > u16 varname[] = L"Boot0000"; > u16 hexmap[] = L"0123456789ABCDEF"; > - void *load_option, *image = NULL; > + void *load_option; > efi_uintn_t size; > + efi_status_t ret; > > varname[4] = hexmap[(n & 0xf000) >> 12]; > varname[5] = hexmap[(n & 0x0f00) >> 8]; > @@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > > load_option = get_var(varname, &efi_global_variable_guid, &size); > if (!load_option) > - return NULL; > + return EFI_LOAD_ERROR; > > efi_deserialize_load_option(&lo, load_option); > > if (lo.attributes & LOAD_OPTION_ACTIVE) { > u32 attributes; > - efi_status_t ret; > > debug("%s: trying to load \"%ls\" from %pD\n", > __func__, lo.label, lo.file_path); > > - ret = efi_load_image_from_path(lo.file_path, &image, &size); > - > + ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */, In cmd/booefi.c you use efi_root. So we shall do the same here. I will fix that. No need to resend the patch. > + lo.file_path, NULL, 0, > + handle)); > if (ret != EFI_SUCCESS) > goto error; TODO: Now that you have the loaded image protocol you should update the load options with the value in load_option. I guess you do not want to overwrite it with the content of bootargs. A the problem was not introduced with this patch series: Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > @@ -159,17 +159,22 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > L"BootCurrent", > (efi_guid_t *)&efi_global_variable_guid, > attributes, size, &n)); > - if (ret != EFI_SUCCESS) > + if (ret != EFI_SUCCESS) { > + if (EFI_CALL(efi_unload_image(*handle)) > + != EFI_SUCCESS) > + printf("Unloading image failed\n"); > goto error; > + } > > printf("Booting: %ls\n", lo.label); > - efi_dp_split_file_path(lo.file_path, device_path, file_path); > + } else { > + ret = EFI_LOAD_ERROR; > } > > error: > free(load_option); > > - return image; > + return ret; > } > > /* > @@ -177,12 +182,10 @@ error: > * EFI variable, the available load-options, finding and returning > * the first one that can be loaded successfully. > */ > -void *efi_bootmgr_load(struct efi_device_path **device_path, > - struct efi_device_path **file_path) > +efi_status_t efi_bootmgr_load(efi_handle_t *handle) > { > u16 bootnext, *bootorder; > efi_uintn_t size; > - void *image = NULL; > int i, num; > efi_status_t ret; > > @@ -209,10 +212,9 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, > /* load BootNext */ > if (ret == EFI_SUCCESS) { > if (size == sizeof(u16)) { > - image = try_load_entry(bootnext, device_path, > - file_path); > - if (image) > - return image; > + ret = try_load_entry(bootnext, handle); > + if (ret == EFI_SUCCESS) > + return ret; > } > } else { > printf("Deleting BootNext failed\n"); > @@ -223,19 +225,20 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, > bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); > if (!bootorder) { > printf("BootOrder not defined\n"); > + ret = EFI_NOT_FOUND; > goto error; > } > > num = size / sizeof(uint16_t); > for (i = 0; i < num; i++) { > debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]); > - image = try_load_entry(bootorder[i], device_path, file_path); > - if (image) > + ret = try_load_entry(bootorder[i], handle); > + if (ret == EFI_SUCCESS) > break; > } > > free(bootorder); > > error: > - return image; > + return ret; > } > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index abc295e392e9..19a58144cf4b 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -1591,6 +1591,7 @@ failure: > * @size: size of the loaded image > * Return: status code > */ > +static > efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, > void **buffer, efi_uintn_t *size) > { > @@ -2664,6 +2665,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > } > > current_image = image_handle; > + EFI_PRINT("Jumping into 0x%p\n", image_obj->entry); > ret = EFI_CALL(image_obj->entry(image_handle, &systab)); > > /* >
On 4/20/19 10:37 AM, Heinrich Schuchardt wrote: > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: >> In the current implementation, bootefi command and EFI boot manager >> don't use load_image API, instead, use more primitive and internal >> functions. This will introduce duplicated code and potentially >> unknown bugs as well as inconsistent behaviours. >> >> With this patch, do_efibootmgr() and do_boot_efi() are completely >> overhauled and re-implemented using load_image API. Hello Takahiro, with this patch applied iPXE cannot find a chained file on the EFI partition. Did you test booting GRUB with this patch? Best regards Heinrich EFI: Entry efi_load_image(0, 00000000b9f92020, /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x800512bd,0x800,0x200000)/dtb, 0000000040080000, 21738, 00000000b9f33fc8) EFI: Exit: efi_load_image: 0 EFI: Entry efi_start_image(00000000b9f95360, 0000000000000000, 0000000000000000) EFI: Call: efi_open_protocol(image_handle, &efi_guid_loaded_image, &info, NULL, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL) EFI: 0 returned by efi_open_protocol(image_handle, &efi_guid_loaded_image, &info, NULL, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL) EFI: Jumping into 0x00000000b8e7861c EFI: Call: image_obj->entry(image_handle, &systab) EFI: Entry efi_load_image(0, 00000000b9f95360, /, 00000000b8e9fbc0, 177, 00000000b9f33e38) efi_load_pe: Invalid DOS Signature EFI: Exit: efi_load_image: 1 iPXE initialising devices...ok iPXE 1.0.0+ (ac4fb) -- Open Source Network Boot Firmware -- http://ipxe.org Features: DNS FTP HTTP HTTPS iSCSI NFS TFTP VLAN AoE EFI Menu iPXE script started Trying to chain file:/boot.ipxe Could not start download: No such file or directory (http://ipxe.org/2d4de08e) Could not find file:/boot.ipxe Opening shell iPXE> >> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> --- >> cmd/bootefi.c | 187 ++++++++++++++++++---------------- >> include/efi_loader.h | 5 +- >> lib/efi_loader/efi_bootmgr.c | 43 ++++---- >> lib/efi_loader/efi_boottime.c | 2 + >> 4 files changed, 127 insertions(+), 110 deletions(-) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index e2ca68c4dc12..8c84fff590a7 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -242,89 +242,36 @@ static efi_status_t efi_install_fdt(const char >> *fdt_opt) >> /** >> * 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 >> + * @handle: handle of loaded image >> * 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) >> +static efi_status_t do_bootefi_exec(efi_handle_t handle) >> { >> - efi_handle_t mem_handle = NULL; >> - struct efi_device_path *memdp = NULL; >> efi_status_t ret; >> - struct efi_loaded_image_obj *image_obj = NULL; >> - struct efi_loaded_image *loaded_image_info = NULL; >> - >> - /* >> - * Special case for efi payload not loaded from disk, such as >> - * 'bootefi hello' or for example payload loaded directly into >> - * memory via JTAG, etc: >> - */ >> - if (!device_path && !image_path) { >> - printf("WARNING: using memory device/image path, this may >> confuse some payloads!\n"); >> - /* actual addresses filled in after efi_load_pe() */ >> - memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); >> - device_path = image_path = memdp; >> - /* >> - * Grub expects that the device path of the loaded image is >> - * installed on a handle. >> - */ >> - ret = efi_create_handle(&mem_handle); >> - if (ret != EFI_SUCCESS) >> - return ret; /* TODO: leaks device_path */ >> - ret = efi_add_protocol(mem_handle, &efi_guid_device_path, >> - device_path); >> - if (ret != EFI_SUCCESS) >> - goto err_add_protocol; >> - } else { >> - assert(device_path && image_path); >> - } >> - >> - ret = efi_setup_loaded_image(device_path, image_path, &image_obj, >> - &loaded_image_info); >> - if (ret) >> - goto err_prepare; >> >> /* Transfer environment variable as load options */ >> - ret = set_load_options((efi_handle_t)image_obj, "bootargs"); >> - if (ret != EFI_SUCCESS) >> - goto err_prepare; >> - >> - /* Load the EFI payload */ >> - ret = efi_load_pe(image_obj, efi, loaded_image_info); >> + ret = set_load_options(handle, "bootargs"); >> if (ret != EFI_SUCCESS) >> - goto err_prepare; >> - >> - 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->end_address = mdp->start_address + >> - loaded_image_info->image_size; >> - } >> + return ret; >> >> /* we don't support much: */ >> >> env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", >> >> "{ro,boot}(blob)0000000000000000"); >> >> /* Call our payload! */ >> - debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry); >> - ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL)); >> + ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); >> >> -err_prepare: >> - /* image has returned, loaded-image obj goes *poof*: */ >> efi_restore_gd(); >> - free(loaded_image_info->load_options); >> - efi_delete_handle(&image_obj->header); >> >> -err_add_protocol: >> - if (mem_handle) >> - efi_delete_handle(mem_handle); >> + /* >> + * FIXME: Who is responsible for >> + * free(loaded_image_info->load_options); >> + * Once efi_exit() is implemented correctly, >> + * handle itself doesn't exist here. >> + */ >> >> return ret; >> } >> @@ -339,8 +286,7 @@ err_add_protocol: >> */ >> static int do_efibootmgr(const char *fdt_opt) >> { >> - struct efi_device_path *device_path, *file_path; >> - void *addr; >> + efi_handle_t handle; >> efi_status_t ret; >> >> /* Allow unaligned memory access */ >> @@ -362,19 +308,19 @@ static int do_efibootmgr(const char *fdt_opt) >> else if (ret != EFI_SUCCESS) >> return CMD_RET_FAILURE; >> >> - addr = efi_bootmgr_load(&device_path, &file_path); >> - if (!addr) >> - return 1; >> + ret = efi_bootmgr_load(&handle); >> + if (ret != EFI_SUCCESS) { >> + printf("EFI boot manager: Cannot load any image\n"); >> + return CMD_RET_FAILURE; >> + } >> >> - printf("## Starting EFI application at %p ...\n", addr); >> - ret = do_bootefi_exec(addr, device_path, file_path); >> - printf("## Application terminated, r = %lu\n", >> - ret & ~EFI_ERROR_MASK); >> + ret = do_bootefi_exec(handle); >> + printf("## Application terminated, r = %lu\n", ret & >> ~EFI_ERROR_MASK); >> >> if (ret != EFI_SUCCESS) >> - return 1; >> + return CMD_RET_FAILURE; >> >> - return 0; >> + return CMD_RET_SUCCESS; >> } >> >> /* >> @@ -389,7 +335,12 @@ static int do_efibootmgr(const char *fdt_opt) >> */ >> static int do_bootefi_image(const char *image_opt, const char *fdt_opt) >> { >> - unsigned long addr; >> + void *image_buf; >> + struct efi_device_path *device_path, *image_path; >> + struct efi_device_path *memdp = NULL, *file_path = NULL; >> + unsigned long addr, size; >> + const char *size_str; >> + efi_handle_t mem_handle = NULL, handle; >> efi_status_t ret; >> >> /* Allow unaligned memory access */ >> @@ -414,33 +365,90 @@ static int do_bootefi_image(const char >> *image_opt, const char *fdt_opt) >> #ifdef CONFIG_CMD_BOOTEFI_HELLO >> if (!strcmp(image_opt, "hello")) { >> char *saddr; >> - ulong size = __efi_helloworld_end - __efi_helloworld_begin; >> >> saddr = env_get("loadaddr"); >> + size = __efi_helloworld_end - __efi_helloworld_begin; >> + >> if (saddr) >> addr = simple_strtoul(saddr, NULL, 16); >> else >> addr = CONFIG_SYS_LOAD_ADDR; >> - memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); >> + >> + image_buf = map_sysmem(addr, size); >> + memcpy(image_buf, __efi_helloworld_begin, size); >> + >> + device_path = NULL; >> + image_path = NULL; >> } else >> #endif >> { >> + size_str = env_get("filesize"); >> + if (size_str) >> + size = simple_strtoul(size_str, NULL, 16); >> + else >> + size = 0; >> + >> addr = simple_strtoul(image_opt, NULL, 16); >> /* Check that a numeric value was passed */ >> if (!addr && *image_opt != '0') >> return CMD_RET_USAGE; >> + >> + image_buf = map_sysmem(addr, size); >> + >> + device_path = bootefi_device_path; >> + image_path = bootefi_image_path; >> } >> >> - printf("## Starting EFI application at %08lx ...\n", addr); >> - ret = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path, >> - bootefi_image_path); >> - printf("## Application terminated, r = %lu\n", >> - ret & ~EFI_ERROR_MASK); >> + if (!device_path && !image_path) { >> + /* >> + * Special case for efi payload not loaded from disk, >> + * such as 'bootefi hello' or for example payload >> + * loaded directly into memory via JTAG, etc: >> + */ >> + memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, >> + (uintptr_t)image_buf, size); >> + file_path = memdp; /* append(memdp, NULL) */ >> + >> + /* >> + * Make sure that device for device_path exist >> + * in load_image(). Otherwise, shell and grub will fail. >> + */ >> + ret = efi_create_handle(&mem_handle); >> + if (ret != EFI_SUCCESS) >> + /* TODO: leaks device_path */ >> + goto err_add_protocol; >> + >> + ret = efi_add_protocol(mem_handle, &efi_guid_device_path, >> + memdp); >> + if (ret != EFI_SUCCESS) >> + goto err_add_protocol; >> + } else { >> + assert(device_path && image_path); >> + file_path = efi_dp_append(device_path, image_path); >> + } >> + >> + ret = EFI_CALL(efi_load_image(false, efi_root, >> + file_path, image_buf, size, &handle)); >> + if (ret != EFI_SUCCESS) >> + goto err_prepare; >> + >> + ret = do_bootefi_exec(handle); >> + printf("## Application terminated, r = %lu\n", ret & >> ~EFI_ERROR_MASK); >> + >> +err_prepare: >> + if (device_path) >> + efi_free_pool(file_path); >> + >> +err_add_protocol: >> + if (mem_handle) >> + efi_delete_handle(mem_handle); >> + if (file_path) /* hence memdp */ >> + efi_free_pool(file_path); >> >> if (ret != EFI_SUCCESS) >> return CMD_RET_FAILURE; >> - else >> - return CMD_RET_SUCCESS; >> + >> + return CMD_RET_SUCCESS; >> } >> >> #ifdef CONFIG_CMD_BOOTEFI_SELFTEST >> @@ -598,7 +606,7 @@ static char bootefi_help_text[] = >> " Use environment variable efi_selftest to select a single >> test.\n" >> " Use 'setenv efi_selftest list' to enumerate all tests.\n" >> #endif >> - "bootefi bootmgr [fdt addr]\n" >> + "bootefi bootmgr [fdt address]\n" >> " - load and boot EFI payload based on BootOrder/BootXXXX >> variables.\n" >> "\n" >> " If specified, the device tree located at <fdt address> gets\n" >> @@ -623,6 +631,13 @@ void efi_set_bootdev(const char *dev, const char >> *devnr, const char *path) >> ret = efi_dp_from_name(dev, devnr, path, &device, &image); >> if (ret == EFI_SUCCESS) { >> bootefi_device_path = device; >> + if (image) { >> + /* FIXME: image should not contain device */ >> + struct efi_device_path *image_tmp = image; >> + >> + efi_dp_split_file_path(image, &device, &image); >> + efi_free_pool(image_tmp); >> + } >> bootefi_image_path = image; >> } else { >> bootefi_device_path = NULL; >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index 93f7672aecbc..39ed8a6fa592 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -412,8 +412,6 @@ efi_status_t efi_setup_loaded_image(struct >> efi_device_path *device_path, >> struct efi_device_path *file_path, >> struct efi_loaded_image_obj **handle_ptr, >> struct efi_loaded_image **info_ptr); >> -efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, >> - void **buffer, efi_uintn_t *size); >> /* Print information about all loaded images */ >> void efi_print_image_infos(void *pc); >> >> @@ -567,8 +565,7 @@ struct efi_load_option { >> >> void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); >> unsigned long efi_serialize_load_option(struct efi_load_option *lo, >> u8 **data); >> -void *efi_bootmgr_load(struct efi_device_path **device_path, >> - struct efi_device_path **file_path); >> +efi_status_t efi_bootmgr_load(efi_handle_t *handle); >> >> #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ >> >> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c >> index 4fccadc5483d..13ec79b2098b 100644 >> --- a/lib/efi_loader/efi_bootmgr.c >> +++ b/lib/efi_loader/efi_bootmgr.c >> @@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t >> *vendor, >> * if successful. This checks that the EFI_LOAD_OPTION is active >> (enabled) >> * and that the specified file to boot exists. >> */ >> -static void *try_load_entry(uint16_t n, struct efi_device_path >> **device_path, >> - struct efi_device_path **file_path) >> +static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) >> { >> struct efi_load_option lo; >> u16 varname[] = L"Boot0000"; >> u16 hexmap[] = L"0123456789ABCDEF"; >> - void *load_option, *image = NULL; >> + void *load_option; >> efi_uintn_t size; >> + efi_status_t ret; >> >> varname[4] = hexmap[(n & 0xf000) >> 12]; >> varname[5] = hexmap[(n & 0x0f00) >> 8]; >> @@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct >> efi_device_path **device_path, >> >> load_option = get_var(varname, &efi_global_variable_guid, &size); >> if (!load_option) >> - return NULL; >> + return EFI_LOAD_ERROR; >> >> efi_deserialize_load_option(&lo, load_option); >> >> if (lo.attributes & LOAD_OPTION_ACTIVE) { >> u32 attributes; >> - efi_status_t ret; >> >> debug("%s: trying to load \"%ls\" from %pD\n", >> __func__, lo.label, lo.file_path); >> >> - ret = efi_load_image_from_path(lo.file_path, &image, &size); >> - >> + ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */, > > In cmd/booefi.c you use efi_root. So we shall do the same here. > > I will fix that. No need to resend the patch. > > >> + lo.file_path, NULL, 0, >> + handle)); >> if (ret != EFI_SUCCESS) >> goto error; > TODO: > Now that you have the loaded image protocol you should update the load > options with the value in load_option. I guess you do not want to > overwrite it with the content of bootargs. > > A the problem was not introduced with this patch series: > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> >> @@ -159,17 +159,22 @@ static void *try_load_entry(uint16_t n, struct >> efi_device_path **device_path, >> L"BootCurrent", >> (efi_guid_t *)&efi_global_variable_guid, >> attributes, size, &n)); >> - if (ret != EFI_SUCCESS) >> + if (ret != EFI_SUCCESS) { >> + if (EFI_CALL(efi_unload_image(*handle)) >> + != EFI_SUCCESS) >> + printf("Unloading image failed\n"); >> goto error; >> + } >> >> printf("Booting: %ls\n", lo.label); >> - efi_dp_split_file_path(lo.file_path, device_path, file_path); >> + } else { >> + ret = EFI_LOAD_ERROR; >> } >> >> error: >> free(load_option); >> >> - return image; >> + return ret; >> } >> >> /* >> @@ -177,12 +182,10 @@ error: >> * EFI variable, the available load-options, finding and returning >> * the first one that can be loaded successfully. >> */ >> -void *efi_bootmgr_load(struct efi_device_path **device_path, >> - struct efi_device_path **file_path) >> +efi_status_t efi_bootmgr_load(efi_handle_t *handle) >> { >> u16 bootnext, *bootorder; >> efi_uintn_t size; >> - void *image = NULL; >> int i, num; >> efi_status_t ret; >> >> @@ -209,10 +212,9 @@ void *efi_bootmgr_load(struct efi_device_path >> **device_path, >> /* load BootNext */ >> if (ret == EFI_SUCCESS) { >> if (size == sizeof(u16)) { >> - image = try_load_entry(bootnext, device_path, >> - file_path); >> - if (image) >> - return image; >> + ret = try_load_entry(bootnext, handle); >> + if (ret == EFI_SUCCESS) >> + return ret; >> } >> } else { >> printf("Deleting BootNext failed\n"); >> @@ -223,19 +225,20 @@ void *efi_bootmgr_load(struct efi_device_path >> **device_path, >> bootorder = get_var(L"BootOrder", &efi_global_variable_guid, >> &size); >> if (!bootorder) { >> printf("BootOrder not defined\n"); >> + ret = EFI_NOT_FOUND; >> goto error; >> } >> >> num = size / sizeof(uint16_t); >> for (i = 0; i < num; i++) { >> debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]); >> - image = try_load_entry(bootorder[i], device_path, file_path); >> - if (image) >> + ret = try_load_entry(bootorder[i], handle); >> + if (ret == EFI_SUCCESS) >> break; >> } >> >> free(bootorder); >> >> error: >> - return image; >> + return ret; >> } >> diff --git a/lib/efi_loader/efi_boottime.c >> b/lib/efi_loader/efi_boottime.c >> index abc295e392e9..19a58144cf4b 100644 >> --- a/lib/efi_loader/efi_boottime.c >> +++ b/lib/efi_loader/efi_boottime.c >> @@ -1591,6 +1591,7 @@ failure: >> * @size: size of the loaded image >> * Return: status code >> */ >> +static >> efi_status_t efi_load_image_from_path(struct efi_device_path >> *file_path, >> void **buffer, efi_uintn_t *size) >> { >> @@ -2664,6 +2665,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t >> image_handle, >> } >> >> current_image = image_handle; >> + EFI_PRINT("Jumping into 0x%p\n", image_obj->entry); >> ret = EFI_CALL(image_obj->entry(image_handle, &systab)); >> >> /* >> > >
On 4/20/19 10:37 AM, Heinrich Schuchardt wrote: > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: >> In the current implementation, bootefi command and EFI boot manager >> don't use load_image API, instead, use more primitive and internal >> functions. This will introduce duplicated code and potentially >> unknown bugs as well as inconsistent behaviours. >> >> With this patch, do_efibootmgr() and do_boot_efi() are completely >> overhauled and re-implemented using load_image API. >> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> <snip /> >> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c >> index 4fccadc5483d..13ec79b2098b 100644 >> --- a/lib/efi_loader/efi_bootmgr.c >> +++ b/lib/efi_loader/efi_bootmgr.c >> @@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t >> *vendor, >> * if successful. This checks that the EFI_LOAD_OPTION is active >> (enabled) >> * and that the specified file to boot exists. >> */ >> -static void *try_load_entry(uint16_t n, struct efi_device_path >> **device_path, >> - struct efi_device_path **file_path) >> +static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) >> { >> struct efi_load_option lo; >> u16 varname[] = L"Boot0000"; >> u16 hexmap[] = L"0123456789ABCDEF"; >> - void *load_option, *image = NULL; >> + void *load_option; >> efi_uintn_t size; >> + efi_status_t ret; >> >> varname[4] = hexmap[(n & 0xf000) >> 12]; >> varname[5] = hexmap[(n & 0x0f00) >> 8]; >> @@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct >> efi_device_path **device_path, >> >> load_option = get_var(varname, &efi_global_variable_guid, &size); >> if (!load_option) >> - return NULL; >> + return EFI_LOAD_ERROR; >> >> efi_deserialize_load_option(&lo, load_option); >> >> if (lo.attributes & LOAD_OPTION_ACTIVE) { >> u32 attributes; >> - efi_status_t ret; >> >> debug("%s: trying to load \"%ls\" from %pD\n", >> __func__, lo.label, lo.file_path); >> >> - ret = efi_load_image_from_path(lo.file_path, &image, &size); >> - >> + ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */, boot_policy should be true for requests originating from the boot manager. Best regards Heinrich
On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > In the current implementation, bootefi command and EFI boot manager > don't use load_image API, instead, use more primitive and internal > functions. This will introduce duplicated code and potentially > unknown bugs as well as inconsistent behaviours. > > With this patch, do_efibootmgr() and do_boot_efi() are completely > overhauled and re-implemented using load_image API. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> <snip /> > +err_prepare: > + if (device_path) > + efi_free_pool(file_path); This free is duplicate to the one below. And why should freeing file_path depend on device_path? Regards Heinrich > + > +err_add_protocol: > + if (mem_handle) > + efi_delete_handle(mem_handle); > + if (file_path) /* hence memdp */ > + efi_free_pool(file_path); > > if (ret != EFI_SUCCESS) > return CMD_RET_FAILURE; > - else > - return CMD_RET_SUCCESS; > + > + return CMD_RET_SUCCESS; > }
On Sat, Apr 20, 2019 at 07:37:47PM +0200, Heinrich Schuchardt wrote: > On 4/20/19 10:37 AM, Heinrich Schuchardt wrote: > > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > >> In the current implementation, bootefi command and EFI boot manager > >> don't use load_image API, instead, use more primitive and internal > >> functions. This will introduce duplicated code and potentially > >> unknown bugs as well as inconsistent behaviours. > >> > >> With this patch, do_efibootmgr() and do_boot_efi() are completely > >> overhauled and re-implemented using load_image API. > > Hello Takahiro, > > with this patch applied iPXE cannot find a chained file on the EFI > partition. Did you test booting GRUB with this patch? No. I test my patch only with Shell.efi right now. -Takahiro Akashi > Best regards > > Heinrich > > EFI: Entry efi_load_image(0, 00000000b9f92020, > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x800512bd,0x800,0x200000)/dtb, > 0000000040080000, 21738, 00000000b9f33fc8) > EFI: Exit: efi_load_image: 0 > EFI: Entry efi_start_image(00000000b9f95360, 0000000000000000, > 0000000000000000) > EFI: Call: efi_open_protocol(image_handle, &efi_guid_loaded_image, > &info, NULL, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL) > EFI: 0 returned by efi_open_protocol(image_handle, > &efi_guid_loaded_image, &info, NULL, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL) > EFI: Jumping into 0x00000000b8e7861c > EFI: Call: image_obj->entry(image_handle, &systab) > EFI: Entry efi_load_image(0, 00000000b9f95360, /, 00000000b8e9fbc0, > 177, 00000000b9f33e38) > efi_load_pe: Invalid DOS Signature > EFI: Exit: efi_load_image: 1 > iPXE initialising devices...ok > iPXE 1.0.0+ (ac4fb) -- Open Source Network Boot Firmware -- http://ipxe.org > Features: DNS FTP HTTP HTTPS iSCSI NFS TFTP VLAN AoE EFI Menu > iPXE script started > Trying to chain file:/boot.ipxe > Could not start download: No such file or directory > (http://ipxe.org/2d4de08e) > Could not find file:/boot.ipxe > Opening shell > iPXE> > > > >> > >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >> --- > >> cmd/bootefi.c | 187 ++++++++++++++++++---------------- > >> include/efi_loader.h | 5 +- > >> lib/efi_loader/efi_bootmgr.c | 43 ++++---- > >> lib/efi_loader/efi_boottime.c | 2 + > >> 4 files changed, 127 insertions(+), 110 deletions(-) > >> > >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c > >> index e2ca68c4dc12..8c84fff590a7 100644 > >> --- a/cmd/bootefi.c > >> +++ b/cmd/bootefi.c > >> @@ -242,89 +242,36 @@ static efi_status_t efi_install_fdt(const char > >> *fdt_opt) > >> /** > >> * 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 > >> + * @handle: handle of loaded image > >> * 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) > >> +static efi_status_t do_bootefi_exec(efi_handle_t handle) > >> { > >> - efi_handle_t mem_handle = NULL; > >> - struct efi_device_path *memdp = NULL; > >> efi_status_t ret; > >> - struct efi_loaded_image_obj *image_obj = NULL; > >> - struct efi_loaded_image *loaded_image_info = NULL; > >> - > >> - /* > >> - * Special case for efi payload not loaded from disk, such as > >> - * 'bootefi hello' or for example payload loaded directly into > >> - * memory via JTAG, etc: > >> - */ > >> - if (!device_path && !image_path) { > >> - printf("WARNING: using memory device/image path, this may > >> confuse some payloads!\n"); > >> - /* actual addresses filled in after efi_load_pe() */ > >> - memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); > >> - device_path = image_path = memdp; > >> - /* > >> - * Grub expects that the device path of the loaded image is > >> - * installed on a handle. > >> - */ > >> - ret = efi_create_handle(&mem_handle); > >> - if (ret != EFI_SUCCESS) > >> - return ret; /* TODO: leaks device_path */ > >> - ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > >> - device_path); > >> - if (ret != EFI_SUCCESS) > >> - goto err_add_protocol; > >> - } else { > >> - assert(device_path && image_path); > >> - } > >> - > >> - ret = efi_setup_loaded_image(device_path, image_path, &image_obj, > >> - &loaded_image_info); > >> - if (ret) > >> - goto err_prepare; > >> > >> /* Transfer environment variable as load options */ > >> - ret = set_load_options((efi_handle_t)image_obj, "bootargs"); > >> - if (ret != EFI_SUCCESS) > >> - goto err_prepare; > >> - > >> - /* Load the EFI payload */ > >> - ret = efi_load_pe(image_obj, efi, loaded_image_info); > >> + ret = set_load_options(handle, "bootargs"); > >> if (ret != EFI_SUCCESS) > >> - goto err_prepare; > >> - > >> - 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->end_address = mdp->start_address + > >> - loaded_image_info->image_size; > >> - } > >> + return ret; > >> > >> /* we don't support much: */ > >> > >> env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", > >> > >> "{ro,boot}(blob)0000000000000000"); > >> > >> /* Call our payload! */ > >> - debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry); > >> - ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL)); > >> + ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); > >> > >> -err_prepare: > >> - /* image has returned, loaded-image obj goes *poof*: */ > >> efi_restore_gd(); > >> - free(loaded_image_info->load_options); > >> - efi_delete_handle(&image_obj->header); > >> > >> -err_add_protocol: > >> - if (mem_handle) > >> - efi_delete_handle(mem_handle); > >> + /* > >> + * FIXME: Who is responsible for > >> + * free(loaded_image_info->load_options); > >> + * Once efi_exit() is implemented correctly, > >> + * handle itself doesn't exist here. > >> + */ > >> > >> return ret; > >> } > >> @@ -339,8 +286,7 @@ err_add_protocol: > >> */ > >> static int do_efibootmgr(const char *fdt_opt) > >> { > >> - struct efi_device_path *device_path, *file_path; > >> - void *addr; > >> + efi_handle_t handle; > >> efi_status_t ret; > >> > >> /* Allow unaligned memory access */ > >> @@ -362,19 +308,19 @@ static int do_efibootmgr(const char *fdt_opt) > >> else if (ret != EFI_SUCCESS) > >> return CMD_RET_FAILURE; > >> > >> - addr = efi_bootmgr_load(&device_path, &file_path); > >> - if (!addr) > >> - return 1; > >> + ret = efi_bootmgr_load(&handle); > >> + if (ret != EFI_SUCCESS) { > >> + printf("EFI boot manager: Cannot load any image\n"); > >> + return CMD_RET_FAILURE; > >> + } > >> > >> - printf("## Starting EFI application at %p ...\n", addr); > >> - ret = do_bootefi_exec(addr, device_path, file_path); > >> - printf("## Application terminated, r = %lu\n", > >> - ret & ~EFI_ERROR_MASK); > >> + ret = do_bootefi_exec(handle); > >> + printf("## Application terminated, r = %lu\n", ret & > >> ~EFI_ERROR_MASK); > >> > >> if (ret != EFI_SUCCESS) > >> - return 1; > >> + return CMD_RET_FAILURE; > >> > >> - return 0; > >> + return CMD_RET_SUCCESS; > >> } > >> > >> /* > >> @@ -389,7 +335,12 @@ static int do_efibootmgr(const char *fdt_opt) > >> */ > >> static int do_bootefi_image(const char *image_opt, const char *fdt_opt) > >> { > >> - unsigned long addr; > >> + void *image_buf; > >> + struct efi_device_path *device_path, *image_path; > >> + struct efi_device_path *memdp = NULL, *file_path = NULL; > >> + unsigned long addr, size; > >> + const char *size_str; > >> + efi_handle_t mem_handle = NULL, handle; > >> efi_status_t ret; > >> > >> /* Allow unaligned memory access */ > >> @@ -414,33 +365,90 @@ static int do_bootefi_image(const char > >> *image_opt, const char *fdt_opt) > >> #ifdef CONFIG_CMD_BOOTEFI_HELLO > >> if (!strcmp(image_opt, "hello")) { > >> char *saddr; > >> - ulong size = __efi_helloworld_end - __efi_helloworld_begin; > >> > >> saddr = env_get("loadaddr"); > >> + size = __efi_helloworld_end - __efi_helloworld_begin; > >> + > >> if (saddr) > >> addr = simple_strtoul(saddr, NULL, 16); > >> else > >> addr = CONFIG_SYS_LOAD_ADDR; > >> - memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); > >> + > >> + image_buf = map_sysmem(addr, size); > >> + memcpy(image_buf, __efi_helloworld_begin, size); > >> + > >> + device_path = NULL; > >> + image_path = NULL; > >> } else > >> #endif > >> { > >> + size_str = env_get("filesize"); > >> + if (size_str) > >> + size = simple_strtoul(size_str, NULL, 16); > >> + else > >> + size = 0; > >> + > >> addr = simple_strtoul(image_opt, NULL, 16); > >> /* Check that a numeric value was passed */ > >> if (!addr && *image_opt != '0') > >> return CMD_RET_USAGE; > >> + > >> + image_buf = map_sysmem(addr, size); > >> + > >> + device_path = bootefi_device_path; > >> + image_path = bootefi_image_path; > >> } > >> > >> - printf("## Starting EFI application at %08lx ...\n", addr); > >> - ret = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path, > >> - bootefi_image_path); > >> - printf("## Application terminated, r = %lu\n", > >> - ret & ~EFI_ERROR_MASK); > >> + if (!device_path && !image_path) { > >> + /* > >> + * Special case for efi payload not loaded from disk, > >> + * such as 'bootefi hello' or for example payload > >> + * loaded directly into memory via JTAG, etc: > >> + */ > >> + memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, > >> + (uintptr_t)image_buf, size); > >> + file_path = memdp; /* append(memdp, NULL) */ > >> + > >> + /* > >> + * Make sure that device for device_path exist > >> + * in load_image(). Otherwise, shell and grub will fail. > >> + */ > >> + ret = efi_create_handle(&mem_handle); > >> + if (ret != EFI_SUCCESS) > >> + /* TODO: leaks device_path */ > >> + goto err_add_protocol; > >> + > >> + ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > >> + memdp); > >> + if (ret != EFI_SUCCESS) > >> + goto err_add_protocol; > >> + } else { > >> + assert(device_path && image_path); > >> + file_path = efi_dp_append(device_path, image_path); > >> + } > >> + > >> + ret = EFI_CALL(efi_load_image(false, efi_root, > >> + file_path, image_buf, size, &handle)); > >> + if (ret != EFI_SUCCESS) > >> + goto err_prepare; > >> + > >> + ret = do_bootefi_exec(handle); > >> + printf("## Application terminated, r = %lu\n", ret & > >> ~EFI_ERROR_MASK); > >> + > >> +err_prepare: > >> + if (device_path) > >> + efi_free_pool(file_path); > >> + > >> +err_add_protocol: > >> + if (mem_handle) > >> + efi_delete_handle(mem_handle); > >> + if (file_path) /* hence memdp */ > >> + efi_free_pool(file_path); > >> > >> if (ret != EFI_SUCCESS) > >> return CMD_RET_FAILURE; > >> - else > >> - return CMD_RET_SUCCESS; > >> + > >> + return CMD_RET_SUCCESS; > >> } > >> > >> #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > >> @@ -598,7 +606,7 @@ static char bootefi_help_text[] = > >> " Use environment variable efi_selftest to select a single > >> test.\n" > >> " Use 'setenv efi_selftest list' to enumerate all tests.\n" > >> #endif > >> - "bootefi bootmgr [fdt addr]\n" > >> + "bootefi bootmgr [fdt address]\n" > >> " - load and boot EFI payload based on BootOrder/BootXXXX > >> variables.\n" > >> "\n" > >> " If specified, the device tree located at <fdt address> gets\n" > >> @@ -623,6 +631,13 @@ void efi_set_bootdev(const char *dev, const char > >> *devnr, const char *path) > >> ret = efi_dp_from_name(dev, devnr, path, &device, &image); > >> if (ret == EFI_SUCCESS) { > >> bootefi_device_path = device; > >> + if (image) { > >> + /* FIXME: image should not contain device */ > >> + struct efi_device_path *image_tmp = image; > >> + > >> + efi_dp_split_file_path(image, &device, &image); > >> + efi_free_pool(image_tmp); > >> + } > >> bootefi_image_path = image; > >> } else { > >> bootefi_device_path = NULL; > >> diff --git a/include/efi_loader.h b/include/efi_loader.h > >> index 93f7672aecbc..39ed8a6fa592 100644 > >> --- a/include/efi_loader.h > >> +++ b/include/efi_loader.h > >> @@ -412,8 +412,6 @@ efi_status_t efi_setup_loaded_image(struct > >> efi_device_path *device_path, > >> struct efi_device_path *file_path, > >> struct efi_loaded_image_obj **handle_ptr, > >> struct efi_loaded_image **info_ptr); > >> -efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, > >> - void **buffer, efi_uintn_t *size); > >> /* Print information about all loaded images */ > >> void efi_print_image_infos(void *pc); > >> > >> @@ -567,8 +565,7 @@ struct efi_load_option { > >> > >> void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); > >> unsigned long efi_serialize_load_option(struct efi_load_option *lo, > >> u8 **data); > >> -void *efi_bootmgr_load(struct efi_device_path **device_path, > >> - struct efi_device_path **file_path); > >> +efi_status_t efi_bootmgr_load(efi_handle_t *handle); > >> > >> #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ > >> > >> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > >> index 4fccadc5483d..13ec79b2098b 100644 > >> --- a/lib/efi_loader/efi_bootmgr.c > >> +++ b/lib/efi_loader/efi_bootmgr.c > >> @@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t > >> *vendor, > >> * if successful. This checks that the EFI_LOAD_OPTION is active > >> (enabled) > >> * and that the specified file to boot exists. > >> */ > >> -static void *try_load_entry(uint16_t n, struct efi_device_path > >> **device_path, > >> - struct efi_device_path **file_path) > >> +static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) > >> { > >> struct efi_load_option lo; > >> u16 varname[] = L"Boot0000"; > >> u16 hexmap[] = L"0123456789ABCDEF"; > >> - void *load_option, *image = NULL; > >> + void *load_option; > >> efi_uintn_t size; > >> + efi_status_t ret; > >> > >> varname[4] = hexmap[(n & 0xf000) >> 12]; > >> varname[5] = hexmap[(n & 0x0f00) >> 8]; > >> @@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct > >> efi_device_path **device_path, > >> > >> load_option = get_var(varname, &efi_global_variable_guid, &size); > >> if (!load_option) > >> - return NULL; > >> + return EFI_LOAD_ERROR; > >> > >> efi_deserialize_load_option(&lo, load_option); > >> > >> if (lo.attributes & LOAD_OPTION_ACTIVE) { > >> u32 attributes; > >> - efi_status_t ret; > >> > >> debug("%s: trying to load \"%ls\" from %pD\n", > >> __func__, lo.label, lo.file_path); > >> > >> - ret = efi_load_image_from_path(lo.file_path, &image, &size); > >> - > >> + ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */, > > > > In cmd/booefi.c you use efi_root. So we shall do the same here. > > > > I will fix that. No need to resend the patch. > > > > > >> + lo.file_path, NULL, 0, > >> + handle)); > >> if (ret != EFI_SUCCESS) > >> goto error; > > TODO: > > Now that you have the loaded image protocol you should update the load > > options with the value in load_option. I guess you do not want to > > overwrite it with the content of bootargs. > > > > A the problem was not introduced with this patch series: > > > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > >> > >> @@ -159,17 +159,22 @@ static void *try_load_entry(uint16_t n, struct > >> efi_device_path **device_path, > >> L"BootCurrent", > >> (efi_guid_t *)&efi_global_variable_guid, > >> attributes, size, &n)); > >> - if (ret != EFI_SUCCESS) > >> + if (ret != EFI_SUCCESS) { > >> + if (EFI_CALL(efi_unload_image(*handle)) > >> + != EFI_SUCCESS) > >> + printf("Unloading image failed\n"); > >> goto error; > >> + } > >> > >> printf("Booting: %ls\n", lo.label); > >> - efi_dp_split_file_path(lo.file_path, device_path, file_path); > >> + } else { > >> + ret = EFI_LOAD_ERROR; > >> } > >> > >> error: > >> free(load_option); > >> > >> - return image; > >> + return ret; > >> } > >> > >> /* > >> @@ -177,12 +182,10 @@ error: > >> * EFI variable, the available load-options, finding and returning > >> * the first one that can be loaded successfully. > >> */ > >> -void *efi_bootmgr_load(struct efi_device_path **device_path, > >> - struct efi_device_path **file_path) > >> +efi_status_t efi_bootmgr_load(efi_handle_t *handle) > >> { > >> u16 bootnext, *bootorder; > >> efi_uintn_t size; > >> - void *image = NULL; > >> int i, num; > >> efi_status_t ret; > >> > >> @@ -209,10 +212,9 @@ void *efi_bootmgr_load(struct efi_device_path > >> **device_path, > >> /* load BootNext */ > >> if (ret == EFI_SUCCESS) { > >> if (size == sizeof(u16)) { > >> - image = try_load_entry(bootnext, device_path, > >> - file_path); > >> - if (image) > >> - return image; > >> + ret = try_load_entry(bootnext, handle); > >> + if (ret == EFI_SUCCESS) > >> + return ret; > >> } > >> } else { > >> printf("Deleting BootNext failed\n"); > >> @@ -223,19 +225,20 @@ void *efi_bootmgr_load(struct efi_device_path > >> **device_path, > >> bootorder = get_var(L"BootOrder", &efi_global_variable_guid, > >> &size); > >> if (!bootorder) { > >> printf("BootOrder not defined\n"); > >> + ret = EFI_NOT_FOUND; > >> goto error; > >> } > >> > >> num = size / sizeof(uint16_t); > >> for (i = 0; i < num; i++) { > >> debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]); > >> - image = try_load_entry(bootorder[i], device_path, file_path); > >> - if (image) > >> + ret = try_load_entry(bootorder[i], handle); > >> + if (ret == EFI_SUCCESS) > >> break; > >> } > >> > >> free(bootorder); > >> > >> error: > >> - return image; > >> + return ret; > >> } > >> diff --git a/lib/efi_loader/efi_boottime.c > >> b/lib/efi_loader/efi_boottime.c > >> index abc295e392e9..19a58144cf4b 100644 > >> --- a/lib/efi_loader/efi_boottime.c > >> +++ b/lib/efi_loader/efi_boottime.c > >> @@ -1591,6 +1591,7 @@ failure: > >> * @size: size of the loaded image > >> * Return: status code > >> */ > >> +static > >> efi_status_t efi_load_image_from_path(struct efi_device_path > >> *file_path, > >> void **buffer, efi_uintn_t *size) > >> { > >> @@ -2664,6 +2665,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t > >> image_handle, > >> } > >> > >> current_image = image_handle; > >> + EFI_PRINT("Jumping into 0x%p\n", image_obj->entry); > >> ret = EFI_CALL(image_obj->entry(image_handle, &systab)); > >> > >> /* > >> > > > > >
On Sat, Apr 20, 2019 at 10:07:20AM +0200, Heinrich Schuchardt wrote: > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > >+ /* > >+ * FIXME: Who is responsible for > >+ * free(loaded_image_info->load_options); > >+ * Once efi_exit() is implemented correctly, > >+ * handle itself doesn't exist here. > >+ */ > > Load option can only be freed when the image is unloaded. For > applications that will happen when the application call Exit(). For > drivers there is not guarantee that they will be ever unloaded. Should we be able to utilize "bootargs" even for drivers? > In Unload() we should not free load options if they were not allocated > by us but by an EFI binary. > > In your implementation you have allocated the load options via calloc() > and not from the EFI pool. So this may allow us to determine whether we > allocated the load options in the Unload() service. DO you mean: efi_unload_image(...) ret = EFI_CALL(efi_pool_free(info->load_options)); if (ret != EFI_SUCCESS) /* * Don't care, assuming that load_options have been allocated * by application. */ It looks fragile. -Takahiro Akashi > Best regards > > Heinrich
On Sat, Apr 20, 2019 at 10:37:54AM +0200, Heinrich Schuchardt wrote: > On 4/19/19 5:22 AM, AKASHI Takahiro wrote: > >In the current implementation, bootefi command and EFI boot manager > >don't use load_image API, instead, use more primitive and internal > >functions. This will introduce duplicated code and potentially > >unknown bugs as well as inconsistent behaviours. > > > >With this patch, do_efibootmgr() and do_boot_efi() are completely > >overhauled and re-implemented using load_image API. > > > >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >--- > > cmd/bootefi.c | 187 ++++++++++++++++++---------------- > > include/efi_loader.h | 5 +- > > lib/efi_loader/efi_bootmgr.c | 43 ++++---- > > lib/efi_loader/efi_boottime.c | 2 + > > 4 files changed, 127 insertions(+), 110 deletions(-) > > > >diff --git a/cmd/bootefi.c b/cmd/bootefi.c > >index e2ca68c4dc12..8c84fff590a7 100644 > >--- a/cmd/bootefi.c > >+++ b/cmd/bootefi.c > >@@ -242,89 +242,36 @@ static efi_status_t efi_install_fdt(const char *fdt_opt) > > /** > > * 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 > >+ * @handle: handle of loaded image > > * 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) > >+static efi_status_t do_bootefi_exec(efi_handle_t handle) > > { > >- efi_handle_t mem_handle = NULL; > >- struct efi_device_path *memdp = NULL; > > efi_status_t ret; > >- struct efi_loaded_image_obj *image_obj = NULL; > >- struct efi_loaded_image *loaded_image_info = NULL; > >- > >- /* > >- * Special case for efi payload not loaded from disk, such as > >- * 'bootefi hello' or for example payload loaded directly into > >- * memory via JTAG, etc: > >- */ > >- if (!device_path && !image_path) { > >- printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); > >- /* actual addresses filled in after efi_load_pe() */ > >- memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); > >- device_path = image_path = memdp; > >- /* > >- * Grub expects that the device path of the loaded image is > >- * installed on a handle. > >- */ > >- ret = efi_create_handle(&mem_handle); > >- if (ret != EFI_SUCCESS) > >- return ret; /* TODO: leaks device_path */ > >- ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > >- device_path); > >- if (ret != EFI_SUCCESS) > >- goto err_add_protocol; > >- } else { > >- assert(device_path && image_path); > >- } > >- > >- ret = efi_setup_loaded_image(device_path, image_path, &image_obj, > >- &loaded_image_info); > >- if (ret) > >- goto err_prepare; > > > > /* Transfer environment variable as load options */ > >- ret = set_load_options((efi_handle_t)image_obj, "bootargs"); > >- if (ret != EFI_SUCCESS) > >- goto err_prepare; > >- > >- /* Load the EFI payload */ > >- ret = efi_load_pe(image_obj, efi, loaded_image_info); > >+ ret = set_load_options(handle, "bootargs"); > > if (ret != EFI_SUCCESS) > >- goto err_prepare; > >- > >- 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->end_address = mdp->start_address + > >- loaded_image_info->image_size; > >- } > >+ return ret; > > > > /* we don't support much: */ > > env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", > > "{ro,boot}(blob)0000000000000000"); > > > > /* Call our payload! */ > >- debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry); > >- ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL)); > >+ ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); > > > >-err_prepare: > >- /* image has returned, loaded-image obj goes *poof*: */ > > efi_restore_gd(); > >- free(loaded_image_info->load_options); > >- efi_delete_handle(&image_obj->header); > > > >-err_add_protocol: > >- if (mem_handle) > >- efi_delete_handle(mem_handle); > >+ /* > >+ * FIXME: Who is responsible for > >+ * free(loaded_image_info->load_options); > >+ * Once efi_exit() is implemented correctly, > >+ * handle itself doesn't exist here. > >+ */ > > > > return ret; > > } > >@@ -339,8 +286,7 @@ err_add_protocol: > > */ > > static int do_efibootmgr(const char *fdt_opt) > > { > >- struct efi_device_path *device_path, *file_path; > >- void *addr; > >+ efi_handle_t handle; > > efi_status_t ret; > > > > /* Allow unaligned memory access */ > >@@ -362,19 +308,19 @@ static int do_efibootmgr(const char *fdt_opt) > > else if (ret != EFI_SUCCESS) > > return CMD_RET_FAILURE; > > > >- addr = efi_bootmgr_load(&device_path, &file_path); > >- if (!addr) > >- return 1; > >+ ret = efi_bootmgr_load(&handle); > >+ if (ret != EFI_SUCCESS) { > >+ printf("EFI boot manager: Cannot load any image\n"); > >+ return CMD_RET_FAILURE; > >+ } > > > >- printf("## Starting EFI application at %p ...\n", addr); > >- ret = do_bootefi_exec(addr, device_path, file_path); > >- printf("## Application terminated, r = %lu\n", > >- ret & ~EFI_ERROR_MASK); > >+ ret = do_bootefi_exec(handle); > >+ printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); > > > > if (ret != EFI_SUCCESS) > >- return 1; > >+ return CMD_RET_FAILURE; > > > >- return 0; > >+ return CMD_RET_SUCCESS; > > } > > > > /* > >@@ -389,7 +335,12 @@ static int do_efibootmgr(const char *fdt_opt) > > */ > > static int do_bootefi_image(const char *image_opt, const char *fdt_opt) > > { > >- unsigned long addr; > >+ void *image_buf; > >+ struct efi_device_path *device_path, *image_path; > >+ struct efi_device_path *memdp = NULL, *file_path = NULL; > >+ unsigned long addr, size; > >+ const char *size_str; > >+ efi_handle_t mem_handle = NULL, handle; > > efi_status_t ret; > > > > /* Allow unaligned memory access */ > >@@ -414,33 +365,90 @@ static int do_bootefi_image(const char *image_opt, const char *fdt_opt) > > #ifdef CONFIG_CMD_BOOTEFI_HELLO > > if (!strcmp(image_opt, "hello")) { > > char *saddr; > >- ulong size = __efi_helloworld_end - __efi_helloworld_begin; > > > > saddr = env_get("loadaddr"); > >+ size = __efi_helloworld_end - __efi_helloworld_begin; > >+ > > if (saddr) > > addr = simple_strtoul(saddr, NULL, 16); > > else > > addr = CONFIG_SYS_LOAD_ADDR; > >- memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); > >+ > >+ image_buf = map_sysmem(addr, size); > >+ memcpy(image_buf, __efi_helloworld_begin, size); > >+ > >+ device_path = NULL; > >+ image_path = NULL; > > } else > > #endif > > { > >+ size_str = env_get("filesize"); > >+ if (size_str) > >+ size = simple_strtoul(size_str, NULL, 16); > >+ else > >+ size = 0; > >+ > > addr = simple_strtoul(image_opt, NULL, 16); > > /* Check that a numeric value was passed */ > > if (!addr && *image_opt != '0') > > return CMD_RET_USAGE; > >+ > >+ image_buf = map_sysmem(addr, size); > >+ > >+ device_path = bootefi_device_path; > >+ image_path = bootefi_image_path; > > } > > > >- printf("## Starting EFI application at %08lx ...\n", addr); > >- ret = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path, > >- bootefi_image_path); > >- printf("## Application terminated, r = %lu\n", > >- ret & ~EFI_ERROR_MASK); > >+ if (!device_path && !image_path) { > >+ /* > >+ * Special case for efi payload not loaded from disk, > >+ * such as 'bootefi hello' or for example payload > >+ * loaded directly into memory via JTAG, etc: > >+ */ > >+ memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, > >+ (uintptr_t)image_buf, size); > >+ file_path = memdp; /* append(memdp, NULL) */ > >+ > >+ /* > >+ * Make sure that device for device_path exist > >+ * in load_image(). Otherwise, shell and grub will fail. > >+ */ > >+ ret = efi_create_handle(&mem_handle); > >+ if (ret != EFI_SUCCESS) > >+ /* TODO: leaks device_path */ > >+ goto err_add_protocol; > >+ > >+ ret = efi_add_protocol(mem_handle, &efi_guid_device_path, > >+ memdp); > >+ if (ret != EFI_SUCCESS) > >+ goto err_add_protocol; > >+ } else { > >+ assert(device_path && image_path); > >+ file_path = efi_dp_append(device_path, image_path); > >+ } > >+ > >+ ret = EFI_CALL(efi_load_image(false, efi_root, > >+ file_path, image_buf, size, &handle)); > >+ if (ret != EFI_SUCCESS) > >+ goto err_prepare; > >+ > >+ ret = do_bootefi_exec(handle); > >+ printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); > >+ > >+err_prepare: > >+ if (device_path) > >+ efi_free_pool(file_path); > >+ > >+err_add_protocol: > >+ if (mem_handle) > >+ efi_delete_handle(mem_handle); > >+ if (file_path) /* hence memdp */ > >+ efi_free_pool(file_path); > > > > if (ret != EFI_SUCCESS) > > return CMD_RET_FAILURE; > >- else > >- return CMD_RET_SUCCESS; > >+ > >+ return CMD_RET_SUCCESS; > > } > > > > #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > >@@ -598,7 +606,7 @@ static char bootefi_help_text[] = > > " Use environment variable efi_selftest to select a single test.\n" > > " Use 'setenv efi_selftest list' to enumerate all tests.\n" > > #endif > >- "bootefi bootmgr [fdt addr]\n" > >+ "bootefi bootmgr [fdt address]\n" > > " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" > > "\n" > > " If specified, the device tree located at <fdt address> gets\n" > >@@ -623,6 +631,13 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path) > > ret = efi_dp_from_name(dev, devnr, path, &device, &image); > > if (ret == EFI_SUCCESS) { > > bootefi_device_path = device; > >+ if (image) { > >+ /* FIXME: image should not contain device */ > >+ struct efi_device_path *image_tmp = image; > >+ > >+ efi_dp_split_file_path(image, &device, &image); > >+ efi_free_pool(image_tmp); > >+ } > > bootefi_image_path = image; > > } else { > > bootefi_device_path = NULL; > >diff --git a/include/efi_loader.h b/include/efi_loader.h > >index 93f7672aecbc..39ed8a6fa592 100644 > >--- a/include/efi_loader.h > >+++ b/include/efi_loader.h > >@@ -412,8 +412,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > > struct efi_device_path *file_path, > > struct efi_loaded_image_obj **handle_ptr, > > struct efi_loaded_image **info_ptr); > >-efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, > >- void **buffer, efi_uintn_t *size); > > /* Print information about all loaded images */ > > void efi_print_image_infos(void *pc); > > > >@@ -567,8 +565,7 @@ struct efi_load_option { > > > > void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); > > unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); > >-void *efi_bootmgr_load(struct efi_device_path **device_path, > >- struct efi_device_path **file_path); > >+efi_status_t efi_bootmgr_load(efi_handle_t *handle); > > > > #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ > > > >diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > >index 4fccadc5483d..13ec79b2098b 100644 > >--- a/lib/efi_loader/efi_bootmgr.c > >+++ b/lib/efi_loader/efi_bootmgr.c > >@@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t *vendor, > > * if successful. This checks that the EFI_LOAD_OPTION is active (enabled) > > * and that the specified file to boot exists. > > */ > >-static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > >- struct efi_device_path **file_path) > >+static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) > > { > > struct efi_load_option lo; > > u16 varname[] = L"Boot0000"; > > u16 hexmap[] = L"0123456789ABCDEF"; > >- void *load_option, *image = NULL; > >+ void *load_option; > > efi_uintn_t size; > >+ efi_status_t ret; > > > > varname[4] = hexmap[(n & 0xf000) >> 12]; > > varname[5] = hexmap[(n & 0x0f00) >> 8]; > >@@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > > > > load_option = get_var(varname, &efi_global_variable_guid, &size); > > if (!load_option) > >- return NULL; > >+ return EFI_LOAD_ERROR; > > > > efi_deserialize_load_option(&lo, load_option); > > > > if (lo.attributes & LOAD_OPTION_ACTIVE) { > > u32 attributes; > >- efi_status_t ret; > > > > debug("%s: trying to load \"%ls\" from %pD\n", > > __func__, lo.label, lo.file_path); > > > >- ret = efi_load_image_from_path(lo.file_path, &image, &size); > >- > >+ ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */, > > In cmd/booefi.c you use efi_root. So we shall do the same here. > > I will fix that. No need to resend the patch. Thanks, but only if we need more fixes in this patch. > > >+ lo.file_path, NULL, 0, > >+ handle)); > > if (ret != EFI_SUCCESS) > > goto error; > TODO: > Now that you have the loaded image protocol you should update the load > options with the value in load_option. I guess you do not want to > overwrite it with the content of bootargs. Right, I have not noticed this issue. -Takahiro Akashi > A the problem was not introduced with this patch series: > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > > >@@ -159,17 +159,22 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > > L"BootCurrent", > > (efi_guid_t *)&efi_global_variable_guid, > > attributes, size, &n)); > >- if (ret != EFI_SUCCESS) > >+ if (ret != EFI_SUCCESS) { > >+ if (EFI_CALL(efi_unload_image(*handle)) > >+ != EFI_SUCCESS) > >+ printf("Unloading image failed\n"); > > goto error; > >+ } > > > > printf("Booting: %ls\n", lo.label); > >- efi_dp_split_file_path(lo.file_path, device_path, file_path); > >+ } else { > >+ ret = EFI_LOAD_ERROR; > > } > > > > error: > > free(load_option); > > > >- return image; > >+ return ret; > > } > > > > /* > >@@ -177,12 +182,10 @@ error: > > * EFI variable, the available load-options, finding and returning > > * the first one that can be loaded successfully. > > */ > >-void *efi_bootmgr_load(struct efi_device_path **device_path, > >- struct efi_device_path **file_path) > >+efi_status_t efi_bootmgr_load(efi_handle_t *handle) > > { > > u16 bootnext, *bootorder; > > efi_uintn_t size; > >- void *image = NULL; > > int i, num; > > efi_status_t ret; > > > >@@ -209,10 +212,9 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, > > /* load BootNext */ > > if (ret == EFI_SUCCESS) { > > if (size == sizeof(u16)) { > >- image = try_load_entry(bootnext, device_path, > >- file_path); > >- if (image) > >- return image; > >+ ret = try_load_entry(bootnext, handle); > >+ if (ret == EFI_SUCCESS) > >+ return ret; > > } > > } else { > > printf("Deleting BootNext failed\n"); > >@@ -223,19 +225,20 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, > > bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); > > if (!bootorder) { > > printf("BootOrder not defined\n"); > >+ ret = EFI_NOT_FOUND; > > goto error; > > } > > > > num = size / sizeof(uint16_t); > > for (i = 0; i < num; i++) { > > debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]); > >- image = try_load_entry(bootorder[i], device_path, file_path); > >- if (image) > >+ ret = try_load_entry(bootorder[i], handle); > >+ if (ret == EFI_SUCCESS) > > break; > > } > > > > free(bootorder); > > > > error: > >- return image; > >+ return ret; > > } > >diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > >index abc295e392e9..19a58144cf4b 100644 > >--- a/lib/efi_loader/efi_boottime.c > >+++ b/lib/efi_loader/efi_boottime.c > >@@ -1591,6 +1591,7 @@ failure: > > * @size: size of the loaded image > > * Return: status code > > */ > >+static > > efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, > > void **buffer, efi_uintn_t *size) > > { > >@@ -2664,6 +2665,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > > } > > > > current_image = image_handle; > >+ EFI_PRINT("Jumping into 0x%p\n", image_obj->entry); > > ret = EFI_CALL(image_obj->entry(image_handle, &systab)); > > > > /* > > >
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index e2ca68c4dc12..8c84fff590a7 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -242,89 +242,36 @@ static efi_status_t efi_install_fdt(const char *fdt_opt) /** * 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 + * @handle: handle of loaded image * 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) +static efi_status_t do_bootefi_exec(efi_handle_t handle) { - efi_handle_t mem_handle = NULL; - struct efi_device_path *memdp = NULL; efi_status_t ret; - struct efi_loaded_image_obj *image_obj = NULL; - struct efi_loaded_image *loaded_image_info = NULL; - - /* - * Special case for efi payload not loaded from disk, such as - * 'bootefi hello' or for example payload loaded directly into - * memory via JTAG, etc: - */ - if (!device_path && !image_path) { - printf("WARNING: using memory device/image path, this may confuse some payloads!\n"); - /* actual addresses filled in after efi_load_pe() */ - memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); - device_path = image_path = memdp; - /* - * Grub expects that the device path of the loaded image is - * installed on a handle. - */ - ret = efi_create_handle(&mem_handle); - if (ret != EFI_SUCCESS) - return ret; /* TODO: leaks device_path */ - ret = efi_add_protocol(mem_handle, &efi_guid_device_path, - device_path); - if (ret != EFI_SUCCESS) - goto err_add_protocol; - } else { - assert(device_path && image_path); - } - - ret = efi_setup_loaded_image(device_path, image_path, &image_obj, - &loaded_image_info); - if (ret) - goto err_prepare; /* Transfer environment variable as load options */ - ret = set_load_options((efi_handle_t)image_obj, "bootargs"); - if (ret != EFI_SUCCESS) - goto err_prepare; - - /* Load the EFI payload */ - ret = efi_load_pe(image_obj, efi, loaded_image_info); + ret = set_load_options(handle, "bootargs"); if (ret != EFI_SUCCESS) - goto err_prepare; - - 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->end_address = mdp->start_address + - loaded_image_info->image_size; - } + return ret; /* we don't support much: */ env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", "{ro,boot}(blob)0000000000000000"); /* Call our payload! */ - debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry); - ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL)); + ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); -err_prepare: - /* image has returned, loaded-image obj goes *poof*: */ efi_restore_gd(); - free(loaded_image_info->load_options); - efi_delete_handle(&image_obj->header); -err_add_protocol: - if (mem_handle) - efi_delete_handle(mem_handle); + /* + * FIXME: Who is responsible for + * free(loaded_image_info->load_options); + * Once efi_exit() is implemented correctly, + * handle itself doesn't exist here. + */ return ret; } @@ -339,8 +286,7 @@ err_add_protocol: */ static int do_efibootmgr(const char *fdt_opt) { - struct efi_device_path *device_path, *file_path; - void *addr; + efi_handle_t handle; efi_status_t ret; /* Allow unaligned memory access */ @@ -362,19 +308,19 @@ static int do_efibootmgr(const char *fdt_opt) else if (ret != EFI_SUCCESS) return CMD_RET_FAILURE; - addr = efi_bootmgr_load(&device_path, &file_path); - if (!addr) - return 1; + ret = efi_bootmgr_load(&handle); + if (ret != EFI_SUCCESS) { + printf("EFI boot manager: Cannot load any image\n"); + return CMD_RET_FAILURE; + } - printf("## Starting EFI application at %p ...\n", addr); - ret = do_bootefi_exec(addr, device_path, file_path); - printf("## Application terminated, r = %lu\n", - ret & ~EFI_ERROR_MASK); + ret = do_bootefi_exec(handle); + printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); if (ret != EFI_SUCCESS) - return 1; + return CMD_RET_FAILURE; - return 0; + return CMD_RET_SUCCESS; } /* @@ -389,7 +335,12 @@ static int do_efibootmgr(const char *fdt_opt) */ static int do_bootefi_image(const char *image_opt, const char *fdt_opt) { - unsigned long addr; + void *image_buf; + struct efi_device_path *device_path, *image_path; + struct efi_device_path *memdp = NULL, *file_path = NULL; + unsigned long addr, size; + const char *size_str; + efi_handle_t mem_handle = NULL, handle; efi_status_t ret; /* Allow unaligned memory access */ @@ -414,33 +365,90 @@ static int do_bootefi_image(const char *image_opt, const char *fdt_opt) #ifdef CONFIG_CMD_BOOTEFI_HELLO if (!strcmp(image_opt, "hello")) { char *saddr; - ulong size = __efi_helloworld_end - __efi_helloworld_begin; saddr = env_get("loadaddr"); + size = __efi_helloworld_end - __efi_helloworld_begin; + if (saddr) addr = simple_strtoul(saddr, NULL, 16); else addr = CONFIG_SYS_LOAD_ADDR; - memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); + + image_buf = map_sysmem(addr, size); + memcpy(image_buf, __efi_helloworld_begin, size); + + device_path = NULL; + image_path = NULL; } else #endif { + size_str = env_get("filesize"); + if (size_str) + size = simple_strtoul(size_str, NULL, 16); + else + size = 0; + addr = simple_strtoul(image_opt, NULL, 16); /* Check that a numeric value was passed */ if (!addr && *image_opt != '0') return CMD_RET_USAGE; + + image_buf = map_sysmem(addr, size); + + device_path = bootefi_device_path; + image_path = bootefi_image_path; } - printf("## Starting EFI application at %08lx ...\n", addr); - ret = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path, - bootefi_image_path); - printf("## Application terminated, r = %lu\n", - ret & ~EFI_ERROR_MASK); + if (!device_path && !image_path) { + /* + * Special case for efi payload not loaded from disk, + * such as 'bootefi hello' or for example payload + * loaded directly into memory via JTAG, etc: + */ + memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, + (uintptr_t)image_buf, size); + file_path = memdp; /* append(memdp, NULL) */ + + /* + * Make sure that device for device_path exist + * in load_image(). Otherwise, shell and grub will fail. + */ + ret = efi_create_handle(&mem_handle); + if (ret != EFI_SUCCESS) + /* TODO: leaks device_path */ + goto err_add_protocol; + + ret = efi_add_protocol(mem_handle, &efi_guid_device_path, + memdp); + if (ret != EFI_SUCCESS) + goto err_add_protocol; + } else { + assert(device_path && image_path); + file_path = efi_dp_append(device_path, image_path); + } + + ret = EFI_CALL(efi_load_image(false, efi_root, + file_path, image_buf, size, &handle)); + if (ret != EFI_SUCCESS) + goto err_prepare; + + ret = do_bootefi_exec(handle); + printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); + +err_prepare: + if (device_path) + efi_free_pool(file_path); + +err_add_protocol: + if (mem_handle) + efi_delete_handle(mem_handle); + if (file_path) /* hence memdp */ + efi_free_pool(file_path); if (ret != EFI_SUCCESS) return CMD_RET_FAILURE; - else - return CMD_RET_SUCCESS; + + return CMD_RET_SUCCESS; } #ifdef CONFIG_CMD_BOOTEFI_SELFTEST @@ -598,7 +606,7 @@ static char bootefi_help_text[] = " Use environment variable efi_selftest to select a single test.\n" " Use 'setenv efi_selftest list' to enumerate all tests.\n" #endif - "bootefi bootmgr [fdt addr]\n" + "bootefi bootmgr [fdt address]\n" " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" "\n" " If specified, the device tree located at <fdt address> gets\n" @@ -623,6 +631,13 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path) ret = efi_dp_from_name(dev, devnr, path, &device, &image); if (ret == EFI_SUCCESS) { bootefi_device_path = device; + if (image) { + /* FIXME: image should not contain device */ + struct efi_device_path *image_tmp = image; + + efi_dp_split_file_path(image, &device, &image); + efi_free_pool(image_tmp); + } bootefi_image_path = image; } else { bootefi_device_path = NULL; diff --git a/include/efi_loader.h b/include/efi_loader.h index 93f7672aecbc..39ed8a6fa592 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -412,8 +412,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, struct efi_device_path *file_path, struct efi_loaded_image_obj **handle_ptr, struct efi_loaded_image **info_ptr); -efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, - void **buffer, efi_uintn_t *size); /* Print information about all loaded images */ void efi_print_image_infos(void *pc); @@ -567,8 +565,7 @@ struct efi_load_option { void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); -void *efi_bootmgr_load(struct efi_device_path **device_path, - struct efi_device_path **file_path); +efi_status_t efi_bootmgr_load(efi_handle_t *handle); #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 4fccadc5483d..13ec79b2098b 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t *vendor, * if successful. This checks that the EFI_LOAD_OPTION is active (enabled) * and that the specified file to boot exists. */ -static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, - struct efi_device_path **file_path) +static efi_status_t try_load_entry(u16 n, efi_handle_t *handle) { struct efi_load_option lo; u16 varname[] = L"Boot0000"; u16 hexmap[] = L"0123456789ABCDEF"; - void *load_option, *image = NULL; + void *load_option; efi_uintn_t size; + efi_status_t ret; varname[4] = hexmap[(n & 0xf000) >> 12]; varname[5] = hexmap[(n & 0x0f00) >> 8]; @@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, load_option = get_var(varname, &efi_global_variable_guid, &size); if (!load_option) - return NULL; + return EFI_LOAD_ERROR; efi_deserialize_load_option(&lo, load_option); if (lo.attributes & LOAD_OPTION_ACTIVE) { u32 attributes; - efi_status_t ret; debug("%s: trying to load \"%ls\" from %pD\n", __func__, lo.label, lo.file_path); - ret = efi_load_image_from_path(lo.file_path, &image, &size); - + ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */, + lo.file_path, NULL, 0, + handle)); if (ret != EFI_SUCCESS) goto error; @@ -159,17 +159,22 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, L"BootCurrent", (efi_guid_t *)&efi_global_variable_guid, attributes, size, &n)); - if (ret != EFI_SUCCESS) + if (ret != EFI_SUCCESS) { + if (EFI_CALL(efi_unload_image(*handle)) + != EFI_SUCCESS) + printf("Unloading image failed\n"); goto error; + } printf("Booting: %ls\n", lo.label); - efi_dp_split_file_path(lo.file_path, device_path, file_path); + } else { + ret = EFI_LOAD_ERROR; } error: free(load_option); - return image; + return ret; } /* @@ -177,12 +182,10 @@ error: * EFI variable, the available load-options, finding and returning * the first one that can be loaded successfully. */ -void *efi_bootmgr_load(struct efi_device_path **device_path, - struct efi_device_path **file_path) +efi_status_t efi_bootmgr_load(efi_handle_t *handle) { u16 bootnext, *bootorder; efi_uintn_t size; - void *image = NULL; int i, num; efi_status_t ret; @@ -209,10 +212,9 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, /* load BootNext */ if (ret == EFI_SUCCESS) { if (size == sizeof(u16)) { - image = try_load_entry(bootnext, device_path, - file_path); - if (image) - return image; + ret = try_load_entry(bootnext, handle); + if (ret == EFI_SUCCESS) + return ret; } } else { printf("Deleting BootNext failed\n"); @@ -223,19 +225,20 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) { printf("BootOrder not defined\n"); + ret = EFI_NOT_FOUND; goto error; } num = size / sizeof(uint16_t); for (i = 0; i < num; i++) { debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]); - image = try_load_entry(bootorder[i], device_path, file_path); - if (image) + ret = try_load_entry(bootorder[i], handle); + if (ret == EFI_SUCCESS) break; } free(bootorder); error: - return image; + return ret; } diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index abc295e392e9..19a58144cf4b 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1591,6 +1591,7 @@ failure: * @size: size of the loaded image * Return: status code */ +static efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, void **buffer, efi_uintn_t *size) { @@ -2664,6 +2665,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, } current_image = image_handle; + EFI_PRINT("Jumping into 0x%p\n", image_obj->entry); ret = EFI_CALL(image_obj->entry(image_handle, &systab)); /*
In the current implementation, bootefi command and EFI boot manager don't use load_image API, instead, use more primitive and internal functions. This will introduce duplicated code and potentially unknown bugs as well as inconsistent behaviours. With this patch, do_efibootmgr() and do_boot_efi() are completely overhauled and re-implemented using load_image API. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- cmd/bootefi.c | 187 ++++++++++++++++++---------------- include/efi_loader.h | 5 +- lib/efi_loader/efi_bootmgr.c | 43 ++++---- lib/efi_loader/efi_boottime.c | 2 + 4 files changed, 127 insertions(+), 110 deletions(-)