[coreboot-gerrit] Patch set updated for coreboot: a37cd61 vboot: use only offsets for tracking firmware components

Aaron Durbin (adurbin@chromium.org) gerrit at coreboot.org
Mon May 18 22:20:19 CEST 2015


Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/10221

-gerrit

commit a37cd6123a5e9d7230d1c94a5c2adf7e786101e4
Author: Aaron Durbin <adurbin at chromium.org>
Date:   Fri May 15 15:57:51 2015 -0500

    vboot: use only offsets for tracking firmware components
    
    Because of the fmap API returning pointers to represent
    regions within the boot device a vboot_region structure
    was used to track the case where offsets could be pointers
    on x86 but not on !x86. Normalize this tracking to use
    offsets only as it provides consistency in the code.
    
    Change-Id: I63c08b31ace3bd0e66ebc17e308f87eb5f857c86
    Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
 src/vendorcode/google/chromeos/vbnv_flash.c        | 14 ++---
 src/vendorcode/google/chromeos/vboot2/misc.h       | 12 ++--
 .../google/chromeos/vboot2/vboot_handoff.c         |  4 +-
 .../google/chromeos/vboot2/vboot_loader.c          | 73 ++++++++++++----------
 src/vendorcode/google/chromeos/vboot2/verstage.c   | 21 +++----
 src/vendorcode/google/chromeos/vboot_common.c      | 64 ++++++++++---------
 src/vendorcode/google/chromeos/vboot_common.h      | 12 ++--
 7 files changed, 105 insertions(+), 95 deletions(-)

diff --git a/src/vendorcode/google/chromeos/vbnv_flash.c b/src/vendorcode/google/chromeos/vbnv_flash.c
index 4cc87b6..c0d0fe2 100644
--- a/src/vendorcode/google/chromeos/vbnv_flash.c
+++ b/src/vendorcode/google/chromeos/vbnv_flash.c
@@ -32,7 +32,7 @@
 #define BLOB_SIZE VB2_NVDATA_SIZE
 
 /* FMAP descriptor of the NVRAM area */
-static struct vboot_region nvram_region;
+static struct region nvram_region;
 
 /* offset of the current nvdata in SPI flash */
 static int blob_offset = -1;
@@ -74,7 +74,7 @@ static int init_vbnv(void)
 	int i;
 
 	vboot_locate_region("RW_NVRAM", &nvram_region);
-	if (nvram_region.size < BLOB_SIZE) {
+	if (region_sz(&nvram_region) < BLOB_SIZE) {
 		printk(BIOS_ERR, "%s: failed to locate NVRAM\n", __func__);
 		return 1;
 	}
@@ -83,8 +83,8 @@ static int init_vbnv(void)
 	for (i = 0; i < BLOB_SIZE; i++)
 		empty_blob[i] = erase_value();
 
-	offset = nvram_region.offset_addr;
-	top_offset = nvram_region.offset_addr + nvram_region.size - BLOB_SIZE;
+	offset = region_offset(&nvram_region);
+	top_offset = offset + region_sz(&nvram_region) - BLOB_SIZE;
 
 	/*
 	 * after the loop, offset is supposed to point the blob right before
@@ -130,8 +130,8 @@ static int erase_nvram(void)
 	if (vbnv_flash_probe())
 		return 1;
 
-	if (spi_flash->erase(spi_flash, nvram_region.offset_addr,
-			     nvram_region.size)) {
+	if (spi_flash->erase(spi_flash, region_offset(&nvram_region),
+			     region_sz(&nvram_region))) {
 		printk(BIOS_ERR, "failed to erase nvram\n");
 		return 1;
 	}
@@ -171,7 +171,7 @@ void save_vbnv(const uint8_t *vbnv_copy)
 			if (new_offset > top_offset) {
 				if (erase_nvram())
 					return;  /* error */
-				new_offset = nvram_region.offset_addr;
+				new_offset = region_offset(&nvram_region);
 			}
 			break;
 		}
