[PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"
Tom Rini
trini at konsulko.com
Fri Jan 10 18:05:31 CET 2025
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.
> > > 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".
> 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.
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250110/3c3a81e4/attachment.sig>
More information about the U-Boot
mailing list