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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 7413 bytes --]

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@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@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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]