diff --git a/src/vendorcode/google/chromeos/vboot2/misc.h b/src/vendorcode/google/chromeos/vboot2/misc.h
index 175e2c2..edeaf19 100644
--- a/src/vendorcode/google/chromeos/vboot2/misc.h
+++ b/src/vendorcode/google/chromeos/vboot2/misc.h
@@ -25,7 +25,7 @@
 void vboot_fill_handoff(void);
 void verstage_main(void);
 void *vboot_load_stage(int stage_index,
-		       struct vboot_region *fw_main,
+		       struct region *fw_main,
 		       struct vboot_components *fw_info);
 
 /*
@@ -47,17 +47,17 @@ size_t vb2_working_data_size(void);
 void *vboot_get_work_buffer(struct vb2_working_data *wd);
 
 static inline void vb2_get_selected_region(struct vb2_working_data *wd,
-					   struct vboot_region *region)
+					   struct region *region)
 {
-	region->offset_addr = wd->selected_region_offset;
+	region->offset = wd->selected_region_offset;
 	region->size = wd->selected_region_size;
 }
 
 static inline void vb2_set_selected_region(struct vb2_working_data *wd,
-					   struct vboot_region *region)
+					   struct region *region)
 {
-	wd->selected_region_offset = region->offset_addr;
-	wd->selected_region_size = region->size;
+	wd->selected_region_offset = region_offset(region);
+	wd->selected_region_size = region_sz(region);
 }
 
 static inline int vboot_is_slot_selected(struct vb2_working_data *wd)
diff --git a/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c b/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c
index 417a496..9952c00 100644
--- a/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c
+++ b/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c
@@ -127,7 +127,7 @@ void vboot_fill_handoff(void)
 	int i;
 	struct vboot_handoff *vh;
 	struct vb2_shared_data *sd;
-	struct vboot_region fw_main;
+	struct region fw_main;
 	struct vboot_components *fw_info;
 	struct vb2_working_data *wd = vboot_get_working_data();
 
@@ -159,7 +159,7 @@ void vboot_fill_handoff(void)
 	/* these offset & size are used to load a rw boot loader */
 	for (i = 0; i < fw_info->num_components; i++) {
 		vh->components[i].address =
-			fw_main.offset_addr + fw_info->entries[i].offset;
+			region_offset(&fw_main) + fw_info->entries[i].offset;
 		vh->components[i].size = fw_info->entries[i].size;
 	}
 }
diff --git a/src/vendorcode/google/chromeos/vboot2/vboot_loader.c b/src/vendorcode/google/chromeos/vboot2/vboot_loader.c
index fd64798..1129cd1 100644
--- a/src/vendorcode/google/chromeos/vboot2/vboot_loader.c
+++ b/src/vendorcode/google/chromeos/vboot2/vboot_loader.c
@@ -121,10 +121,10 @@ static int vboot_loader_active(struct prog *prog)
 		if (IS_ENABLED(CONFIG_MULTIPLE_CBFS_INSTANCES) &&
 		    run_verification) {
 			/* RW A or B */
-			struct vboot_region fw_main;
+			struct region fw_main;
 
 			vb2_get_selected_region(wd, &fw_main);
-			cbfs_set_header_offset(fw_main.offset_addr);
+			cbfs_set_header_offset(region_offset(&fw_main));
 		}
 		return 1;
 	}
@@ -132,27 +132,30 @@ static int vboot_loader_active(struct prog *prog)
 	return 0;
 }
 
