Re: [PATCH v7 4/4] Load option with short device path for boot vars

Re: [PATCH v7 4/4] Load option with short device path for boot vars

[Ilias Apalodimas]

On Wed, 31 May 2023 at 20:47, Raymond Mao <raymond.mao@linaro.org> wrote:
>
> The boot variables automatically generated for removable medias
> should be with short form of device path without device nodes.
> This is a requirement for the case that a removable media is
> plugged into a different port but is still able to work with the
> existing boot variables.
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> Changes in v2
> - Ignore EFI_NOT_FOUND returned from
>   efi_bootmgr_update_media_device_boot_option which means no boot
>   options scanned.
> Changes in v3
> - Split the patch into moving and renaming functions and
>   individual patches for each changed functionality
> Changes in v4
> - Revert v2 change of introducing a bool parameter when updating
>   the boot option. Use short-form of device path by default
> Changes in v5
> - Add warning log when a short-form device path doesn't exist
> Changes in v6
> - Revert v5 change and use original device path if short-form does
>   not exist
>
>  lib/efi_loader/efi_bootmgr.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 95559ad6dd..90151f45d7 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -371,6 +371,7 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
>                 struct efi_load_option lo;
>                 char buf[BOOTMENU_DEVICE_NAME_MAX];
>                 struct efi_device_path *device_path;
> +               struct efi_device_path *short_dp;
>
>                 ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);
>                 if (ret != EFI_SUCCESS)
> @@ -387,6 +388,11 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
>                 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;
> +
>                 lo.label = dev_name;
>                 lo.attributes = LOAD_OPTION_ACTIVE;
>                 lo.file_path = device_path;
> --
> 2.25.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v7 1/4] Move bootorder and bootoption apis to lib

[Heinrich Schuchardt]

