From aeb652a4a04226f467eb8e850b2096d772c6e31e Mon Sep 17 00:00:00 2001 From: Yu-Ping Wu Date: Thu, 14 Nov 2019 15:42:25 +0800 Subject: security/vboot: Remove selected_region from struct vboot_working_data Since we already have pre-RAM cache for FMAP (CB:36657), calling load_firmware() multiple times is no longer a problem. This patch replaces vboot_get_selected_region() usage with vboot_locate_firmware(), which locates the firmware by reading from the CBMEM cache. In addition, returning false from vboot_is_slot_selected() implies the recovery path was requested, i.e., vb2_shared_data.recovery_reason was set. Therefore, we simply remove the vboot_is_slot_selected() check from vboot_check_recovery_request(). BRANCH=none BUG=chromium:1021452 TEST=emerge-kukui coreboot Change-Id: I27cb1a2175beb189053fc3e44b17b60aba474bb0 Signed-off-by: Yu-Ping Wu Reviewed-on: https://review.coreboot.org/c/coreboot/+/36845 Tested-by: build bot (Jenkins) Reviewed-by: Julius Werner --- src/security/vboot/bootmode.c | 12 ++++------- src/security/vboot/common.c | 42 +++++++++------------------------------ src/security/vboot/misc.h | 22 ++++++++++---------- src/security/vboot/vboot_loader.c | 14 +++++++++---- src/security/vboot/vboot_logic.c | 26 ++++-------------------- 5 files changed, 39 insertions(+), 77 deletions(-) (limited to 'src/security') diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c index 0cab0c8559..83baa815c7 100644 --- a/src/security/vboot/bootmode.c +++ b/src/security/vboot/bootmode.c @@ -71,9 +71,8 @@ BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, * VB2_RECOVERY_RO_MANUAL. * 2. Checks if recovery request is present in VBNV and returns the code read * from it. - * 3. Checks if vboot verification is done and looks up selected region - * to identify if vboot_reference library has requested recovery path. - * If yes, return the reason code from shared data. + * 3. Checks if vboot verification is done. If yes, return the reason code from + * shared data. * 4. If nothing applies, return 0 indicating no recovery request. */ int vboot_check_recovery_request(void) @@ -88,11 +87,8 @@ int vboot_check_recovery_request(void) if ((reason = get_recovery_mode_from_vbnv()) != 0) return reason; - /* - * Identify if vboot verification is already complete and no slot - * was selected i.e. recovery path was requested. - */ - if (vboot_logic_executed() && !vboot_is_slot_selected()) + /* Identify if vboot verification is already complete. */ + if (vboot_logic_executed()) return vboot_get_recovery_reason_shared_data(); return 0; diff --git a/src/security/vboot/common.c b/src/security/vboot/common.c index 3f57602cb1..bad01ff57f 100644 --- a/src/security/vboot/common.c +++ b/src/security/vboot/common.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -83,42 +84,17 @@ struct vb2_context *vboot_get_context(void) return *vboot_ctx_ptr; } -int vboot_get_selected_region(struct region *region) +int vboot_locate_firmware(const struct vb2_context *ctx, + struct region_device *fw) { - const struct selected_region *reg = - &vboot_get_working_data()->selected_region; + const char *name; - if (reg == NULL) - return -1; + if (vboot_is_firmware_slot_a(ctx)) + name = "FW_MAIN_A"; + else + name = "FW_MAIN_B"; - if (reg->offset == 0 && reg->size == 0) - return -1; - - region->offset = reg->offset; - region->size = reg->size; - - return 0; -} - -void vboot_set_selected_region(const struct region *region) -{ - struct selected_region *reg = - &vboot_get_working_data()->selected_region; - - assert(reg != NULL); - - reg->offset = region_offset(region); - reg->size = region_sz(region); -} - -int vboot_is_slot_selected(void) -{ - struct selected_region *reg = - &vboot_get_working_data()->selected_region; - - assert(reg != NULL); - - return reg->size > 0; + return fmap_locate_area_as_rdev(name, fw); } #if CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index e438848635..1b147992d8 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -23,11 +23,6 @@ struct vb2_context; struct vb2_shared_data; -struct selected_region { - uint32_t offset; - uint32_t size; -}; - /* * 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 @@ -36,7 +31,6 @@ struct selected_region { * Keep the struct CPU architecture agnostic as it crosses stage boundaries. */ struct vboot_working_data { - struct selected_region selected_region; /* offset of the buffer from the start of this struct */ uint16_t buffer_offset; }; @@ -47,11 +41,19 @@ struct vboot_working_data { struct vboot_working_data *vboot_get_working_data(void); struct vb2_context *vboot_get_context(void); -/* Returns 0 on success. < 0 on failure. */ -int vboot_get_selected_region(struct region *region); +/* + * Returns 1 if firmware slot A is used, 0 if slot B is used. + */ +static inline int vboot_is_firmware_slot_a(const struct vb2_context *ctx) +{ + return !(ctx->flags & VB2_CONTEXT_FW_SLOT_B); +} -void vboot_set_selected_region(const struct region *region); -int vboot_is_slot_selected(void); +/* + * Locates firmware as a region device. Returns 0 on success, -1 on failure. + */ +int vboot_locate_firmware(const struct vb2_context *ctx, + struct region_device *fw); /* * Source: security/vboot/vboot_handoff.c diff --git a/src/security/vboot/vboot_loader.c b/src/security/vboot/vboot_loader.c index 2b7ba83503..3903f18f6b 100644 --- a/src/security/vboot/vboot_loader.c +++ b/src/security/vboot/vboot_loader.c @@ -73,17 +73,23 @@ void vboot_run_logic(void) static int vboot_locate(struct cbfs_props *props) { - struct region selected_region; + const struct vb2_context *ctx; + struct region_device fw_main; /* Don't honor vboot results until the vboot logic has run. */ if (!vboot_logic_executed()) return -1; - if (vboot_get_selected_region(&selected_region)) + ctx = vboot_get_context(); + + if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) + return -1; + + if (vboot_locate_firmware(ctx, &fw_main)) return -1; - props->offset = region_offset(&selected_region); - props->size = region_sz(&selected_region); + props->offset = region_device_offset(&fw_main); + props->size = region_device_sz(&fw_main); return 0; } diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 71371cdb75..6ee4d948f3 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -35,11 +35,6 @@ #define TODO_BLOCK_SIZE 1024 -static int is_slot_a(struct vb2_context *ctx) -{ - return !(ctx->flags & VB2_CONTEXT_FW_SLOT_B); -} - /* exports */ void vb2ex_printf(const char *func, const char *fmt, ...) @@ -70,7 +65,7 @@ vb2_error_t vb2ex_read_resource(struct vb2_context *ctx, name = "GBB"; break; case VB2_RES_FW_VBLOCK: - if (is_slot_a(ctx)) + if (vboot_is_firmware_slot_a(ctx)) name = "VBLOCK_A"; else name = "VBLOCK_B"; @@ -256,19 +251,6 @@ static vb2_error_t hash_body(struct vb2_context *ctx, return VB2_SUCCESS; } -static int locate_firmware(struct vb2_context *ctx, - struct region_device *fw_main) -{ - const char *name; - - if (is_slot_a(ctx)) - name = "FW_MAIN_A"; - else - name = "FW_MAIN_B"; - - return fmap_locate_area_as_rdev(name, fw_main); -} - /** * Save non-volatile and/or secure data if needed. */ @@ -417,7 +399,7 @@ void verstage_main(void) } printk(BIOS_INFO, "Phase 4\n"); - rv = locate_firmware(ctx, &fw_main); + rv = vboot_locate_firmware(ctx, &fw_main); if (rv) die_with_post_code(POST_INVALID_ROM, "Failed to read FMAP to locate firmware"); @@ -468,8 +450,8 @@ void verstage_main(void) } } - printk(BIOS_INFO, "Slot %c is selected\n", is_slot_a(ctx) ? 'A' : 'B'); - vboot_set_selected_region(region_device_region(&fw_main)); + printk(BIOS_INFO, "Slot %c is selected\n", + vboot_is_firmware_slot_a(ctx) ? 'A' : 'B'); verstage_main_exit: /* If CBMEM is not up yet, let the ROMSTAGE_CBMEM_INIT_HOOK take care -- cgit v1.2.3