Attention is currently required from: Nico Huber. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56210 )
Change subject: console: Handle verstage as "first non-bootblock stage" ......................................................................
Patch Set 1:
(1 comment)
File src/console/init.c:
https://review.coreboot.org/c/coreboot/+/56210/comment/5e7b495a_c37632ad PS1, Line 15: #endif
Given the back and forth here I'm thinking of making all this explicit, even though I dislike option […]
No, please, let's not overkill it. The arch/arm requirement is more of a non-x86 (or non-BOOT_DEVICE_MEMORY_MAPPED) requirement anyway. And I think for x86 there are other good reasons not to rely on this in the bootblock too, more similar to what Nico mentioned, because CBFS *is* actually a pretty complicated system where it would be good to have output if something goes wrong (e.g. with CBFS_VERIFICATION, it will print an error and die on a hash mismatch... would be good if that was always visible). The thing is just that for CBFS, of course it's going to get used anyway whether BOOTBLOCK_CONSOLE is enabled or not, so if you have a CBFS problem and NO_BOOTBLOCK_CONSOLE you're not gonna see anything either way. For get_option(), I guess that's not necessarily true (but it might).
I would prefer to use ENV_INITIAL_STAGE to guard the CBFS stuff, and then whether get_option() is also guarded by that or not at all I don't care too much about. I'd assume the "using get_option() + using NO_BOOTBLOCK_CONSOLE + error in get_option() + get_option() not already used anyway by other bootblock stuff" intersection is probably not really big enough to worry that much about.
-
Julius Werner (Code Review)