Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31887 )
Change subject: vboot: make vboot workbuf available to payload ......................................................................
Patch Set 9:
(6 comments)
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/coreboot... File payloads/libpayload/include/coreboot_tables.h:
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/coreboot... PS9, Line 205: 0x0034 Should this be at the bottom for ordering?
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/sysinfo.... File payloads/libpayload/include/sysinfo.h:
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/sysinfo.... PS9, Line 100: void *vboot_workbuf; Does coreboot have a convention of commenting struct members? It doesn't look like it, but I thought I'd check.
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/libc/coreboot.c File payloads/libpayload/libc/coreboot.c:
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/libc/coreboot.c@... PS9, Line 91: struct const?
https://review.coreboot.org/#/c/31887/5/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/#/c/31887/5/src/lib/coreboot_table.c@212 PS5, Line 212: #if CONFIG(VBOOT)
I indeed did misunderstand. But sounds like we can just remove the guards and leave as is.
Re your penultimate comment, yes, the compiler will drop the code if it can tell it isn't used. At least it does if you don't use -O0
https://review.coreboot.org/#/c/31887/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/#/c/31887/9/src/lib/coreboot_table.c@549 PS9, Line 549: CONFIG(VBOOT) I saw the thread on this and it seems like it is decided to use the CONFIG() macro. One problem with it is hard to search for CONFIG options without getting a lot of unwanted matches.
https://review.coreboot.org/#/c/31887/9/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/31887/9/src/security/vboot/misc.h@32 PS9, Line 32: CPU architecture agnostic CPU-architecture-agnostic
Are all these comments still true?