From 0d7634ebc13ee176f9252864c8d3408dfb07bbdd Mon Sep 17 00:00:00 2001 From: Wei Ming Chen Date: Fri, 19 Jan 2024 09:34:14 +0800 Subject: doc: uefi: remove duplicate word "has" There should be only one "has" instead of "has has" Signed-off-by: Wei Ming Chen Reviewed-by: Heinrich Schuchardt --- doc/develop/uefi/uefi.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index 6bc9d92..c739242 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -830,7 +830,7 @@ driver on a device the ConnectController service is called. In this context controller refers to the device for which the driver is installed. The relevant drivers are identified using the EFI_DRIVER_BINDING_PROTOCOL. This -protocol has has three functions: +protocol has three functions: * supported - determines if the driver is compatible with the device * start - installs the driver by opening the relevant protocol with -- cgit v1.1 From e75e4cf5d91012fa8af327177a7927daabfcf931 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 16 Jan 2024 15:00:20 +0100 Subject: part: correct documentation of part_get_bootable() We have to use 'Return:' to render the description of the return value in the HTML documentation. Fixes: f55aa4454ac3 ("part: Add a fallback for part_get_bootable()") Fixes: dcffa4428d03 ("part: Add a function to find the first bootable partition") Signed-off-by: Heinrich Schuchardt --- include/part.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/part.h b/include/part.h index db34bc6..32ee404 100644 --- a/include/part.h +++ b/include/part.h @@ -685,8 +685,8 @@ int part_get_type_by_name(const char *name); /** * part_get_bootable() - Find the first bootable partition * - * @desc: Block-device descriptor - * @return first bootable partition, or 0 if there is none + * @desc: Block-device descriptor + * Return: first bootable partition, or 0 if there is none */ int part_get_bootable(struct blk_desc *desc); -- cgit v1.1 From eb2f0867a179c4f6cef4dd67deb7fb2bf2faf923 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 12 Jan 2024 09:19:21 +0900 Subject: efi_loader: rename check_disk_has_default_file function check_disk_has_default_file() function checks if the architecture-specific default file exists on the block device, and fills the default file device path if it exists. Rename the function name to fill_default_file_path(). Signed-off-by: Masahisa Kojima Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_bootmgr.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 68d7db5..404af28 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -294,14 +294,17 @@ err: } /** - * check_disk_has_default_file() - load the default file + * fill_default_file_path() - get fallback boot device path for block device + * + * Provide the device path to the fallback UEFI boot file, e.g. + * EFI/BOOT/BOOTAA64.EFI if that file exists on the block device @blk. * * @blk: pointer to the UCLASS_BLK udevice - * @dp: pointer to default file device path + * @dp: pointer to store the fallback boot device path * Return: status code */ -static efi_status_t check_disk_has_default_file(struct udevice *blk, - struct efi_device_path **dp) +static efi_status_t fill_default_file_path(struct udevice *blk, + struct efi_device_path **dp) { efi_status_t ret; struct udevice *partition; @@ -348,7 +351,7 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size, if (!ramdisk_blk) return EFI_LOAD_ERROR; - ret = check_disk_has_default_file(ramdisk_blk, dp); + ret = fill_default_file_path(ramdisk_blk, dp); if (ret != EFI_SUCCESS) { log_info("Cannot boot from downloaded image\n"); goto err; -- cgit v1.1 From f86fba8adbf38f9520b3913880c3e6703d014d10 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 12 Jan 2024 09:19:22 +0900 Subject: efi_loader: auto-generate boot option for each blkio device Current efibootmgr auto-generates the boot option for all disks and partitions installing EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, while EDK II reference implementation auto-generates the boot option for all devices installing EFI_BLOCK_IO_PROTOCOL with eliminating logical partitions. This commit modifies the efibootmgr to get aligned to EDK II. Signed-off-by: Masahisa Kojima Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_bootmgr.c | 101 ++++++++++++++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 26 deletions(-) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 404af28..bd86f09a 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -550,6 +550,50 @@ err: } /** + * try_load_from_media() - load file from media + * + * @file_path: file path + * @handle_img: on return handle for the newly installed image + * + * If @file_path contains a file name, load the file. + * If @file_path does not have a file name, search the architecture-specific + * fallback boot file and load it. + * TODO: If the FilePathList[0] device does not support + * EFI_SIMPLE_FILE_SYSTEM_PROTOCOL but supports EFI_BLOCK_IO_PROTOCOL, + * call EFI_BOOT_SERVICES.ConnectController() + * TODO: FilePathList[0] device supports the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL + * not based on EFI_BLOCK_IO_PROTOCOL + * + * Return: status code + */ +static efi_status_t try_load_from_media(struct efi_device_path *file_path, + efi_handle_t *handle_img) +{ + efi_handle_t handle_blkdev; + efi_status_t ret = EFI_SUCCESS; + struct efi_device_path *rem, *dp = NULL; + struct efi_device_path *final_dp = file_path; + + handle_blkdev = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem); + if (handle_blkdev) { + if (rem->type == DEVICE_PATH_TYPE_END) { + /* no file name present, try default file */ + ret = fill_default_file_path(handle_blkdev->dev, &dp); + if (ret != EFI_SUCCESS) + return ret; + + final_dp = dp; + } + } + + ret = EFI_CALL(efi_load_image(true, efi_root, final_dp, NULL, 0, handle_img)); + + efi_free_pool(dp); + + return ret; +} + +/** * try_load_entry() - try to load image for boot option * * Attempt to load load-option number 'n', returning device_path and file_path @@ -583,7 +627,6 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, } if (lo.attributes & LOAD_OPTION_ACTIVE) { - struct efi_device_path *file_path; u32 attributes; log_debug("trying to load \"%ls\" from %pD\n", lo.label, @@ -600,10 +643,7 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, else ret = EFI_LOAD_ERROR; } else { - file_path = expand_media_path(lo.file_path); - ret = EFI_CALL(efi_load_image(true, efi_root, file_path, - NULL, 0, handle)); - efi_free_pool(file_path); + ret = try_load_from_media(lo.file_path, handle); } if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n", @@ -734,22 +774,23 @@ error: } /** - * efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media + * efi_bootmgr_enumerate_boot_options() - enumerate the possible bootable media * * @opt: pointer to the media boot option structure - * @volume_handles: pointer to the efi handles - * @count: number of efi handle + * @handles: pointer to block device handles + * @count: On entry number of handles to block devices. + * On exit number of boot options. * Return: status code */ -static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt, - efi_handle_t *volume_handles, - efi_status_t count) +static efi_status_t +efi_bootmgr_enumerate_boot_options(struct eficonfig_media_boot_option *opt, + efi_handle_t *handles, efi_uintn_t *count) { - u32 i; + u32 i, num = 0; struct efi_handler *handler; efi_status_t ret = EFI_SUCCESS; - for (i = 0; i < count; i++) { + for (i = 0; i < *count; i++) { u16 *p; u16 dev_name[BOOTMENU_DEVICE_NAME_MAX]; char *optional_data; @@ -757,8 +798,15 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo char buf[BOOTMENU_DEVICE_NAME_MAX]; struct efi_device_path *device_path; struct efi_device_path *short_dp; + struct efi_block_io *blkio; + + ret = efi_search_protocol(handles[i], &efi_block_io_guid, &handler); + blkio = handler->protocol_interface; - ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler); + if (blkio->media->logical_partition) + continue; + + ret = efi_search_protocol(handles[i], &efi_guid_device_path, &handler); if (ret != EFI_SUCCESS) continue; ret = efi_protocol_open(handler, (void **)&device_path, @@ -766,7 +814,7 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo if (ret != EFI_SUCCESS) continue; - ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX); + ret = efi_disk_get_device_name(handles[i], buf, BOOTMENU_DEVICE_NAME_MAX); if (ret != EFI_SUCCESS) continue; @@ -790,17 +838,20 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo * to store guid, instead of realloc the load_option. */ lo.optional_data = "1234567"; - opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); - if (!opt[i].size) { + opt[num].size = efi_serialize_load_option(&lo, (u8 **)&opt[num].lo); + if (!opt[num].size) { ret = EFI_OUT_OF_RESOURCES; goto out; } /* set the guid */ - optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567")); + optional_data = (char *)opt[num].lo + (opt[num].size - u16_strsize(u"1234567")); memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); + num++; } out: + *count = num; + return ret; } @@ -1029,8 +1080,7 @@ efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index) /** * efi_bootmgr_update_media_device_boot_option() - generate the media device boot option * - * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL - * and generate the bootmenu entries. + * This function enumerates all BlockIo devices and add the boot option for it. * This function also provide the BOOT#### variable maintenance for * the media device entries. * - Automatically create the BOOT#### variable for the newly detected device, @@ -1046,13 +1096,13 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) u32 i; efi_status_t ret; efi_uintn_t count; - efi_handle_t *volume_handles = NULL; + efi_handle_t *handles = NULL; struct eficonfig_media_boot_option *opt = NULL; ret = efi_locate_handle_buffer_int(BY_PROTOCOL, - &efi_simple_file_system_protocol_guid, + &efi_block_io_guid, NULL, &count, - (efi_handle_t **)&volume_handles); + (efi_handle_t **)&handles); if (ret != EFI_SUCCESS) goto out; @@ -1062,8 +1112,7 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) goto out; } - /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ - ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count); + ret = efi_bootmgr_enumerate_boot_options(opt, handles, &count); if (ret != EFI_SUCCESS) goto out; @@ -1111,7 +1160,7 @@ out: free(opt[i].lo); } free(opt); - efi_free_pool(volume_handles); + efi_free_pool(handles); if (ret == EFI_NOT_FOUND) return EFI_SUCCESS; -- cgit v1.1 From 3f7822bf9f03e6decb8e698fb4f9ac31d23725f7 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 12 Jan 2024 09:19:23 +0900 Subject: efi_loader: auto-generate removable media boot option first This commit auto-generates the boot option for removable block io devices followed by fixed block io devices. This is what EDK II reference implementation does. Signed-off-by: Masahisa Kojima Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_bootmgr.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index bd86f09a..4ac5192 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -777,16 +777,19 @@ error: * efi_bootmgr_enumerate_boot_options() - enumerate the possible bootable media * * @opt: pointer to the media boot option structure + * @index: index of the opt array to store the boot option * @handles: pointer to block device handles * @count: On entry number of handles to block devices. * On exit number of boot options. + * @removable: flag to parse removable only * Return: status code */ static efi_status_t efi_bootmgr_enumerate_boot_options(struct eficonfig_media_boot_option *opt, - efi_handle_t *handles, efi_uintn_t *count) + efi_uintn_t index, efi_handle_t *handles, + efi_uintn_t *count, bool removable) { - u32 i, num = 0; + u32 i, num = index; struct efi_handler *handler; efi_status_t ret = EFI_SUCCESS; @@ -806,6 +809,9 @@ efi_bootmgr_enumerate_boot_options(struct eficonfig_media_boot_option *opt, if (blkio->media->logical_partition) continue; + if (removable != (blkio->media->removable_media != 0)) + continue; + ret = efi_search_protocol(handles[i], &efi_guid_device_path, &handler); if (ret != EFI_SUCCESS) continue; @@ -847,6 +853,9 @@ efi_bootmgr_enumerate_boot_options(struct eficonfig_media_boot_option *opt, optional_data = (char *)opt[num].lo + (opt[num].size - u16_strsize(u"1234567")); memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t)); num++; + + if (num >= *count) + break; } out: @@ -1095,7 +1104,7 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) { u32 i; efi_status_t ret; - efi_uintn_t count; + efi_uintn_t count, num, total; efi_handle_t *handles = NULL; struct eficonfig_media_boot_option *opt = NULL; @@ -1112,22 +1121,32 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) goto out; } - ret = efi_bootmgr_enumerate_boot_options(opt, handles, &count); + /* parse removable block io followed by fixed block io */ + num = count; + ret = efi_bootmgr_enumerate_boot_options(opt, 0, handles, &num, true); if (ret != EFI_SUCCESS) goto out; + total = num; + num = count; + ret = efi_bootmgr_enumerate_boot_options(opt, total, handles, &num, false); + if (ret != EFI_SUCCESS) + goto out; + + total = num; + /* * System hardware configuration may vary depending on the user setup. * The boot option is automatically added by the bootmenu. * If the device is not attached to the system, the boot option needs * to be deleted. */ - ret = efi_bootmgr_delete_invalid_boot_option(opt, count); + ret = efi_bootmgr_delete_invalid_boot_option(opt, total); if (ret != EFI_SUCCESS) goto out; /* add non-existent boot option */ - for (i = 0; i < count; i++) { + for (i = 0; i < total; i++) { u32 boot_index; u16 var_name[9]; @@ -1156,7 +1175,7 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) out: if (opt) { - for (i = 0; i < count; i++) + for (i = 0; i < total; i++) free(opt[i].lo); } free(opt); -- cgit v1.1 From f674a2f9a9f9c28fddde53f0a0b2f3e3c3b342ee Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 19 Jan 2024 09:45:44 +0900 Subject: efi_loader: avoid pointer access after calling efi_delete_handle efi_delete_handle() calls efi_purge_handle(), then it finally frees the EFI handle. Both diskobj and handle variables in efi_disk_remove() have the same pointer, we can not access diskobj->dp after calling efi_delete_handle(). This commit saves the struct efi_device_path pointer before calling efi_delete_handle(). This commit also fixes the missing free for volume member in struct efi_disk_obj. This commit also removes the container_of() calls, and adds the TODO comment of missing efi_close_protocol() call for the parent EFI_BLOCK_IO_PROTOCOL. Signed-off-by: Masahisa Kojima Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_disk.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 013842f..105f080 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -707,7 +707,9 @@ int efi_disk_remove(void *ctx, struct event *event) struct udevice *dev = event->data.dm.dev; efi_handle_t handle; struct blk_desc *desc; + struct efi_device_path *dp = NULL; struct efi_disk_obj *diskobj = NULL; + struct efi_simple_file_system_protocol *volume = NULL; efi_status_t ret; if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)) @@ -718,24 +720,30 @@ int efi_disk_remove(void *ctx, struct event *event) case UCLASS_BLK: desc = dev_get_uclass_plat(dev); if (desc && desc->uclass_id != UCLASS_EFI_LOADER) - diskobj = container_of(handle, struct efi_disk_obj, - header); + diskobj = (struct efi_disk_obj *)handle; break; case UCLASS_PARTITION: - diskobj = container_of(handle, struct efi_disk_obj, header); + diskobj = (struct efi_disk_obj *)handle; + + /* TODO: closing the parent EFI_BLOCK_IO_PROTOCOL is missing. */ + break; default: return 0; } + if (diskobj) { + dp = diskobj->dp; + volume = diskobj->volume; + } + ret = efi_delete_handle(handle); /* Do not delete DM device if there are still EFI drivers attached. */ if (ret != EFI_SUCCESS) return -1; - if (diskobj) - efi_free_pool(diskobj->dp); - + efi_free_pool(dp); + free(volume); dev_tag_del(dev, DM_TAG_EFI); return 0; -- cgit v1.1 From 0351b659dd0283062d91ab0dd752887bedd53278 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 19 Jan 2024 09:45:45 +0900 Subject: efi_loader: create common function to free struct efi_disk_obj Current error handling of creating raw disk/partition has following issues. - duplicate free for EFI handle, EFI handle is already freed in efi_delete_handle() - missing free for struct efi_device_path and struct efi_simple_file_system_protocol in some error paths To address those issues, this commit creates the common function to free the struct efi_disk_obj resources and calls it in case of error. Signed-off-by: Masahisa Kojima Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_disk.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 105f080..e2edc69 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -371,6 +371,20 @@ static int efi_fs_exists(struct blk_desc *desc, int part) return 1; } +static void efi_disk_free_diskobj(struct efi_disk_obj *diskobj) +{ + struct efi_device_path *dp = diskobj->dp; + struct efi_simple_file_system_protocol *volume = diskobj->volume; + + /* + * ignore error of efi_delete_handle() since this function + * is expected to be called in error path. + */ + efi_delete_handle(&diskobj->header); + efi_free_pool(dp); + free(volume); +} + /** * efi_disk_add_dev() - create a handle for a partition or disk * @@ -528,9 +542,7 @@ static efi_status_t efi_disk_add_dev( } return EFI_SUCCESS; error: - efi_delete_handle(&diskobj->header); - free(diskobj->volume); - free(diskobj); + efi_disk_free_diskobj(diskobj); return ret; } @@ -569,8 +581,7 @@ static int efi_disk_create_raw(struct udevice *dev, efi_handle_t agent_handle) return ret; } if (efi_link_dev(&disk->header, dev)) { - efi_free_pool(disk->dp); - efi_delete_handle(&disk->header); + efi_disk_free_diskobj(disk); return -EINVAL; } @@ -624,8 +635,9 @@ static int efi_disk_create_part(struct udevice *dev, efi_handle_t agent_handle) return -1; } if (efi_link_dev(&disk->header, dev)) { - efi_free_pool(disk->dp); - efi_delete_handle(&disk->header); + efi_disk_free_diskobj(disk); + + /* TODO: closing the parent EFI_BLOCK_IO_PROTOCOL is missing. */ return -1; } -- cgit v1.1 From 2c98f7435cc5edb2a0b96e9f398c0b7f084e83cd Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 19 Jan 2024 09:45:46 +0900 Subject: efi_loader: return immediately in UCLASS_EFI_LOADER removal In case of UCLASS_EFI_LOADER, EFI handles are managed by EFI application/driver, we must not delete EFI handles. Signed-off-by: Masahisa Kojima Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_disk.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index e2edc69..b1739d9 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -731,8 +731,14 @@ int efi_disk_remove(void *ctx, struct event *event) switch (id) { case UCLASS_BLK: desc = dev_get_uclass_plat(dev); - if (desc && desc->uclass_id != UCLASS_EFI_LOADER) - diskobj = (struct efi_disk_obj *)handle; + if (desc && desc->uclass_id == UCLASS_EFI_LOADER) + /* + * EFI application/driver manages the EFI handle, + * no need to delete EFI handle. + */ + return 0; + + diskobj = (struct efi_disk_obj *)handle; break; case UCLASS_PARTITION: diskobj = (struct efi_disk_obj *)handle; @@ -744,10 +750,8 @@ int efi_disk_remove(void *ctx, struct event *event) return 0; } - if (diskobj) { - dp = diskobj->dp; - volume = diskobj->volume; - } + dp = diskobj->dp; + volume = diskobj->volume; ret = efi_delete_handle(handle); /* Do not delete DM device if there are still EFI drivers attached. */ -- cgit v1.1