Attention is currently required from: Arthur Heymans, Felix Singer, Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Lean Sheng Tan, Werner Zeh, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60405 )
Change subject: soc/intel/common/cse: Add `finalize` operation for CSE ......................................................................
Patch Set 25:
(1 comment)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/60405/comment/4ab67c50_b3c576a9 PS24, Line 1195: static void cse_final(struct device *dev) : { : if (CONFIG(SKIP_FSP_NOTIFY_PHASE_READY_TO_BOOT)) { : cse_send_end_of_post(); : : cse_control_global_reset_lock(); : : if (CONFIG(DISABLE_HECI1_AT_PRE_BOOT)) { : cse_set_to_d0i3(); : heci1_disable(); : } : } : : if (CONFIG(SKIP_FSP_NOTIFY_PHASE_END_OF_FIRMWARE)) : heci_set_to_d0i3(); : }
hi Sridhar, I am not following your statement, but seems like you are not too familiar with FSP and coreboot design.. Or else you would have known that any new changes in FSP spec will only comes with next version, and then we can then design accordingly based on latest FSP spec, but the current implementation should be consistent across FSP2.0.
+1 to what Sheng said, I'm unable to understand how you will able to change the FSP design and make those API call non-sequentially compared to how it's design in FSP2.0 driver (https://review.coreboot.org/c/coreboot/+/15855/) 6 year ago.
Even if you could build Intel UEFI FSP-Wrapper code in API mode, you will see those APIs are getting executed sequentially.
I don't want to say here, but looking at your review comments in this patch trend, it looks like you have some concern in mind which you are not sharing. Are you against this approach of coreboot implementing notify phase natively. If yes, then here is read for you.
FSPv2.1 release note in coreboot mailing list https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/AJUY6...
Dispatch Mode – A new optional FSP boot mode intended to ease integration of FSP with PI spec bootloaders. In dispatch mode, FSP-M and FSP-S are containers that expose firmware volumes (FVs) directly to the bootloader. The PEIMs in these FVs are executed directly in the context of the PEI environment provided by the bootloader. In dispatch mode, the FspMemoryInit(), FspSiliconInit(), and NotifyPhase() API’s are not used.
***NotifyPhase() is replaced by FSP-S containing DXE drivers that provide a native implementation of equivalent events for each of the NotifyPhase() invocations.***
FSP in dispatch mode is not using PEI implementation of Notify Phase instead uses DXE driver. Then what is wrong if coreboot decides to implement a native code of managing notify work. As I have mentioned, all these registers are part of EDS and FAS section 12 has mentioned about the OEM/ODM fundamental right about implementing those lockdowns. I'm even planning to land all the Silicon recommendations as per FAS over incremental CLs.