Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Kapil Porwal.
Subrata Banik has posted comments on this change by Dinesh Gehlot. ( https://review.coreboot.org/c/coreboot/+/86153?usp=email )
Change subject: soc/intel/cmn/blk: Remove boot partition check for forced cse sync ......................................................................
Patch Set 13:
(3 comments)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/86153/comment/b5c6485f_f3071db0?usp... : PS12, Line 751: if ((vb2api_gbb_get_flags(ctx) & VB2_GBB_FLAG_FORCE_CSE_SYNC) && : cse_get_current_bp() == RO) {
The current flag-based implementation has backward compatibility and will always function regardless of the current boot partition. Therefore, I believe we may not need to check if the platform is newer or older than PTL. WDYT.
what is the point returning `return cse_info_in_cmos.cse_sync_status & CSE_ENFORCED_SYNC_REQUEST` as true if the CSE slot is RW? We won't be able to enforce CSE sync when CSE is booting from RW slot ? we might need to push CSE to go into RO slot so we can enforce CSE update
https://review.coreboot.org/c/coreboot/+/86153/comment/3922cd55_4723b0de?usp... : PS12, Line 765: is_cse_sync_enforced
The ESOL screen function at romstage also calls `is_cse_sync_enforced()`. At that stage, the enforce CSE sync status flags have not been updated. Therefore, an update to these status flags is required.
I'm unable to follow. if you take a look into `is_cse_sync_enforced` API used previously then this API just meant to tell if CSE sync being enforced but now you have used this function to also store the status of CSE info crc, CSE specific information etc. What i'm asking is to decouple this API. What you need here is just return `return cse_info_in_cmos.cse_sync_status & CSE_ENFORCED_SYNC_REQUEST`
if you need to update the CSE info CMOS status during `is_cse_sync_enforced`, then you can add `update_cse_info()` from all caller of `is_cse_sync_enforced` API.
https://review.coreboot.org/c/coreboot/+/86153/comment/70eadfe7_a61bfc66?usp... : PS12, Line 792: n cse_info_in_cmos.cse_sync_status & CSE_ENFORCED_SYNC_REQ;
Yes, the sync status update exhibits a distinct behavior and warrants relocation to a dedicated function, `store_cse_sync_status()`.
However, `is_cse_sync_enforced()` also determines if ESOL should be displayed or not. If we move the sync status update elsewhere, ESOL execution during ROMSTAGE might not receive the correct indication to display the ESOL screen.
you just need to decouple `is_cse_sync_enforced` API to ensure storing into CSE info can be handled by the caller of the API.