Attention is currently required from: Krishna P Bhat D, Rizwan Qureshi.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78053?usp=email )
Change subject: soc/intel/cse: Add function to get cse_bp_info early ......................................................................
Patch Set 3:
(4 comments)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/78053/comment/d0d2666b_e2688ef0 : PS1, Line 468: is_cse_bp_info_valid
anyway had introduced the boot time impact (which we don't measure today) across the global reset but user will feel the same
the impact is across cold reset. This change will nullify the impact across warm/global resets.
There is no benefit of sending CSE cmd on every boot if u know the CBMEM data is persistent and CSE version can't be changed across warm reboot
we can take it up as a follow-up patch, if you insist.
lets do that later
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/78053/comment/61486147_6f827529 : PS2, Line 424: store_cse_bp_info_in_cbmem
store_cse_bp_info_in_cbmem() is being called by https://review.coreboot.org/c/coreboot/+/78053/2/src/soc/intel/common/block/... to store response in the case of late romstage cse_fw_sync() and also called by https://review.coreboot.org/c/coreboot/+/78053/2/src/soc/intel/common/block/... in CBMEM_CREATION_HOOK. cbmem_find(CBMEM_ID_CSE_BP_INFO) is only used at https://review.coreboot.org/c/coreboot/+/78053/2/src/soc/intel/common/block/..., better not to move it to common function.
i'm unable to follow the concern ?
you are trying to fetch the cse bp info from cbmem. if the information is readily available then fetch it or else, prepare the data into the cbmem. I don't understand why it's better not to move things into an unified function ?
take a look into cbmem_add implementation, it first do cbmem_find and on failure it makes a new entry.
https://review.coreboot.org/c/coreboot/+/78053/comment/f8582ac9_90bb7bae : PS2, Line 470: cse_bp_info_in_cbmem
https://review.coreboot.org/c/coreboot/+/78053/comment/2f9f5241_046c81c8/ As Rizwan suggested.
lets do that later as discussed https://review.coreboot.org/c/coreboot/+/78053/comment/2f9f5241_046c81c8/
https://review.coreboot.org/c/coreboot/+/78053/comment/96f90ffd_35143c41 : PS2, Line 507: if (cbmem_online()) : store_cse_bp_info_in_cbmem();
We have to consider the case of cse_fw_sync() being called in late romstage also like in JSL, TGL. The cbmem init hooks would have been run prior to cse_get_bp_info being called (after fsp memory init) and we would have to then copy the get_bp_info_rsp to cbmem.
Calling fetch_cse_bp_info_from_cbmem() from line 469 would eliminate the need to store the cse bp info later (basically do the cbmem add, like what this function is doing).