Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36300 )
Change subject: vboot: use vboot persistent context ......................................................................
Patch Set 15:
(13 comments)
https://review.coreboot.org/c/coreboot/+/36300/1/3rdparty/blobs File 3rdparty/blobs:
https://review.coreboot.org/c/coreboot/+/36300/1/3rdparty/blobs@1 PS1, Line 1: Subproject commit 678b4c4a81069bb6e10e2e59f5374b83d727cd2b
Done
Done
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/bootmode... File src/security/vboot/bootmode.c:
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/bootmode... PS1, Line 30: (struct vb2_shared_data *)vboot_get_workbuf();
Actually, I already did for the depthcharge netboot case >< Let's use it here too.
Done
https://review.coreboot.org/c/coreboot/+/36300/7/src/security/vboot/bootmode... File src/security/vboot/bootmode.c:
https://review.coreboot.org/c/coreboot/+/36300/7/src/security/vboot/bootmode... PS7, Line 29: struct vb2_shared_data *sd = vb2_get_sd(vboot_get_context());
Yeah... the recovery reason stuff is still something we need to untangle.
Ack
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 49: ctx
Sure, let's do that.
Done
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/common.c... PS1, Line 131: /* TODO: We don't know the workbuf size with the current API... */
@Julius, yes, this is actually the current plan as listed in the design document. […]
Done
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/common.c... PS1, Line 140: memcpy(wd_cbmem, wd_preram, cbmem_size);
Right -- I was just unsure of what to do because of the whole inability to get workbuf_used issue ab […]
Done
https://review.coreboot.org/c/coreboot/+/36300/2/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/2/src/security/vboot/common.c... PS2, Line 146: VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE
It'd be nice if we could know at this point how much extra we need for struct vboot_working_data + p […]
Right now it's guaranteed to be 32 byte aligned (via imd_create_tiered_empty() setting up alignment variables and its callers setting up 1024 and 32 respectively)
https://review.coreboot.org/c/coreboot/+/36300/6/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/6/src/security/vboot/common.c... PS6, Line 140: "VBOOT: copying vboot_working_data (%zu bytes) to CBMEM...\n",
Let's just drop it.
Done
https://review.coreboot.org/c/coreboot/+/36300/7/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/7/src/security/vboot/common.c... PS7, Line 75: (void *)wd + wd->buffer_offset
¯_(ツ)_/¯ […]
Done
https://review.coreboot.org/c/coreboot/+/36300/7/src/security/vboot/common.c... PS7, Line 77: car_get_var_ptr(&vboot_ctx)
Thanks for your excellent explanation of the situation, Julius. […]
Done
https://review.coreboot.org/c/coreboot/+/36300/7/src/security/vboot/common.c... PS7, Line 139: memcpy(wd_cbmem, wd_preram, sizeof(struct vboot_working_data));
Yes -- although we currently don't make use of the buffer_size field after this point. […]
Done
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;
Sure, let's do that.
Done
https://review.coreboot.org/c/coreboot/+/36300/6/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/c/coreboot/+/36300/6/src/security/vboot/misc.h@3... PS6, Line 35: vboot_init_context
There was for a short, brief patchset. […]
can you make a follow-up CL to fix the comment?