Attention is currently required from: Jakub Czapiga, Jan Dabros, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60080 )
Change subject: libpayload: Enable vboot integration ......................................................................
Patch Set 3:
(7 comments)
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/60080/comment/0100d66c_7c584f26 PS3, Line 230: endmenu Should we put vboot in this menu with the other libraries, maybe?
File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/60080/comment/8923e6eb_b927937b PS2, Line 82: $(addsuffix .a,$(addprefix $(obj)/,$(libraries)))
Doesn't this also ask for an $(obj)/vboot. […]
No, I'm saying there shouldn't be a vboot.a. There should only be vboot_fw.a and tlcl.a. Maybe rather than making `vboot` an extra class, just add the two libraries to libc-objs?
File payloads/libpayload/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/60080/comment/c4fc6271_d63ede93 PS3, Line 13: config VBOOT_DEBUG Forgot to take this out?
https://review.coreboot.org/c/coreboot/+/60080/comment/c9c80a4c_bb558887 PS3, Line 30: This option enables SHA256 implementation using x86 SHA processor extension. Maybe list the instructions (sha256msg1, sha256msg2, sha256rnds2) like in the coreboot Kconfig, for clarity.
File payloads/libpayload/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/60080/comment/eb6cb443_6f83a1d9 PS3, Line 12: kconfig-to-binary=$(if $(1),1,0) (FWIW, this is fine, but the vboot Makefile is also specifically written such that OPTION= and OPTION=y also work as expected... so not really necessary.)
https://review.coreboot.org/c/coreboot/+/60080/comment/9024cb6a_8a706aea PS3, Line 18: VBOOT_CFLAGS += -I$(abspath $(obj)) -Wno-missing-prototypes Are these two actually needed? I'm not sure what they're there for in coreboot, but it might be vestigal. We shouldn't add it if things seem to work without.
https://review.coreboot.org/c/coreboot/+/60080/comment/839af0cc_c9332f21 PS3, Line 23: VBOOT_FIRMWARE_ARCH-$(CONFIG_LP_ARCH_ARM64) := arm64 Would be good to add an
ifeq ($(VBOOT_FIRMWARE_ARCH-y,)) $(error ...)
assertion here, in case we ever add more architectures.