Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36543 )
Change subject: security/vboot: Removed vboot_prepare from vboot_locator ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36543/4/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/36543/4/src/arch/x86/postcar_loader... PS4, Line 175: ENV_ROMSTAGE nit: would it ever not be?
https://review.coreboot.org/c/coreboot/+/36543/4/src/lib/prog_loaders.c File src/lib/prog_loaders.c:
https://review.coreboot.org/c/coreboot/+/36543/4/src/lib/prog_loaders.c@63 PS4, Line 63: (ENV_BOOTBLOCK || ENV_VERSTAGE) ENV_VERSTAGE is wrong here, that's when you explicitly *don't* want to run it (because then it already ran). However, vboot_run_logic() already checks for that (with the verification_should_run()/verification_should_load() checks). So I think it might be best to just remove all the checks here (including the VBOOT_STARTS_IN checks), call the function vboot_may_run(), and let the function itself figure out when it actually needs to run (like vboot_prepare() already does today).
https://review.coreboot.org/c/coreboot/+/36543/4/src/security/vboot/vboot_co... File src/security/vboot/vboot_common.h:
https://review.coreboot.org/c/coreboot/+/36543/4/src/security/vboot/vboot_co... PS4, Line 73: void vboot_run_logic(void); nit: If you remove the checks like I suggest, you should move this under the #if CONFIG(VBOOT) below and add an empty stub for it when vboot is not configured in.