[PATCH 0/2] fix auto-generated boot options

This series aims to get aligned to the EDK II reference implementation for auto-generated boot options, and tries to address the following issue. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/13
This series is based on the efi-next-20231217 tag.
Masahisa Kojima (2): efi_loader: auto-generate boot option for each blkio device efi_loader: auto-generate removable media boot option first
lib/efi_loader/efi_bootmgr.c | 123 +++++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 27 deletions(-)

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 masahisa.kojima@linaro.org --- lib/efi_loader/efi_bootmgr.c | 94 +++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 56d97f2382..636f01e2eb 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -560,6 +560,50 @@ err: return ret; }
+/** + * try_load_from_media() - load file from media + * + * @fp: file path + * @handle: pointer to handle for newly installed image + * + * If @fp contains a file name, load the file. + * If @fp does not have a file name, search the architecture-specific + * default 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 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) +{ + efi_handle_t h; + efi_status_t ret = EFI_SUCCESS; + struct efi_device_path *rem, *dp = NULL; + struct efi_device_path *final_dp = file_path; + + h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem); + if (h) { + if (rem->type == DEVICE_PATH_TYPE_END) { + /* no file name present, try default file */ + ret = check_disk_has_default_file(h->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)); + + efi_free_pool(dp); + + return ret; +} + /** * try_load_entry() - try to load image for boot option * @@ -594,7 +638,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, @@ -611,10 +654,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", @@ -748,19 +788,19 @@ error: * efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media * * @opt: pointer to the media boot option structure - * @volume_handles: pointer to the efi handles + * @handles: pointer to the efi handles * @count: number of efi handle * 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) + 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; @@ -768,8 +808,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, @@ -777,7 +824,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;
@@ -801,17 +848,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; }
@@ -1040,8 +1090,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, @@ -1057,13 +1106,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;
@@ -1073,8 +1122,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_option(opt, handles, &count); if (ret != EFI_SUCCESS) goto out;
@@ -1122,7 +1170,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;

On 12/20/23 09:41, Masahisa Kojima wrote:
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 masahisa.kojima@linaro.org
lib/efi_loader/efi_bootmgr.c | 94 +++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 56d97f2382..636f01e2eb 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -560,6 +560,50 @@ err: return ret; }
+/**
- try_load_from_media() - load file from media
- @fp: file path
- @handle: pointer to handle for newly installed image
- If @fp contains a file name, load the file.
- If @fp does not have a file name, search the architecture-specific
- default 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 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)
+{
- efi_handle_t h;
- efi_status_t ret = EFI_SUCCESS;
- struct efi_device_path *rem, *dp = NULL;
- struct efi_device_path *final_dp = file_path;
- h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem);
- if (h) {
if (rem->type == DEVICE_PATH_TYPE_END) {
/* no file name present, try default file */
ret = check_disk_has_default_file(h->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));
- efi_free_pool(dp);
- return ret;
+}
- /**
- try_load_entry() - try to load image for boot option
@@ -594,7 +638,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,
@@ -611,10 +654,7 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, else ret = EFI_LOAD_ERROR;
This line does not exist in efi/next. Am I missing a prerequisite patch?
Best regards
Heinrich
} 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);
} if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n",ret = try_load_from_media(lo.file_path, handle);
@@ -748,19 +788,19 @@ error:
- efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media
- @opt: pointer to the media boot option structure
- @volume_handles: pointer to the efi handles
*/ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt,
- @handles: pointer to the efi handles
- @count: number of efi handle
- Return: status code
efi_handle_t *volume_handles,
efi_status_t count)
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;
@@ -768,8 +808,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;
if (ret != EFI_SUCCESS) continue; ret = efi_protocol_open(handler, (void **)&device_path,ret = efi_search_protocol(handles[i], &efi_guid_device_path, &handler);
@@ -777,7 +824,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);
if (ret != EFI_SUCCESS) continue;ret = efi_disk_get_device_name(handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
@@ -801,17 +848,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);
} /* set the guid */if (!opt[num].size) { ret = EFI_OUT_OF_RESOURCES; goto out;
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; }
@@ -1040,8 +1090,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,
@@ -1057,13 +1106,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);
if (ret != EFI_SUCCESS) goto out;(efi_handle_t **)&handles);
@@ -1073,8 +1122,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_option(opt, handles, &count); if (ret != EFI_SUCCESS) goto out;
@@ -1122,7 +1170,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;

