[coreboot-gerrit] Patch set updated for coreboot: drivers/intel/fsp2_0: separate component validation from loading

Aaron Durbin (adurbin@chromium.org) gerrit at coreboot.org
Mon Jul 18 22:18:23 CEST 2016


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

-gerrit

commit 645d3f2f6022d73cd86cce2fe36fd76ecb8f315c
Author: Aaron Durbin <adurbin at chromium.org>
Date:   Sun Jul 17 23:06:03 2016 -0500

    drivers/intel/fsp2_0: separate component validation from loading
    
    The current FSP component loading mechanism doesn't handle all the
    requirements actually needed. Two things need to be added:
    1. XIP support for MemoryInit component
    2. Relocating SiliconInit component to not corrupt OS memory.
    
    In order to accommodate those requirements the validation
    and header initialization needs to be a separate function.
    Therefore, provide fsp_validate_component() to help achieve those
    requirements.
    
    BUG=chrome-os-partner:52679
    
    Change-Id: I53525498b250033f3187c05db248e07b00cc934d
    Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
 src/drivers/intel/fsp2_0/include/fsp/util.h |  5 +++
 src/drivers/intel/fsp2_0/util.c             | 52 +++++++++++++++++------------
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h
index d546c11..0f4d33c 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/util.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/util.h
@@ -14,6 +14,7 @@
 #define _FSP2_0_UTIL_H_
 
 #include <boot/coreboot_tables.h>
+#include <commonlib/region.h>
 #include <fsp/api.h>
 #include <fsp/info_header.h>
 #include <memrange.h>
@@ -34,6 +35,10 @@ enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 void fsp_find_reserved_memory(struct range_entry *re, const void *hob_list);
 void fsp_print_memory_resource_hobs(const void *hob_list);
 
+/* Fill in header and validate sanity of component within region device. */
+enum cb_err fsp_validate_component(struct fsp_header *hdr,
+					const struct region_device *rdev);
+
 /* Load an FSP binary into CBFS, and fill the associated fsp_header struct */
 enum cb_err fsp_load_binary(struct fsp_header *hdr, const char *name,
 			    struct range_entry *r);
diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c
index b47b898..942ea09 100644
--- a/src/drivers/intel/fsp2_0/util.c
+++ b/src/drivers/intel/fsp2_0/util.c
@@ -104,47 +104,57 @@ void fsp_print_header_info(const struct fsp_header *hdr)
 
 }
 
-/* TODO: this won't work for SoC's that need to XIP certain modules. */
-enum cb_err fsp_load_binary(struct fsp_header *hdr,
-			    const char *name,
-			    struct range_entry *range)
+enum cb_err fsp_validate_component(struct fsp_header *hdr,
+					const struct region_device *rdev)
 {
-	struct cbfsf file_desc;
-	struct region_device file_data;
 	void *membase;
 
-	if (cbfs_boot_locate(&file_desc, name, NULL)) {
-		printk(BIOS_ERR, "Could not locate %s in CBFS\n", name);
-		return CB_ERR;
-	}
-
-	cbfs_file_data(&file_data, &file_desc);
-
 	/* Map just enough of the file to be able to parse the header. */
-	membase = rdev_mmap(&file_data, FSP_HDR_OFFSET, FSP_HDR_LEN);
+	membase = rdev_mmap(rdev, FSP_HDR_OFFSET, FSP_HDR_LEN);
 
 	if (membase == NULL) {
-		printk(BIOS_ERR, "Could not mmap() '%s' FSP header.\n", name);
+		printk(BIOS_ERR, "Could not mmap() FSP header.\n");
 		return CB_ERR;
 	}
 
 	if (fsp_identify(hdr, membase) != CB_SUCCESS) {
-		rdev_munmap(&file_data, membase);
-		printk(BIOS_ERR, "%s did not have a valid FSP header\n", name);
+		rdev_munmap(rdev, membase);
+		printk(BIOS_ERR, "No valid FSP header\n");
 		return CB_ERR;
 	}
 
-	rdev_munmap(&file_data, membase);
+	rdev_munmap(rdev, membase);
 
 	fsp_print_header_info(hdr);
 
 	/* Check if size specified in the header matches the cbfs file size */
-	if (region_device_sz(&file_data) < hdr->image_size) {
-		printk(BIOS_ERR, "%s size bigger than cbfs file.\n", name);
+	if (region_device_sz(rdev) < hdr->image_size) {
+		printk(BIOS_ERR, "Component size bigger than cbfs file.\n");
 		return CB_ERR;
 	}
 
-	/* Check if the binary load address is within expected range */
+	return CB_SUCCESS;
+}
+
+/* TODO: this won't work for SoC's that need to XIP certain modules. */
+enum cb_err fsp_load_binary(struct fsp_header *hdr,
+			    const char *name,
+			    struct range_entry *range)
+{
+	struct cbfsf file_desc;
+	struct region_device file_data;
+
+	if (cbfs_boot_locate(&file_desc, name, NULL)) {
+		printk(BIOS_ERR, "Could not locate %s in CBFS\n", name);
+		return CB_ERR;
+	}
+
+	cbfs_file_data(&file_data, &file_desc);
+
+	if (fsp_validate_component(hdr, &file_data) != CB_SUCCESS)
+		return CB_ERR;
+
+	/* Check if the component load address is within expected range */
 	/* TODO: this doesn't check the current running program footprint. */
 	if (range_entry_base(range) > hdr->image_base ||
 	    range_entry_end(range) <= hdr->image_base + hdr->image_size) {



More information about the coreboot-gerrit mailing list