> 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
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.
>
> Best regards
>
> Heinrich
>
