Attention is currently required from: Andrey Petrov, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, 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_DOES_NOT_NEED_TEMP_RAM ......................................................................
Patch Set 3:
(1 comment)
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80328/comment/5c552852_dea5576d : PS1, Line 156: goto temp_ram_assigned;
Yeah, for the RC sample fixes (thanks for providing the inputs :-)), we made careful evaluations. However, due to RC baseline logic changes, it cannot be directly applied to GNR, and also the performance scaling aspects needs to be considered along with the increasing of socket counts. The other related factor is thorough tests needed to be executed on all related configurations, include FSP dispatch mode and UEFI BIOS. Due to these opens, the agendas will be addressed on the next generation.
Well you need to synchronize the CAR heaps across sockets. It's not hard to do that for both heaps separately which would have 0 performance impact and should be very straightforward to test: do both coreboot and UEFI still boot. Is avoiding 30 minutes of testing really worth it over the many meetings and email and friction to get workarounds in place at the wrong level (it is breaking the FSP spec!)?
Note that this issue and fix was reported for SPR-SP early in the bringup, long before GNR was announced or even worked on. A proper fix was promised for GNR and now you're saying that's not happening either?! The process of working with Intel and fixing issues is really not how it should be (not blaming you ofc, but this issue is frustrating)!
Anyway to move forward: let's call it a workaround and make the memory allocations explicit such that it's clear from the code and the comments what is going on.
As to the patch commit message, sure, let me try to add more info around for the ease of technically maintenance. BTW, we could also revise FSP integration guides for related generations to address this usage.
Or admit that this is a workaround... Documenting it here in code is good enough IMO.