[PATCH 1/1] efi_loader: boot options should relate to the ESP

[Mark Kettenis]

> From: Peter Robinson <pbrobinson at gmail.com>
> Date: Wed, 6 Dec 2023 10:44:59 +0000
> 
> On Wed, Dec 6, 2023 at 9:31 AM Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
> >
> > On 05.12.23 07:23, Masahisa Kojima wrote:
> > > Hi Heinrich,
> > >
> > > On Tue, 5 Dec 2023 at 11:38, Heinrich Schuchardt
> > > <heinrich.schuchardt at canonical.com> wrote:
> > >>
> > >> * Only generate removable media entries for EFI system partitions.
> > >> * Only offer EFI system partitions for boot options in the eficonfig
> > >>    command.
> > >>
> > >> Fixes: c416f1c0bcab ("bootmenu: add removable media entries")
> > >> Fixes: 87d791423ac6 ("eficonfig: menu-driven addition of UEFI boot option")
> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> > >> ---
> > >>   cmd/eficonfig.c              | 6 ++++--
> > >>   lib/efi_loader/efi_bootmgr.c | 3 ++-
> > >>   2 files changed, 6 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > >> index 34a59cb15d..c066b28b6b 100644
> > >> --- a/cmd/eficonfig.c
> > >> +++ b/cmd/eficonfig.c
> > >> @@ -626,8 +626,10 @@ static efi_status_t eficonfig_select_volume(struct eficonfig_select_file_info *f
> > >>          efi_handle_t *volume_handles = NULL;
> > >>          struct efi_simple_file_system_protocol *v;
> > >>
> > >> -       ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
> > >> -                                          NULL, &count, (efi_handle_t **)&volume_handles);
> > >> +       /* Find all EFI system partitions */
> > >> +       ret = efi_locate_handle_buffer_int(BY_PROTOCOL,
> > >> +                                          &efi_system_partition_guid,
> > >> +                                          NULL, &count, &volume_handles);
> > >
> > > UEFI specification v2.10 section '3.5 Boot Mechanisms' says
> > > "EFI can boot from a device using the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the
> > > EFI_LOAD_FILE_PROTOCOL."
> > >
> > > So in my understanding of the UEFI spec, I think
> > > efi_simple_file_system_protocol_guid
> > > is correct here.
> > >
> > >>          if (ret != EFI_SUCCESS) {
> > >>                  eficonfig_print_msg("No block device found!");
> > >>                  return ret;
> > >> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > >> index 48153bd5ff..9e84391186 100644
> > >> --- a/lib/efi_loader/efi_bootmgr.c
> > >> +++ b/lib/efi_loader/efi_bootmgr.c
> > >> @@ -1047,8 +1047,9 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void)
> > >>          efi_handle_t *volume_handles = NULL;
> > >>          struct eficonfig_media_boot_option *opt = NULL;
> > >>
> > >> +       /* Find all EFI system partitions */
> > >>          ret = efi_locate_handle_buffer_int(BY_PROTOCOL,
> > >> -                                          &efi_simple_file_system_protocol_guid,
> > >> +                                          &efi_system_partition_guid,
> > >
> > > In EDK II reference implementation[0], it enumerates all
> > > block_io_protocol devices
> > > and eliminates logical partitions.
> > > Should we do the same? If yes, I can prepare the patch on this.
> >
> > This is what the boot options look like on an EDK II system:
> >
> > $ efibootmgr -v
> > BootCurrent: 000D
> > Timeout: 3 seconds
> > BootOrder:
> > 000D,000B,000C,0003,0004,0005,0006,0007,0008,0009,000A,0000,0001,0002
> > Boot0000* UiApp
> > FvVol(5eda4200-2c5f-43cb-9da3-0baf74b1b30c)/FvFile(462caa21-7614-4503-836e-8ab6f4662331)
> > Boot0001* helloworld
> > VenHw(0d51905b-b77e-452a-a2c0-eca0cc8d514a,000054f20000000000)/Sata(1,65535,0)/HD(1,GPT,40c323d0-6b76-47c4-bc82-e05bf5d23c9f,0x2000,0x200000)/File(\helloworld.efi)h.e.l.l.o.
> > .w.o.r.l.d...
> > Boot0002* UEFI Crucial_CT128M550SSD1 14270C7CFEFE
> > VenHw(0d51905b-b77e-452a-a2c0-eca0cc8d514a,000054f20000000000)/Sata(1,65535,0)N.....YM....R,Y.
> > Boot0003* UEFI eMMC Device
> > VenHw(0d51905b-b77e-452a-a2c0-eca0cc8d514a,00006ef00000000000)/eMMC(0)/Ctrl(0x0)N.....YM....R,Y.
> > Boot0004* UEFI eMMC Device 2
> > VenHw(0d51905b-b77e-452a-a2c0-eca0cc8d514a,00006ef00000000000)/eMMC(0)/Ctrl(0x1)N.....YM....R,Y.
> > Boot0005* UEFI eMMC Device 3
> > VenHw(0d51905b-b77e-452a-a2c0-eca0cc8d514a,00006ef00000000000)/eMMC(0)/Ctrl(0x2)N.....YM....R,Y.
> > Boot0006* UEFI PXEv4 (MAC:000000000001)
> > MAC(000000000001,1)/IPv4(0.0.0.00.0.0.0,0,0)N.....YM....R,Y.
> > Boot0007* UEFI PXEv4 (MAC:000000000002)
> > MAC(000000000002,1)/IPv4(0.0.0.00.0.0.0,0,0)N.....YM....R,Y.
> > Boot0008* UEFI PXEv4 (MAC:000000000003)
> > MAC(000000000003,1)/IPv4(0.0.0.00.0.0.0,0,0)N.....YM....R,Y.
> > Boot0009* UEFI PXEv4 (MAC:000000000004)
> > MAC(000000000004,1)/IPv4(0.0.0.00.0.0.0,0,0)N.....YM....R,Y.
> > Boot000A* UEFI Shell
> > FvVol(5eda4200-2c5f-43cb-9da3-0baf74b1b30c)/FvFile(7c04a583-9e3e-4f1c-ad65-e05268d0b4d1)
> > Boot000B* UEFI HL-DT-ST DVDRAM GTC0N
> > VenHw(0d51905b-b77e-452a-a2c0-eca0cc8d514a,000054f40000000000)/Sata(1,65535,0)N.....YM....R,Y.
> > Boot000C* ubuntu
> > HD(1,GPT,5a1ba4fc-f9b4-4da2-b5e1-065e9cd10e3a,0x800,0x100000)/File(\EFI\ubuntu\grubaa64.efi)
> > Boot000D* grub
> > HD(1,GPT,5a1ba4fc-f9b4-4da2-b5e1-065e9cd10e3a,0x800,0x100000)/File(\EFI\grub\shimaa64.efi)
> > Boot1001* helloworld
> > HD(1,GPT,40c323d0-6b76-47c4-bc82-e05bf5d23c8f,0x2000,0x200000)/File(helloworld.efi)
> >
> > As you described autogenerated boot options are on block device level
> > irrespective of the existence of an ESP.
> >
> > EDK II does not to care if /EFI/BOOT/BOOT<ARCH>.EFI is on a partition
> > with the ESP partition type.
> >
> > We should try to mimic the EDK II behavior.
> 
> I believe the UEFI spec allows VFAT partition type as well ESP
> partition type. For example a lot of Raspberry Pis won't load their
> firmware of a ESP partition type but will off VFAT even though they
> are essentially the same thing.

Right.  And therefore OpenBSD/arm64 uses partition type 0x0C; this
works with all EFI implementations that we have tested so far
(including EDK II and some proprietary implementations).