On 5/31/23 19:45, Raymond Mao wrote:
> Rename and move bootorder and bootoption apis from cmd to lib
> for re-use between eficonfig and bootmgr
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> Changes in v3
> - Split the patch into moving and renaming functions and
>    individual patches for each changed functionality
> Changes in v6
> - Revert v3 changes in function
>    efi_bootmgr_update_media_device_boot_option and
>    efi_bootmgr_delete_invalid_boot_option
>
>   cmd/bootmenu.c               |   2 +-
>   cmd/eficonfig.c              | 408 +----------------------------------
>   include/efi_config.h         |   5 -
>   include/efi_loader.h         |  11 +
>   lib/efi_loader/efi_bootmgr.c | 375 ++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_helper.c  |  25 +++
>   6 files changed, 418 insertions(+), 408 deletions(-)
>
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 6baeedc69f..01daddca7b 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -351,7 +351,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
>   		 * UEFI specification requires booting from removal media using
>   		 * a architecture-specific default image name such as BOOTAA64.EFI.
>   		 */
> -		efi_ret = eficonfig_generate_media_device_boot_option();
> +		efi_ret = efi_bootmgr_update_media_device_boot_option();
>   		if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND)
>   			goto cleanup;
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 720f52b48b..82a80306f4 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -1134,43 +1134,6 @@ out:
>   	return ret;
>   }
>
> -/**
> - * eficonfig_get_unused_bootoption() - get unused "Boot####" index
> - *
> - * @buf:	pointer to the buffer to store boot option variable name
> - * @buf_size:	buffer size
> - * @index:	pointer to store the index in the BootOrder variable
> - * Return:	status code
> - */
> -efi_status_t eficonfig_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size,
> -					     unsigned int *index)
> -{
> -	u32 i;
> -	efi_status_t ret;
> -	efi_uintn_t size;
> -
> -	if (buf_size < u16_strsize(u"Boot####"))
> -		return EFI_BUFFER_TOO_SMALL;
> -
> -	for (i = 0; i <= 0xFFFF; i++) {
> -		size = 0;
> -		efi_create_indexed_name(buf, buf_size, "Boot", i);
> -		ret = efi_get_variable_int(buf, &efi_global_variable_guid,
> -					   NULL, &size, NULL, NULL);
> -		if (ret == EFI_BUFFER_TOO_SMALL)
> -			continue;
> -		else
> -			break;
> -	}
> -
> -	if (i > 0xFFFF)
> -		return EFI_OUT_OF_RESOURCES;
> -
> -	*index = i;
> -
> -	return EFI_SUCCESS;
> -}
> -
>   /**
>    * eficonfig_set_boot_option() - set boot option
>    *
> @@ -1208,46 +1171,6 @@ static efi_status_t eficonfig_set_boot_option(u16 *varname, struct efi_device_pa
>   	return ret;
>   }
>
> -/**
> - * eficonfig_append_bootorder() - append new boot option in BootOrder variable
> - *
> - * @index:	"Boot####" index to append to BootOrder variable
> - * Return:	status code
> - */
> -efi_status_t eficonfig_append_bootorder(u16 index)
> -{
> -	u16 *bootorder;
> -	efi_status_t ret;
> -	u16 *new_bootorder = NULL;
> -	efi_uintn_t last, size, new_size;
> -
> -	/* append new boot option */
> -	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> -	last = size / sizeof(u16);
> -	new_size = size + sizeof(u16);
> -	new_bootorder = calloc(1, new_size);
> -	if (!new_bootorder) {
> -		ret = EFI_OUT_OF_RESOURCES;
> -		goto out;
> -	}
> -	memcpy(new_bootorder, bootorder, size);
> -	new_bootorder[last] = index;
> -
> -	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> -				   EFI_VARIABLE_NON_VOLATILE |
> -				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -				   EFI_VARIABLE_RUNTIME_ACCESS,
> -				   new_size, new_bootorder, false);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -out:
> -	free(bootorder);
> -	free(new_bootorder);
> -
> -	return ret;
> -}
> -
>   /**
>    * create_boot_option_entry() - create boot option entry
>    *
> @@ -1619,7 +1542,7 @@ static efi_status_t eficonfig_process_add_boot_option(void *data)
>   	if (!bo)
>   		return EFI_OUT_OF_RESOURCES;
>
> -	ret = eficonfig_get_unused_bootoption(varname, sizeof(varname), &bo->boot_index);
> +	ret = efi_bootmgr_get_unused_bootoption(varname, sizeof(varname), &bo->boot_index);
>   	if (ret != EFI_SUCCESS)
>   		return ret;
>
> @@ -1627,7 +1550,7 @@ static efi_status_t eficonfig_process_add_boot_option(void *data)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
> -	ret = eficonfig_append_bootorder((u16)bo->boot_index);
> +	ret = efi_bootmgr_append_bootorder((u16)bo->boot_index);
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
> @@ -1656,31 +1579,6 @@ static efi_status_t eficonfig_process_boot_selected(void *data)
>   	return EFI_SUCCESS;
>   }
>
> -/**
> - * search_bootorder() - search the boot option index in BootOrder
> - *
> - * @bootorder:	pointer to the BootOrder variable
> - * @num:	number of BootOrder entry
> - * @target:	target boot option index to search
> - * @index:	pointer to store the index of BootOrder variable
> - * Return:	true if exists, false otherwise
> - */
> -static bool search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index)
> -{
> -	u32 i;
> -
> -	for (i = 0; i < num; i++) {
> -		if (target == bootorder[i]) {
> -			if (index)
> -				*index = i;
> -
> -			return true;
> -		}
> -	}
> -
> -	return false;
> -}
> -
>   /**
>    * eficonfig_add_boot_selection_entry() - add boot option menu entry
>    *
> @@ -1805,7 +1703,7 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected)
>
>   		if (efi_varname_is_load_option(var_name16, &index)) {
>   			/* If the index is included in the BootOrder, skip it */
> -			if (search_bootorder(bootorder, num, index, NULL))
> +			if (efi_search_bootorder(bootorder, num, index, NULL))
>   				continue;
>
>   			ret = eficonfig_add_boot_selection_entry(efi_menu, index, selected);
> @@ -2202,7 +2100,7 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
>
>   		if (efi_varname_is_load_option(var_name16, &index)) {
>   			/* If the index is included in the BootOrder, skip it */
> -			if (search_bootorder(bootorder, num, index, NULL))
> +			if (efi_search_bootorder(bootorder, num, index, NULL))
>   				continue;
>
>   			ret = eficonfig_add_change_boot_order_entry(efi_menu, index, false);
> @@ -2304,50 +2202,6 @@ out:
>   	return ret;
>   }
>
> -/**
> - * delete_boot_option() - delete selected boot option
> - *
> - * @boot_index:	boot option index to delete
> - * Return:	status code
> - */
> -static efi_status_t delete_boot_option(u16 boot_index)
> -{
> -	u16 *bootorder;
> -	u16 varname[9];
> -	efi_status_t ret;
> -	unsigned int index;
> -	efi_uintn_t num, size;
> -
> -	efi_create_indexed_name(varname, sizeof(varname),
> -				"Boot", boot_index);
> -	ret = efi_set_variable_int(varname, &efi_global_variable_guid,
> -				   0, 0, NULL, false);
> -	if (ret != EFI_SUCCESS) {
> -		log_err("delete boot option(%ls) failed\n", varname);
> -		return ret;
> -	}
> -
> -	/* update BootOrder if necessary */
> -	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> -	if (!bootorder)
> -		return EFI_SUCCESS;
> -
> -	num = size / sizeof(u16);
> -	if (!search_bootorder(bootorder, num, boot_index, &index))
> -		return EFI_SUCCESS;
> -
> -	memmove(&bootorder[index], &bootorder[index + 1],
> -		(num - index - 1) * sizeof(u16));
> -	size -= sizeof(u16);
> -	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> -				   EFI_VARIABLE_NON_VOLATILE |
> -				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -				   EFI_VARIABLE_RUNTIME_ACCESS,
> -				   size, bootorder, false);
> -
> -	return ret;
> -}
> -
>   /**
>    * eficonfig_process_delete_boot_option() - handler to delete boot option
>    *
> @@ -2362,7 +2216,7 @@ static efi_status_t eficonfig_process_delete_boot_option(void *data)
>   	while (1) {
>   		ret = eficonfig_show_boot_selection(&selected);
>   		if (ret == EFI_SUCCESS)
> -			ret = delete_boot_option(selected);
> +			ret = efi_bootmgr_delete_boot_option(selected);
>
>   		if (ret != EFI_SUCCESS)
>   			break;
> @@ -2374,256 +2228,6 @@ static efi_status_t eficonfig_process_delete_boot_option(void *data)
>   	return ret;
>   }
>
> -/**
> - * eficonfig_enumerate_boot_option() - 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
> - * Return:		status code
> - */
> -efi_status_t eficonfig_enumerate_boot_option(struct eficonfig_media_boot_option *opt,
> -					     efi_handle_t *volume_handles, efi_status_t count)
> -{
> -	u32 i;
> -	struct efi_handler *handler;
> -	efi_status_t ret = EFI_SUCCESS;
> -
> -	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;
> -
> -		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);
> -		if (ret != EFI_SUCCESS)
> -			continue;
> -
> -		ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
> -		if (ret != EFI_SUCCESS)
> -			continue;
> -
> -		p = dev_name;
> -		utf8_utf16_strncpy(&p, buf, strlen(buf));
> -
> -		lo.label = dev_name;
> -		lo.attributes = LOAD_OPTION_ACTIVE;
> -		lo.file_path = device_path;
> -		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.
> -		 * 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;
> -		}
> -		/* set the guid */
> -		optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567"));
> -		memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t));
> -	}
> -
> -out:
> -	return ret;
> -}
> -
> -/**
> - * eficonfig_delete_invalid_boot_option() - delete non-existing boot option
> - *
> - * @opt:		pointer to the media boot option structure
> - * @count:		number of media boot option structure
> - * Return:		status code
> - */
> -efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt,
> -						  efi_status_t count)
> -{
> -	efi_uintn_t size;
> -	void *load_option;
> -	u32 i, list_size = 0;
> -	struct efi_load_option lo;
> -	u16 *var_name16 = NULL;
> -	u16 varname[] = u"Boot####";
> -	efi_status_t ret = EFI_SUCCESS;
> -	u16 *delete_index_list = NULL, *p;
> -	efi_uintn_t buf_size;
> -
> -	buf_size = 128;
> -	var_name16 = malloc(buf_size);
> -	if (!var_name16)
> -		return EFI_OUT_OF_RESOURCES;
> -
> -	var_name16[0] = 0;
> -	for (;;) {
> -		int index;
> -		efi_guid_t guid;
> -		efi_uintn_t tmp;
> -
> -		ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
> -		if (ret == EFI_NOT_FOUND) {
> -			/*
> -			 * EFI_NOT_FOUND indicates we retrieved all EFI variables.
> -			 * This should be treated as success.
> -			 */
> -			ret = EFI_SUCCESS;
> -			break;
> -		}
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -
> -		if (!efi_varname_is_load_option(var_name16, &index))
> -			continue;
> -
> -		efi_create_indexed_name(varname, sizeof(varname), "Boot", index);
> -		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> -		if (!load_option)
> -			continue;
> -
> -		tmp = size;
> -		ret = efi_deserialize_load_option(&lo, load_option, &size);
> -		if (ret != EFI_SUCCESS)
> -			goto next;
> -
> -		if (size >= sizeof(efi_guid_bootmenu_auto_generated) &&
> -		    !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated)) {
> -			for (i = 0; i < count; i++) {
> -				if (opt[i].size == tmp &&
> -				    memcmp(opt[i].lo, load_option, tmp) == 0) {
> -					opt[i].exist = true;
> -					break;
> -				}
> -			}
> -
> -			/*
> -			 * The entire list of variables must be retrieved by
> -			 * efi_get_next_variable_name_int() before deleting the invalid
> -			 * boot option, just save the index here.
> -			 */
> -			if (i == count) {
> -				p = realloc(delete_index_list, sizeof(u32) *
> -					    (list_size + 1));
> -				if (!p) {
> -					ret = EFI_OUT_OF_RESOURCES;
> -					goto out;
> -				}
> -				delete_index_list = p;
> -				delete_index_list[list_size++] = index;
> -			}
> -		}
> -next:
> -		free(load_option);
> -	}
> -
> -	/* delete all invalid boot options */
> -	for (i = 0; i < list_size; i++) {
> -		ret = delete_boot_option(delete_index_list[i]);
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -	}
> -
> -out:
> -	free(var_name16);
> -	free(delete_index_list);
> -
> -	return ret;
> -}
> -
> -/**
> - * eficonfig_generate_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 also provide the BOOT#### variable maintenance for
> - * the media device entries.
> - *   - Automatically create the BOOT#### variable for the newly detected device,
> - *     this BOOT#### variable is distinguished by the special GUID
> - *     stored in the EFI_LOAD_OPTION.optional_data
> - *   - If the device is not attached to the system, the associated BOOT#### variable
> - *     is automatically deleted.
> - *
> - * Return:	status code
> - */
> -efi_status_t eficonfig_generate_media_device_boot_option(void)
> -{
> -	u32 i;
> -	efi_status_t ret;
> -	efi_uintn_t count;
> -	efi_handle_t *volume_handles = NULL;
> -	struct eficonfig_media_boot_option *opt = NULL;
> -
> -	ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
> -					   NULL, &count, (efi_handle_t **)&volume_handles);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> -
> -	opt = calloc(count, sizeof(struct eficonfig_media_boot_option));
> -	if (!opt)
> -		goto out;
> -
> -	/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
> -	ret = eficonfig_enumerate_boot_option(opt, volume_handles, count);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -	/*
> -	 * 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 = eficonfig_delete_invalid_boot_option(opt, count);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -	/* add non-existent boot option */
> -	for (i = 0; i < count; i++) {
> -		u32 boot_index;
> -		u16 var_name[9];
> -
> -		if (!opt[i].exist) {
> -			ret = eficonfig_get_unused_bootoption(var_name, sizeof(var_name),
> -							      &boot_index);
> -			if (ret != EFI_SUCCESS)
> -				goto out;
> -
> -			ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
> -						   EFI_VARIABLE_NON_VOLATILE |
> -						   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -						   EFI_VARIABLE_RUNTIME_ACCESS,
> -						   opt[i].size, opt[i].lo, false);
> -			if (ret != EFI_SUCCESS)
> -				goto out;
> -
> -			ret = eficonfig_append_bootorder(boot_index);
> -			if (ret != EFI_SUCCESS) {
> -				efi_set_variable_int(var_name, &efi_global_variable_guid,
> -						     0, 0, NULL, false);
> -				goto out;
> -			}
> -		}
> -	}
> -
> -out:
> -	if (opt) {
> -		for (i = 0; i < count; i++)
> -			free(opt[i].lo);
> -	}
> -	free(opt);
> -	efi_free_pool(volume_handles);
> -
> -	return ret;
> -}
> -
>   /**
>    * eficonfig_init() - do required initialization for eficonfig command
>    *
> @@ -2709,7 +2313,7 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
>   	if (ret != EFI_SUCCESS)
>   		return CMD_RET_FAILURE;
>
> -	ret = eficonfig_generate_media_device_boot_option();
> +	ret = efi_bootmgr_update_media_device_boot_option();
>   	if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
>   		return ret;
>
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 01ce9b2b06..d7c1601137 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -105,11 +105,6 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu,
>   				      void (*item_data_print)(void *),
>   				      char *(*item_choice)(void *));
>   efi_status_t eficonfig_process_select_file(void *data);
> -efi_status_t eficonfig_get_unused_bootoption(u16 *buf,
> -					     efi_uintn_t buf_size, u32 *index);
> -efi_status_t eficonfig_append_bootorder(u16 index);
> -efi_status_t eficonfig_generate_media_device_boot_option(void);
> -
>   efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
>   					 char *title, eficonfig_entry_func func,
>   					 void *data);
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 1542b4b625..31ca1f5d1d 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -516,6 +516,17 @@ extern struct list_head efi_register_notify_events;
>   int efi_init_early(void);
>   /* Initialize efi execution environment */
>   efi_status_t efi_init_obj_list(void);
> +/* Append new boot option in BootOrder variable */
> +efi_status_t efi_bootmgr_append_bootorder(u16 index);
> +/* Get unused "Boot####" index */
> +efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
> +					       efi_uintn_t buf_size, u32 *index);
> +/* Generate the media device boot option */
> +efi_status_t efi_bootmgr_update_media_device_boot_option(void);
> +/* Delete selected boot option */
> +efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
> +/* search the boot option index in BootOrder */
> +bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index);
>   /* Set up console modes */
>   void efi_setup_console_size(void);
>   /* Install device tree */
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 4b24b41047..48cd5f0636 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -347,3 +347,378 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
>   error:
>   	return ret;
>   }
> +
> +/**
> + * 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
> + * @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)
> +{
> +	u32 i;
> +	struct efi_handler *handler;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	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;
> +
> +		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);
> +		if (ret != EFI_SUCCESS)
> +			continue;
> +
> +		ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
> +		if (ret != EFI_SUCCESS)
> +			continue;
> +
> +		p = dev_name;
> +		utf8_utf16_strncpy(&p, buf, strlen(buf));
> +
> +		lo.label = dev_name;
> +		lo.attributes = LOAD_OPTION_ACTIVE;
> +		lo.file_path = device_path;
> +		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.
> +		 * 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;
> +		}
> +		/* set the guid */
> +		optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567"));
> +		memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t));
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_delete_invalid_boot_option() - delete non-existing boot option
> + *
> + * @opt:		pointer to the media boot option structure
> + * @count:		number of media boot option structure
> + * Return:		status code
> + */
> +static efi_status_t efi_bootmgr_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt,
> +							   efi_status_t count)
> +{
> +	efi_uintn_t size;
> +	void *load_option;
> +	u32 i, list_size = 0;
> +	struct efi_load_option lo;
> +	u16 *var_name16 = NULL;
> +	u16 varname[] = u"Boot####";
> +	efi_status_t ret = EFI_SUCCESS;
> +	u16 *delete_index_list = NULL, *p;
> +	efi_uintn_t buf_size;
> +
> +	buf_size = 128;
> +	var_name16 = malloc(buf_size);
> +	if (!var_name16)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	var_name16[0] = 0;
> +	for (;;) {
> +		int index;
> +		efi_guid_t guid;
> +		efi_uintn_t tmp;
> +
> +		ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
> +		if (ret == EFI_NOT_FOUND) {
> +			/*
> +			 * EFI_NOT_FOUND indicates we retrieved all EFI variables.
> +			 * This should be treated as success.
> +			 */
> +			ret = EFI_SUCCESS;
> +			break;
> +		}
> +
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +
> +		if (!efi_varname_is_load_option(var_name16, &index))
> +			continue;
> +
> +		efi_create_indexed_name(varname, sizeof(varname), "Boot", index);
> +		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> +		if (!load_option)
> +			continue;
> +
> +		tmp = size;
> +		ret = efi_deserialize_load_option(&lo, load_option, &size);
> +		if (ret != EFI_SUCCESS)
> +			goto next;
> +
> +		if (size >= sizeof(efi_guid_bootmenu_auto_generated) &&
> +		    !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated)) {
> +			for (i = 0; i < count; i++) {
> +				if (opt[i].size == tmp &&
> +				    memcmp(opt[i].lo, load_option, tmp) == 0) {
> +					opt[i].exist = true;
> +					break;
> +				}
> +			}
> +
> +			/*
> +			 * The entire list of variables must be retrieved by
> +			 * efi_get_next_variable_name_int() before deleting the invalid
> +			 * boot option, just save the index here.
> +			 */
> +			if (i == count) {
> +				p = realloc(delete_index_list, sizeof(u32) *
> +					    (list_size + 1));
> +				if (!p) {
> +					ret = EFI_OUT_OF_RESOURCES;
> +					goto out;
> +				}
> +				delete_index_list = p;
> +				delete_index_list[list_size++] = index;
> +			}
> +		}
> +next:
> +		free(load_option);
> +	}
> +
> +	/* delete all invalid boot options */
> +	for (i = 0; i < list_size; i++) {
> +		ret = efi_bootmgr_delete_boot_option(delete_index_list[i]);
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
> +out:
> +	free(var_name16);
> +	free(delete_index_list);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_get_unused_bootoption() - get unused "Boot####" index
> + *
> + * @buf:	pointer to the buffer to store boot option variable name
> + * @buf_size:	buffer size
> + * @index:	pointer to store the index in the BootOrder variable
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size,
> +					       unsigned int *index)
> +{
> +	u32 i;
> +	efi_status_t ret;
> +	efi_uintn_t size;
> +
> +	if (buf_size < u16_strsize(u"Boot####"))
> +		return EFI_BUFFER_TOO_SMALL;
> +
> +	for (i = 0; i <= 0xFFFF; i++) {
> +		size = 0;
> +		efi_create_indexed_name(buf, buf_size, "Boot", i);
> +		ret = efi_get_variable_int(buf, &efi_global_variable_guid,
> +					   NULL, &size, NULL, NULL);
> +		if (ret == EFI_BUFFER_TOO_SMALL)
> +			continue;
> +		else
> +			break;
> +	}
> +
> +	if (i > 0xFFFF)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	*index = i;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_bootmgr_append_bootorder() - append new boot option in BootOrder variable
> + *
> + * @index:	"Boot####" index to append to BootOrder variable
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_append_bootorder(u16 index)
> +{
> +	u16 *bootorder;
> +	efi_status_t ret;
> +	u16 *new_bootorder = NULL;
> +	efi_uintn_t last, size, new_size;
> +
> +	/* append new boot option */
> +	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> +	last = size / sizeof(u16);
> +	new_size = size + sizeof(u16);
> +	new_bootorder = calloc(1, new_size);
> +	if (!new_bootorder) {
> +		ret = EFI_OUT_OF_RESOURCES;
> +		goto out;
> +	}
> +	memcpy(new_bootorder, bootorder, size);
> +	new_bootorder[last] = index;
> +
> +	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> +				   EFI_VARIABLE_NON_VOLATILE |
> +				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +				   EFI_VARIABLE_RUNTIME_ACCESS,
> +				   new_size, new_bootorder, false);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +out:
> +	free(bootorder);
> +	free(new_bootorder);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_delete_boot_option() - delete selected boot option
> + *
> + * @boot_index:	boot option index to delete
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index)
> +{
> +	u16 *bootorder;
> +	u16 varname[9];
> +	efi_status_t ret;
> +	unsigned int index;
> +	efi_uintn_t num, size;
> +
> +	efi_create_indexed_name(varname, sizeof(varname),
> +				"Boot", boot_index);
> +	ret = efi_set_variable_int(varname, &efi_global_variable_guid,
> +				   0, 0, NULL, false);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("delete boot option(%ls) failed\n", varname);
> +		return ret;
> +	}
> +
> +	/* update BootOrder if necessary */
> +	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> +	if (!bootorder)
> +		return EFI_SUCCESS;
> +
> +	num = size / sizeof(u16);
> +	if (!efi_search_bootorder(bootorder, num, boot_index, &index))
> +		return EFI_SUCCESS;
> +
> +	memmove(&bootorder[index], &bootorder[index + 1],
> +		(num - index - 1) * sizeof(u16));
> +	size -= sizeof(u16);
> +	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> +				   EFI_VARIABLE_NON_VOLATILE |
> +				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +				   EFI_VARIABLE_RUNTIME_ACCESS,
> +				   size, bootorder, false);
> +
> +	return ret;
> +}
> +
> +/**
> + * 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 also provide the BOOT#### variable maintenance for
> + * the media device entries.
> + *   - Automatically create the BOOT#### variable for the newly detected device,

With this patch applied to origin/next I get:

./lib/efi_loader/efi_bootmgr.c:651:Unexpected indentation.

We have
doc/api/efi.rst:44:
.. kernel-doc:: lib/efi_loader/efi_bootmgr.c

So this file goes into documentation.

Please, provide a follow up patch to fix 'make htmldocs'.
See https://u-boot.readthedocs.io/en/latest/build/documentation.html .

Best regards

Heinrich

> + *     this BOOT#### variable is distinguished by the special GUID
> + *     stored in the EFI_LOAD_OPTION.optional_data
> + *   - If the device is not attached to the system, the associated BOOT#### variable
> + *     is automatically deleted.
> + *
> + * Return:	status code
> + */
> +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;
> +	struct eficonfig_media_boot_option *opt = NULL;
> +
> +	ret = efi_locate_handle_buffer_int(BY_PROTOCOL,
> +					   &efi_simple_file_system_protocol_guid,
> +					   NULL, &count,
> +					   (efi_handle_t **)&volume_handles);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	opt = calloc(count, sizeof(struct eficonfig_media_boot_option));
> +	if (!opt)
> +		goto out;
> +
> +	/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
> +	ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	/*
> +	 * 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);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	/* add non-existent boot option */
> +	for (i = 0; i < count; i++) {
> +		u32 boot_index;
> +		u16 var_name[9];
> +
> +		if (!opt[i].exist) {
> +			ret = efi_bootmgr_get_unused_bootoption(var_name, sizeof(var_name),
> +								&boot_index);
> +			if (ret != EFI_SUCCESS)
> +				goto out;
> +
> +			ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
> +						   EFI_VARIABLE_NON_VOLATILE |
> +						   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +						   EFI_VARIABLE_RUNTIME_ACCESS,
> +						   opt[i].size, opt[i].lo, false);
> +			if (ret != EFI_SUCCESS)
> +				goto out;
> +
> +			ret = efi_bootmgr_append_bootorder(boot_index);
> +			if (ret != EFI_SUCCESS) {
> +				efi_set_variable_int(var_name, &efi_global_variable_guid,
> +						     0, 0, NULL, false);
> +				goto out;
> +			}
> +		}
> +	}
> +
> +out:
> +	if (opt) {
> +		for (i = 0; i < count; i++)
> +			free(opt[i].lo);
> +	}
> +	free(opt);
> +	efi_free_pool(volume_handles);
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> index 1f4ab2b419..cdfd16ea77 100644
> --- a/lib/efi_loader/efi_helper.c
> +++ b/lib/efi_loader/efi_helper.c
> @@ -257,3 +257,28 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *gu
>
>   	return ret;
>   }
> +
> +/**
> + * efi_search_bootorder() - search the boot option index in BootOrder
> + *
> + * @bootorder:	pointer to the BootOrder variable
> + * @num:	number of BootOrder entry
> + * @target:	target boot option index to search
> + * @index:	pointer to store the index of BootOrder variable
> + * Return:	true if exists, false otherwise
> + */
> +bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index)
> +{
> +	u32 i;
> +
> +	for (i = 0; i < num; i++) {
> +		if (target == bootorder[i]) {
> +			if (index)
> +				*index = i;
> +
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}

Re: [PATCH v7 3/4] Boot var automatic management for removable medias

[Ilias Apalodimas]

On Wed, May 31, 2023 at 10:45:17AM -0700, Raymond Mao wrote:
> Changes for complying to EFI spec §3.5.1.1
> 'Removable Media Boot Behavior'.
> Boot variables can be automatically generated during a removable
> media is probed. At the same time, unused boot variables will be
> detected and removed.
>

The code seems fine, but I think there's some things that need to be
clarified on the commit message for future reference.
Can you add a short text describing *why* you only do that in
efi_disk_probe() and not when the disk is signalled to be removed as well?
IOW mention the fact that the _remove() callback is always called when we
are exiting U-Boot trying to boot and OS, so running the same function in
remove doesn't currently make sense.

With that addition
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
> Changes in v3
> - Split the patch into moving and renaming functions and
>   individual patches for each changed functionality
> Changes in v5
> - Move function call of efi_bootmgr_update_media_device_boot_option()
>   from efi_init_variables() to efi_init_obj_list()
> Changes in v6
> - Revert unrelated changes
> Changes in v7
> - adapt the return code of function
>   efi_bootmgr_update_media_device_boot_option()
>
>  lib/efi_loader/efi_disk.c  | 7 +++++++
>  lib/efi_loader/efi_setup.c | 5 +++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index d2256713a8..d9b48ecf64 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
>  			return -1;
>  	}
>
> +	/* only do the boot option management when UEFI sub-system is initialized */
> +	if (efi_obj_list_initialized == EFI_SUCCESS) {
> +		ret = efi_bootmgr_update_media_device_boot_option();
> +		if (ret != EFI_SUCCESS)
> +			return -1;
> +	}
> +
>  	return 0;
>  }
>
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 58d4e13402..877f3878d6 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -245,6 +245,11 @@ efi_status_t efi_init_obj_list(void)
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>
> +	/* update boot option after variable service initialized */
> +	ret = efi_bootmgr_update_media_device_boot_option();
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
>  	/* Define supported languages */
>  	ret = efi_init_platform_lang();
>  	if (ret != EFI_SUCCESS)
> --
> 2.25.1
>

