From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 58D59C32793 for ; Wed, 24 Aug 2022 01:57:44 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id AEFFE844B5; Wed, 24 Aug 2022 03:57:41 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="awx9RFuz"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 77711844E3; Wed, 24 Aug 2022 03:57:40 +0200 (CEST) Received: from mail-pg1-x533.google.com (mail-pg1-x533.google.com [IPv6:2607:f8b0:4864:20::533]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2CD1E84429 for ; Wed, 24 Aug 2022 03:57:36 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pg1-x533.google.com with SMTP id 202so13763651pgc.8 for ; Tue, 23 Aug 2022 18:57:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc; bh=VWBGnqbEu6VUGn11Kx4s12xiaT8meQoQknFufm1etiI=; b=awx9RFuzwlT4DntKvvOASypg9tVNVWsP8zZ98CK3RpwDkPhawzw8ZW9XG4DPoS/RON YWpHjBP1A93PRtdFo56DtMTRIU8XrLSkfQE2HsDh6ib1Cqoho6SjUx4RwYKS21L9nvCQ hXXQ9jkmAftyPYZAMtXpiSK2LX3ntbI6wqRfgUNWJGZzdcXakaRI/Kia6tAIYKqLlHrJ 8pMZiQFIM0zfosr3O9r3/45KJKVkaYMwTSOFOxngAkbbe8kAfcmtet8ME2vBQmJDCqnP Skq9dnvrQ4sekByZhSQqzBooeLjYsWLxMYKFDYTVh6vd/2imfrpvTVMO2ftQilOIbLEU 6QeQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc; bh=VWBGnqbEu6VUGn11Kx4s12xiaT8meQoQknFufm1etiI=; b=6bYqYDVQsS+jvu4K/r/Cyxr6RBERmpg19PGEUpwiw/TvVskwq6kD60q+EJOhrxD0lY RoT1FD7S0Bg4fXIGXkR/7Yxdp2ZGi3U4QosGrf7XsH6RiVFCttHb+IrkCETODLI5vEdS lfVICH9g0PkVzBEOMGnI34ynSqcKtu1o54KNUGhCwrt0+8gICtjRmOeAb+Nm5c2Uke1m AVIz6dYcbPWlYWCYxKDVC/XriKVn8E4ACvqTkBLcJpVvqWpXez+hlDyFEvUixqrHBvxz tHpbMsqm+Fc8GxxBQIWr2YR3k3EZlVJUn/wJZUV3FeKbEn+Q5bkw+AgcerYaWJc4OtJN ihCQ== X-Gm-Message-State: ACgBeo0LsgKUCYnBN2c/ii3xkff7tOwPY+6hu1v0Ji9/5MZ7sY+juMFe TwOb9BDzh7/6/n2UqJkRWQQyJg== X-Google-Smtp-Source: AA6agR5CzOUa0i1MPo7rr980FIaF+yJKREbiK+tt4nTdDQroK9wFFo2sKlenNkr1fftmsEke8rFA/A== X-Received: by 2002:a65:46c4:0:b0:41d:e36b:1e59 with SMTP id n4-20020a6546c4000000b0041de36b1e59mr22284199pgr.494.1661306254118; Tue, 23 Aug 2022 18:57:34 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:a705:144d:4f4b:4a05]) by smtp.gmail.com with ESMTPSA id a5-20020a624d05000000b00536b67c6a80sm4612408pfb.109.2022.08.23.18.57.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Aug 2022 18:57:33 -0700 (PDT) Date: Wed, 24 Aug 2022 10:57:28 +0900 From: Takahiro Akashi To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Simon Glass , Mark Kettenis Subject: Re: [PATCH v11 6/9] bootmenu: add removable media entries Message-ID: <20220824015728.GA45149@laputa> Mail-Followup-To: Takahiro Akashi , Masahisa Kojima , u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Simon Glass , Mark Kettenis References: <20220817093614.32266-1-masahisa.kojima@linaro.org> <20220817093614.32266-7-masahisa.kojima@linaro.org> <20220819013100.GA61057@laputa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Fri, Aug 19, 2022 at 12:05:50PM +0900, Masahisa Kojima wrote: > Hi Akashi-san, > > On Fri, 19 Aug 2022 at 10:31, Takahiro Akashi > wrote: > > > > On Wed, Aug 17, 2022 at 06:36:11PM +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 > > > > Should this feature belong to bootmenu command? > > Under the current implementation, those boot options are > > generated only by bootmenu, and so if eficonfig is invoked > > prior to bootmenu, we won't see them (under "Change Boot Order"). > > > > I expect that the functionality be also provided in eficonfig > > (or even as part of system initialization?). > > OK, generating the (removable) media boot options will be added > in "Change Boot Order". I found another wrong behavior. What I did was - eficonfig it shows no boot options. - scsi rescan One disk with two partitions was detected. - eficonfig Now it shows two options for removal media. I disabled one of two partitions from BootOrder. - bootmenu It still shows both boot options. -> Probably okay? - eficonfig Then a duplicated option comes up: ** Change Boot Order ** [*] scsi 0:1 [*] scsi 0:2 [ ] scsi 0:2 Save Quit Internally there exist three boot options now. -Takahiro Akashi > Thanks, > Masahisa Kojima > > > > > -Takahiro Akashi > > > > > > > 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. > > > > > > Signed-off-by: Masahisa Kojima > > > --- > > > 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 | 106 +++++++++++++++++++++++++-- > > > cmd/eficonfig.c | 135 +++++++++++++++++++++++++++++++++++ > > > include/efi_loader.h | 20 ++++++ > > > lib/efi_loader/efi_bootmgr.c | 4 ++ > > > 4 files changed, 260 insertions(+), 5 deletions(-) > > > > > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c > > > index 704d36debe..04df41a0cb 100644 > > > --- a/cmd/bootmenu.c > > > +++ b/cmd/bootmenu.c > > > @@ -220,7 +220,93 @@ 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)) > > > +/** > > > + * 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 > > > + */ > > > +static efi_status_t 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; > > > +} > > > + > > > /** > > > * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries > > > * > > > @@ -340,11 +426,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 = 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 6e39c0cd4d..c7f55c62fb 100644 > > > --- a/cmd/eficonfig.c > > > +++ b/cmd/eficonfig.c > > > @@ -2080,6 +2080,141 @@ 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) > > > +{ > > > + u16 *bootorder; > > > + u32 i, j; > > > + efi_status_t ret; > > > + efi_uintn_t num, size, bootorder_size; > > > + void *load_option; > > > + struct efi_load_option lo; > > > + u16 varname[] = u"Boot####"; > > > + > > > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); > > > + if (!bootorder) > > > + return EFI_SUCCESS; /* BootOrder is not defined, nothing to do */ > > > + > > > + num = bootorder_size / sizeof(u16); > > > + for (i = 0; i < num;) { > > > + efi_uintn_t tmp; > > > + > > > + efi_create_indexed_name(varname, sizeof(varname), > > > + "Boot", bootorder[i]); > > > + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); > > > + if (!load_option) > > > + goto next; > > > + > > > + 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(bootorder[i]); > > > + if (ret != EFI_SUCCESS) { > > > + free(load_option); > > > + goto out; > > > + } > > > + > > > + num--; > > > + bootorder_size -= sizeof(u16); > > > + free(load_option); > > > + continue; > > > + } > > > + } > > > + } > > > +next: > > > + free(load_option); > > > + i++; > > > + } > > > + > > > +out: > > > + return ret; > > > +} > > > + > > > /** > > > * eficonfig_init() - do required initialization for eficonfig command > > > * > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > > index 49e7d1e613..a5a0448fa0 100644 > > > --- a/include/efi_loader.h > > > +++ b/include/efi_loader.h > > > @@ -955,6 +955,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, > > > @@ -1104,6 +1120,10 @@ efi_status_t efi_console_get_u16_string > > > 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_enumerate_boot_option(struct eficonfig_media_boot_option *opt, > > > + efi_handle_t *volume_handles, efi_status_t count); > > > +efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt, > > > + efi_status_t count); > > > > > > efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int size); > > > > > > 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 > > >