Attention is currently required from: Andrey Petrov, Bora Guvendik, Chen, Gang C, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Paul Menzel, Ronak Kanabar, Shuo Liu, Subrata Banik, Tarun.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81212?usp=email )
Change subject: drivers/intel/fsp2_0: Support FSP-M execution from CBFS cache ......................................................................
Patch Set 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81212/comment/badbe9d0_0faa36f2 : PS6, Line 42: 2 MB
This commit goal is to offer an alternative to `FSP_M_ADDR` solution allowing the use FSP-M compression. The commit message also provides some information on the impact when used on particular board design for illustration. With this scope in mind, what would you suggest to add to this commit to help with your point ?
I think this is a step in the right direction. Both AMD FSP code and Intel APL/GLK code uses a static addr for FSP-M. They should be moved to this solution if possible?
OEMs/ODMs often face challenges when testing devices across a wide range of configurations (from Celeron to i7) using a unified AP FW image. Hidden dependencies in the AP FW, such as minimum cache size requirements, can be easily overlooked, causing issues on low-end devices.
We need a way for OEMs/ODMs to statically assess whether their SoC designs meet the requirements for specific AP FW features, which i don't believe is the process.
I suppose it indeed does not make sense to set any default but 'n' on the SOC level, unless all SKUs would support this bootboth / have enough cache (server? although their FSP are massive). On the mainboard level however it could be an interesting option that users could carefully enable, possibly per variant? Codewise
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/81212/comment/2f4ea969_10fc1732 : PS8, Line 229: It requires the PRERAM_CBFS_CACHE_SIZE to be large enough to : accommodate the entire FSP-M binary and therefore the SoC to : support a large enough Cache-As-RAM (DCACHE_RAM_SIZE). : The commit message says something about needing adjustments for this to work. I think that's worth mentioning.