On 12/25/23 12:00, Heinrich Schuchardt wrote:
On 12/20/23 09:41, Masahisa Kojima wrote:
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 masahisa.kojima@linaro.org
lib/efi_loader/efi_bootmgr.c | 94 +++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 56d97f2382..636f01e2eb 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -560,6 +560,50 @@ err: return ret; }
+/**
- try_load_from_media() - load file from media
- @fp: file path
- @handle: pointer to handle for newly installed image
- If @fp contains a file name, load the file.
- If @fp does not have a file name, search the architecture-specific
- default 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 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) +{ + efi_handle_t h; + efi_status_t ret = EFI_SUCCESS; + struct efi_device_path *rem, *dp = NULL; + struct efi_device_path *final_dp = file_path;
+ h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem); + if (h) { + if (rem->type == DEVICE_PATH_TYPE_END) { + /* no file name present, try default file */ + ret = check_disk_has_default_file(h->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));
+ efi_free_pool(dp);
+ return ret; +}
/** * try_load_entry() - try to load image for boot option * @@ -594,7 +638,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, @@ -611,10 +654,7 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, else ret = EFI_LOAD_ERROR;
This line does not exist in efi/next. Am I missing a prerequisite patch?
My fault.
Best regards
Heinrich
} 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", @@ -748,19 +788,19 @@ error: * efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media * * @opt: pointer to the media boot option structure
- @volume_handles: pointer to the efi handles
- @handles: pointer to the efi handles
* @count: number of efi handle * 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) + 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; @@ -768,8 +808,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, @@ -777,7 +824,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;
@@ -801,17 +848,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; }
@@ -1040,8 +1090,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, @@ -1057,13 +1106,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;
@@ -1073,8 +1122,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_option(opt, handles, &count); if (ret != EFI_SUCCESS) goto out;
@@ -1122,7 +1170,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;

On 12/20/23 09:41, Masahisa Kojima wrote:
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 masahisa.kojima@linaro.org
lib/efi_loader/efi_bootmgr.c | 94 +++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 56d97f2382..636f01e2eb 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -560,6 +560,50 @@ err: return ret; }
+/**
- try_load_from_media() - load file from media
- @fp: file path
This does not match the parameter name of the function leading to a 'make htmldocs' build error.
- @handle: pointer to handle for newly installed image
- If @fp contains a file name, load the file.
- If @fp does not have a file name, search the architecture-specific
ditto
Best regards
Heinrich
- default 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 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)
+{
- efi_handle_t h;
- efi_status_t ret = EFI_SUCCESS;
- struct efi_device_path *rem, *dp = NULL;
- struct efi_device_path *final_dp = file_path;
- h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem);
- if (h) {
if (rem->type == DEVICE_PATH_TYPE_END) {
/* no file name present, try default file */
ret = check_disk_has_default_file(h->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));
- efi_free_pool(dp);
- return ret;
+}
- /**
- try_load_entry() - try to load image for boot option
@@ -594,7 +638,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,
@@ -611,10 +654,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);
} if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n",ret = try_load_from_media(lo.file_path, handle);
@@ -748,19 +788,19 @@ error:
- efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media
- @opt: pointer to the media boot option structure
- @volume_handles: pointer to the efi handles
*/ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt,
- @handles: pointer to the efi handles
- @count: number of efi handle
- Return: status code
efi_handle_t *volume_handles,
efi_status_t count)
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;
@@ -768,8 +808,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;
if (ret != EFI_SUCCESS) continue; ret = efi_protocol_open(handler, (void **)&device_path,ret = efi_search_protocol(handles[i], &efi_guid_device_path, &handler);
@@ -777,7 +824,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);
if (ret != EFI_SUCCESS) continue;ret = efi_disk_get_device_name(handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
@@ -801,17 +848,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);
} /* set the guid */if (!opt[num].size) { ret = EFI_OUT_OF_RESOURCES; goto out;
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; }
@@ -1040,8 +1090,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,
@@ -1057,13 +1106,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);
if (ret != EFI_SUCCESS) goto out;(efi_handle_t **)&handles);
@@ -1073,8 +1122,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_option(opt, handles, &count); if (ret != EFI_SUCCESS) goto out;
@@ -1122,7 +1170,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;

