On Tue, 6 May 2025 at 17:29, Tom Rini <tr...@konsulko.com> wrote:
>
> On Tue, May 06, 2025 at 12:24:24PM +0200, Mark Kettenis 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.
> >
> > 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, I recall Heinrich mentioned it could (might often be?) a GUID
> instead and so we might need to hunt, check what we found for a GUID,
> repeat. I am assuming that would not be an expensive operation however.
Yes and that's the main problem here.
In EFI you have long and short options. The long one is the full
device path, which we can easily map to a device. The short is a bit
trickier as Heinrich described -- but because it's more flexible
that's what people tend to use.
Thanks
/Ilias
>
> --
> Tom
