Hi Tom,
On Thu, 28 Oct 2021 at 16:50, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 28, 2021 at 04:22:11PM -0600, Simon Glass wrote:
Hi Tom,
On Thu, 28 Oct 2021 at 13:19, Tom Rini trini@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@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@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.
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?
I'm not sure if EFI bootmgr is going to cover the cases, or not. But yes, in so far as I have problems with the current distro boot, it's that dealing with strings in C kinda sucks. Having that be plain text will be good.
I'm sure EFI bootmgr will cover all the *EFI* cases, by definition. No one will understand it, though...
Yes the scripts themselves are not ideal. It is great that we have this feature but it will be even better to have proper U-Boot support for booting, I think. But anyway, I'll do the analysis I talked about and deal with any other feedback and we can park it for now.
I'd still like you to think about what would be an acceptable increment in terms of code size (you cannot say zero!)
Is the new env framework agreed? I'd love to see some reviews from Wolfgang.
I believe the new env framework is agreed, yes. So long as "complex" environments can be constructed and dropped in some way or another outside of the new tooling, that's good enough.
Well I was planning on migrating env_default.h but we can leave that for later I suppose.
I am pretty sure we will need to enhance it, but we can see how it goes. I'd like to get rid of the sandbox config file, at least. I got someway down the path there, so will dig up the patches.
Regards, Simon