From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>,
u-boot@lists.denx.de, Simon Glass <sjg@chromium.org>,
Takahiro Akashi <takahiro.akashi@linaro.org>,
Michal Simek <michal.simek@amd.com>
Subject: Re: [PATCH v7 4/9] efi_loader: create default file boot option
Date: Tue, 17 Oct 2023 22:29:31 +0200 [thread overview]
Message-ID: <10caa00d-8d2f-4b56-8dff-8535b2bb8563@gmx.de> (raw)
In-Reply-To: <CADQ0-X9n24HVJ_AoMszTUQy1wk+q=TfZG915mzd3gd=GScxjMA@mail.gmail.com>
On 17.10.23 07:32, Masahisa Kojima wrote:
> Hi Heinrich,
>
> On Mon, 16 Oct 2023 at 23:52, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 16.10.23 15:00, Masahisa Kojima wrote:
>>> On Mon, 16 Oct 2023 at 21:46, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>
>>>> On 16.10.23 14:31, Ilias Apalodimas wrote:
>>>>> Hi Heinrich,
>>>>>
>>>>> On Mon, 16 Oct 2023 at 10:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Am 16. Oktober 2023 08:45:21 MESZ schrieb Masahisa Kojima <masahisa.kojima@linaro.org>:
>>>>>>> Current efibootmgr automatically creates the
>>>>>>> boot options of all disks and partitions installing
>>>>>>> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
>>>>>>> Some of the automatically created boot options are
>>>>>>> useless if the disk and partition does not have
>>>>>>> the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
>>>>>>>
>>>>>>> This commit only creates the boot option if the disk and
>>>>>>> partition have the default file so that system can directly
>>>>>>> boot from it.
>>>>>>
>>>>>> I don't directly see the user benefit.
>>>>>
>>>>> The user can add an HTTP boot option now and the installer will
>>>>> automatically start. That would allow products to ship with a single
>>>>> boot option provisioned and run an installer on first boot
>>>>
>>>> This commit is not about HTTP. It changes how boot options for block
>>>> devices are created.
>>>>
>>>>>
>>>>>>
>>>>>> Reading all file systems will increase the boot time. Shouldn't we avoid this?
>>>>>
>>>>> Any idea what would be an alternative? But when we added the
>>>>> automatic boot options we said we should avoid dynamic scanning and
>>>>> store results in a file. This is doing a similar thing. The only
>>>>> difference is that it mounts the iso image before adding the boot
>>>>> option.
>>>>
>>>> The alternative is to keep showing boot options for block devices even
>>>> if there is no BOOTxxxx.EFI file.
>>>>
>>>>>
>>>>>>
>>>>>> What does EDK II do?
>>>>>
>>>>> No Idea :)
>>>>
>>>>
>>>> On my workstation I get generated boot options
>>>>
>>>> Boot0001* UEFI:CD/DVD Drive BBS(129,,0x0)
>>>> Boot0003* UEFI:Removable Device BBS(130,,0x0)
>>>> Boot0004* UEFI:Network Device BBS(131,,0x0)
>>>>
>>>> without any media inserted and without any PXE server available.
>>>
>>> It is just information about how the EDK2 works.
>>> When I attach the Fedora installation media on EDK2(OVMF),
>>> the automatically generated boot option is as follows.
>>>
>>> UEFI QEMU HARDDISK QM00001 : PciRoot(0x0)/Pci(0x1,0x1)/Ata(Primary,Master,0x0)
>>
>> An ATAPI drive typically is not removable. So I wonder why it is listed.
>> Did you set the removable flag on the command line?
>
> I guess it is not removable(actually I don't know how to set the
> device as removable).
> I just attached the iso image to QEMU with something like '-hda
> Fedora_netinst.iso".
>
> According to the EDK II implementation[1], the boot option is
> enumerated with the following order.
> 1. Removable BlockIo
> 2. Fixed BlockIo
> 3. Non-BlockIo SimpleFileSystem
> 4. LoadFile
> So boot option for the fixed device such as HDD is also automatically created.
>
> [1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c#L2150
>
>>
>>>
>>> When this boot option is selected, Fedora installer automatically starts.
>>> So EDK II is searching the default file on the fly.
>>
>> What is shown if you attach a medium without Bootaa64.efi?
>
> The same boot option is created.
> UEFI QEMU HARDDISK QM00001 : PciRoot(0x0)/Pci(0x1,0x1)/Ata(Primary,Master,0x0)
For USB devices there is a flag controlling if the device is removable:
qemu-system-riscv64 -M virt -kernel u-boot.bin -nographic -device
qemu-xhci -drive if=none,file=disk.img,format=raw,id=USB1 -device
usb-storage,drive=USB1,removable=on
This allows us to test the handling of removable devices in the boot
manager.
Best regards
Heinrich
>
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Thanks,
>>> Masahisa Kojima
>>>
>>>>
>>>> Best regards
>>>>
>>>> Heinrich
>>>>
>>>>>
>>>>> Thanks
>>>>> /Ilias
>>>>>>
>>>>>> Does the UEFI specification warrant this change?
>>>>>>
>>>>>> Best regards
>>>>>>
>>>>>> Heinrich
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
>>>>>>> ---
>>>>>>> lib/efi_loader/efi_bootmgr.c | 86 ++++++++++++++++++++++++++----------
>>>>>>> 1 file changed, 63 insertions(+), 23 deletions(-)
>>>>>>>
>>>>>>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>>>>>>> index a40762c74c..c8cf1c5506 100644
>>>>>>> --- a/lib/efi_loader/efi_bootmgr.c
>>>>>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>>>>>> @@ -355,40 +355,70 @@ error:
>>>>>>> */
>>>>>>> static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt,
>>>>>>> efi_handle_t *volume_handles,
>>>>>>> - efi_status_t count)
>>>>>>> + efi_uintn_t *count)
>>>>>>> {
>>>>>>> - u32 i;
>>>>>>> + u32 i, num = 0;
>>>>>>> struct efi_handler *handler;
>>>>>>> efi_status_t ret = EFI_SUCCESS;
>>>>>>>
>>>>>>> - for (i = 0; i < count; i++) {
>>>>>>> + 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;
>>>>>>> + struct efi_device_path *device_path, *full_path, *dp, *fp;
>>>>>>> struct efi_device_path *short_dp;
>>>>>>> + struct efi_file_handle *root, *f;
>>>>>>> + struct efi_simple_file_system_protocol *file_system;
>>>>>>> + u16 *default_file_path = NULL;
>>>>>>>
>>>>>>> - ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);
>>>>>>> + 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);
>>>>>>> +
>>>>>>> + device_path = handler->protocol_interface;
>>>>>>> + full_path = efi_dp_from_file(device_path,
>>>>>>> + "/EFI/BOOT/" BOOTEFI_NAME);
>>>>>>> +
>>>>>>> + /* check whether the partition or disk have the default file */
>>>>>>> + ret = efi_dp_split_file_path(full_path, &dp, &fp);
>>>>>>> + if (ret != EFI_SUCCESS || !fp)
>>>>>>> + goto next_entry;
>>>>>>> +
>>>>>>> + default_file_path = efi_dp_str(fp);
>>>>>>> + if (!default_file_path)
>>>>>>> + goto next_entry;
>>>>>>> +
>>>>>>> + ret = efi_search_protocol(volume_handles[i],
>>>>>>> + &efi_simple_file_system_protocol_guid,
>>>>>>> + &handler);
>>>>>>> if (ret != EFI_SUCCESS)
>>>>>>> - continue;
>>>>>>> + goto next_entry;
>>>>>>> +
>>>>>>> + file_system = handler->protocol_interface;
>>>>>>> + ret = EFI_CALL(file_system->open_volume(file_system, &root));
>>>>>>> + if (ret != EFI_SUCCESS)
>>>>>>> + goto next_entry;
>>>>>>> +
>>>>>>> + ret = EFI_CALL(root->open(root, &f, default_file_path,
>>>>>>> + EFI_FILE_MODE_READ, 0));
>>>>>>> + if (ret != EFI_SUCCESS)
>>>>>>> + goto next_entry;
>>>>>>> +
>>>>>>> + EFI_CALL(f->close(f));
>>>>>>>
>>>>>>> ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);
>>>>>>> if (ret != EFI_SUCCESS)
>>>>>>> - continue;
>>>>>>> + goto next_entry;
>>>>>>>
>>>>>>> p = dev_name;
>>>>>>> utf8_utf16_strncpy(&p, buf, strlen(buf));
>>>>>>>
>>>>>>> /* prefer to short form device path */
>>>>>>> - short_dp = efi_dp_shorten(device_path);
>>>>>>> - if (short_dp)
>>>>>>> - device_path = short_dp;
>>>>>>> + short_dp = efi_dp_shorten(full_path);
>>>>>>> + device_path = short_dp ? short_dp : full_path;
>>>>>>>
>>>>>>> lo.label = dev_name;
>>>>>>> lo.attributes = LOAD_OPTION_ACTIVE;
>>>>>>> @@ -396,24 +426,35 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
>>>>>>> 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.
>>>>>>> + * the boot option that automatically generated by the efibootmgr.
>>>>>>> * 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;
>>>>>>> + opt[num].size = efi_serialize_load_option(&lo, (u8 **)&opt[num].lo);
>>>>>>> + if (!opt[num].size) {
>>>>>>> + efi_free_pool(full_path);
>>>>>>> + efi_free_pool(dp);
>>>>>>> + efi_free_pool(fp);
>>>>>>> + efi_free_pool(default_file_path);
>>>>>>> + return EFI_OUT_OF_RESOURCES;
>>>>>>> }
>>>>>>> /* set the guid */
>>>>>>> - optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567"));
>>>>>>> + optional_data = (char *)opt[num].lo + (opt[num].size - u16_strsize(u"1234567"));
>>>>>>> memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t));
>>>>>>> + num++;
>>>>>>> +
>>>>>>> +next_entry:
>>>>>>> + efi_free_pool(full_path);
>>>>>>> + efi_free_pool(dp);
>>>>>>> + efi_free_pool(fp);
>>>>>>> + efi_free_pool(default_file_path);
>>>>>>> }
>>>>>>>
>>>>>>> -out:
>>>>>>> - return ret;
>>>>>>> + *count = num;
>>>>>>> +
>>>>>>> + return EFI_SUCCESS;
>>>>>>> }
>>>>>>>
>>>>>>> /**
>>>>>>> @@ -642,7 +683,7 @@ efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index)
>>>>>>> * 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.
>>>>>>> + * and create the boot option with default file if the file exists.
>>>>>>> * This function also provide the BOOT#### variable maintenance for
>>>>>>> * the media device entries.
>>>>>>> * - Automatically create the BOOT#### variable for the newly detected device,
>>>>>>> @@ -674,8 +715,7 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void)
>>>>>>> goto out;
>>>>>>> }
>>>>>>>
>>>>>>> - /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
>>>>>>> - ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
>>>>>>> + ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, &count);
>>>>>>> if (ret != EFI_SUCCESS)
>>>>>>> goto out;
>>>>>>>
>>>>
>>
next prev parent reply other threads:[~2023-10-17 20:29 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-16 6:45 [PATCH v7 0/9] Add EFI HTTP boot support Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 1/9] net: wget: prevent overwriting reserved memory Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 2/9] net: wget: add wget with dns utility function Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 3/9] blk: blkmap: add ramdisk creation " Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 4/9] efi_loader: create default file boot option Masahisa Kojima
2023-10-16 7:06 ` Heinrich Schuchardt
2023-10-16 12:31 ` Ilias Apalodimas
2023-10-16 12:46 ` Heinrich Schuchardt
2023-10-16 12:57 ` Ilias Apalodimas
2023-10-16 13:00 ` Masahisa Kojima
2023-10-16 14:47 ` Heinrich Schuchardt
2023-10-17 5:32 ` Masahisa Kojima
2023-10-17 19:46 ` Ilias Apalodimas
2023-10-18 9:07 ` Masahisa Kojima
2023-10-18 9:12 ` Ilias Apalodimas
2023-10-18 9:30 ` Heinrich Schuchardt
2023-10-19 6:45 ` Masahisa Kojima
2023-10-19 7:22 ` Ilias Apalodimas
2023-10-19 8:24 ` Masahisa Kojima
2023-10-17 20:29 ` Heinrich Schuchardt [this message]
2023-10-16 6:45 ` [PATCH v7 5/9] efi_loader: support boot from URI device path Masahisa Kojima
2023-10-18 0:42 ` Heinrich Schuchardt
2023-10-18 8:32 ` Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 6/9] efi_loader: add CDROM short-form " Masahisa Kojima
2023-10-18 10:19 ` Heinrich Schuchardt
2023-10-19 3:21 ` Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 7/9] Boot var automatic management for removable medias Masahisa Kojima
2023-10-16 7:58 ` João Marcos Costa
2023-10-16 19:02 ` João Marcos Costa
2023-10-16 6:45 ` [PATCH v7 8/9] cmd: efidebug: add uri device path Masahisa Kojima
2023-10-17 19:27 ` Ilias Apalodimas
2023-10-18 8:38 ` Masahisa Kojima
2023-10-16 6:45 ` [PATCH v7 9/9] doc: uefi: add HTTP Boot support Masahisa Kojima
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=10caa00d-8d2f-4b56-8dff-8535b2bb8563@gmx.de \
--to=xypron.glpk@gmx.de \
--cc=ilias.apalodimas@linaro.org \
--cc=masahisa.kojima@linaro.org \
--cc=michal.simek@amd.com \
--cc=sjg@chromium.org \
--cc=takahiro.akashi@linaro.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox