Hi Tom,
On Fri, 10 Jan 2025 at 10:05, Tom Rini <trini at konsulko.com> wrote:
>
> On Fri, Jan 10, 2025 at 06:41:00AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 9 Jan 2025 at 08:22, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Thu, Jan 09, 2025 at 08:11:58AM -0700, Simon Glass wrote:
> > > > 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.
> > >
> > > Yes, the single EFI bootmeth. Because part of the issue with that is to
> > > be compliant a bunch of things need to be probed. And rather than have
> > > an option to be only semi-compliant, the preference is to be compliant
> > > and just tried later in the boot sequence.
> >
> > Well then you need a way to disable the single EFI bootmeth?
>
> We have BOOTMETH_EFILOADER already.
>
> > > > 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.
> > >
> > > In the case of no specified boot order, after block (and after very
> > > special things like FEL) and before network is what makes the most
> > > sense. We don't need to try and DHCP/etc for it, just need to know if
> > > the interface exists (and so the user can say / have configured, to try
> > > and boot it).
> >
> > Apart from FEL, we also have ChromeOS, Android, QFW.
>
> OK? Yes, bootstd needs to handle "we're in a special update things
> state". I'm not sure if QFW counts as a faux-block device or not. And
> ChromeOS I would have thought is "try and boot like $this from $device".
> I don't know if by "Android" you mean booting the OS, or the AVB state
> machine. The former would be like ChromeOS and the latter the special
> state machine case.
Standard boot is designed as a generalisation of the boot process and
is able to cope with most use cases. Everything uses the same
algorithm, at a high level.
>
> > > > 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.
> > >
> > > Yes, that should work just fine, especially if we're in the middle?
> > >
> > > The big challenge here is that conceptually, "bootstd" and "efi
> > > bootmanager" are duplicate functionality. They are both "figure out what
> > > on the system we should boot". The compromise is to try "bootstd" on
> > > block devices first (the common case).
> >
> > Are we trying to make boot slower?
>
> We're already in the slow path, because we're making guesses. But no, we
> aren't trying to make anything slower. This should potentially faster in
> the non-EFI case since we won't be trying to EFI boot every block device
> until we have exhausted the non-EFI methods.
>
> > A better way to design this would be to integrate the EFI stuff with
> > bootstd, rather than duplicating things. For example, a way to convert
> > a BOOTxxxx device into a bootdev would permit faster booting, since we
> > can just work through the boot order and request each device in turn.
> >
> > Once we give up on that and ask the user, we need to probe all devices.
>
> No, we're trying to untangle "EFI", "EFI bootmanager" and "bootstd".
By neutering bootstd? It would be better to update EFI to make use of
bootdev, for example. Heinrich did a patch to use the 'hunt' feature
of bootdev.
>
> > The monolithic init of EFI_LOADER is creating quite a few problems. Am
> > I the only one that sees it?
>
> Yes, because your "monolithic init is a problem" is the subsystem
> maintainers "monolithic init makes us spec compliant".
>
> And when we're in some sort of "probe the device and see what can be
> booted" the subsystem maintainers would rather be spec compliant.
We don't need to be noncompliant with spec. But some thoughtful design
would help us a lot.
>
> And for the role I played in this confusion by suggesting how to
> integrate things privately and has now gone off the rails, I am sorry
> for the confusion and wasted efforts. I don't know if I was unclear or
> misspoke.
I'm not sure what that relates to?
Regards,
Simon