[PATCH v2 00/41] Initial implementation of standard boot

[Simon Glass]

Hi Tom,

On Thu, 28 Oct 2021 at 13:19, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Oct 28, 2021 at 12:48:50PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 28 Oct 2021 at 12:36, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Thu, Oct 28, 2021 at 12:13:56PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 28 Oct 2021 at 11:52, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Thu, Oct 28, 2021 at 11:29:35AM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 28 Oct 2021 at 10:27, Tom Rini <trini at konsulko.com> wrote:
> > > > > > >
> > > > > > > On Sat, Oct 23, 2021 at 05:25:54PM -0600, Simon Glass wrote:
> > > > > > >
> > > > > > > > The bootflow feature provide a built-in way for U-Boot to automatically
> > > > > > > > boot an Operating System without custom scripting and other customisation.
> > > > > > > > This is called 'standard boot' since it provides a standard way for
> > > > > > > > U-Boot to boot a distro, without scripting.
> > > > > > > >
> > > > > > > > It introduces the following concepts:
> > > > > > > >
> > > > > > > >    - bootdev - a device which can hold a distro
> > > > > > > >    - bootmeth - a method to scan a bootdev to find bootflows (owned by
> > > > > > > >                 U-Boot)
> > > > > > > >    - bootflow - a description of how to boot (owned by the distro)
> > > > > > > >
> > > > > > > > This series provides an implementation of these, enabled to scan for
> > > > > > > > bootflows from MMC, USB and Ethernet. It supports the existing distro
> > > > > > > > boot as well as the EFI loader flow (bootefi/bootmgr). It works
> > > > > > > > similiarly to the existing script-based approach, but is native to
> > > > > > > > U-Boot.
> > > > > > >
> > > > > > > I'm going to break my feedback down in to a few threads, to hopefully
> > > > > > > not confuse things too much.  My first comment is that rpi_arm64 grows
> > > > > > > in size by 17 kilobytes, with the whole series (pxe, env, this) applied.
> > > > > > > And while there's a few small changes in the pxe cleanup I'm going to
> > > > > > > re-investigate on their own, it's really just this series, right here,
> > > > > > > adding tons of code.  To replace an admittedly complex bit of
> > > > > > > environment scripting, with C.  It's not even the earlier parts of the
> > > > > > > series to clean up / prepare, it starts at "bootstd: Add the bootstd
> > > > > > > uclass and core implementation" and keeps going from there.
> > > > > >
> > > > > > Yes it does add a lot of code, although it is a lot less if the
> > > > > > commands are disabled or simplified, e.g. to only support 'bootflow
> > > > > > scan -b'. At the moment it enables all dev features.
> > > > >
> > > > > OK, for the next go-round yes, please show what a typical enablement
> > > > > would look like on Pi, for example.
> > > >
> > > > OK. Do understand that conceiving of this and implementing it is quite
> > > > a bit of effort. At some point I just send things out, to get feedback
> > > > and to think some more. Apart from anything else, there is a risk of
> > > > going into the weeds or never finishing it.
> > > >
> > > > >
> > > > > > It does save a small amount of data. E.g. rpi_3_32b environment goes
> > > > > > drops by 3.5KB.
> > > > >
> > > > > So we're replacing 3.5KB of scripts with 17KB of code.  That is not a
> > > > > win.
> > > >
> > > > Certainly not on size! On testing, debug, visibility and control of
> > > > the boot process, there are wins.
> > >
> > > I'm not sure if there's wins on those grounds either, and certainly not
> > > enough to justify what this adds.
> > >
> > > > > > We should compare this with the EFI support which is about 90KB, as I recall.
> > > > >
> > > > > No, we shouldn't.  This isn't about replacing EFI, this is about
> > > > > replacing the generic distro boot macros, so that's what the size
> > > > > comparison is to.  At the end of the day, and looking towards non-legacy
> > > > > uses, a big common use case is "Find the EFI app to run".
> > > >
> > > > I just mean that EFI has been accepted as part of U-Boot and adds 90KB.
> > >
> > > OK?  I still don't see the relevance here.
> > >
> > > > > > If we continue down the path of making this feature use U-Boot
> > > > > > functions directly, instead the command line, I suspect we can save
> > > > > > quite a bit more, since there is a lot of overhead with these
> > > > > > commands. At present it is impossible to boot without CONFIG_CMDLINE
> > > > > > enabled, for example.
> > > > >
> > > > > Yes, this should be using the API and not the command interface.
> > > >
> > > > Right, but that's not something I am taking on right now. The PXE
> > > > refactoring gives an idea of what is needed. I did that work mainly
> > > > because I don't like adding to code that desperately needs
> > > > refactoring. We need to do the same for dhcp, EFI and bootm/zboot, but
> > > > that might take a year.
> > >
> > > Well, maybe this particular part of things get set aside for now, and
> > > the generic distro boot framework just needs to be moved to the env
> > > update you did.
> > >
> > > > > > In any case, I think this feature is filling a gap in U-Boot since at
> > > > > > present everything about boot is ad-hoc. This gives us a base to build
> > > > > > on. Nothing is for free.
> > > > >
> > > > > I disagree.  At present, booting is either intentionally per-board, or
> > > > > it's using the generic distro boot framework.  That framework is what to
> > > > > further build on or perhaps make more readily simplified (for example,
> > > > > making it just be "scan around for EFI" or just be "scan around of
> > > > > extlinux.conf").
> > > >
> > > > Well if the Universal Bootloader is only going to exist to boot EFI,
> > > > then we should rename it :-)
> > > >
> > > > I am not sure that anyone wants something intentionally per-board,
> > >
> > > There's some cases, yes, where the system is supposed to do X (or Y, or
> > > Z).  Otherwise there's the generic framework.  Or...
> > >
> > > > it's just that everything about the boot in U-Boot is really low-level
> > > > (bootm, fixed addresses, etc.) We need the layer on top that can deal
> > > > with these silly details. For example, yes there is a Chrome OS boot
> > > > script in chromebook_coral, but it is the same on all devices. I would
> > > > rather just enable that bootmethod so that if Chrome OS is present it
> > > > will boot.
> > >
> > > there's things like what Chrome OS wants.
> > >
> > > > Re the memory side, i don't like the vars that define the kernel
> > > > address, FDT address, etc. It seems to me that most/all are
> > > > unnecessary, if there is something able to deal with memory allocatoin
> > > > automatically. Perhaps we should use malloc() more, or use LMB more
> > > > proactively.
> > >
> > > Well, in that for Linux, arm64 the Image format has a header that lets
> > > us avoid a number of problems that weren't possible on arm32, there's a
> > > tiny bit more flexibility there.  But it sounds like you're talking
> > > about "bootm_size" and "bootm_low" now.
> > >
> > > > Even for custom flows, creating a bootmethod will have advantages, I think.
> > > >
> > > > The other thing is that this allows further innovation. For verified
> > > > boot, it makes it possible to sensibly deal with A/B;recovery, whereas
> > > > at present that is all just scripting, certainly not handled by distro
> > > > boot.
> > >
> > > No distros implement A/B updates directly.  When implemented on top of
> > > them it's done via the environment, yes.  I don't think adding Mender
> > > and RAUC and swupdate specific bootflow commands is a step in the right
> > > direction at all.
> > >
> > > > > > Anyway, I can look at what the minimum size is with the above points
> > > > > > and send that info through.
> > > > >
> > > > > I looked at the PXE stuff, and I think the minimal growth there ends up
> > > > > being reasonable, fwiw.  It comes down to adding sanity checks in places
> > > > > where the code wasn't always sanity checking, as you reduced
> > > > > duplication.
> > > >
> > > > Yes and perhaps the growth can be reduced, too.
> > >
> > > It must be.  You need to be a lot closer to parity with the existing
> > > generic boot mechanism and around that size.  I am not liking what I'm
> > > seeing here so far.
> >
> > Just on this point, I'm pretty sure this will kill it. We cannot add
> > code without increasing the size! It sounds like 17KB on ARM64 is too
> > much (perhaps 13.5KB including env reduction). Firstly, why, and
> > secondly, how much is acceptable? If the answer is zero, we are not
> > going to go anywhere with this, at least without a huge amount of
> > future refactoring, which is not going to happen since we cannot land
> > 100s of patches all at once.
> >
> > The big prize would be to be able to disable CONFIG_CMDLINE and still
> > boot. That saves 200KB or more.
>
> I think the problem is this just needs to get put on hold for a while.
> I am not convinced this is moving things in the right direction.
> Conceptually, it's duplicating a lot of the efi bootmgr functionality,
> but also adding in non-block devices.  And I am not liking configuring
> this via device tree, rather than environment.  Lets move the generic
> distro framework to the new env framework and continue finishing off
> other technical debts that we have, before we re-visit a brand new way
> of handling booting.

OMG, please, no :-)  That would be a mess IMO. We should have done a
proper boot framework when distroboot was created, instead of all the
scripts.

Now it's seems we need to stick with that, or move to EFI bootmgr?

Is the new env framework agreed? I'd love to see some reviews from Wolfgang.

Regards,
Simon