Hi Mark, Tom,
On Tue, 6 May 2025 at 12:24, Mark Kettenis <mark.kette...@xs4all.nl> wrote:
>
> > Date: Mon, 5 May 2025 17:43:19 -0600
> > From: Tom Rini <tr...@konsulko.com>
> >
> > On Tue, May 06, 2025 at 01:18:16AM +0200, Heinrich Schuchardt wrote:
> > > On 5/6/25 00:10, Tom Rini wrote:
> > > > On Tue, May 06, 2025 at 12:07:20AM +0200, Heinrich Schuchardt wrote:
> > > > > Tom Rini <tr...@konsulko.com> schrieb am Mo., 5. Mai 2025, 21:54:
> > > > >
> > > > > > On Mon, May 05, 2025 at 09:51:52PM +0200, Heinrich Schuchardt wrote:
> > > > > > > Mark Kettenis <mark.kette...@xs4all.nl> schrieb am Mo., 5. Mai
> > > > > > > 2025,
> > > > > > 21:03:
> > > > > > >
> > > > > > > > > Date: Fri, 2 May 2025 18:08:56 +0200
> > > > > > > > > From: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> > > > > > > > >
> > > > > > > > > On 5/2/25 16:49, Simon Glass wrote:
> > > > > > > > > > Hi Heinrich,
> > > > > > > > > >
> > > > > > > > > > On Mon, 21 Apr 2025 at 10:26, Heinrich Schuchardt
> > > > > > > > > > <heinrich.schucha...@canonical.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > The EFI boot manager bootmeth does not require variable
> > > > > > > > > > > BootOrder
> > > > > > to
> > > > > > > > be
> > > > > > > > > > > preexisting. It creates this variable.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Heinrich Schuchardt <
> > > > > > heinrich.schucha...@canonical.com
> > > > > > > > >
> > > > > > > > > > > ---
> > > > > > > > > > > boot/bootmeth_efi_mgr.c | 21 +++------------------
> > > > > > > > > > > 1 file changed, 3 insertions(+), 18 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/boot/bootmeth_efi_mgr.c
> > > > > > > > > > > b/boot/bootmeth_efi_mgr.c
> > > > > > > > > > > index 42b8863815e..1669cbed5bd 100644
> > > > > > > > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > > > > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > > > > > > > @@ -47,30 +47,15 @@ static int efi_mgr_check(struct
> > > > > > > > > > > udevice *dev,
> > > > > > > > struct bootflow_iter *iter)
> > > > > > > > > > >
> > > > > > > > > > > static int efi_mgr_read_bootflow(struct udevice *dev,
> > > > > > > > > > > struct
> > > > > > > > bootflow *bflow)
> > > > > > > > > > > {
> > > > > > > > > > > - struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > > > > > > > - efi_status_t ret;
> > > > > > > > > > > - efi_uintn_t size;
> > > > > > > > > > > - u16 *bootorder;
> > > > > > > > > > > -
> > > > > > > > > > > - if (priv->fake_dev) {
> > > > > > > > > > > - bflow->state = BOOTFLOWST_READY;
> > > > > > > > > > > - return 0;
> > > > > > > > > > > - }
> > > > > > > > > > > + int ret
> > > > > > > > > > >
> > > > > > > > > > > ret = efi_init_obj_list();
> > > > > > > > > > > if (ret)
> > > > > > > > > > > return log_msg_ret("init", ret);
> > > > > > > > > > >
> > > > > > > > > > > - /* Enable this method if the "BootOrder" UEFI
> > > > > > > > > > > exists. */
> > > > > > > > > > > - bootorder = efi_get_var(u"BootOrder",
> > > > > > > > &efi_global_variable_guid,
> > > > > > > > > > > - &size);
> > > > > > > > > > > - if (bootorder) {
> > > > > > > > > > > - free(bootorder);
> > > > > > > > > > > - bflow->state = BOOTFLOWST_READY;
> > > > > > > > > > > - return 0;
> > > > > > > > > > > - }
> > > > > > > > > > > + bflow->state = BOOTFLOWST_READY;
> > > > > > > > > > >
> > > > > > > > > > > - return -EINVAL;
> > > > > > > > > > > + return 0;
> > > > > > > > > > > }
> > > > > > > > > > >
> > > > > > > > > > > static int efi_mgr_read_file(struct udevice *dev,
> > > > > > > > > > > struct
> > > > > > bootflow
> > > > > > > > *bflow,
> > > > > > > > > > > --
> > > > > > > > > > > 2.48.1
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > How do we know if the board is using EFI bootmgr? My
> > > > > > > > > > understanding
> > > > > > was
> > > > > > > > > > that this was a way to find out?
> > > > > > > > >
> > > > > > > > > The boot manager must always run.
> > > > > > > > >
> > > > > > > > > The check for the BootOrder variable introduced in commit
> > > > > > f2bfa0cb1794
> > > > > > > > > is a bug.
> > > > > > > >
> > > > > > > > Well, at the time the boot manager did not attempt to boot the
> > > > > > > > default
> > > > > > > > path. So there was no point in running the boot manager code
> > > > > > > > unless
> > > > > > > > BootOrder (or BootNext) was set. And of course before that
> > > > > > > > commit the
> > > > > > > > boot manager didn't run at all on non-sandbox builds that had
> > > > > > > > the
> > > > > > > > standard boot stuff enebaled.
> > > > > > > >
> > > > > > > > Anyway, I believe the thinking behind that commit is still
> > > > > > > > sound. As
> > > > > > > > I explained earlier, I think that...
> > > > > > > >
> > > > > > > > > The boot manager handles in sequence:
> > > > > > > > >
> > > > > > > > > * Try to boot as indicated by BootNext.
> > > > > > > > > * Try to boot as indicated by BootOrder.
> > > > > > > > > * Try to boot default path for available media.
> > > > > > > > > This will add Boot#### entries and update BootOrder.
> > > > > > > >
> > > > > > > > ...doing this all in a monolithic sequence isn't the best way to
> > > > > > > > handle EFI boot in the u-boot ecosystem.
> > > > > > > >
> > > > > > > > Your series moves the boot manager further down the list
> > > > > > > > because the
> > > > > > > > third step in the sequence has to happen late. But as a result
> > > > > > > > BootNext and BootOrder processing will also happen late. So
> > > > > > > > what
> > > > > > > > happens if you have a board with two OS installations:
> > > > > > > >
> > > > > > > > 1. A generic Linux distro that boots via EFI.
> > > > > > > >
> > > > > > > > 2. Something like Armbian that provides an extlinux.conf file.
> > > > > > > >
> > > > > > > > Currently such a system will probably boot OS #1. But after
> > > > > > > > your
> > > > > > > >
> > > > > > >
> > > > > > > This did not happen with distoboot. So migration from distroboot
> > > > > > > to
> > > > > > > standard boot results in a change that you want to avoid.
> > > > > > >
> > > > > > > changes it will probably boot OS #2. And if OS #1 sets BootOrder
> > > > > > > or
> > > > > > > > BootNext that will not change anything.
> > > > > > > >
> > > > > > > > So I think we need a solution where BootNext and BootOrder
> > > > > > > > processing
> > > > > > > > happens early, like we do now.
> > > > > > > >
> > > > > > >
> > > > > > > As of today BootNext does not invoke the boot manager.
> > > > > > >
> > > > > > > If BootOrder is set the boot manager may fail because not all
> > > > > > > devices are
> > > > > > > detected as "hunters" have not been running. E.g. nvme scan and
> > > > > > > usb start
> > > > > > > are only invoked after the EFI boot manager.
> > > > > > >
> > > > > > > I originally suggested to probe all boot devices before the boot
> > > > > > > manager
> > > > > > > runs, but users complained that this slows down their non-EFI
> > > > > > > boot flows.
> > > > > > > This is why I now move it after all boot methods but PXE.
> > > > > >
> > > > > > Can we not see what BootOrder is and then ensure it's been "hunted"
> > > > > > ?
> > > > > >
> > > > > > --
> > > > > > Tom
> > > > > >
> > > > >
> > > > > A load option may only contain the partition GUID and the file path.
> > > > > In
> > > > > this case you wouldn't be able to tell whether it is for an NVMe
> > > > > drive, or
> > > > > a USB stick.
> > > > >
> > > > > As numbering of devices cannot be expected to be stable, it is
> > > > > preferable
> > > > > to use this short form of device paths for load options.
> > > >
> > > > Ah, OK. So could we check and if not found print a human understandable
> > > > error message, and continue on? I want to figure out some path that does
> > > > not change the pre-bootstd behavior.
> > > >
> > >
> > > In distroboot scan_dev_for_efi is invoked for every boot device and each
> > > time calls the boot manager. As usb start is only called in usb_boot this
> > > will result in behavior that is as non-compliant with the UEFI
> > > specification.
> > >
> > > We should reach a behavior that complies with the UEFI specification.
> > > Not changing former distro boot behavior is not a valid option.
> >
> > Can we also not figure out some way to boot promptly? I was thinking
> > something along the lines of if we know BootOrder (or similar), we try
> > it. If we don't find it, we try again a bit later on when we can run all
> > the hunting that might be slow. If the only option is slow-but-compliant
> > I feel like a lot of use cases will opt out of using EFI boot instead.
BTW this is what I have been proposing for a year or so. But my
understanding was that you were happy enough with Heinrich's
workaround for now.
>
> I think that is possible. But it will require tighter integration
> integration between the EFI loader and the standard boot device
> hunting code such that devices are probed as we walk to options
> specified by the BootOrder.
>
> A complication here is that a Boot#### option doesn't necessearily
> have the device encoded into the boot option. An option can reference
> just the GUID of a partition and many OS installers create such
> options. So if we see such an option we do need to probe devices
> until we find the right partition.
Right. It might be feasible to just use bootstd to produce partitions
one by one, e.g. by calling iter_incr(), then looking at what was
received and seeing if it matches the first bootorder var, booting if
so. That might be enough for a first pass?
Somewhat related, it would be nice if bootmgr could move to using expo
and show its menu while it is still scanning things.
>
> Another complication is that currently the
> efi_bootmgr_update_media_device_boot_option() function will append
> Boot#### options it creates to the BootOrder variable.
>
> This is used to implement the following requirement in the spec:
>
> The boot manager must also support booting from a short-form device
> path that starts with the first element being a File Path Media
> Device Path (File Path Media Device Path ). When the boot manager
> attempts to boot a short-form File Path Media Device Path, it will
> enumerate all removable media devices, followed by all fixed media
> devices, creating boot options for each device.
>
> However, a the spec also says that:
>
> These new boot options must not be saved to non volatile storage,
> and may not be added to BootOrder.
>
> But we we do add those options to BootOrder and we I believe we will
> save those new Boot#### options as well.
Regards,
Simon
