Attention is currently required from: Dinesh Gehlot, Julius Werner, Paul Menzel.
Subrata Banik has posted comments on this change by Dinesh Gehlot. ( https://review.coreboot.org/c/coreboot/+/84120?usp=email )
Change subject: vc/google/chromeos: Skip boot info logging if cse sync at payload ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Do we really need to do this? We've had issues where vboot state is getting confused across additional platform resets (like CSE sync) in the past, I don't think it's necessarily a bad thing to just log the in-between state as well. Either way I don't think a double log is worth making all this more complicated and porting all this code (and adding the future maintenance burden of updating this code in two places, etc.).
We logged vboot_info so late in the boot process to ensure that there were no other agents that could trigger a known reset of the platform. These known agents include FSP boot flow, CSE slot change, and others. We wanted to register vboot information as part of elog so that we could understand if there was any abnormal behavior that caused the boot process to be reinitialized. For example, we wanted to know if the boot process failed from the previous boot, what slot was in use, and so on.
With the planned move of CSE sync to payload, there is another agent that could trigger a platform reset. This is because the CSE slot switch from RO to RW could cause a reset. Therefore, it does not make sense to register vboot information twice in elog, before (coreboot) and after (payload) the CSE slot change. Additionally, if we do not register the vboot information from payload, then we are not capturing the actual boot information event. Instead, we are using information that was interrupted by a CSE slot switch or global reset. Therefore, it makes more sense to avoid the coreboot attempt to log vboot information (when the platform decided to perform CSE switch at payload) and simply add one more entry to the payload. This way, we can still capture one instance of vboot boot information in normal scenarios.
The maintenance burden is one time, like we kept the CSE sync implementation both in coreboot and depthcharge. If time permits later, we shall try to use commonlib to keep only one instance of CSE sync to avoid maintenance overhaul.