Attention is currently required from: Michał Żygowski, Yu-Ping Wu.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74343 )
Change subject: security/vboot: Add function to clear recovery request ......................................................................
Patch Set 3:
(1 comment)
File src/security/vboot/bootmode.c:
https://review.coreboot.org/c/coreboot/+/74343/comment/31cded7d_ca439294 PS3, Line 31: save_vbnv
Do we need to save nvdata here in this function? Usually it should be sufficient to save nvdata righ […]
I mean, the way this is designed to run (at the end of ramstage, with the assumption that there's no vboot-aware payload coming after it), it needs to save here because there won't be another save afterwards. Whether the save_vbnv() call should be in this function or the caller is debatable. I'd suggest (also see my comment in CB:74402) just transforming this function into a boot state hook that's not called by anything else and just does it's thing (clear recovery reason and safe vbnv) at the end of ramstage in one go.
back_up_vbnv_cmos() is a different thing since it only runs for the CMOS backend, so I think it should stay independent (it just needs to be ordered right to ensure the clearing recovery reason happens before the backup).
-
Julius Werner (Code Review)