-static uintptr_t vboot_fw_region(int fw_index, struct vboot_region *fw_main,
-				struct vboot_components *fw_info, size_t *size)
+static int vboot_fw_region(int fw_index, struct region *fw_main,
+				struct vboot_components *fw_info,
+				struct region *fc)
 {
-	uintptr_t fc_addr;
-	uint32_t fc_size;
+	size_t fw_main_end;
+	size_t fc_end;
 
 	if (fw_index >= fw_info->num_components) {
 		printk(BIOS_INFO, "invalid stage index: %d\n", fw_index);
-		return 0;
+		return -1;
 	}
 
-	fc_addr = fw_main->offset_addr + fw_info->entries[fw_index].offset;
-	fc_size = fw_info->entries[fw_index].size;
-	if (fc_size == 0 ||
-	    fc_addr + fc_size > fw_main->offset_addr + fw_main->size) {
+	fc->offset = region_offset(fw_main) + fw_info->entries[fw_index].offset;
+	fc->size = fw_info->entries[fw_index].size;
+
+	fw_main_end = region_offset(fw_main) + region_sz(fw_main);
+	fc_end = region_offset(fc) + region_sz(fc);
+
+	if (region_sz(fc) == 0 || fc_end > fw_main_end) {
 		printk(BIOS_INFO, "invalid stage address or size\n");
-		return 0;
+		return -1;
 	}
 
-	*size = fc_size;
-	return fc_addr;
+	return 0;
 }
 
 /* This function is only called when vboot_loader_active() returns 1. That
@@ -160,7 +163,7 @@ static uintptr_t vboot_fw_region(int fw_index, struct vboot_region *fw_main,
 static int vboot_prepare(struct prog *prog)
 {
 	struct vb2_working_data *wd;
-	struct vboot_region fw_main;
+	struct region fw_main;
 	struct vboot_components *fw_info;
 
 	/* Code size optimization. We'd never actually get called under the
@@ -202,25 +205,22 @@ static int vboot_prepare(struct prog *prog)
 
 	/* Load payload in ramstage. */
 	if (ENV_RAMSTAGE) {
-		uintptr_t payload;
+		struct region payload;
 		void *payload_ptr;
-		size_t size;
-
-		payload = vboot_fw_region(CONFIG_VBOOT_BOOT_LOADER_INDEX,
-						&fw_main, fw_info, &size);
 
-		if (payload == 0)
+		if (vboot_fw_region(CONFIG_VBOOT_BOOT_LOADER_INDEX,
+						&fw_main, fw_info, &payload))
 			die("Couldn't load payload.");
 
-		payload_ptr = vboot_get_region(payload, size, NULL);
+		payload_ptr = vboot_get_region(region_offset(&payload),
+						region_sz(&payload), NULL);
 
 		if (payload_ptr == NULL)
 			die("Couldn't load payload.");
 
-		prog_set_area(prog, payload_ptr, size);
+		prog_set_area(prog, payload_ptr, region_sz(&payload));
 	} else {
-		uintptr_t stage;
-		size_t size;
+		struct region stage;
 		int stage_index = 0;
 
 		if (prog->type == PROG_ROMSTAGE)
@@ -230,22 +230,31 @@ static int vboot_prepare(struct prog *prog)
 		else
 			die("Invalid program type for vboot.");
 
-		stage = vboot_fw_region(stage_index, &fw_main, fw_info, &size);
-
-		if (stage == 0)
+		if (vboot_fw_region(stage_index, &fw_main, fw_info, &stage))
 			die("Vboot stage load failed.");
 
 		if (ENV_ROMSTAGE && IS_ENABLED(CONFIG_RELOCATABLE_RAMSTAGE)) {
+			void *stage_ptr;
 			struct rmod_stage_load rmod_ram = {
 				.cbmem_id = CBMEM_ID_RAMSTAGE,
 				.prog = prog,
 			};
 
-			if (rmodule_stage_load(&rmod_ram, (void *)stage))
+			stage_ptr = vboot_get_region(region_offset(&stage),
+						region_sz(&stage), NULL);
+
+			if (stage_ptr == NULL)
+				die("Vboot couldn't load stage.");
+
+			if (rmodule_stage_load(&rmod_ram, stage_ptr))
 				die("Vboot couldn't load stage");
-		} else if (cbfs_load_prog_stage_by_offset(CBFS_DEFAULT_MEDIA,
-							prog, stage) < 0)
-			die("Vboot couldn't load stage");
+		} else {
+			size_t offset = region_offset(&stage);
+
+			if (cbfs_load_prog_stage_by_offset(CBFS_DEFAULT_MEDIA,
+								prog, offset))
+				die("Vboot couldn't load stage");
+		}
 	}
 
 	return 0;
