Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37562 )
Change subject: EC sync: Properly handle VBERROR return codes from vb2api_ec_sync ......................................................................
Patch Set 1:
(4 comments)
Whoops, yeah, not sure how we all missed that...
https://review.coreboot.org/c/coreboot/+/37562/1/src/security/vboot/ec_sync.... File src/security/vboot/ec_sync.c:
https://review.coreboot.org/c/coreboot/+/37562/1/src/security/vboot/ec_sync.... PS1, Line 69: vboot_reboot(); This seems a bit odd for error handling? If you do want to reboot, you should at least vb2api_fail() first so that we'll drop into recovery mode. Otherwise, I think a die() would also be fine here (it shouldn't really happen).
https://review.coreboot.org/c/coreboot/+/37562/1/src/security/vboot/ec_sync.... PS1, Line 71: poweroff(); poweroff() is not available on all coreboot platforms, so putting this here would prevent us from using it more widely. There's no need to do it anyway, you can just halt() and wait for the EC to reboot us.
https://review.coreboot.org/c/coreboot/+/37562/1/src/security/vboot/ec_sync.... PS1, Line 86: poweroff(); This has the same problem with poweroff(). Luckily, I don't think the EC sync code needs it?
https://review.coreboot.org/c/coreboot/+/37562/1/src/security/vboot/ec_sync.... PS1, Line 95: if (retval != VB2_SUCCESS) { nit: maybe just put a
case VB2_SUCCESS: break;
at the top?