Attention is currently required from: Furquan Shaikh, Michał Żygowski, Marshall Dawson, Nikolai Vyssotski, Andrey Petrov, Patrick Rudolph, Nathaniel L Desimone. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56190 )
Change subject: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header ......................................................................
Patch Set 5:
(2 comments)
File src/drivers/intel/fsp2_0/util.c:
https://review.coreboot.org/c/coreboot/+/56190/comment/82204001_b402c6ca PS5, Line 17: if (CONFIG(PLATFORM_USES_FSP2_2)) Somewhat unrelated to this patch, but does it make sense to switch this from a config to a runtime option? I could see this being a problem if at some point in the future, we updated a platform's FSP version. Say 6 months from now, we decided to update the Cezanne FSP from 2.0 to 2.2.
Or should we just not worry about that? Either way, I think that's for a future patch if we do decide to change it.
https://review.coreboot.org/c/coreboot/+/56190/comment/fb0ee6f4_30ea485a PS5, Line 30: Should we check the bytes 10 & 11 as well? If we're not changing the version at runtime, Byte 10 should directly match the FSP version config in BCD. For Byte 11 I'd expect that FSP 2.2 would use header version 5, 2.1 would be 4, and 2.0 would use header version 3.
I'd be good with this being done in a follow-on patch as well if we decide to do it.
-
Martin Roth (Code Review)