Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37231 )
Change subject: security/vboot: Remove struct vboot_working_data ......................................................................
security/vboot: Remove struct vboot_working_data
After CB:36808, CB:36844 and CB:36845, all fields except buffer_offset were removed from struct vboot_working_data. Since buffer_offset is used to record the offset of the workbuf relative to the whole structure, it is no longer needed.
This patch removes the structure, and renames vboot_get_working_data() to vboot_get_workbuf().
BRANCH=none BUG=chromium:1021452 TEST=emerge-nami coreboot
Change-Id: I304a5e4236f13b1aecd64b88ca5c8fbc1526e592 Signed-off-by: Yu-Ping Wu yupingso@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/37231 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Joel Kitching kitching@google.com --- M src/lib/coreboot_table.c M src/security/vboot/common.c M src/security/vboot/misc.h 3 files changed, 28 insertions(+), 73 deletions(-)
Approvals: build bot (Jenkins): Verified Joel Kitching: Looks good to me, approved
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 7245a63..8b18dfb 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -221,20 +221,18 @@ static void lb_vboot_workbuf(struct lb_header *header) { struct lb_range *vbwb; - struct vboot_working_data *wd = vboot_get_working_data(); + void *wb = vboot_get_workbuf();
vbwb = (struct lb_range *)lb_new_record(header); vbwb->tag = LB_TAG_VBOOT_WORKBUF; vbwb->size = sizeof(*vbwb); - vbwb->range_start = (uintptr_t)wd + wd->buffer_offset; + vbwb->range_start = (uintptr_t)wb; /* * TODO(chromium:1021452): Since cbmem size of vboot workbuf is now * always a known value, we hardcode the value of range_size here. - * Ultimately we'll want to move this to add_cbmem_pointers() below, - * but we'll have to get rid of the vboot_working_data struct first. + * Ultimately we'll want to move this to add_cbmem_pointers() below. */ - vbwb->range_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE - - wd->buffer_offset; + vbwb->range_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE; }
__weak uint32_t board_id(void) { return UNDEFINED_STRAPPING_ID; } diff --git a/src/security/vboot/common.c b/src/security/vboot/common.c index 290fa5e..517a1d4 100644 --- a/src/security/vboot/common.c +++ b/src/security/vboot/common.c @@ -27,57 +27,42 @@
static struct vb2_context *vboot_ctx;
-struct vboot_working_data *vboot_get_working_data(void) +void *vboot_get_workbuf(void) { - struct vboot_working_data *wd = NULL; + void *wb = NULL;
if (cbmem_possibly_online()) - wd = cbmem_find(CBMEM_ID_VBOOT_WORKBUF); + wb = cbmem_find(CBMEM_ID_VBOOT_WORKBUF);
- if (wd == NULL && CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) && + if (wb == NULL && CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) && preram_symbols_available()) - wd = (struct vboot_working_data *)_vboot2_work; + wb = _vboot2_work;
- assert(wd != NULL); + assert(wb != NULL);
- return wd; -} - -static inline void *vboot_get_workbuf(struct vboot_working_data *wd) -{ - return (void *)((uintptr_t)wd + wd->buffer_offset); + return wb; }
struct vb2_context *vboot_get_context(void) { - struct vboot_working_data *wd; + void *wb;
/* Return if context has already been initialized/restored. */ if (vboot_ctx) return vboot_ctx;
- wd = vboot_get_working_data(); + wb = vboot_get_workbuf();
/* Restore context from a previous stage. */ if (vboot_logic_executed()) { - assert(vb2api_reinit(vboot_get_workbuf(wd), - &vboot_ctx) == VB2_SUCCESS); + assert(vb2api_reinit(wb, &vboot_ctx) == VB2_SUCCESS); return vboot_ctx; }
assert(verification_should_run());
- /* - * vboot prefers 16-byte alignment. This takes away 16 bytes - * from the VBOOT2_WORK region, but the vboot devs said that's okay. - */ - memset(wd, 0, sizeof(*wd)); - wd->buffer_offset = ALIGN_UP(sizeof(*wd), 16); - /* Initialize vb2_shared_data and friends. */ - assert(vb2api_init(vboot_get_workbuf(wd), - VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE - - wd->buffer_offset, + assert(vb2api_init(wb, VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE, &vboot_ctx) == VB2_SUCCESS);
return vboot_ctx; @@ -96,35 +81,19 @@ return fmap_locate_area_as_rdev(name, fw); }
-#if CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) -/* - * For platforms that do not employ VBOOT_STARTS_IN_ROMSTAGE, vboot - * verification occurs before CBMEM is brought online, using pre-RAM. - * In order to make vboot data structures available downstream, copy - * vboot_working_data from SRAM/CAR into CBMEM. - */ -static void vboot_migrate_cbmem(int unused) -{ - const size_t cbmem_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE; - struct vboot_working_data *wd_preram = - (struct vboot_working_data *)_vboot2_work; - struct vboot_working_data *wd_cbmem = - cbmem_add(CBMEM_ID_VBOOT_WORKBUF, cbmem_size); - assert(wd_cbmem != NULL); - memcpy(wd_cbmem, wd_preram, sizeof(struct vboot_working_data)); - vb2api_relocate(vboot_get_workbuf(wd_cbmem), - vboot_get_workbuf(wd_preram), - cbmem_size - wd_cbmem->buffer_offset, - &vboot_ctx); -} -ROMSTAGE_CBMEM_INIT_HOOK(vboot_migrate_cbmem) -#else static void vboot_setup_cbmem(int unused) { - struct vboot_working_data *wd_cbmem = - cbmem_add(CBMEM_ID_VBOOT_WORKBUF, - VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE); - assert(wd_cbmem != NULL); + const size_t cbmem_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE; + void *wb_cbmem = cbmem_add(CBMEM_ID_VBOOT_WORKBUF, cbmem_size); + assert(wb_cbmem != NULL); + /* + * For platforms where VBOOT_STARTS_IN_BOOTBLOCK, vboot verification + * occurs before CBMEM is brought online, using pre-RAM. In order to + * make vboot data structures available downstream, copy vboot workbuf + * from SRAM/CAR into CBMEM. + */ + if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK)) + assert(vb2api_relocate(wb_cbmem, _vboot2_work, cbmem_size, + &vboot_ctx) == VB2_SUCCESS); } ROMSTAGE_CBMEM_INIT_HOOK(vboot_setup_cbmem) -#endif diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 9f681f6..9dd482e 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -23,21 +23,9 @@ struct vb2_shared_data;
/* - * Stores vboot-related information. selected_region is used by verstage to - * store the location of the selected slot. buffer is used by vboot to store - * its work buffer. vb2_context is contained within this work buffer, and is - * accessible via vboot_get_context() declared below. - * Keep the struct CPU architecture agnostic as it crosses stage boundaries. - */ -struct vboot_working_data { - /* offset of the buffer from the start of this struct */ - uint16_t buffer_offset; -}; - -/* * Source: security/vboot/common.c */ -struct vboot_working_data *vboot_get_working_data(void); +void *vboot_get_workbuf(void); struct vb2_context *vboot_get_context(void);
/*