| Message ID | 20231108182625.46563-1-alpernebiyasak@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | firmware: coreboot: framebuffer: Avoid invalid zero physical address | expand |
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.
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 &&
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(+) base-commit: 2220f68f4504aa1ccce0fac721ccdb301e9da32f