Furquan Shaikh 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 18:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36208/17//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36208/17//COMMIT_MSG@12 PS17, Line 12: This patch assumes that the EC image is added to CBFS : uncompressed.
I have been waiting for these changes to land so that the Kconfig name is set, but the ebuild looks […]
Oh okay. Cool. Thanks!
https://review.coreboot.org/c/coreboot/+/36208/17/src/security/vboot/ec_sync... File src/security/vboot/ec_sync.c:
https://review.coreboot.org/c/coreboot/+/36208/17/src/security/vboot/ec_sync... PS17, Line 268: (burst / resp_flash.write_block_size) * : resp_flash.write_block_size;
Doesn't that mean that resp_flash.write_block_size must be a power of two?
Yes, that is the requirement.
Is there a guarantee of that in the EC?
Write block size seems to be the minimum number of bytes required in a write which currently seems to be a power of 2 in the EC. But, I don't see anything enforcing that check in the EC.
It might be okay just leaving this as is. If you use ALIGN_DOWN(), it would be good to at least add an assert() ensuring that the write_block_size is a power of 2.