Re: [PATCH v7 2/4] Fix incorrect return code of boot option update

[Ilias Apalodimas]

On Wed, May 31, 2023 at 10:45:15AM -0700, Raymond Mao wrote:
> Correct the return code for out-of-memory and no boot option found
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
> Changes in v7
> - new patch file created
>
>  cmd/bootmenu.c               | 2 +-
>  cmd/eficonfig.c              | 2 +-
>  lib/efi_loader/efi_bootmgr.c | 8 ++++++--
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 01daddca7b..987b16889f 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -352,7 +352,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
>  		 * a architecture-specific default image name such as BOOTAA64.EFI.
>  		 */
>  		efi_ret = efi_bootmgr_update_media_device_boot_option();
> -		if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND)
> +		if (efi_ret != EFI_SUCCESS)
>  			goto cleanup;
>
>  		ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 82a80306f4..e6e8a0a488 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -2314,7 +2314,7 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
>  		return CMD_RET_FAILURE;
>
>  	ret = efi_bootmgr_update_media_device_boot_option();
> -	if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
> +	if (ret != EFI_SUCCESS)
>  		return ret;
>
>  	while (1) {
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 48cd5f0636..95559ad6dd 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -663,11 +663,13 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void)
>  					   NULL, &count,
>  					   (efi_handle_t **)&volume_handles);
>  	if (ret != EFI_SUCCESS)
> -		return ret;
> +		goto out;
>
>  	opt = calloc(count, sizeof(struct eficonfig_media_boot_option));
> -	if (!opt)
> +	if (!opt) {
> +		ret = EFI_OUT_OF_RESOURCES;
>  		goto out;
> +	}
>
>  	/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
>  	ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
> @@ -720,5 +722,7 @@ out:
>  	free(opt);
>  	efi_free_pool(volume_handles);
>
> +	if (ret == EFI_NOT_FOUND)
> +		return EFI_SUCCESS;
>  	return ret;
>  }
> --
> 2.25.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v7 1/4] Move bootorder and bootoption apis to lib

