[L] Change in coreboot[main]: Introduce a coreboot Control Block (CCB)

[Julius Werner (Code Review)]

Attention is currently required from: Jérémy Compostella, Simon Glass.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79317?usp=email )

Change subject: Introduce a coreboot Control Block (CCB)
......................................................................

Patch Set 2: Code-Review-2

(5 comments)

Patchset:

PS2: 
Sorry but I do not understand why we are suddenly proceeding to implementation with this. Everywhere this was discussed before in both the leadership meetings and on the mailing list, you have repeatedly received feedback from multiple people that we shouldn't add yet another configuration backend and instead just add serial control to any of the existing ones. I feel like you have consistently just ignored that feedback and kept bringing up the same proposal again and again without even really engaging the objections.

I still maintain that this is not good for coreboot, the added complexity to firmware layout and cbfstool is in no way worth the benefit, this interacts badly with edge cases on certain platforms or other feature configurations (e.g. verification), and we should focus on consolidating the existing configuration backends we have rather than adding yet another one that is super narrow and will in practice only be used for a single thing (especially when it is so invasive). People have proposed plenty of alternatives that could be used for post-build serial control including option table (CMOS), VPD, FW_CONFIG (CBI) and a CBFS file — in fact, the option table mechanism for this has always existed, and for CBFS files we have an almost finished patch series which would just need to be rebased and cleaned up (CB:45208). The only real argument for adding all this extra stuff instead is to control serial output in a very narrow time window in the early bootblock before CBFS access is available, and it seems incredibly unlikely that something goes wrong in that tiny early window during a development phase where you don't still have serial console enabled by default anyway.

If you are adamant to keep pushing this approach then I guess maybe we'll need to have another big discussion and force a decision in the leadership meeting or something. But I don't think this should be decided by just continuously pushing the same thing until the people with objections get tired or happen to be on vacation. So I'll -2 this until I'm convinced there is a majority in favor.

File Documentation/technotes/ccb.md:

https://review.coreboot.org/c/coreboot/+/79317/comment/d2670d71_88bd445f :
PS2, Line 79: Some boards used a signed bootblock which cannot easily be modified after
            : building, since it requires resigning parts of the image. To address this, the
            : CCB could be stored in the romstage instead. This means it is unable to affect
            : the operation of bootblock, of course. This could be controlled by a new
            : ROMSTAGE_CCB option.
This would literally invalidate the only reason you've used to justify this approach (over other post-build configuration options like CBFS files).

File src/arch/x86/car.ld:

https://review.coreboot.org/c/coreboot/+/79317/comment/53d43d0b_7ad2e5f2 :
PS2, Line 45: 	CCB(., 0x10)
This file only affects x86 devices (excluding AMD). Expanding this to other platforms puts the burden of finding room for this in the memlayout (and remembering to do so) on every single platform owner, which will lead to a fragmented landscape where only a random selection of platforms supports this. That's another good reason to reject this solution over the alternatives that don't need to pass the setting forward between stages.

File src/arch/x86/postcar.c:

https://review.coreboot.org/c/coreboot/+/79317/comment/c2ec411e_cea94f82 :
PS2, Line 33: 	console_init();
Moving console_init() down here makes it impossible to debug CBMEM initialization handlers, of which there are many and which can sometimes be non-trivial.

File src/console/Kconfig:

https://review.coreboot.org/c/coreboot/+/79317/comment/dc0e3bfe_6f107e3b :
PS2, Line 433: 	default y
Any feature for runtime control of console behavior should probably be based on loglevel as it was suggested back in CB:45208, not create yet another orthogonal setting.