Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36300 )
Change subject: vboot: use vboot persistent context ......................................................................
Patch Set 10:
(4 comments)
Seems like we are pretty much done with this CL.
Now we need to commit this into Chromium repo first, along with the depthcharge and vboot_reference Cq-Depends. Then I can uprev vboot_reference submodule in this CL.
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/common.c... PS1, Line 55: if (wd->buffer_offset == ALIGN_UP(sizeof(*wd), 16)) {
Well, we can introduce another static vboot_started variable if you want. […]
Perhaps we should just punt this for now, and deal with it along with cleaning up all the recovery_reason functions?
https://review.coreboot.org/c/coreboot/+/36300/9/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/9/src/security/vboot/common.c... PS9, Line 52: struct vboot_working_data *wd;
nit: […]
Sure, let's do that.
https://review.coreboot.org/c/coreboot/+/36300/9/src/security/vboot/common.c... PS9, Line 140: wd_cbmem->buffer_size = 0;
... […]
Added the TODO. I added your note about add_cbmem_pointers() to the bug I created here: https://bugs.chromium.org/p/chromium/issues/detail?id=1021452
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/misc.h@5... PS1, Line 58: void *vboot_get_workbuf(void);
Sure -- I'm definitely not suggesting that we do that in this CL.
Julius, could you comment on https://bugs.chromium.org/p/chromium/issues/detail?id=1021452 to make sure I'm understanding the FMAP lookup issue correctly?