[PATCH v7 4/9] efi_loader: create default file boot option

[Heinrich Schuchardt]

Am 16. Oktober 2023 08:45:21 MESZ schrieb Masahisa Kojima <masahisa.kojima at linaro.org>:
>Current efibootmgr automatically creates the
>boot options of all disks and partitions installing
>EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
>Some of the automatically created boot options are
>useless if the disk and partition does not have
>the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
>
>This commit only creates the boot option if the disk and
>partition have the default file so that system can directly
>boot from it.

I don't directly see the user benefit.

Reading all file systems will increase the boot time. Shouldn't we avoid this?

What does EDK II do?

Does the UEFI specification warrant this change?

Best regards

Heinrich

>
>Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
>---
> lib/efi_loader/efi_bootmgr.c | 86 ++++++++++++++++++++++++++----------
> 1 file changed, 63 insertions(+), 23 deletions(-)
>
>diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>index a40762c74c..c8cf1c5506 100644
>--- a/lib/efi_loader/efi_bootmgr.c
>+++ b/lib/efi_loader/efi_bootmgr.c
>@@ -355,40 +355,70 @@ error:
>  */
> 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_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;
> 		struct efi_load_option lo;
> 		char buf[BOOTMENU_DEVICE_NAME_MAX];
>-		struct efi_device_path *device_path;
>+		struct efi_device_path *device_path, *full_path, *dp, *fp;
> 		struct efi_device_path *short_dp;
>+		struct efi_file_handle *root, *f;
>+		struct efi_simple_file_system_protocol *file_system;
>+		u16 *default_file_path = NULL;
> 
>-		ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);
>+		ret = efi_search_protocol(volume_handles[i],
>+					  &efi_guid_device_path, &handler);
> 		if (ret != EFI_SUCCESS)
> 			continue;
>-		ret = efi_protocol_open(handler, (void **)&device_path,
>-					efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>+
>+		device_path = handler->protocol_interface;
>+		full_path = efi_dp_from_file(device_path,
>+					     "/EFI/BOOT/" BOOTEFI_NAME);
>+
>+		/* check whether the partition or disk have the default file */
>+		ret = efi_dp_split_file_path(full_path, &dp, &fp);
>+		if (ret != EFI_SUCCESS || !fp)
>+			goto next_entry;
>+
>+		default_file_path = efi_dp_str(fp);
>+		if (!default_file_path)
>+			goto next_entry;
>+
>+		ret = efi_search_protocol(volume_handles[i],
>+					  &efi_simple_file_system_protocol_guid,
>+					  &handler);
> 		if (ret != EFI_SUCCESS)
>-			continue;
>+			goto next_entry;
>+
>+		file_system = handler->protocol_interface;
>+		ret = EFI_CALL(file_system->open_volume(file_system, &root));
>+		if (ret != EFI_SUCCESS)
>+			goto next_entry;
>+
>+		ret = EFI_CALL(root->open(root, &f, default_file_path,
>+					  EFI_FILE_MODE_READ, 0));
>+		if (ret != EFI_SUCCESS)
>+			goto next_entry;
>+
>+		EFI_CALL(f->close(f));
> 
> 		ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
> 		if (ret != EFI_SUCCESS)
>-			continue;
>+			goto next_entry;
> 
> 		p = dev_name;
> 		utf8_utf16_strncpy(&p, buf, strlen(buf));
> 
> 		/* prefer to short form device path */
>-		short_dp = efi_dp_shorten(device_path);
>-		if (short_dp)
>-			device_path = short_dp;
>+		short_dp = efi_dp_shorten(full_path);
>+		device_path = short_dp ? short_dp : full_path;
> 
> 		lo.label = dev_name;
> 		lo.attributes = LOAD_OPTION_ACTIVE;
>@@ -396,24 +426,35 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
> 		lo.file_path_length = efi_dp_size(device_path) + sizeof(END);
> 		/*
> 		 * Set the dedicated guid to optional_data, it is used to identify
>-		 * the boot option that automatically generated by the bootmenu.
>+		 * the boot option that automatically generated by the efibootmgr.
> 		 * efi_serialize_load_option() expects optional_data is null-terminated
> 		 * utf8 string, so set the "1234567" string to allocate enough space
> 		 * 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) {
>-			ret = EFI_OUT_OF_RESOURCES;
>-			goto out;
>+		opt[num].size = efi_serialize_load_option(&lo, (u8 **)&opt[num].lo);
>+		if (!opt[num].size) {
>+			efi_free_pool(full_path);
>+			efi_free_pool(dp);
>+			efi_free_pool(fp);
>+			efi_free_pool(default_file_path);
>+			return EFI_OUT_OF_RESOURCES;
> 		}
> 		/* 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++;
>+
>+next_entry:
>+		efi_free_pool(full_path);
>+		efi_free_pool(dp);
>+		efi_free_pool(fp);
>+		efi_free_pool(default_file_path);
> 	}
> 
>-out:
>-	return ret;
>+	*count = num;
>+
>+	return EFI_SUCCESS;
> }
> 
> /**
>@@ -642,7 +683,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.
>+ * and create the boot option with default file if the file exists.
>  * This function also provide the BOOT#### variable maintenance for
>  * the media device entries.
>  * - Automatically create the BOOT#### variable for the newly detected device,
>@@ -674,8 +715,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, volume_handles, &count);
> 	if (ret != EFI_SUCCESS)
> 		goto out;
>