Hi Tom, On Fri, 10 Jan 2025 at 10:05, Tom Rini <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 > > > > > > <[email protected]> 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 <[email protected]> > > > > > > > > --- > > > > > > > > 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
