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/8f5930cd_f24ef36d PS1, Line 15: #endif
For one, some stages are _pretty_ packed, e.g. verstage on google/veyron. […]
Okay, I think we're all confused about what we're talking about now. ^^ What I mean is using ENV_INITIAL_STAGE instead of the the FIRST_CONSOLE macro that gets defined here. So the code in init_log_level() should just read:
console_loglevel = get_console_loglevel();
if (!ENV_INITIAL_STAGE) console_loglevel = get_uint_option("debug_level", console_loglevel);
(and then the stuff Patrick adds in the next patch would also fall under that). That way we don't need an extra FIRST_CONSOLE macro and all the vboot stage ordering stuff has already been taken care of correctly in the definition of ENV_INITIAL_STAGE.
If the question is why we need any check at all and can't just call get_uint_option() in every stage... I don't know, I'm not super familiar with how CMOS stuff works on x86. Are there any platforms where accessing CMOS requires any sort of platform-specific setup beforehand? For CBFS that is definitely the case, so at least for CBFS we need to prevent accesses in the first stage. For CMOS, if you're sure that no point to call it would be too early, I guess it doesn't matter.
-
Julius Werner (Code Review)