Attention is currently required from: Arthur Heymans, Raul Rangel, Furquan Shaikh, Philipp Hug, ron minnich, Stefan Reinauer, Angel Pons, Subrata Banik, Julius Werner, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Piotr Król, Jason Glenesk, Michał Żygowski, Martin Roth, Marshall Dawson, Ron Minnich, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55068 )
Change subject: [WIP]Allow to build romstage sources inside the bootblock ......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Why are we architecturally going back to 2004 in order to save 10-20kb? With all due respect but that seems like a very questionable micro-optimization at best to put it mildly. Are you really suggesting creating another code path for a platform that only runs on at least 2mb worth of blobs is worth it for 10kb on a 32MB flash chip?
I wasn't around then but 2004 coreboot looks very different from 2021 coreboot. There were romcc compiled stages and a bootblock/romstage separation just made more sense then. Also the x86 bootblock as limited to 64K which is not the case anymore. Now coreboot already supports tons of different bootpaths: just take a look at the rules.h file! Coreboot supports statically linked code, XIP code, relocatable code, separate or not verstage either linked in bootblock or in romstage, decompressor stages in bootblock to make things smaller and faster, ... This just adds one extra bootpath. I don't get why it's so scary. Keeping the complexity increase limited for different use cases is a price to pay and however often it leads to better abstractions and APIs, therefore increasing the general quality of the codebase. See the previous patch that makes for a common romstage entry. Besides this patch barely changes 20 lines and could probably be even less! I think this is a solid argument that this bootpath won't be hard to maintain.
I don't get why the size of some big blobs required on some platform is a good argument for anything. It's just being pessimistic about any optimization one can make in coreboot.
Implementing this on x86 is just a POC. The concept is not x86 specific though and requires only small changes for other platforms. On x86 often makes more sense for a 'big' bootblock than on other platforms since there is a lot of care and complexity needed to keep CAR symbols in sync between stages. On AMD systems it's even worse as APs need to communicate across stages stages without having common CAR. FWIW I could just as easily turn your argument around and ask why bootblock/romstage separation is needed or good. For a lot of use cases bootblock/romstage is architecturally over engineered and a historical artifact. That solution had a lot of merits at the time for sure, but things are different now as there is no romcc anymore. It's currently useful for VBOOT and some other edge cases and that's it. Why should 'one' usecase determine how all other images should look and function?
If you want to save space, drop all the bloat that made it into the bootblock that shouldn't be in there in the first place (like console drivers)
Or the bloat that came with our so called generic drivers.
The code pulled in bootblock is very often the same code as the one in romstage and not just more bloated things like console drivers but 'essential' things required to continue booting, like fmap, cbfs, decompression. That code is present in all stages so less stages is often a good idea. https://review.coreboot.org/c/coreboot/+/52788/4 for instance links verstage inside the bootblock.
-
Arthur Heymans (Code Review)