[PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Simon Glass]

Hi Tom,

On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 11/13/24 16:09, Simon Glass wrote:
> > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > better since it will work for all boards. We can then figure out how to
> > > > automatically and deterministicaly decide when bootmgr should be used.
> > > >
> > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > ---
> > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > >
> > > > (no changes since v1)
> > > >
> > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > >
> > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > index 23ae1e610ac..095fa74fc60 100644
> > > > --- a/boot/bootmeth_efi_mgr.c
> > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > @@ -14,8 +14,6 @@
> > > >   #include <command.h>
> > > >   #include <dm.h>
> > > >   #include <efi_loader.h>
> > > > -#include <efi_variable.h>
> > > > -#include <malloc.h>
> > > >
> > > >   /**
> > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > @@ -48,27 +46,13 @@ 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;
> > > >       }
> > > >
> > > > -     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;
> > > > -     }
> > > > +     /* To be implemented */
> > >
> > > The EFI boot manager can boot based on:
> > >
> > > * variable BootOrder
> > > * variable BootNext
> > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > >
> > > It obsoletes bootsmeth_efi.
> > >
> > > >
> > > >       return -EINVAL;
> > >
> > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > wrong here.
> >
> > Well, I don't believe you have a solution, then.
> >
> > You did suggest putting bootmgr later, as we discussed on irc.
> >
> > For now, I think we should apply this patch (and series), while we
> > sort out how to make bootmgr more incremental. It should not be
> > scanning every available device before it starts, since that can be
> > very slow.
>
> Heinrich and I talked the other day, and we think the right path is the
> "later" path, where we don't try and use efi bootmanager until
> everything has been probed, and also drop the single "efi" option. This
> should mean that by the time we would be trying efi bootmanager most if
> not everything that needs to be probed has been probed. It doesn't make
> any sense to have "efi bootmanger" be more incremental as conceptually
> it's point is to show the user all the options.

What is the single 'efi' option? I hope you don't mean the EFI bootmeth.

Putting bootmanager at the very end is OK with me, if we really cannot
figure out a way to know whether the system is using it or now. It is
'putting it in the middle' that I don't like.

How to handle the case where bootorder only specifies a subset of
options? Ideally we would just probe devices related to that subset.
That is what I was getting at.

Regards,
Simon