Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38536 )
Change subject: libpayload/corebootfb: Fix character buffer relocation ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38536/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/video/corebootfb.c:
https://review.coreboot.org/c/coreboot/+/38536/1/payloads/libpayload/drivers... PS1, Line 71: #define CHARS (chars) nit: Can we just use 'chars' in the code now?
https://review.coreboot.org/c/coreboot/+/38536/1/payloads/libpayload/drivers... PS1, Line 246: chars = malloc(coreboot_video_console.rows * So you're saying this call happens before relocation, right? Then wouldn't 'chars' hold the physical address of the allocated buffer? And then later when the code is accessing it, the buffer will be mapped at a different address, so you'd need a phys_to_virt() again? Maybe the virt_to_phys() here was essentially a no-op (although I don't think that's necessarily true when CONFIG_LP_SKIP_CONSOLE_INIT is set and the payload does it at a later time), but the phys_to_virt() still had a purpose?