On Mon, 25 Dec 2023 at 21:22, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/20/23 09:41, Masahisa Kojima wrote:
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 masahisa.kojima@linaro.org
lib/efi_loader/efi_bootmgr.c | 94 +++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 56d97f2382..636f01e2eb 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -560,6 +560,50 @@ err: return ret; }
+/**
- try_load_from_media() - load file from media
- @fp: file path
This does not match the parameter name of the function leading to a 'make htmldocs' build error.
Sorry, I will fix it.
- @handle: pointer to handle for newly installed image
- If @fp contains a file name, load the file.
- If @fp does not have a file name, search the architecture-specific
ditto
OK.
Thanks, Masahisa Kojima
Best regards
Heinrich
- default 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 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)
+{
efi_handle_t h;
efi_status_t ret = EFI_SUCCESS;
struct efi_device_path *rem, *dp = NULL;
struct efi_device_path *final_dp = file_path;
h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem);
if (h) {
if (rem->type == DEVICE_PATH_TYPE_END) {
/* no file name present, try default file */
ret = check_disk_has_default_file(h->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));
efi_free_pool(dp);
return ret;
+}
- /**
- try_load_entry() - try to load image for boot option
@@ -594,7 +638,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,
@@ -611,10 +654,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",
@@ -748,19 +788,19 @@ error:
- efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media
- @opt: pointer to the media boot option structure
- @volume_handles: pointer to the efi handles
*/ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt,
- @handles: pointer to the efi handles
- @count: number of efi handle
- Return: status code
efi_handle_t *volume_handles,
efi_status_t count)
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;
@@ -768,8 +808,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,
@@ -777,7 +824,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;
@@ -801,17 +848,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;
}
@@ -1040,8 +1090,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,
@@ -1057,13 +1106,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;
@@ -1073,8 +1122,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_option(opt, handles, &count); if (ret != EFI_SUCCESS) goto out;
@@ -1122,7 +1170,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;

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 masahisa.kojima@linaro.org --- lib/efi_loader/efi_bootmgr.c | 37 ++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 636f01e2eb..6b02abcf0d 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -584,7 +584,6 @@ static efi_status_t try_load_from_media(struct efi_device_path *file_path, efi_status_t ret = EFI_SUCCESS; struct efi_device_path *rem, *dp = NULL; struct efi_device_path *final_dp = file_path; - h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem); if (h) { if (rem->type == DEVICE_PATH_TYPE_END) { @@ -788,15 +787,19 @@ error: * efi_bootmgr_enumerate_boot_option() - 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 the efi handles * @count: number of efi handle + * @removable: flag to parse removable only * Return: status code */ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt, + efi_uintn_t index, efi_handle_t *handles, - efi_uintn_t *count) + efi_uintn_t *count, + bool removable) { - u32 i, num = 0; + u32 i, num = index; struct efi_handler *handler; efi_status_t ret = EFI_SUCCESS;
@@ -810,12 +813,20 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo struct efi_device_path *short_dp; struct efi_block_io *blkio;
+ if (num >= *count) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + ret = efi_search_protocol(handles[i], &efi_block_io_guid, &handler); blkio = handler->protocol_interface;
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; @@ -1105,7 +1116,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;
@@ -1122,22 +1133,32 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) goto out; }
- ret = efi_bootmgr_enumerate_boot_option(opt, handles, &count); + /* parse removable block io followed by fixed block io */ + num = count; + ret = efi_bootmgr_enumerate_boot_option(opt, 0, handles, &num, true); if (ret != EFI_SUCCESS) goto out;
+ total = num; + num = count; + ret = efi_bootmgr_enumerate_boot_option(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];
@@ -1166,7 +1187,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);

On 12/20/23 09:41, Masahisa Kojima wrote:
This series aims to get aligned to the EDK II reference implementation for auto-generated boot options, and tries to address the following issue. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/13
This series is based on the efi-next-20231217 tag.
Masahisa Kojima (2): efi_loader: auto-generate boot option for each blkio device efi_loader: auto-generate removable media boot option first
lib/efi_loader/efi_bootmgr.c | 123 +++++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 27 deletions(-)
Hello Masahisa,
please, have a look at the test failures in https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19089
Best regards
Heinrich

On Tue, 26 Dec 2023 at 02:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/20/23 09:41, Masahisa Kojima wrote:
This series aims to get aligned to the EDK II reference implementation for auto-generated boot options, and tries to address the following issue. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/13
This series is based on the efi-next-20231217 tag.
Masahisa Kojima (2): efi_loader: auto-generate boot option for each blkio device efi_loader: auto-generate removable media boot option first
lib/efi_loader/efi_bootmgr.c | 123 +++++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 27 deletions(-)
Hello Masahisa,
please, have a look at the test failures in https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19089
I fixed the error, v2 will be sent shortly.
Thanks, Masahis Kojima
Best regards
Heinrich
participants (2)
-
Heinrich Schuchardt
-
Masahisa Kojima