Change in coreboot[master]: console: Allow configuring log behavior on slow coreboot consoles

[Julius Werner (Code Review)]

Attention is currently required from: Marc Jones.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45208 )

Change subject: console: Allow configuring log behavior on slow coreboot consoles
......................................................................

Patch Set 14:

(4 comments)

File src/console/Kconfig:

https://review.coreboot.org/c/coreboot/+/45208/comment/a380aa10_4ea7c44b 
PS14, Line 43: config OPTIONAL_SLOW_CONSOLE
nit: In line with the suggestion of merging all this into the "main" loglevel, I'd consider just renaming this to LOGLEVEL_FROM_CBFS or something like that.

https://review.coreboot.org/c/coreboot/+/45208/comment/2044abbf_a1bd87d5 
PS14, Line 63: config ADD_SLOW_CONSOLE_LOGLEVEL_CONFIG
You seem to imply that not setting this doesn't add the file, but I don't think(?) it works that way because in the Makefile you're just testing `ifneq ($(CONFIG_SLOW_CONSOLE_LOGLEVEL),)`. In Kconfig, `if xxx` blocks are simply equivalent to adding `depends on xxx` to every option within them, and the final output config file always contains values for all options -- the ones with unmet dependencies are simply forced to the zero value for the respective type. For an `int` option like SLOW_CONSOLE_LOGLEVEL, that should be 0, so testing it for emptiness doesn't work. (FWIW I'm not sure if this option is necessary, I'd just always add the file when the whole feature is enabled in general.)

File src/console/init.c:

https://review.coreboot.org/c/coreboot/+/45208/comment/a756bfff_19a4e101 
PS14, Line 31: 		console_loglevel = get_uint_option("debug_level", console_loglevel);
The more I stare at this, the more I think this should really just all go into here instead. There's no reason to have a main loglevel and a "slow loglevel" that are separate from each other. We already have a "fast loglevel" (that's basically hardcoded to BIOS_DEBUG, unless the main one is higher), so the existing main loglevel *is* already only for the slow consoles, I don't think there's a point in introducing another slow loglevel concept separate from it. There should only be one loglevel and it should be determined in this function, based on what stage we're in and what different kinds of loglevel sources are enabled by Kconfig. (The priority order should probably be: option table highest, CBFS second highest, then whatever get_console_loglevel() returned as the ultimate fallback.)

You also shouldn't need the extra "status" enum then because console_inited already manages the initialization/recursion here, the USE_OWN vs. GLOBAL distinction disappears, and DISABLED can just be represented as log level -1 (or maybe it would make sense to tell people to just set it to BIOS_EMERG to "disable" the console, since by definition that shouldn't print anything unless you're already in a situation where boot time is the least of your concerns).

File tests/console/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/45208/comment/642b395a_e375252a 
PS14, Line 10: routing-with-cbmemcons-test-cflags += -I 3rdparty/vboot/firmware/include
That's... odd? Why is this needed here? Sounds like if you need to add this here to make things compile, we should fix the test framework's general include list instead?