Hi Heinrich,

On Fri, 26 May 2023 at 03:18, Heinrich Schuchardt <xypron.g...@gmx.de>
wrote:

> On 5/26/23 00:04, 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....@linaro.org>
>
> The commit message says that this patch does not change the code!
> But there are code changes, e.g.
>
> <               for (i = 0; i < count; i++)
> <                       free(opt[i].lo);
> ---
>  >               for (i = 0; i < count; i++) {
>  >                       if (opt[i].lo)
>  >                               free(opt[i].lo);
>  >               }
>
> Please, provide a patch that only renames and moves the functions as
> indicated in the commit message. Please, avoid reformatting as that
> makes checking the diff hard.
>
> Besides, the 'if' that you introduced is superfluous as free() already
> checks for NULL.
>
[RM] It is a leftover line during debugging. I will clean it all up.

>
> > ---
> > 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 the change of introducing a bool parameter when updating
> >    the boot option. Use short-form of device path by default
> > Changes in v5
> > - Move function call of efi_bootmgr_update_media_device_boot_option()
> >    from efi_init_variables() to efi_init_obj_list()
> > - Add warning log when a short-form device path doesn't exist
>
> This change history does not match changes in patch 1/3. Please, use a
> cover-letter to describe the series and overall changes.
>
[RM] I catched all changes of the complete patch set here. I will separate
them
for each patch respectively.

>
> >
> >   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_helper.c  |  25 +++
> >   6 files changed, 423 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..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"));
>
>
> We should replace 'u16_strsize(u"1234567")' by 'sizeof(efi_guid_t)' in a
> follow-up patch to avoid the superfluous function call.
>
[RM] Will address in follow-up patch.

>
> Best regards
>
> Heinrich
>
> > +             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) {
> > +                     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)
> > +{
> > +     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_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;
> > +}
>
>

Reply via email to