Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38706 )
Change subject: security/vboot: relocate vb2ex_abort and vb2ex_printf ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38706/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38706/4//COMMIT_MSG@13 PS4, Line 13: which is linked into ramstage.
Maybe go even further and change compilation of vboot_common. […]
I agree -- but perhaps we should hold off on this until we have cleaned up/merged security/vboot/common.c and security/vboot/vboot_common.c?
Or perhaps we can have one file for "vboot common" and one for "vboot lib common"?
https://review.coreboot.org/c/coreboot/+/38706/4/src/security/vboot/vboot_co... File src/security/vboot/vboot_common.c:
https://review.coreboot.org/c/coreboot/+/38706/4/src/security/vboot/vboot_co... PS4, Line 29: #if CONFIG(VBOOT)
Why does this need to be guarded? If this function isn't used it will get dropped at link time.
This is to match the CONFIG(VBOOT) conditional in vboot_common.h. Unfortunately the set of functions defined in the conditional are spread all over the place. The "vboot utility code" in coreboot could probably use some shuffling around.
https://qa.coreboot.org/job/coreboot-gerrit/115915/ failure:
src/security/vboot/vboot_common.c:30:5: error: redefinition of 'vboot_can_enable_udc' int vboot_can_enable_udc(void) ^~~~~~~~~~~~~~~~~~~~ In file included from src/security/vboot/misc.h:20, from src/security/vboot/vboot_common.c:24: src/security/vboot/vboot_common.h:80:19: note: previous definition of 'vboot_can_enable_udc' was here static inline int vboot_can_enable_udc(void) { return 1; }