Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, Christian Walter, Jincheng Li, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar, Tim Chu.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80684?usp=email )
Change subject: soc/intel/xeon_sp: Add fsp_headers.h ......................................................................
Patch Set 11: Code-Review+1
(1 comment)
This change is ready for review.
Patchset:
PS11:
Increases complexity and decreases maintainability and is based on non-existing specifications. […]
For SKX to SPR, there are 2 class of FSP headers. One class is common headers, like <IioUniversalDataHob.h> which is commonly used for all bootloaders, include UEFI and SlimBoot. Another class is coreboot extended FSP headers, like <hob_iiouds.h>. These extended headers are used in coreboot only and maintained in coreboot repo (actually they are not released with initial FSP, but added during coreboot dev cycles).
In GNR and future generations, the generality of FSP headers is improved and these extended headers are not needed. Hence files like <hob_iiouds.h> will not be used. Accordingly, we need to remove the references of these headers from Xeon-SP common codes, otherwise, compilation cannot pass for GNR.
The solution this patch is not the only solution to deal with the problem. We have alternatives, e.g.,
1) to still keep <hob_iiouds.h> for GNR, but leave them empty so that could have coreboot pass compilation, or,
2) use macro/Kconfig, e.g. (USE_COREBOOT_SPECIFIC_FSP_HEADERS) to include these references only for SoCs that need this, e.g. SKX to SPR.
My preference is either that we use this solution, where the FSP header reference is centralized in a clean way for Xeon-SP codes (fsp_upd.h already provides a centralized place to handle similar stuffs). Either we choose 2) to use a macro to explicitly to control this.
Please let me know your opinion.
-
Shuo Liu (Code Review)