On Wed, 24 Aug 2022 at 22:25, Ilias Apalodimas <[email protected]> wrote: > > 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 ?
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". > - 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]. [1] https://lore.kernel.org/u-boot/[email protected]/ Thanks, Masahisa Kojima > > Thanks > /Ilias > > Signed-off-by: Masahisa Kojima <[email protected]> > > --- > > 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 > >
