Re: [PATCH] efi_loader: fix efi_bootmgr_enumerate_boot_option

Hi Mark,

On Tue, 17 Oct 2023 at 22:25, Mark Kettenis mark.kettenis@xs4all.nl wrote:

...

From: Heinrich Schuchardt heinrich.schuchardt@canonical.com Date: Tue, 17 Oct 2023 19:51:48 +0200

Only auto-generate boot options for removable media.

For testing the different handling of removable and non-removable devices an emulated USB device can be used:

make qemu-riscv64_smode_defconfig; make
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

Unless I misunderstand what this patch does, this may result in systems that don't boot after installation.

On most boards U-Boot does not provide a way to create persistent EFI variables after ExitBootServices() is called. This means that an OS installer cannot create a boot entry for the OS installation it just created. Therefore the EFI boot manager will not boot the newly installed system.

Now as long as the traditional distro boot mechanism or the EFI distro boot bootmeth is enabled the system will probobly still attempt to boot the installed system. But do want to rely on that?

No, we don't. We already discussed this with Heinrich (offline) and I think we should not merge that. The EFI spec doesn't limit the boot manager to removable entries only. In [0] it mentions

"if the boot image is not loaded via EFI_BOOT_SERVICES.LoadImage() the boot manager is required to check for a default application to boot. Searching for a default application to boot happens on ** both removable and fixed media types**. This search occurs when the device path of the boot image listed in any boot option points directly to an EFI_SIMPLE_FILE_SYSTEM_PROTOCOL device and does not specify the exact file to load. The file discovery method is explained in Boot Option Recovery. The default media boot case of a protocol other than EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is handled by the EFI_LOAD_FILE_PROTOCOL for the target device path and does not need to be handled by the boot manager."

So scanning both is required.

[0] https://uefi.org/specs/UEFI/2.10/03_Boot_Manager.html#load-option-processing

Regards /Ilias

...

Fixes: c416f1c0bcab ("bootmenu: add removable media entries") Fixes: 339b527bd450 ("Move bootorder and bootoption apis to lib") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

lib/efi_loader/efi_bootmgr.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a40762c74c..96d45157b3 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -346,7 +346,7 @@ error: }

/**

• • efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media

• • efi_bootmgr_enumerate_boot_option() - boot options for removable boot media
  • @opt: pointer to the media boot option structure
  • @volume_handles: pointer to the efi handles

@@ -370,6 +370,9 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo struct efi_device_path *device_path; struct efi_device_path *short_dp;

•         if (!efi_disk_is_removable(volume_handles[i]))
  

•                 continue;
  

•         ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);
          if (ret != EFI_SUCCESS)
                  continue;
  

@@ -674,7 +677,10 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void) goto out; }

• /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
  

• /*
  

•  * Enumerate all removable devices supporting the
  

•  * EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
  

•  */
  ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
  if (ret != EFI_SUCCESS)
          goto out;
  

-- 2.40.1