Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34355 )
Change subject: nb/via/vx900: Ensure framebuffer size is within limits ......................................................................
Patch Set 2: Code-Review+1
(2 comments)
I don't know if this chipset's code took inspiration from italian cuisine, or if I'm just very cursed myself, but I find this function rather weird
https://review.coreboot.org/c/coreboot/+/34355/2/src/northbridge/via/vx900/m... File src/northbridge/via/vx900/memmap.c:
https://review.coreboot.org/c/coreboot/+/34355/2/src/northbridge/via/vx900/m... PS2, Line 84: 0 After that assert, you know the first iteration of the loop will always run. This could be a 1, or the loop could be a do-while. That way, it's easier to prove that decrementing fb_pow will not cause an underflow.
https://review.coreboot.org/c/coreboot/+/34355/2/src/northbridge/via/vx900/m... PS2, Line 89: size_mb = (1U << fb_pow); I see why you would want to change this, but what if fb_pow is, e.g., 255?