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 <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36845
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

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
+ * 3. Checks if vboot verification is done. If yes, return the reason code from
- * to identify if vboot_reference library has requested recovery path.
+ * shared data.
- * 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. */
-	 * Identify if vboot verification is already complete and no slot
+	if (vboot_logic_executed())
-	 * was selected i.e. recovery path was requested.
-	 */
-	if (vboot_logic_executed() && !vboot_is_slot_selected())
 		return vboot_get_recovery_reason_shared_data();
 
 	return 0;

src/security/vboot/common.c

@@ -16,6 +16,7 @@
 #include <assert.h>
 #include <cbmem.h>
 #include <console/console.h>
+#include <fmap.h>
 #include <stdint.h>
 #include <string.h>
 #include <symbols.h>
@@ -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 =
+	const char *name;
-		&vboot_get_working_data()->selected_region;
 
-	if (reg == NULL)
+	if (vboot_is_firmware_slot_a(ctx))
-		return -1;
+		name = "FW_MAIN_A";
+	else
+		name = "FW_MAIN_B";
 
-	if (reg->offset == 0 && reg->size == 0)
+	return fmap_locate_area_as_rdev(name, fw);
-		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;
 }
 
 #if CONFIG(VBOOT_STARTS_IN_BOOTBLOCK)

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

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;
 
-	props->offset = region_offset(&selected_region);
+	if (vboot_locate_firmware(ctx, &fw_main))
-	props->size = region_sz(&selected_region);
+		return -1;
+
+	props->offset = region_device_offset(&fw_main);
+	props->size = region_device_sz(&fw_main);
 
 	return 0;
 }

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');
+	printk(BIOS_INFO, "Slot %c is selected\n",
-	vboot_set_selected_region(region_device_region(&fw_main));
+	       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