Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c@125 PS3, Line 125: int vboot_declares_display(void)
Yes, I think the latter option is reasonable.
Ack
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
nit: maybe change these two to u16 so the whole thing can still fit 16 bytes (which we align the wor […]
You're talking about buffer_offset and buffer_size? I'm not so sure what you mean by "so the whole thing can still fit 16 bytes" ...
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@47 PS4, Line 47: #define VBOOT_FLAG_DISPLAY_REQUESTED (1 << 0)
Maybe prefix this VB_WD_FLAG instead to have clearer differentiation from the dozens of other kinds […]
Sure. I'd still like to keep the front as VBOOT to differentiate from vboot internal stuff (just like we moved vb2_working_data to vboot_working_data).
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h@37 PS3, Line 37: uint8_t declares_display;
I think either is fine. […]
Ack