On ARM64 systems coreboot defers framebuffer allocation to its payload, to be done by a libpayload function call. In this case, coreboot tables still include a framebuffer entry with display format details, but the physical address field is set to zero (as in [1], for example).
Unfortunately, this field is not automatically updated when the framebuffer is initialized through libpayload, citing that doing so would invalidate checksums over the entire coreboot table [2].
This can be observed on ARM64 Chromebooks with stock firmware. On a Google Kevin (RK3399), trying to use coreboot framebuffer driver as built-in to the kernel results in a benign error. But on Google Hana (MT8173) and Google Cozmo (MT8183) it causes a hang.
When the framebuffer physical address field in the coreboot table is zero, we have no idea where coreboot initialized a framebuffer, or even if it did. Instead of trying to set up a framebuffer located at zero, return ENODEV to indicate that there isn't one.
[1] https://review.coreboot.org/c/coreboot/+/17109 [2] https://review.coreboot.org/c/coreboot/+/8797
Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com --- (I was previously told on #coreboot IRC that I could add coreboot mailing list to CC for kernel patches related to coreboot.)
drivers/firmware/google/framebuffer-coreboot.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c index c323a818805c..5c84bbebfef8 100644 --- a/drivers/firmware/google/framebuffer-coreboot.c +++ b/drivers/firmware/google/framebuffer-coreboot.c @@ -36,6 +36,9 @@ static int framebuffer_probe(struct coreboot_device *dev) .format = NULL, };
+ if (!fb->physical_address) + return -ENODEV; + for (i = 0; i < ARRAY_SIZE(formats); ++i) { if (fb->bits_per_pixel == formats[i].bits_per_pixel && fb->red_mask_pos == formats[i].red.offset &&
base-commit: 2220f68f4504aa1ccce0fac721ccdb301e9da32f
Yeah, if the kernel wanted to make use of coreboot display init on those boards, it would have to reserve its own framebuffer space and need to have at least enough of a driver for the display controller to be able to tell it which address it picked. Until someone implements that, erroring out for those cases makes sense.
Reviewed-by: Julius Werner jwerner@chromium.org
On Wed, Nov 08, 2023 at 09:25:13PM +0300, Alper Nebi Yasak wrote:
On ARM64 systems coreboot defers framebuffer allocation to its payload, to be done by a libpayload function call. In this case, coreboot tables still include a framebuffer entry with display format details, but the physical address field is set to zero (as in [1], for example).
Unfortunately, this field is not automatically updated when the framebuffer is initialized through libpayload, citing that doing so would invalidate checksums over the entire coreboot table [2].
[...]
Applied, thanks!
[1/1] firmware: coreboot: framebuffer: Avoid invalid zero physical address commit: ecea08916418a94f99f89c543303877cb6e08a11
Just realized the bot didn't send the mail for the patch.
-
Alper Nebi Yasak -
Julius Werner -
Tzung-Bi Shih