[M] Change in coreboot[master]: soc/intel/common: Introduce API to get the FSP Reset Status
Attention is currently required from: Derek Huang, Paul Menzel, Kapil Porwal, Andrey Petrov.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74783 )
Change subject: soc/intel/common: Introduce API to get the FSP Reset Status ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74783/comment/af95d1fb_cbb029cd PS2, Line 14: Ideally FSP API should be able to return the status (both success and : error code) upon exiting the FSP API but unfortunately there are some : scenarios in ADL/RPL FSP where MultiPhaseSiInit API is unable to return : any ERROR status. Hence, this function provides an additional hook to : read the FSP reset status by reading the dedicated HOB without relying : on the FSP API exit status code.
Is that dependent on the FSP version, that means, will it be fixed in a future version?
Can this FSP bug detected at runtime?
nop, there is no way to detect if the FSP API is broken and can't return the status in proper or not.
IMO, this was broken since the day of MultiPhaseSiInit API integrated inside FSP (meaning TGL)
File src/soc/intel/common/fsp_reset.c:
https://review.coreboot.org/c/coreboot/+/74783/comment/45b5d47a_eebfc56f PS2, Line 56: size_t size = 0;
You do not seem to use `size`, so there is no need if it’s uninitialized.
It is important to note that passing an uninitialized variable to a function is still undefined behavior. However, by passing a pointer, you can reduce the risk of unexpected results.
File src/soc/intel/common/reset.h:
https://review.coreboot.org/c/coreboot/+/74783/comment/c07583ed_a077d3da PS2, Line 22: th
the
Ack
-
Subrata Banik (Code Review)