[PATCH v2 1/2] Boot var automatic management for removable medias
Heinrich Schuchardt
xypron.glpk at gmx.de
Fri Apr 7 12:13:36 CEST 2023
On 4/5/23 02:06, 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.
> Related APIs are renamed and moved from cmd to lib for re-use
> between eficonfig and bootmgr.
>
> Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
We should split the patch into moving and renaming functions and
individual patches for each changed functionality.
> ---
> Changes in v2
> - Ignore EFI_NOT_FOUND returned from
> efi_bootmgr_update_media_device_boot_option which means no boot
> options scanned.
>
> cmd/bootmenu.c | 2 +-
> cmd/eficonfig.c | 408 +-----------------------------
> include/efi_config.h | 5 -
> include/efi_loader.h | 11 +
> lib/efi_loader/efi_bootmgr.c | 380 ++++++++++++++++++++++++++++
> lib/efi_loader/efi_disk.c | 7 +
> lib/efi_loader/efi_helper.c | 25 ++
> lib/efi_loader/efi_variable.c | 6 +-
> lib/efi_loader/efi_variable_tee.c | 3 +-
> 9 files changed, 437 insertions(+), 410 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);
Please, pass the number in the Boot#### variable and not an index in
BootOrder.
>
> 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..c329428973 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -347,3 +347,383 @@ 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
You can't delete what does not exist.
> + *
> + * @opt: pointer to the media boot option structure
> + * @count: number of media boot option structure
Why would you need such a parameter?
> + * Return: status code
> + */
> +static efi_status_t efi_bootmgr_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt,
> + efi_status_t count)
Why don't you pass in the boot option num
> +{
> + 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####";
Why would you initialize the variable?
> + 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);
Can't we simply put the boot option number into struct
eficonfig_media_boot_option and save 100 lines code?
> + if (ret == EFI_NOT_FOUND) {
> + log_debug("last efi var\n");
> + /*
> + * 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) {
> + log_debug("found index %d can be deleted\n", index);
> + 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++) {
> + log_debug("deleting index %d\n", delete_index_list[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)
This function should be static. The exported functions should not take
the inddex in the BootOrder variable as a parameter.
> +{
> + 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++) {
> + if (opt[i].lo)
> + free(opt[i].lo);
> + }
> + free(opt);
> + }
> + efi_free_pool(volume_handles);
> +
> + return ret;
> +}
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index d2256713a8..1309e28134 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();
Why should we rescan all devices if a single device is added?
The only place where we need the list of removable media devices is when
executing the boot manager.
> + if (ret)
> + return -1;
> + }
> +
> return 0;
> }
>
> 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)
This function should be static. The exported function to delete a boot
option should not use the index in the boot order in their interface but
the number encoded in the variable name.
We can simplify the function:
Return -1 if the entry is not found and the index otherwise.
> +{
> + u32 i;
> +
> + for (i = 0; i < num; i++) {
> + if (target == bootorder[i]) {
> + if (index)
> + *index = i;
> +
> + return true;
> + }
> + }
> +
> + return false;
> +}
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index be95ed44e6..2f251553e1 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -476,6 +476,10 @@ efi_status_t efi_init_variables(void)
> log_err("Invalid EFI variable seed\n");
> }
>
> + ret = efi_init_secure_state();
> + if (ret != EFI_SUCCESS)
> + return ret;
>
> - return efi_init_secure_state();
> + /* update boot option management after variable service initialized */
> + return efi_bootmgr_update_media_device_boot_option();
The right place to call the function might be efi_init_obj_list().
Best regards
Heinrich
> }
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index dfef18435d..a48d313ef0 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -748,5 +748,6 @@ efi_status_t efi_init_variables(void)
> if (ret != EFI_SUCCESS)
> return ret;
>
> - return EFI_SUCCESS;
> + /* update boot option management after variable service initialized */
> + return efi_bootmgr_update_media_device_boot_option();
> }
More information about the U-Boot
mailing list