Attention is currently required from: Kangheui Won, Maulik V Vaghela, Paul Menzel, Tim Wawrzynczak, Nick Vaccaro, Andrey Petrov. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62942 )
Change subject: drivers/intel/fsp2_0: Add provision to extract FSP Performance Data ......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62942/comment/b636ed44_f1425dac PS2, Line 9: This patch enriches coreboot FSP2.0 driver to extract the FSP timestamp : from FPDT (Firmware Performance Data Table) and display as part of : ramstage boot stage if the SoC user selects the required : `GET_FSP_TIMESTAMP` config.
Extracting this data sounds great. But instead of printing it to the CBMEM console, could you please add them to the CBMEM timestamp table?
we already have FSP TS data as part of cbmem -t table. what this CL does is to have detailed break down of the FSP timestamp which might be useful to debug or optimise the FSP per component boot time.
Is the FSP data theoretically also accessible from the operating sytsem?
Not sure, if I understood your comments, but cbmem -t is able to show the FSP API wise boot time impact. where this CL is more deep dive into those FSP APIs.
950:calling FspMemoryInit 1,936,192 (59,275) 951:returning from FspMemoryInit 81,024,105 (79,087,913) 971:loading FSP-S 82,941,127 (21,318) 954:calling FspSiliconInit 83,166,997 (219,188) 955:returning from FspSiliconInit 102,564,474 (19,397,476) 962:calling FspMultiPhaseSiInit 102,564,627 (152) 963:returning from FspMultiPhaseSiInit 108,645,466 (6,080,839)
File src/drivers/intel/fsp2_0/fsp_timestamp.c:
https://review.coreboot.org/c/coreboot/+/62942/comment/750b9716_806100de PS2, Line 101: memcpy((void *)rec, (uint8_t *)hdr + sizeof(struct fpdt_pei_ext_perf_header),
Since we're only reading data, can we avoid malloc/memcpy/free and just iterate over table?
Ack