Change in coreboot[master]: console: Allow VPD to disable an otherwise enabled coreboot console

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45208 )

Change subject: console: Allow VPD to disable an otherwise enabled coreboot console
......................................................................

Patch Set 4: Code-Review-1

(1 comment)

https://review.coreboot.org/c/coreboot/+/45208/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45208/4//COMMIT_MSG@18 
PS4, Line 18: entry is added.
No, that's not true. The patch makes an unconditional call with very complicated dependencies very early in the boot process. It may logically not change behavior but it pulls in a lot of code into stages that otherwise wouldn't link it, adds boot time impact to every pre-RAM stage (VPD is not meant to be accessed pre-RAM and not made to be efficient there), and adds a big risk of hanging the firmware before it can output anything on the console if there is any issue in that large chain of dependencies.

This issue has already been discussed in several places recently (e.g. CB:45408, CB:45405, CB:45774). This patch has basically the same issues and I still think we shouldn't do something like this at all, because VPD is just a system that isn't well-suited for this super early use. But if enough people think we do need something like this, it should absolutely be behind a Kconfig so it can be disabled to the point of not even linking the extra code, and it should be default-off.

I think Nico's attempt of tying it into the get_option() API and having a default-off Kconfig to allow options to be read from VPD (CB:45774) was the best approach yet if we really want to go down that route. Trying to expand/fix the option table API seems cleaner than adding yet another one-off control somewhere else.