Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33534 )
Change subject: vboot: relocate code to log and clear recovery mode switch ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/33534/1/src/security/vboot/vboot_logic.c File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/#/c/33534/1/src/security/vboot/vboot_logic.c@285 PS1, Line 285: vboot_log_and_clear_recovery_mode_switch
Two questions about saving recovery reason to VBNV: […]
From the history of how vboot_save_recovery_reason_vbnv() was added, it looks like it was done to address an issue on x86 platforms specifically. These platforms never return from prog_run() in verification_should_load() clause and so the vboot_save_recovery_reason_vbnv() ended up being added to verification_should_run() clause only. vboot_save_recovery_reason_vbnv truly does something only if VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT is selected which is currently done only on x86 platforms. But I agree that in order to keep both paths consistent, it would make sense to take the action of saving recovery reason to vbnv in both clauses.