Re: [PATCH v13 5/9] bootmenu: add removable media entries

On Thu, 25 Aug 2022 at 17:03, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

[...]

...

...

...

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 ?

In current design, we can not edit/delete the auto-generated boot options. I can move automatic scanning of boot options into the eficonfig startup, but it does not affect edit/delete boot options. I just do automatic scanning of boot options just before it is used in "Change Boot Order".

Fair enough. Isn't it better to still scan on startup ? If we ever fix the automatic detected boot options to show up, that would make sense I guess

I agree. I will move scan on eficonfig startup.

Thanks, Masahisa Kojima

...

...

• 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

Is it the auto-generated boot option? The auto-generate boot option is not listed in the edit/delete menu, discussed with Heinrich before[1].

Ok thanks!

Regards /Ilias

...

[1] https://lore.kernel.org/u-boot/2d19f001-a3cb-735e-d615-2f23cb5b254b@gmx.de/

Thanks, Masahisa Kojima

...

Thanks /Ilias

...

Signed-off-by: Masahisa Kojima masahisa.kojima@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