[Ilias Apalodimas]

On Wed, May 31, 2023 at 10:45:13AM -0700, Raymond Mao wrote:
> Rename and move bootorder and bootoption apis from cmd to lib
> for re-use between eficonfig and bootmgr
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> Changes in v3
> - Split the patch into moving and renaming functions and
>   individual patches for each changed functionality
> Changes in v6
> - Revert v3 changes in function
>   efi_bootmgr_update_media_device_boot_option and
>   efi_bootmgr_delete_invalid_boot_option
>
>  cmd/bootmenu.c               |   2 +-
>  cmd/eficonfig.c              | 408 +----------------------------------
>  include/efi_config.h         |   5 -
>  include/efi_loader.h         |  11 +
>  lib/efi_loader/efi_bootmgr.c | 375 ++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_helper.c  |  25 +++
>  6 files changed, 418 insertions(+), 408 deletions(-)
>
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 6baeedc69f..01daddca7b 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -351,7 +351,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
>  		 * UEFI specification requires booting from removal media using
>  		 * a architecture-specific default image name such as BOOTAA64.EFI.
>  		 */
> -		efi_ret = eficonfig_generate_media_device_boot_option();
> +		efi_ret = efi_bootmgr_update_media_device_boot_option();
>  		if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND)
>  			goto cleanup;
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 720f52b48b..82a80306f4 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -1134,43 +1134,6 @@ out:
>  	return ret;
>  }
>
> -/**
> - * eficonfig_get_unused_bootoption() - get unused "Boot####" index
> - *
> - * @buf:	pointer to the buffer to store boot option variable name
> - * @buf_size:	buffer size
> - * @index:	pointer to store the index in the BootOrder variable
> - * Return:	status code
> - */
> -efi_status_t eficonfig_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size,
> -					     unsigned int *index)
> -{
> -	u32 i;
> -	efi_status_t ret;
> -	efi_uintn_t size;
> -
> -	if (buf_size < u16_strsize(u"Boot####"))
> -		return EFI_BUFFER_TOO_SMALL;
> -
> -	for (i = 0; i <= 0xFFFF; i++) {
> -		size = 0;
> -		efi_create_indexed_name(buf, buf_size, "Boot", i);
> -		ret = efi_get_variable_int(buf, &efi_global_variable_guid,
> -					   NULL, &size, NULL, NULL);
> -		if (ret == EFI_BUFFER_TOO_SMALL)
> -			continue;
> -		else
> -			break;
> -	}
> -
> -	if (i > 0xFFFF)
> -		return EFI_OUT_OF_RESOURCES;
> -
> -	*index = i;
> -
> -	return EFI_SUCCESS;
> -}
> -
>  /**
>   * eficonfig_set_boot_option() - set boot option
>   *
> @@ -1208,46 +1171,6 @@ static efi_status_t eficonfig_set_boot_option(u16 *varname, struct efi_device_pa
>  	return ret;
>  }
>
> -/**
> - * eficonfig_append_bootorder() - append new boot option in BootOrder variable
> - *
> - * @index:	"Boot####" index to append to BootOrder variable
> - * Return:	status code
> - */
> -efi_status_t eficonfig_append_bootorder(u16 index)
> -{
> -	u16 *bootorder;
> -	efi_status_t ret;
> -	u16 *new_bootorder = NULL;
> -	efi_uintn_t last, size, new_size;
> -
> -	/* append new boot option */
> -	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> -	last = size / sizeof(u16);
> -	new_size = size + sizeof(u16);
> -	new_bootorder = calloc(1, new_size);
> -	if (!new_bootorder) {
> -		ret = EFI_OUT_OF_RESOURCES;
> -		goto out;
> -	}
> -	memcpy(new_bootorder, bootorder, size);
> -	new_bootorder[last] = index;
> -
> -	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> -				   EFI_VARIABLE_NON_VOLATILE |
> -				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -				   EFI_VARIABLE_RUNTIME_ACCESS,
> -				   new_size, new_bootorder, false);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -out:
> -	free(bootorder);
> -	free(new_bootorder);
> -
> -	return ret;
> -}
> -
>  /**
>   * create_boot_option_entry() - create boot option entry
>   *
> @@ -1619,7 +1542,7 @@ static efi_status_t eficonfig_process_add_boot_option(void *data)
>  	if (!bo)
>  		return EFI_OUT_OF_RESOURCES;
>
> -	ret = eficonfig_get_unused_bootoption(varname, sizeof(varname), &bo->boot_index);
> +	ret = efi_bootmgr_get_unused_bootoption(varname, sizeof(varname), &bo->boot_index);
>  	if (ret != EFI_SUCCESS)
>  		return ret;
>
> @@ -1627,7 +1550,7 @@ static efi_status_t eficonfig_process_add_boot_option(void *data)
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>
> -	ret = eficonfig_append_bootorder((u16)bo->boot_index);
> +	ret = efi_bootmgr_append_bootorder((u16)bo->boot_index);
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>
> @@ -1656,31 +1579,6 @@ static efi_status_t eficonfig_process_boot_selected(void *data)
>  	return EFI_SUCCESS;
>  }
>
> -/**
> - * search_bootorder() - search the boot option index in BootOrder
> - *
> - * @bootorder:	pointer to the BootOrder variable
> - * @num:	number of BootOrder entry
> - * @target:	target boot option index to search
> - * @index:	pointer to store the index of BootOrder variable
> - * Return:	true if exists, false otherwise
> - */
> -static bool search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index)
> -{
> -	u32 i;
> -
> -	for (i = 0; i < num; i++) {
> -		if (target == bootorder[i]) {
> -			if (index)
> -				*index = i;
> -
> -			return true;
> -		}
> -	}
> -
> -	return false;
> -}
> -
>  /**
>   * eficonfig_add_boot_selection_entry() - add boot option menu entry
>   *
> @@ -1805,7 +1703,7 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected)
>
>  		if (efi_varname_is_load_option(var_name16, &index)) {
>  			/* If the index is included in the BootOrder, skip it */
> -			if (search_bootorder(bootorder, num, index, NULL))
> +			if (efi_search_bootorder(bootorder, num, index, NULL))
>  				continue;
>
>  			ret = eficonfig_add_boot_selection_entry(efi_menu, index, selected);
> @@ -2202,7 +2100,7 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
>
>  		if (efi_varname_is_load_option(var_name16, &index)) {
>  			/* If the index is included in the BootOrder, skip it */
> -			if (search_bootorder(bootorder, num, index, NULL))
> +			if (efi_search_bootorder(bootorder, num, index, NULL))
>  				continue;
>
>  			ret = eficonfig_add_change_boot_order_entry(efi_menu, index, false);
> @@ -2304,50 +2202,6 @@ out:
>  	return ret;
>  }
>
> -/**
> - * delete_boot_option() - delete selected boot option
> - *
> - * @boot_index:	boot option index to delete
> - * Return:	status code
> - */
> -static efi_status_t delete_boot_option(u16 boot_index)
> -{
> -	u16 *bootorder;
> -	u16 varname[9];
> -	efi_status_t ret;
> -	unsigned int index;
> -	efi_uintn_t num, size;
> -
> -	efi_create_indexed_name(varname, sizeof(varname),
> -				"Boot", boot_index);
> -	ret = efi_set_variable_int(varname, &efi_global_variable_guid,
> -				   0, 0, NULL, false);
> -	if (ret != EFI_SUCCESS) {
> -		log_err("delete boot option(%ls) failed\n", varname);
> -		return ret;
> -	}
> -
> -	/* update BootOrder if necessary */
> -	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> -	if (!bootorder)
> -		return EFI_SUCCESS;
> -
> -	num = size / sizeof(u16);
> -	if (!search_bootorder(bootorder, num, boot_index, &index))
> -		return EFI_SUCCESS;
> -
> -	memmove(&bootorder[index], &bootorder[index + 1],
> -		(num - index - 1) * sizeof(u16));
> -	size -= sizeof(u16);
> -	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> -				   EFI_VARIABLE_NON_VOLATILE |
> -				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -				   EFI_VARIABLE_RUNTIME_ACCESS,
> -				   size, bootorder, false);
> -
> -	return ret;
> -}
> -
>  /**
>   * eficonfig_process_delete_boot_option() - handler to delete boot option
>   *
> @@ -2362,7 +2216,7 @@ static efi_status_t eficonfig_process_delete_boot_option(void *data)
>  	while (1) {
>  		ret = eficonfig_show_boot_selection(&selected);
>  		if (ret == EFI_SUCCESS)
> -			ret = delete_boot_option(selected);
> +			ret = efi_bootmgr_delete_boot_option(selected);
>
>  		if (ret != EFI_SUCCESS)
>  			break;
> @@ -2374,256 +2228,6 @@ static efi_status_t eficonfig_process_delete_boot_option(void *data)
>  	return ret;
>  }
>
> -/**
> - * eficonfig_enumerate_boot_option() - 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
> - * Return:		status code
> - */
> -efi_status_t eficonfig_enumerate_boot_option(struct eficonfig_media_boot_option *opt,
> -					     efi_handle_t *volume_handles, efi_status_t count)
> -{
> -	u32 i;
> -	struct efi_handler *handler;
> -	efi_status_t ret = EFI_SUCCESS;
> -
> -	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;
> -
> -		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);
> -		if (ret != EFI_SUCCESS)
> -			continue;
> -
> -		ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
> -		if (ret != EFI_SUCCESS)
> -			continue;
> -
> -		p = dev_name;
> -		utf8_utf16_strncpy(&p, buf, strlen(buf));
> -
> -		lo.label = dev_name;
> -		lo.attributes = LOAD_OPTION_ACTIVE;
> -		lo.file_path = device_path;
> -		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.
> -		 * 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;
> -		}
> -		/* set the guid */
> -		optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567"));
> -		memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t));
> -	}
> -
> -out:
> -	return ret;
> -}
> -
> -/**
> - * eficonfig_delete_invalid_boot_option() - delete non-existing boot option
> - *
> - * @opt:		pointer to the media boot option structure
> - * @count:		number of media boot option structure
> - * Return:		status code
> - */
> -efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt,
> -						  efi_status_t count)
> -{
> -	efi_uintn_t size;
> -	void *load_option;
> -	u32 i, list_size = 0;
> -	struct efi_load_option lo;
> -	u16 *var_name16 = NULL;
> -	u16 varname[] = u"Boot####";
> -	efi_status_t ret = EFI_SUCCESS;
> -	u16 *delete_index_list = NULL, *p;
> -	efi_uintn_t buf_size;
> -
> -	buf_size = 128;
> -	var_name16 = malloc(buf_size);
> -	if (!var_name16)
> -		return EFI_OUT_OF_RESOURCES;
> -
> -	var_name16[0] = 0;
> -	for (;;) {
> -		int index;
> -		efi_guid_t guid;
> -		efi_uintn_t tmp;
> -
> -		ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
> -		if (ret == EFI_NOT_FOUND) {
> -			/*
> -			 * EFI_NOT_FOUND indicates we retrieved all EFI variables.
> -			 * This should be treated as success.
> -			 */
> -			ret = EFI_SUCCESS;
> -			break;
> -		}
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -
> -		if (!efi_varname_is_load_option(var_name16, &index))
> -			continue;
> -
> -		efi_create_indexed_name(varname, sizeof(varname), "Boot", index);
> -		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> -		if (!load_option)
> -			continue;
> -
> -		tmp = size;
> -		ret = efi_deserialize_load_option(&lo, load_option, &size);
> -		if (ret != EFI_SUCCESS)
> -			goto next;
> -
> -		if (size >= sizeof(efi_guid_bootmenu_auto_generated) &&
> -		    !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated)) {
> -			for (i = 0; i < count; i++) {
> -				if (opt[i].size == tmp &&
> -				    memcmp(opt[i].lo, load_option, tmp) == 0) {
> -					opt[i].exist = true;
> -					break;
> -				}
> -			}
> -
> -			/*
> -			 * The entire list of variables must be retrieved by
> -			 * efi_get_next_variable_name_int() before deleting the invalid
> -			 * boot option, just save the index here.
> -			 */
> -			if (i == count) {
> -				p = realloc(delete_index_list, sizeof(u32) *
> -					    (list_size + 1));
> -				if (!p) {
> -					ret = EFI_OUT_OF_RESOURCES;
> -					goto out;
> -				}
> -				delete_index_list = p;
> -				delete_index_list[list_size++] = index;
> -			}
> -		}
> -next:
> -		free(load_option);
> -	}
> -
> -	/* delete all invalid boot options */
> -	for (i = 0; i < list_size; i++) {
> -		ret = delete_boot_option(delete_index_list[i]);
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -	}
> -
> -out:
> -	free(var_name16);
> -	free(delete_index_list);
> -
> -	return ret;
> -}
> -
> -/**
> - * eficonfig_generate_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 also provide the BOOT#### variable maintenance for
> - * the media device entries.
> - *   - Automatically create the BOOT#### variable for the newly detected device,
> - *     this BOOT#### variable is distinguished by the special GUID
> - *     stored in the EFI_LOAD_OPTION.optional_data
> - *   - If the device is not attached to the system, the associated BOOT#### variable
> - *     is automatically deleted.
> - *
> - * Return:	status code
> - */
> -efi_status_t eficonfig_generate_media_device_boot_option(void)
> -{
> -	u32 i;
> -	efi_status_t ret;
> -	efi_uintn_t count;
> -	efi_handle_t *volume_handles = NULL;
> -	struct eficonfig_media_boot_option *opt = NULL;
> -
> -	ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
> -					   NULL, &count, (efi_handle_t **)&volume_handles);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> -
> -	opt = calloc(count, sizeof(struct eficonfig_media_boot_option));
> -	if (!opt)
> -		goto out;
> -
> -	/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
> -	ret = eficonfig_enumerate_boot_option(opt, volume_handles, count);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -	/*
> -	 * 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 = eficonfig_delete_invalid_boot_option(opt, count);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -	/* add non-existent boot option */
> -	for (i = 0; i < count; i++) {
> -		u32 boot_index;
> -		u16 var_name[9];
> -
> -		if (!opt[i].exist) {
> -			ret = eficonfig_get_unused_bootoption(var_name, sizeof(var_name),
> -							      &boot_index);
> -			if (ret != EFI_SUCCESS)
> -				goto out;
> -
> -			ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
> -						   EFI_VARIABLE_NON_VOLATILE |
> -						   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -						   EFI_VARIABLE_RUNTIME_ACCESS,
> -						   opt[i].size, opt[i].lo, false);
> -			if (ret != EFI_SUCCESS)
> -				goto out;
> -
> -			ret = eficonfig_append_bootorder(boot_index);
> -			if (ret != EFI_SUCCESS) {
> -				efi_set_variable_int(var_name, &efi_global_variable_guid,
> -						     0, 0, NULL, false);
> -				goto out;
> -			}
> -		}
> -	}
> -
> -out:
> -	if (opt) {
> -		for (i = 0; i < count; i++)
> -			free(opt[i].lo);
> -	}
> -	free(opt);
> -	efi_free_pool(volume_handles);
> -
> -	return ret;
> -}
> -
>  /**
>   * eficonfig_init() - do required initialization for eficonfig command
>   *
> @@ -2709,7 +2313,7 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
>  	if (ret != EFI_SUCCESS)
>  		return CMD_RET_FAILURE;
>
> -	ret = eficonfig_generate_media_device_boot_option();
> +	ret = efi_bootmgr_update_media_device_boot_option();
>  	if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
>  		return ret;
>
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 01ce9b2b06..d7c1601137 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -105,11 +105,6 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu,
>  				      void (*item_data_print)(void *),
>  				      char *(*item_choice)(void *));
>  efi_status_t eficonfig_process_select_file(void *data);
> -efi_status_t eficonfig_get_unused_bootoption(u16 *buf,
> -					     efi_uintn_t buf_size, u32 *index);
> -efi_status_t eficonfig_append_bootorder(u16 index);
> -efi_status_t eficonfig_generate_media_device_boot_option(void);
> -
>  efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
>  					 char *title, eficonfig_entry_func func,
>  					 void *data);
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 1542b4b625..31ca1f5d1d 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -516,6 +516,17 @@ extern struct list_head efi_register_notify_events;
>  int efi_init_early(void);
>  /* Initialize efi execution environment */
>  efi_status_t efi_init_obj_list(void);
> +/* Append new boot option in BootOrder variable */
> +efi_status_t efi_bootmgr_append_bootorder(u16 index);
> +/* Get unused "Boot####" index */
> +efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
> +					       efi_uintn_t buf_size, u32 *index);
> +/* Generate the media device boot option */
> +efi_status_t efi_bootmgr_update_media_device_boot_option(void);
> +/* Delete selected boot option */
> +efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
> +/* search the boot option index in BootOrder */
> +bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index);
>  /* Set up console modes */
>  void efi_setup_console_size(void);
>  /* Install device tree */
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 4b24b41047..48cd5f0636 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -347,3 +347,378 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
>  error:
>  	return ret;
>  }
> +
> +/**
> + * 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
> + * @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)
> +{
> +	u32 i;
> +	struct efi_handler *handler;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	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;
> +
> +		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);
> +		if (ret != EFI_SUCCESS)
> +			continue;
> +
> +		ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
> +		if (ret != EFI_SUCCESS)
> +			continue;
> +
> +		p = dev_name;
> +		utf8_utf16_strncpy(&p, buf, strlen(buf));
> +
> +		lo.label = dev_name;
> +		lo.attributes = LOAD_OPTION_ACTIVE;
> +		lo.file_path = device_path;
> +		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.
> +		 * 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;
> +		}
> +		/* set the guid */
> +		optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567"));
> +		memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t));
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_delete_invalid_boot_option() - delete non-existing boot option
> + *
> + * @opt:		pointer to the media boot option structure
> + * @count:		number of media boot option structure
> + * Return:		status code
> + */
> +static efi_status_t efi_bootmgr_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt,
> +							   efi_status_t count)
> +{
> +	efi_uintn_t size;
> +	void *load_option;
> +	u32 i, list_size = 0;
> +	struct efi_load_option lo;
> +	u16 *var_name16 = NULL;
> +	u16 varname[] = u"Boot####";
> +	efi_status_t ret = EFI_SUCCESS;
> +	u16 *delete_index_list = NULL, *p;
> +	efi_uintn_t buf_size;
> +
> +	buf_size = 128;
> +	var_name16 = malloc(buf_size);
> +	if (!var_name16)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	var_name16[0] = 0;
> +	for (;;) {
> +		int index;
> +		efi_guid_t guid;
> +		efi_uintn_t tmp;
> +
> +		ret = efi_next_variable_name(&buf_size, &var_name16, &guid);
> +		if (ret == EFI_NOT_FOUND) {
> +			/*
> +			 * EFI_NOT_FOUND indicates we retrieved all EFI variables.
> +			 * This should be treated as success.
> +			 */
> +			ret = EFI_SUCCESS;
> +			break;
> +		}
> +
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +
> +		if (!efi_varname_is_load_option(var_name16, &index))
> +			continue;
> +
> +		efi_create_indexed_name(varname, sizeof(varname), "Boot", index);
> +		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> +		if (!load_option)
> +			continue;
> +
> +		tmp = size;
> +		ret = efi_deserialize_load_option(&lo, load_option, &size);
> +		if (ret != EFI_SUCCESS)
> +			goto next;
> +
> +		if (size >= sizeof(efi_guid_bootmenu_auto_generated) &&
> +		    !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated)) {
> +			for (i = 0; i < count; i++) {
> +				if (opt[i].size == tmp &&
> +				    memcmp(opt[i].lo, load_option, tmp) == 0) {
> +					opt[i].exist = true;
> +					break;
> +				}
> +			}
> +
> +			/*
> +			 * The entire list of variables must be retrieved by
> +			 * efi_get_next_variable_name_int() before deleting the invalid
> +			 * boot option, just save the index here.
> +			 */
> +			if (i == count) {
> +				p = realloc(delete_index_list, sizeof(u32) *
> +					    (list_size + 1));
> +				if (!p) {
> +					ret = EFI_OUT_OF_RESOURCES;
> +					goto out;
> +				}
> +				delete_index_list = p;
> +				delete_index_list[list_size++] = index;
> +			}
> +		}
> +next:
> +		free(load_option);
> +	}
> +
> +	/* delete all invalid boot options */
> +	for (i = 0; i < list_size; i++) {
> +		ret = efi_bootmgr_delete_boot_option(delete_index_list[i]);
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
> +out:
> +	free(var_name16);
> +	free(delete_index_list);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_get_unused_bootoption() - get unused "Boot####" index
> + *
> + * @buf:	pointer to the buffer to store boot option variable name
> + * @buf_size:	buffer size
> + * @index:	pointer to store the index in the BootOrder variable
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size,
> +					       unsigned int *index)
> +{
> +	u32 i;
> +	efi_status_t ret;
> +	efi_uintn_t size;
> +
> +	if (buf_size < u16_strsize(u"Boot####"))
> +		return EFI_BUFFER_TOO_SMALL;
> +
> +	for (i = 0; i <= 0xFFFF; i++) {
> +		size = 0;
> +		efi_create_indexed_name(buf, buf_size, "Boot", i);
> +		ret = efi_get_variable_int(buf, &efi_global_variable_guid,
> +					   NULL, &size, NULL, NULL);
> +		if (ret == EFI_BUFFER_TOO_SMALL)
> +			continue;
> +		else
> +			break;
> +	}
> +
> +	if (i > 0xFFFF)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	*index = i;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_bootmgr_append_bootorder() - append new boot option in BootOrder variable
> + *
> + * @index:	"Boot####" index to append to BootOrder variable
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_append_bootorder(u16 index)
> +{
> +	u16 *bootorder;
> +	efi_status_t ret;
> +	u16 *new_bootorder = NULL;
> +	efi_uintn_t last, size, new_size;
> +
> +	/* append new boot option */
> +	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> +	last = size / sizeof(u16);
> +	new_size = size + sizeof(u16);
> +	new_bootorder = calloc(1, new_size);
> +	if (!new_bootorder) {
> +		ret = EFI_OUT_OF_RESOURCES;
> +		goto out;
> +	}
> +	memcpy(new_bootorder, bootorder, size);
> +	new_bootorder[last] = index;
> +
> +	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> +				   EFI_VARIABLE_NON_VOLATILE |
> +				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +				   EFI_VARIABLE_RUNTIME_ACCESS,
> +				   new_size, new_bootorder, false);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +out:
> +	free(bootorder);
> +	free(new_bootorder);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_delete_boot_option() - delete selected boot option
> + *
> + * @boot_index:	boot option index to delete
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index)
> +{
> +	u16 *bootorder;
> +	u16 varname[9];
> +	efi_status_t ret;
> +	unsigned int index;
> +	efi_uintn_t num, size;
> +
> +	efi_create_indexed_name(varname, sizeof(varname),
> +				"Boot", boot_index);
> +	ret = efi_set_variable_int(varname, &efi_global_variable_guid,
> +				   0, 0, NULL, false);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("delete boot option(%ls) failed\n", varname);
> +		return ret;
> +	}
> +
> +	/* update BootOrder if necessary */
> +	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> +	if (!bootorder)
> +		return EFI_SUCCESS;
> +
> +	num = size / sizeof(u16);
> +	if (!efi_search_bootorder(bootorder, num, boot_index, &index))
> +		return EFI_SUCCESS;
> +
> +	memmove(&bootorder[index], &bootorder[index + 1],
> +		(num - index - 1) * sizeof(u16));
> +	size -= sizeof(u16);
> +	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> +				   EFI_VARIABLE_NON_VOLATILE |
> +				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +				   EFI_VARIABLE_RUNTIME_ACCESS,
> +				   size, bootorder, false);
> +
> +	return ret;
> +}
> +
> +/**
> + * 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 also provide the BOOT#### variable maintenance for
> + * the media device entries.
> + *   - Automatically create the BOOT#### variable for the newly detected device,
> + *     this BOOT#### variable is distinguished by the special GUID
> + *     stored in the EFI_LOAD_OPTION.optional_data
> + *   - If the device is not attached to the system, the associated BOOT#### variable
> + *     is automatically deleted.
> + *
> + * Return:	status code
> + */
> +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;
> +	struct eficonfig_media_boot_option *opt = NULL;
> +
> +	ret = efi_locate_handle_buffer_int(BY_PROTOCOL,
> +					   &efi_simple_file_system_protocol_guid,
> +					   NULL, &count,
> +					   (efi_handle_t **)&volume_handles);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	opt = calloc(count, sizeof(struct eficonfig_media_boot_option));
> +	if (!opt)
> +		goto out;
> +
> +	/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
> +	ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	/*
> +	 * 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);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	/* add non-existent boot option */
> +	for (i = 0; i < count; i++) {
> +		u32 boot_index;
> +		u16 var_name[9];
> +
> +		if (!opt[i].exist) {
> +			ret = efi_bootmgr_get_unused_bootoption(var_name, sizeof(var_name),
> +								&boot_index);
> +			if (ret != EFI_SUCCESS)
> +				goto out;
> +
> +			ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
> +						   EFI_VARIABLE_NON_VOLATILE |
> +						   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +						   EFI_VARIABLE_RUNTIME_ACCESS,
> +						   opt[i].size, opt[i].lo, false);
> +			if (ret != EFI_SUCCESS)
> +				goto out;
> +
> +			ret = efi_bootmgr_append_bootorder(boot_index);
> +			if (ret != EFI_SUCCESS) {
> +				efi_set_variable_int(var_name, &efi_global_variable_guid,
> +						     0, 0, NULL, false);
> +				goto out;
> +			}
> +		}
> +	}
> +
> +out:
> +	if (opt) {
> +		for (i = 0; i < count; i++)
> +			free(opt[i].lo);
> +	}
> +	free(opt);
> +	efi_free_pool(volume_handles);
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> index 1f4ab2b419..cdfd16ea77 100644
> --- a/lib/efi_loader/efi_helper.c
> +++ b/lib/efi_loader/efi_helper.c
> @@ -257,3 +257,28 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *gu
>
>  	return ret;
>  }
> +
> +/**
> + * efi_search_bootorder() - search the boot option index in BootOrder
> + *
> + * @bootorder:	pointer to the BootOrder variable
> + * @num:	number of BootOrder entry
> + * @target:	target boot option index to search
> + * @index:	pointer to store the index of BootOrder variable
> + * Return:	true if exists, false otherwise
> + */
> +bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index)
> +{
> +	u32 i;
> +
> +	for (i = 0; i < num; i++) {
> +		if (target == bootorder[i]) {
> +			if (index)
> +				*index = i;
> +
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> --
> 2.25.1
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v7 1/4] Move bootorder and bootoption apis to lib

[Raymond Mao]

Hi Heinrich,

> With this patch applied to origin/next I get:
>
> ./lib/efi_loader/efi_bootmgr.c:651:Unexpected indentation.
>
> We have
> doc/api/efi.rst:44:
> .. kernel-doc:: lib/efi_loader/efi_bootmgr.c
>
> So this file goes into documentation.
>
> Please, provide a follow up patch to fix 'make htmldocs'.
> See https://u-boot.readthedocs.io/en/latest/build/documentation.html .
I think it is better to put this fix into patch 'v8 1/4' if you agree,
as this 'Unexpected indentation' was introduced when function
'###_media_device_boot_option' was moved from eficonfig to efi_bootmgr

Regards,
Raymond

Re: [PATCH v7 3/4] Boot var automatic management for removable medias

[Raymond Mao]

Hi Ilias,

> The code seems fine, but I think there's some things that need to be
> clarified on the commit message for future reference.
> Can you add a short text describing *why* you only do that in
> efi_disk_probe() and not when the disk is signalled to be removed as well?
> IOW mention the fact that the _remove() callback is always called when we
> are exiting U-Boot trying to boot and OS, so running the same function in
> remove doesn't currently make sense.
Agree, I will add this text into the commit log of v8

Raymond

Re: [PATCH v7 1/4] Move bootorder and bootoption apis to lib

[Heinrich Schuchardt]

Am 6. Juni 2023 16:39:47 MESZ schrieb Raymond Mao <raymond.mao@linaro.org>:
>Hi Heinrich,
>
>> With this patch applied to origin/next I get:
>>
>> ./lib/efi_loader/efi_bootmgr.c:651:Unexpected indentation.
>>
>> We have
>> doc/api/efi.rst:44:
>> .. kernel-doc:: lib/efi_loader/efi_bootmgr.c
>>
>> So this file goes into documentation.
>>
>> Please, provide a follow up patch to fix 'make htmldocs'.
>> See https://u-boot.readthedocs.io/en/latest/build/documentation.html .
>I think it is better to put this fix into patch 'v8 1/4' if you agree,
>as this 'Unexpected indentation' was introduced when function
>'###_media_device_boot_option' was moved from eficonfig to efi_bootmgr
>
>Regards,
>Raymond


Ok with me. Please, mention it in the commit message.

Best regards

Heinrich