Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37478 )
Change subject: libpayload: Cache physical cbmem console address ......................................................................
Patch Set 1:
(2 comments)
I just found out that using FILO with DEBUG_ALL enabled still shows the issue.
This seems independent of the cbmem_console, as the problem persists even with
# CONFIG_CONSOLE_CBMEM is not set
As I feared, more debug options enabled => more bugs ;) Maybe there is another console driver suffering in a similar way. If it's the last debug() call in x86/segment.c, a console driver might rely on outdated sysinfo.
https://review.coreboot.org/c/coreboot/+/37478/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/cbmem_console.c:
https://review.coreboot.org/c/coreboot/+/37478/1/payloads/libpayload/drivers... PS1, Line 60: just keep the physical address which won't break on relocation. */
Only few drivers that cache pointers are affected... […]
It makes much more sense, I just don't have the time for this, right now and don't want to leave the default config broken.
https://review.coreboot.org/c/coreboot/+/37478/1/payloads/libpayload/drivers... PS1, Line 64: cbmem_console_p
(This is another good reason to keep it physical as long as possible, because it really screws with […]
Yeah, this is fishy. I did it on purpose but I'm not sure anymore. What do we check here? if the field in sysinfo is NULL or if the value read from coreboot tables was NULL?
Making all sysinfo pointers physical, would solve this too.