Julius Werner 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)
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... […]
Oh okay, yeah, then all this makes a bit more sense. And yeah, that was probably not the best solution. ;)
https://review.coreboot.org/c/coreboot/+/37478/1/payloads/libpayload/drivers... PS1, Line 64: cbmem_console_p
Yeah, this is fishy. I did it on purpose but I'm not sure anymore. What […]
Yes, we're checking if the field in sysinfo was NULL. phys_to_virt() is not checking for NULL, so phys_to_virt(NULL) is just (void *)virt_offset. This code is correct because you're essentially checking (lib_sysinfo.cbmem_cons + virt_offset - virt_offset == NULL), I'm just saying I find the whole thing pretty hard to follow.