Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36208 )
Change subject: security/vboot: Add vboot callbacks to support EC software sync ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... File src/security/vboot/sync_ec.c:
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 267: rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED || rv != VB2_SUCCESS
Shouldn't this just be: […]
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 320: google_chromeec_init();
Yeah this function seems really weird. […]
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 321: if (google_ec_running_ro()) { : *in_rw = 0; : } else { : *in_rw = 1; : }
*in_rw = !google_ec_running_ro();
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 364: cbfs_boot_map_with_leak
This wouldn't work for non-x86 systems. See my comment about using cbfs_boot_locate(), etc.
Yeah this is changed in the next patchset.
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 419: assert(devidx == 0);
Probably goes away once you update vboot.
Done
https://review.coreboot.org/c/coreboot/+/36208/2/src/security/vboot/sync_ec.... PS2, Line 433: -1
VB2_ERROR_UNKNOWN?
Done