Hi Takahiro,
On Sun, 5 Sept 2021 at 19:47, AKASHI Takahiro
<[email protected]> wrote:
>
> Hi Simon,
>
> On Fri, Sep 03, 2021 at 02:53:52AM -0600, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > <[email protected]> wrote:
> > >
> > > Simon,
> > >
> > > On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > > > Hi Takahiro,
> > > >
> > > > On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > > > <[email protected]> wrote:
> > > > >
> > > > > Simon,
> > > > >
> > > > > On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > > > > This is just a demonstration of how to support EFI loader using
> > > > > > bootflow.
> > > > > > Various things need cleaning up, not least that the naming needs to
> > > > > > be
> > > > > > finalised. I will deal with that in the v2 series.
> > > > > >
> > > > > > In order to support multiple methods of booting from the same
> > > > > > device, we
> > > > > > should probably separate out the different implementations
> > > > > > (syslinux,
> > > > > > EFI loader
> > > > >
> > > > > I still believe that we'd better add "removable media" support
> > > > > to UEFI boot manager first (and then probably call this functionality
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >
> > > > > from bootflow?).
> > > > >
> > > > > I admit that, in this case, we will have an issue that we will not
> > > > > recognize any device which is plugged in dynamically after UEFI
> > > > > subsystem is initialized. But this issue will potentially exist
> > > > > even with your approach.
> > > >
> > > > That can be fixed by dropping the UEFI tables and using driver model
> > > > instead. I may have mentioned that :-)
> > >
> > > I'm afraid that you don't get my point above.
> > >
> > > > >
> > > > > > and soon bootmgr,
> > > > >
> > > > > What will you expect in UEFI boot manager case?
> > > > > Boot parameters (options) as well as the boot order are well defined
> > > > > by BootXXXX and BootOrder variables. How are they fit into your
> > > > > scheme?
> > > >
> > > > I haven't looked at boot manager yet, but I can't imagine it
> > > > presenting an insurmountable challenge.
> > >
> > > I don't say it's challenging.
> > > Since you have not yet explained your idea about how to specify
> > > the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> > > be treated and honored.
> > > There might be a parallel world again.
> >
> > Well as I mentioned, I haven't looked at it yet. The original question
> > was how to do EFI LOADER and I did a patch to show that.
> >
> > Are we likely to see mixed-boot environments, that use distro boot for
> > some OSes and EFI for others? I hope not as it would be confusing. EFI
> > is the parallel world, as I see it.
>
> Yeah, I absolutely agree here.
> That is why I constantly insist "removable media" support should be
> implemented in UEFI boot manager in the first place. Please remember
> that "removable media" support is part of UEFI specification,
> UEFI boot manager and hence "EFI LOADER".
>
> # By "removable media" support, I mean that the image to be loaded
> # is determined by searching for the default file path, or /EFI/BOOT/xxx.efi,
> # in the system partition. The media can be, say, a fixed (non-removable)
> # HD on the system.
> # This is definitely part of UEFI environment.
>
OK.
> > It should be easy enough for the 'bootmgr' bootflow to read the EFI
> > variables and select the correct ordering. As I understand it, EFI
> > does not support lazy boot, so it is acceptable to probe all the
> > devices before selecting one?
> > >
> > > > >
> > > > > But anyway, we can use the following commands to run a specific
> > > > > boot flow in UEFI world:
> > > > > => efidebug boot next 1(or whatever else); bootefi bootmgr
> > > >
> > > > OK.
> > > >
> > > > As you probably noticed I was trying to have bootflow connect directly
> > > > to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > > > disabled (e.g. for security reasons) and the boot will still work.
> > >
> > > # Maybe, it sounds kinda chicken and egg.
> > >
> > > Even now, you can code this way :)
> > >
> > > efi_set_variable(u"BootNext", ..., u"Boot0001");
> > > do_efibootmgr();
> > >
> > > That's it. My concern is what I mentioned above.
> >
> > OK. But then you would need to export those functions. I think it
> > would be better to split up the logic a bit and move things out of the
> > cmd/ directory (at some point).
>
> If we don't take "mixed-boot environments", adding efibootmger to
> bootflow mechanism, or implementing efibootmger using bootflow,
> makes little sense.
I'm not convinced of that, actually. Let's leave the discussion for
when bootflow is a little closer. I think it should be possible to
boot without needing CONFIG_CMDLINE, for example. But even without
that, a unified approach to what is available to boot and what to
select could be quite helpful I think. It can support custom flows
too, such as Chrome OS.
>
> > >
> > > Just a note:
> > > In the current distro_bootcmd, UEFI boot manager is also called
> > > *every time* one of boot media in "boot_targets" is scanned/enumerated.
> > > But it will make little sense because the current boot manager only
> > > allows/requires users to specify both the boot device and the image file
> > > path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> > > all the boot options in "BootOrder" until it successfully launches
> > > one of those images.
> >
> > Yes, is the idea of lazy boot entirely impossible? Or is it still
> > possible to do that to some extent, e.g. by scanning until you find
> > the first thing in the boot order?
>
> If I correctly understand what 'lazy boot' means here, you can do
> all the enumeration/scanning first, then call UEFI boot manager:
> => scsi rescan; usb start; mmc rescan; (and whatever else); bootefi bootmger
I mean scan the first device, try to boot it. If that doesn't work,
scan the next one. It can make the boot a lot faster.
>
> The order of scanning commands doesn't matter as "BootOrder" determine
> the priorities among BootXXXX (boot options).
> What the current UEFI cannot provides is to detect a new device that
> is attached to the board once UEFI subsystem is initialized.
>
> That issue, which I have mentioned in the ML from time to time, should be
> separately resolved as part of "integration" efforts between UEFI objects
> and driver model.
Yes, this limitation is a side effect of the implementation method. I
will try to watch out for additional 'parallel' tables being added,
but I would like to see some effort to reduce what is there.
>
> > >
> > > > >
> > > > > > Chromium OS, Android, VBE) into pluggable
> > > > > > drivers and number them as we do with partitions. For now the
> > > > > > sequence
> > > > > > number is used to determine both the partition number and the
> > > > > > implementation to use.
> > > > > >
> > > > > > The same boot command is used as before ('bootflow scan -lb') so
> > > > > > there is
> > > > > > no change to that. It can boot both Fedora 31 and 34, for example.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <[email protected]>
> > > > > > ---
> > > > > > See u-boot-dm/bmea for the tree containing this patch and the series
> > > > > > that it relies on:
> > > > > >
> > > > > >
> > > > > > https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > > > > >
> > > >
> > > > [..]
> > > >
> > > > > > +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > > > > + struct bootflow *bflow)
> > > > > > +{
> > > > > > + const struct udevice *media_dev;
> > > > > > + int size = bflow->size;
> > > > > > + char devnum_str[9];
> > > > > > + char dirname[200];
> > > > > > + loff_t bytes_read;
> > > > > > + char *last_slash;
> > > > > > + ulong addr;
> > > > > > + char *buf;
> > > > > > + int ret;
> > > > > > +
> > > > > > + /* Sadly FS closes the file after fs_size() so we must redo
> > > > > > this */
> > > > > > + ret = fs_set_blk_dev_with_part(desc, partnum);
> > > > > > + if (ret)
> > > > > > + return log_msg_ret("set", ret);
> > > > > > +
> > > > > > + buf = malloc(size + 1);
> > > > > > + if (!buf)
> > > > > > + return log_msg_ret("buf", -ENOMEM);
> > > > > > + addr = map_to_sysmem(buf);
> > > > > > +
> > > > > > + ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > > > > + if (ret) {
> > > > > > + free(buf);
> > > > > > + return log_msg_ret("read", ret);
> > > > > > + }
> > > > > > + if (size != bytes_read)
> > > > > > + return log_msg_ret("bread", -EINVAL);
> > > > > > + buf[size] = '\0';
> > > > > > + bflow->state = BOOTFLOWST_LOADED;
> > > > > > + bflow->buf = buf;
> > > > > > +
> > > > > > + /*
> > > > > > + * This is a horrible hack to tell EFI about this boot
> > > > > > device. Once we
> > > > > > + * unify EFI with the rest of U-Boot we can clean this up.
> > > > > > The same hack
> > > > > > + * exists in multiple places, e.g. in the fs, tftp and load
> > > > > > commands.
> > > > >
> > > > > Which part do you call a "horrible hack"? efi_set_bootdev()?
> > > > > In fact, there are a couple of reason why we need to call this
> > > > > function:
> > > > > 1. to remember a device to create a dummy device path for the loaded
> > > > > image later,
> > > > > 2. to remember a size of loaded image which is used for sanity check
> > > > > and
> > > > > image authentication later,
> > > > > 3. to avoid those parameters being remembered accidentally by
> > > > > "loading" dtb
> > > > > and/or other binaries than the image itself,
> > > > >
> > > > > I hope that (1) and (2) will be avoidable if we modify the current
> > > > > implementation (and bootefi syntax), and then we won't need (3).
> > > >
> > > > Yes thank you...I do understand why it is needed now, but it is
> > > > basically due to the the fat that EFI has its own driver structures.
> > > > Once we stop those, it will go away.
> > >
> > > Here, my point is, even under the current implementation, we will
> > > be able to eliminate efi_set_bootdev() with some tweaks.
> > >
> > > In other words, even you could integrate UEFI into the device model,
> > > the issue (2), for example, would still remain unsolved.
> > > In case of (2), we use the *size* information for sanity check against
> > > image's header information as well as calculating a hash value for
> > > UEFI secure boot when efi_load_image() is called. Even if the integration
> > > is done, we need to pass on the size information to "bootefi <addr>"
> > > command implicitly or explicitly.
> >
> > If you look at 'struct bootflow' you can see that it stores the size.
> > It can easily store other info as needed by EFI. So I don't think we
> > need that hack in the end, when things are using bootflow.
>
> I'm afraid that you miss the context of my discussion.
> We support a command sequence like the following:
> => load scsi 0:1 50000000 /hellowworld.efi
> => bootefi 50000000
>
> distro_bootcmd does use this method to add "removable media" support,
> but even if the distro script is re-implemented using bootflow machanism,
> we cannot remove "horrible hack" in load command as long as we continue
> to use load command.
Actually 'bootflow' does not use the load command so I think it is a
path forward here.
>
> > >
> > > > >
> > > > > > + * Once we can clean up the EFI code to make proper use of
> > > > > > driver model,
> > > > > > + * this can go away.
> > > > >
> > > > > My point is, however, that this kind of cleanup is irrelevant to
> > > > > whether we use driver model or not.
> > > >
> > > > Are you sure? Without driver model how are you going to reference a
> > > > udevice? If not that, how are you going to reference a device? The
> > > > tables in the UEFI implementation were specifically added to avoid
> > > > relying on driver model. It is a crying shame that I did not push back
> > > > harder on this at the time.
> > >
> > > I hope you will get my point in the previous comment now.
> >
> > Well yes I do, but I don't understand why there is such resistance to
> > sorting out the EFI implementation? It is quite a mess at the moment.
>
> As you know, I have a positive opinion to the integration.
> What I object to here is to integrate UEFI boot manager into bootflow.
Well I have not looked at this yet and it is not my immediate focus.
For now I can just make bootflow use the 'bootmgr' command in that
case.
>
> > I think the first step is to drop the non-DM code, but the second is
> > to drop the parallel tables that EFI keeps about each device.
>
> As Heinrich suggested in his reply, there are many concepts that might
> not be easily mapped to U-Boot counterparts. If our only concern is on
> the boot flow, it would be easier, but we would also like to support
> a lot of API, either boot/runtime services or protocols. Filling
> semantic gaps would make it harder to completely drop some sort of
> "parallel tables".
But I'm not asking for that. In fact I sent Heinrich an email recently
asking for just one thing to be removed, as a starting point.
>
> I think that the first thing that we should tackle is to eliminate
> the issue, as I mentioned above, regarding "block devices".
> It would shed light on how we could go further on this issue.
Do you mean drop the parallel tables for block devices? If so, yes
that sounds good.
Regards,
Simon
