Hi Kojima-san

On Wed, Aug 24, 2022 at 03:37:36PM +0900, Masahisa Kojima wrote:
> UEFI specification requires booting from removal media using
> a architecture-specific default image name such as BOOTAA64.EFI.
> This commit adds the removable media entries into bootmenu,
> so that user can select the removable media and boot with
> default image.
> 
> The bootmenu automatically enumerates the possible bootable
> media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL,
> add it as new UEFI boot option(BOOT####) and update BootOrder
> variable. This automatically generated UEFI boot option
> has the dedicated guid in the optional_data to distinguish it from
> the UEFI boot option user adds manually. This optional_data is
> removed when the efi bootmgr loads the selected UEFI boot option.
> 
> This commit also provides the BOOT#### variable maintenance feature.
> Depending on the system hardware setup, some devices
> may not exist at a later system boot, so bootmenu checks the
> available device in each bootmenu invocation and automatically
> removes the BOOT#### variable corrensponding to the non-existent
> media device.
> 

Maybe I am doing something wrong here and this has been discussed in the
past, but here's what I get by testing this.

- The automatic scanning of boot options seems to be happening only when 
  "Change boot order" menu is selected.  Isn't it better to do it on
  eficonfig startup ?
- Although I can see a variable Boot0000 which holds the device path of the
  virtio disk that has the BOOTAA64.EFI file, I can't see that option in
  the edit/delete menus
 
Thanks
/Ilias
> Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> ---
> Changes in v13:
> - remove BootOrder variable dependency
> 
> Changes in v12:
> - move generate_media_device_boot_option into cmd/eficonfig.c and expose it
> - remove unnecessary include file
> 
> Changes in v11:
> - update delete_boot_option() parameter
> 
> Changes in v10:
> - add function comment
> - devname dynamic allocation removes, allocate in stack
> - delete BOOT#### when updating BootOrder fails
> 
> Changes in v9:
> - update efi_disk_get_device_name() parameter to pass efi_handle_t
> - add function comment
> 
> Changes in v8:
> - function and structure prefix is changed to "eficonfig"
> 
> Changes in v7:
> - rename prepare_media_device_entry() to generate_media_device_boot_option()
> 
> Changes in v6:
> - optional_data size is changed to 16bytes
> - check the load option size before comparison
> - remove guid included in optional_data of auto generated
>   entry when loading
> 
> Changes in v5:
> - Return EFI_SUCCESS if there is no BootOrder defined
> - correctly handle the case if no removable device found
> - use guid to identify the automatically generated entry by bootmenu
> 
>  cmd/bootmenu.c               |  22 +++-
>  cmd/eficonfig.c              | 209 +++++++++++++++++++++++++++++++++++
>  include/efi_config.h         |   1 +
>  include/efi_loader.h         |  16 +++
>  lib/efi_loader/efi_bootmgr.c |   4 +
>  5 files changed, 246 insertions(+), 6 deletions(-)
> 
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 704d36debe..3340be1632 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -7,7 +7,7 @@
>  #include <common.h>
>  #include <command.h>
>  #include <ansi.h>
> -#include <efi_loader.h>
> +#include <efi_config.h>
>  #include <efi_variable.h>
>  #include <env.h>
>  #include <log.h>
> @@ -220,7 +220,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data 
> *menu,
>       return 1;
>  }
>  
> -#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR))
> +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) && 
> (CONFIG_IS_ENABLED(CMD_EFICONFIG))
>  /**
>   * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries
>   *
> @@ -340,11 +340,21 @@ static struct bootmenu_data *bootmenu_create(int delay)
>       if (ret < 0)
>               goto cleanup;
>  
> -#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR))
> +#if (CONFIG_IS_ENABLED(CMD_BOOTEFI_BOOTMGR)) && 
> (CONFIG_IS_ENABLED(CMD_EFICONFIG))
>       if (i < MAX_COUNT - 1) {
> -                     ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
> -                     if (ret < 0 && ret != -ENOENT)
> -                             goto cleanup;
> +             efi_status_t efi_ret;
> +
> +             /*
> +              * 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();
> +             if (efi_ret != EFI_SUCCESS && efi_ret != EFI_NOT_FOUND)
> +                     goto cleanup;
> +
> +             ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
> +             if (ret < 0 && ret != -ENOENT)
> +                     goto cleanup;
>       }
>  #endif
>  
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 537f3f2bbc..92171c4894 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -1786,6 +1786,215 @@ 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;
> +                     free(dev_name);
> +                     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)
> +{
> +     u32 i, j;
> +     efi_uintn_t size;
> +     efi_status_t ret;
> +     void *load_option;
> +     struct efi_load_option lo;
> +     u16 varname[] = u"Boot####";
> +
> +     for (i = 0; i <= 0xFFFF; i++) {
> +             efi_uintn_t tmp;
> +
> +             efi_create_indexed_name(varname, sizeof(varname), "Boot", i);
> +             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)) {
> +                     if (guidcmp(lo.optional_data, 
> &efi_guid_bootmenu_auto_generated) == 0) {
> +                             for (j = 0; j < count; j++) {
> +                                     if (opt[j].size == tmp &&
> +                                         memcmp(opt[j].lo, load_option, tmp) 
> == 0) {
> +                                             opt[j].exist = true;
> +                                             break;
> +                                     }
> +                             }
> +
> +                             if (j == count) {
> +                                     ret = delete_boot_option(i);
> +                                     if (ret != EFI_SUCCESS) {
> +                                             free(load_option);
> +                                             goto out;
> +                                     }
> +                             }
> +                     }
> +             }
> +next:
> +             free(load_option);
> +     }
> +
> +out:
> +     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
>   *
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 974d8ce1ee..55ace89e4a 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -92,5 +92,6 @@ efi_status_t eficonfig_select_file_handler(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);
>  
>  #endif
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 4461f721e0..6b63ae8dde 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -953,6 +953,22 @@ struct efi_signature_store {
>  struct x509_certificate;
>  struct pkcs7_message;
>  
> +/**
> + * struct eficonfig_media_boot_option - boot option for (removable) media 
> device
> + *
> + * This structure is used to enumerate possible boot option
> + *
> + * @lo:              Serialized load option data
> + * @size:    Size of serialized load option data
> + * @exist:   Flag to indicate the load option already exists
> + *           in Non-volatile load option
> + */
> +struct eficonfig_media_boot_option {
> +     void *lo;
> +     efi_uintn_t size;
> +     bool exist;
> +};
> +
>  bool efi_hash_regions(struct image_region *regs, int count,
>                     void **hash, const char *hash_algo, int *len);
>  bool efi_signature_lookup_digest(struct efi_image_regions *regs,
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index ede9116b3c..4b24b41047 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -246,6 +246,10 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t 
> *handle,
>       }
>  
>       /* Set load options */
> +     if (size >= sizeof(efi_guid_t) &&
> +         !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated))
> +             size = 0;
> +
>       if (size) {
>               *load_options = malloc(size);
>               if (!*load_options) {
> -- 
> 2.17.1
> 

Reply via email to