Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Fri, Aug 20, 2021 at 12:22:17PM -0600, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 19 Aug 2021 at 21:15, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Aug 19, 2021 at 08:25:33AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 19 Aug 2021 at 07:59, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Aug 18, 2021 at 09:45:33PM -0600, Simon Glass wrote:
> > > >
> > > > > Bootmethod and bootflow provide a built-in way for U-Boot to automatically boot
> > > > > an Operating System without custom scripting and other customisation:
> > > > >
> > > > >   - bootmethod - a method to scan a device to find bootflows (owned by U-Boot)
> > > > >   - bootflow - a description of how to boot (owned by the distro)
> > > > >
> > > > > This series provides an initial implementation of these, enable to scan
> > > > > for bootflows from MMC and Ethernet. The only bootflow supported is
> > > > > distro boot, i.e. an extlinux.conf file included on a filesystem or
> > > > > tftp server. It works similiarly to the existing script-based approach,
> > > > > but is native to U-Boot.
> > > > >
> > > > > With this we can boot on a Raspberry Pi 3 with just one command:
> > > > >
> > > > >    bootflow scan -lb
> > > > >
> > > > > which means to scan, listing (-l) each bootflow and trying to boot each
> > > > > one (-b). The final patch shows this.
> > > > >
> > > > > It is intended that this approach be expanded to support mechanisms other
> > > > > than distro boot, including EFI-related ones. With a standard way to
> > > > > identify boot devices, these features become easier. It also should
> > > > > support U-Boot scripts, for backwards compatibility only.
> > > > >
> > > > > The first patch of this series moves boot-related code out of common/ and
> > > > > into a new boot/ directory. This helps to collect these related files
> > > > > in one place, as common/ is quite large.
> > > > >
> > > > > Like sysboot, this feature makes use of the existing PXE implementation.
> > > > > Much of this series consists of cleaning up that code and refactoring it
> > > > > into something closer to a module that can be called, teasing apart its
> > > > > reliance on the command-line interpreter to access filesystems and the
> > > > > like. Also it now uses function arguments and its own context struct
> > > > > internally rather than environment variables, which is very hard to
> > > > > follow. No core functional change is included in the included PXE patches.
> > > > >
> > > > > For documentation, see the 'doc' patch.
> > > > >
> > > > > There is quite a long list of future work included in the documentation.
> > > > > One question is the choice of naming. Since this is a bootloader, should
> > > > > we just call this a 'method' and a 'flow' ? The 'boot' prefix is already
> > > > > shared by other commands like bootm, booti, etc.
> >
> > Regarding the naming, I'm still a bit confused with bootmethod vs.
> > bootflow. Personally, I prefer a more intuitive name against bootmethod,
> > say, bootmedia or bootdevice (as the original distro_bootcmd uses).
> 
> I quite like bootmedia. Would just 'media' be OK? That would reduce
> the use of the 'boot' prefix which I think is overused.

If 'boot' prefix is removed, I'm afraid that the word, either media,
method or flow, sounds quite generic and so ambiguous.
As far as the commands are concerned, on the other hand, it might be
one way to have sub-commands under 'boot', like "boot media ..." or
"boot flow ..." although I'm not sure it is a preferred style on U-Boot.

# I have no strong opinion here.

> >
> > > > > The design is described here:
> > > > >
> > > > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing
> > > > >
> > > > > The series is available at u-boot-dm/bmea-working
> > > >
> > > > My question / concern is this.  Would the next step here be to
> > > > implement the generic UEFI boot path?  Today, I can write Fedora 34 for
> > > > AArch64 to a USB stick, boot U-Boot off of uSD card and the installer
> > > > automatically boots.  I'm sure I could do the same with the BSDs.
> > > > Reading the documentation left me with the impression that every OSV
> > > > would be expected to write something, so that their installer / OS boot.
> > > > But there's already standards for that, which they do, and we should be
> > > > implementing (and do, via the current distro_boot) or making easier to
> > > > enable.  Thanks!
> >
> > I had the same concern.
> 
> OK see my reply to Tom on this point. The separate of concerns between
> providing media and booting an OS is a key purpose of this series.
> 
> I have Fedora 34 in my lab and will send an update for it next week.
> 
> >
> > > Here you are talking about scanning for a bootflow. It is not actually
> > > OS-specific. If it were, there would be no point to this :-)
> > >
> > > If you look in the distro scripts you will see 'scan_dev_for_efi' (and
> > > also scan_dev_for_scrips). At least the first needs to be implemented
> > > a bit like the distro boot is at present. So far I have only
> > > implemented scan_dev_for_extlinux (plus pxe) as it is enough to show
> > > the concept.
> > >
> > > Adding EFI it's likely to be about the same amount of code as distro.c
> > > at present, perhaps a little less since we don't have the network
> > > case. It is used by Fedora 34, I believe, so is easy enough for me to
> > > do.  But I wanted to get something out as the concept is visible in
> > > this series.
> >
> > If I correctly understand this framework, we will have to
> > write several functions:
> > (Here I will ignore "UEFI boot manager" sequence.)
> >
> > 1)boot/distro_efi.c
> >    distro_boot_efi_setup()  ; almost the same as distro_boot_setup()
> >    distro_boot_efi()        ; search for /EFI/BOOT/BOOTAA64.EFI (and dtb)
> 
> Yes
> 
> >
> > 2)boot/bootflow.c
> >    bootmethod_find_efi_in_blk()
> >       call distro_boot_efi_setup()
> >    bootflow_boot()
> >       CASE BOOTFLOWT_DISTRO_EFI: ; new bootflow type
> >           call distro_boot_efi()
> 
> Yes
> 
> >
> > 3)drivers/mmc/mmc_bootmethod.c
> >    mmc_efi_get_bootflow()
> >       call bootmethod_find_efi_in_blk()
> >    U_BOOT_DRIVER(mmc_efi_bootmethod) =
> >       .name           = "mmc_efi_bootmethod",
> >       .id             = UCLASS_BOOTMETHOD,
> 
> No, this is not needed. Bootmethod provides the media so we don't have
> a separate one for EFI. It is the bootflow detection that needs to be
> changed (as above).

Well, this is what I didn't really understand. How are all the boot flows
enumerated? Looking at do_bootflow_scan(),
        for (i = 0, ret = 0; i < 100 && ret != -ESHUTDOWN; i++) {
                ret = bootmethod_get_bootflow(dev, i, &bflow);

Here, the second argument, i (or seq), is used to indicate a partition
number. So how can we add more than one bootflow to the same bootmethod
(bootmedia in my language :)?

-Takahiro Akashi

> >
> > 4)drivers/mm/Makefile
> >    obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD) += mmc_bootmethod.o mmc_efi_bootmethod.o
> 
> No, this is not needed.
> 
> See also the todo at the end of the bootflow docs. One day we may want
> to expand add sort of interface for locating the files, instead of
> calling it directly from bootflow.
> 
> >
> > Do you think that the above is correct?
> > If so, does (3) (+ (1)/(2)) inevitably increase the code size
> > as we need functions for all devices?
> 
> Yes, of course any feature increases the code size., although I'd
> argue this is a feature that could/should have been added instead of
> the scripts approach. It is much easier to understand and test.
> 
> BTW you can try this out with sandbox.
> 
> $ u-boot -T
> => bootmethod list
> => bootflow scan
> 
> etc.
> 
> Regards,
> Simon