[PATCH] RFC: Support an EFI-loader bootflow
AKASHI Takahiro
takahiro.akashi at linaro.org
Tue Sep 7 05:58:47 CEST 2021
On Mon, Sep 06, 2021 at 04:41:55AM -0600, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 3 Sept 2021 at 10:30, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> >
> >
> > On 9/3/21 10:53 AM, Simon Glass wrote:
> > > Hi Takahiro,
> > >
> > > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > > <takahiro.akashi at linaro.org> 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
> > >>> <takahiro.akashi at linaro.org> 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.
> > >
> > > 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).
> > >
> > >>
> > >> 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?
> > >
> > >>
> > >>>>
> > >>>>> 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 <sjg at chromium.org>
> > >>>>> ---
> > >>>>> 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.
> > >
> > >>
> > >>>>
> > >>>>> + * 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.
> > > 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.
> >
> >
> > Concerning these "parallel" tables. Please, have a look at the UEFI spec
> > to understand what a handle, a protocol interface, and an event are.
> >
> > I also gave as short overview in
> > https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting
> > at slide 10 or 08:00 of the video.
> >
> > Handles may refer to devices, to drivers or anything else.
> > Handles are both to be created by U-Boot and by EFI binaries that we
> > execute.
> >
> > On these handles protocol interfaces can be installed. These may be
> > those defined in the UEFI spec or something completely independent. The
> > protocol interfaces contain pointers to functions and additional data.
> >
> > Further objects are events. These may contain references to an event
> > handler function that is called when the event is triggered.
>
> Thank you for the info. I did read the entire spec some years ago
> (about 6 I think) and have dug into bits of it since. I do find the
> naming confusing so I cannot claim to understand it. The use of void *
> everywhere is a pain.. I will doubtless take another look.
>
> U-Boot does have devices, which have a (single) uclass and API. It
> also has drivers. After all, the EFI tables are created from U-Boot
> data structures, so presumably there is some correspondence.
>
> >
> > None of the above objects matches one to one to objects in the driver
> > model. But we can use some integration:
> >
> > When a U-Boot device is probed we must create the UEFI handle and
> > install the expected protocols on it.
> >
> > When a U-Boot device is removed we must destroy the handle. There are
> > certain conditions under which a protocol must not be removed from a
> > handle and the handle cannot be destroyed. In this case removing a
> > device should not be possible.
>
> I wish this had been done from the beginning, but we may as well start
> now. It is quite a complicated task at this point.
>
> Can we pick one data structure that we can make ephemeral, using only
> the DM structs and features? Can we look at what features need to be
> added (such as partition devices?) and add those to U-Boot?
>
> What would be easiest as a starting point? How about struct
> efi_system_partition?
I have already proposed it as part of my more ambitious approach[1],
in particular, patch#11,#12 and #13.
If you want, I'm willing to re-post those (11-13 only).
But it is not so much beneficial on its own because there are a lot
of U-Boot interfaces that still take parameters, blk_desc + part_num.
> Or perhaps could we drop efi_disk_register() and have it scan the disk
> 'on the fly' when needed?
Patch#13 does this. Heinrich, instead, suggested another way[2].
(Both have a lack for "removing-device" support.)
Either way, we need to have some sort of mechanism (callback or binding)
so that we can bridge UEFI world and U-Boot environment.
-Takahiro Akashi
[1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html
[2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html
> There must be something that can be done here. I am sure there is a
> mismatch of concepts/API, but some of these mismatches are features
> that U-Boot could add and some can be 'thinned down' so that UEFI is
> not such a lot of code.
>
> Regards,
> Simon
More information about the U-Boot
mailing list