Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32532 )
Change subject: security/vboot/vboot_crtm.c: Use ENV_ conditions for vboot_measure_cbfs_hook() ......................................................................
Patch Set 2:
(2 comments)
Patch Set 2:
(1 comment)
I suggest using CONFIG() at function call: if (CONFIG(VBOOT_MEASURED_BOOT) if (vboot_measure_cbfs_hook(fh, name)) return -1;
and removing #if/#else/#endif condition in vboot_crtm.h
https://review.coreboot.org/#/c/32532/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32532/2//COMMIT_MSG@10 PS2, Line 10: is enabled, but this function is defined as 0 in vboot_crtm.h using ENV_
Do we need all the ENV_ checks in the header? There's already a vboot_logic_executed() check in the […]
One or the other way conditions in vboot_crtm.h and vboot_crtm.c should be in sync.
I could not find the info why defines for vboot_measure_cbfs_hook() are used.
https://review.coreboot.org/#/c/32532/2/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/#/c/32532/2/src/security/vboot/vboot_crtm.c@142 PS2, Line 142: #if !ENV_BOOTBLOCK && !ENV_DECOMPRESSOR && !ENV_SMM
Don't you also need CONFIG(VBOOT_MEASURED_BOOT) just like it is done in vboot_crtm.h? […]
The VBOOT_MEASURED_BOOT is already used in makefile.inc to include this file.