Attention is currently required from: Andrey Petrov, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Martin L Roth, Nico Huber, Patrick Rudolph, Paul Menzel, Ronak Kanabar, Shuo Liu.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email )
Change subject: drivers/intel/fsp2_0: Add FSP_ALLOCATES_TEMP_RAM ......................................................................
Patch Set 10:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80328/comment/aa1deafe_d2acb806 : PS10, Line 11: This is required by some FSP implementations : of Xeon SP multi-socket platforms. Please change this to "This works around a flaw in FSP for Xeon SP multi-socket platforms". Also reflect in the Kconfig naming that this is a workaround.
Patchset:
PS10: I still think this is not the right approach. Explicit allocation is desirable or at least some asserts that the default values are not overlapping with anything else used by coreboot. Try CB:80579 ?
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/80328/comment/f1ff0d78_7d10fded : PS10, Line 46: FSP allocates temporary ram in its reserved memory ranges : (covered in FSP_M_RC_HEAP_SIZE) instead of requesting coreboot to : do the allocation. This is required by some FSP implementations of : Xeon SP multi-socket platforms. : This is misleading: You just use the default value. FSP isn't doing allocating anything.
https://review.coreboot.org/c/coreboot/+/80328/comment/05e06446_ca618d75 : PS10, Line 53: default 0x0 if FSP_ALLOCATES_TEMP_RAM I don't think the option
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80328/comment/29460d42_253f1379 : PS10, Line 178: Bootloader should not override these values otherwise multi-socket : * platform booting will be broken. That is simply not true. FSP makes some wrong assumption about where the heap is. Leaving it default is not the only way of solving this.