diff --git a/src/vendorcode/google/chromeos/vboot2/verstage.c b/src/vendorcode/google/chromeos/vboot2/verstage.c
index 019208d..59ead4d 100644
--- a/src/vendorcode/google/chromeos/vboot2/verstage.c
+++ b/src/vendorcode/google/chromeos/vboot2/verstage.c
@@ -67,7 +67,7 @@ int vb2ex_read_resource(struct vb2_context *ctx,
 			void *buf,
 			uint32_t size)
 {
-	struct vboot_region region;
+	struct region region;
 
 	switch (index) {
 	case VB2_RES_GBB:
@@ -83,10 +83,10 @@ int vb2ex_read_resource(struct vb2_context *ctx,
 		return VB2_ERROR_EX_READ_RESOURCE_INDEX;
 	}
 
-	if (offset + size > region.size)
+	if (offset + size > region_sz(&region))
 		return VB2_ERROR_EX_READ_RESOURCE_SIZE;
 
-	if (vboot_get_region(region.offset_addr + offset, size, buf) == NULL)
+	if (vboot_get_region(region_offset(&region) + offset, size, buf) == NULL)
 		return VB2_ERROR_UNKNOWN;
 
 	return VB2_SUCCESS;
@@ -114,13 +114,13 @@ int vb2ex_hwcrypto_digest_finalize(uint8_t *digest, uint32_t digest_size)
 	return VB2_ERROR_UNKNOWN;
 }
 
-static int hash_body(struct vb2_context *ctx, struct vboot_region *fw_main)
+static int hash_body(struct vb2_context *ctx, struct region *fw_main)
 {
 	uint64_t load_ts;
 	uint32_t expected_size;
 	MAYBE_STATIC uint8_t block[TODO_BLOCK_SIZE];
 	size_t block_size = sizeof(block);
-	uintptr_t offset;
+	size_t offset;
 	int rv;
 
 	/*
@@ -132,8 +132,8 @@ static int hash_body(struct vb2_context *ctx, struct vboot_region *fw_main)
 	load_ts = timestamp_get();
 	timestamp_add(TS_START_HASH_BODY, load_ts);
 
-	expected_size = fw_main->size;
-	offset = fw_main->offset_addr;
+	expected_size = region_sz(fw_main);
+	offset = region_offset(fw_main);
 
 	/* Start the body hash */
 	rv = vb2api_init_hash(ctx, VB2_HASH_TAG_FW_BODY, &expected_size);
@@ -174,15 +174,14 @@ static int hash_body(struct vb2_context *ctx, struct vboot_region *fw_main)
 	return VB2_SUCCESS;
 }
 
-static int locate_firmware(struct vb2_context *ctx,
-			   struct vboot_region *fw_main)
+static int locate_firmware(struct vb2_context *ctx, struct region *fw_main)
 {
 	if (is_slot_a(ctx))
 		vboot_locate_region("FW_MAIN_A", fw_main);
 	else
 		vboot_locate_region("FW_MAIN_B", fw_main);
 
-	if (fw_main->size < 0)
+	if (region_sz(fw_main) == 0)
 		return 1;
 
 	return 0;
@@ -220,7 +219,7 @@ static uint32_t extend_pcrs(struct vb2_context *ctx)
 void verstage_main(void)
 {
 	struct vb2_context ctx;
-	struct vboot_region fw_main;
+	struct region fw_main;
 	struct vb2_working_data *wd = vboot_get_working_data();
 	int rv;
 	timestamp_add_now(TS_START_VBOOT);
diff --git a/src/vendorcode/google/chromeos/vboot_common.c b/src/vendorcode/google/chromeos/vboot_common.c
index d3a939c..67e52a7 100644
--- a/src/vendorcode/google/chromeos/vboot_common.c
+++ b/src/vendorcode/google/chromeos/vboot_common.c
@@ -18,7 +18,7 @@
  */
 
 #include <boot/coreboot_tables.h>
-#include <cbfs.h>
+#include <boot_device.h>
 #include <cbmem.h>
 #include <console/cbmem_console.h>
 #include <console/console.h>
@@ -31,36 +31,42 @@
 #include "vboot_common.h"
 #include "vboot_handoff.h"
 
-void vboot_locate_region(const char *name, struct vboot_region *region)
+void vboot_locate_region(const char *name, struct region *region)
 {
-	region->size = find_fmap_entry(name, (void **)&region->offset_addr);
+	const struct fmap_area *area;
+
+	region->size = 0;
+
+	area = find_fmap_area(fmap_find(), name);
+
+	if (area != NULL) {
+		region->offset = area->offset;
+		region->size = area->size;
+	}
 }
 
-void *vboot_get_region(uintptr_t offset_addr, size_t size, void *dest)
+void *vboot_get_region(size_t offset, size_t size, void *dest)
 {
-	if (IS_ENABLED(CONFIG_SPI_FLASH_MEMORY_MAPPED)) {
-		if (dest != NULL)
-			return memcpy(dest, (void *)offset_addr, size);
-		else
-			return (void *)offset_addr;
-	} else {
-		struct cbfs_media default_media, *media = &default_media;
-		void *cache;
-
-		init_default_cbfs_media(media);
-		media->open(media);
-		if (dest != NULL) {
-			cache = dest;
-			if (media->read(media, dest, offset_addr, size) != size)
-				cache = NULL;
-		} else {
-			cache = media->map(media, offset_addr, size);
-			if (cache == CBFS_MEDIA_INVALID_MAP_ADDRESS)
-				cache = NULL;
-		}
-		media->close(media);
-		return cache;
-	}
+	const struct region_device *boot_dev;
+	struct region_device rdev;
+
+	boot_device_init();
+	boot_dev = boot_device_ro();
+
+	if (boot_dev == NULL)
+		return NULL;
+
+	if (rdev_chain(&rdev, boot_dev, offset, size))
+		return NULL;
+
+	/* Each call will leak a mapping. */
+	if (dest == NULL)
+		return rdev_mmap_full(&rdev);
+
+	if (rdev_readat(&rdev, dest, 0, size) != size)
+		return NULL;
+
+	return dest;
 }
 
 int vboot_get_handoff_info(void **addr, uint32_t *size)
@@ -78,7 +84,7 @@ int vboot_get_handoff_info(void **addr, uint32_t *size)
 }
 
 /* This will leak a mapping of a fw region */
-struct vboot_components *vboot_locate_components(struct vboot_region *region)
+struct vboot_components *vboot_locate_components(struct region *region)
 {
 	size_t req_size;
 	struct vboot_components *vbc;
@@ -87,7 +93,7 @@ struct vboot_components *vboot_locate_components(struct vboot_region *region)
 	req_size += sizeof(struct vboot_component_entry) *
 			MAX_PARSED_FW_COMPONENTS;
 
-	vbc = vboot_get_region(region->offset_addr, req_size, NULL);
+	vbc = vboot_get_region(region_offset(region), req_size, NULL);
 	if (vbc && vbc->num_components > MAX_PARSED_FW_COMPONENTS)
 		vbc = NULL;
 
diff --git a/src/vendorcode/google/chromeos/vboot_common.h b/src/vendorcode/google/chromeos/vboot_common.h
index c6c9b50..3430fbe 100644
--- a/src/vendorcode/google/chromeos/vboot_common.h
+++ b/src/vendorcode/google/chromeos/vboot_common.h
@@ -20,11 +20,7 @@
 #define VBOOT_COMMON_H
 
 #include <stdint.h>
-
-struct vboot_region {
-	uintptr_t offset_addr;
-	int32_t size;
-};
+#include <region.h>
 
 /* The FW areas consist of multiple components. At the beginning of
  * each area is the number of total compoments as well as the size and
@@ -40,9 +36,9 @@ struct vboot_components {
 	struct vboot_component_entry entries[0];
 } __attribute__((packed));
 
-void vboot_locate_region(const char *name, struct vboot_region *region);
+void vboot_locate_region(const char *name, struct region *region);
 
-struct vboot_components *vboot_locate_components(struct vboot_region *region);
+struct vboot_components *vboot_locate_components(struct region *region);
 
 /*
  * This is a dual purpose routine. If dest is non-NULL the region at
@@ -50,6 +46,6 @@ struct vboot_components *vboot_locate_components(struct vboot_region *region);
  * is NULL,the region will be mapped to a memory location. NULL is
  * returned on error else the location of the requested region.
  */
-void *vboot_get_region(uintptr_t offset_addr, size_t size, void *dest);
+void *vboot_get_region(size_t offset, size_t size, void *dest);
 
 #endif /* VBOOT_COMMON_H */



More information about the coreboot-gerrit mailing list