[coreboot-gerrit] Patch set updated for coreboot: drivers/intel/fsp2_0: Verify HOBs returned by FspMemoryInit

Lee Leahy (leroy.p.leahy@intel.com) gerrit at coreboot.org
Wed Jul 27 21:14:37 CEST 2016


Lee Leahy (leroy.p.leahy at intel.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15850

-gerrit

commit c9c95691385a5723ffb3c90fc3ad19a18108fd02
Author: Lee Leahy <leroy.p.leahy at intel.com>
Date:   Sun Jul 24 09:52:38 2016 -0700

    drivers/intel/fsp2_0: Verify HOBs returned by FspMemoryInit
    
    Verify that FSP is properly returning:
    * HOB list pointer
    * FSP_BOOTLOADER_TOLUM_HOB
    * FSP_RESERVED_MEMORY_RESOURCE_HOB
    
    TEST=Build and run on Galileo Gen2
    
    Change-Id: I23005d10f7f3ccf06a2e29dab5fa11c7ed79f187
    Signed-off-by: Lee Leahy <leroy.p.leahy at intel.com>
---
 src/drivers/intel/fsp2_0/Kconfig             |  7 ++++
 src/drivers/intel/fsp2_0/Makefile.inc        |  2 +
 src/drivers/intel/fsp2_0/hand_off_block.c    | 15 +++++--
 src/drivers/intel/fsp2_0/hob_verify.c        | 63 ++++++++++++++++++++++++++++
 src/drivers/intel/fsp2_0/include/fsp/debug.h | 31 +++++++++++++-
 src/drivers/intel/fsp2_0/include/fsp/util.h  |  3 +-
 src/drivers/intel/fsp2_0/memory_init.c       |  4 +-
 7 files changed, 117 insertions(+), 8 deletions(-)

diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig
index 01846e3..5cdd33b 100644
--- a/src/drivers/intel/fsp2_0/Kconfig
+++ b/src/drivers/intel/fsp2_0/Kconfig
@@ -66,4 +66,11 @@ config FSP_M_XIP
 	help
 	  Select this value when FSP-M is execute-in-place.
 
+config VERIFY_HOBS
+	bool "Verify the FSP hand-off-blocks"
+	default n
+	help
+	  Verify that the HOBs required by coreboot are returned by FSP and
+	  that the resource HOBs are in the correct order and position.
+
 endif
diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc
index 5b8c663..9982dcb 100644
--- a/src/drivers/intel/fsp2_0/Makefile.inc
+++ b/src/drivers/intel/fsp2_0/Makefile.inc
@@ -18,12 +18,14 @@ ifeq ($(CONFIG_PLATFORM_USES_FSP2_0),y)
 romstage-y += hand_off_block.c
 romstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c
 romstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c
+romstage-$(CONFIG_VERIFY_HOBS) += hob_verify.c
 romstage-y += util.c
 romstage-y += memory_init.c
 
 ramstage-y += graphics.c
 ramstage-y += hand_off_block.c
 ramstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c
+ramstage-$(CONFIG_VERIFY_HOBS) += hob_verify.c
 ramstage-y += notify.c
 ramstage-y += silicon_init.c
 ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c
diff --git a/src/drivers/intel/fsp2_0/hand_off_block.c b/src/drivers/intel/fsp2_0/hand_off_block.c
index d3392cd..9693cd2 100644
--- a/src/drivers/intel/fsp2_0/hand_off_block.c
+++ b/src/drivers/intel/fsp2_0/hand_off_block.c
@@ -43,7 +43,7 @@ static const char *resource_names[] = {
 };
 
 /* UUIDs (GUIDs) in little-endian, so they can be used with memcmp() */
-static const uint8_t uuid_owner_bootloader_tolum[16] = {
+const uint8_t uuid_owner_bootloader_tolum[16] = {
 	0x56, 0x4f, 0xff, 0x73, 0x8e, 0xaa, 0x51, 0x44,
 	0xb3, 0x16, 0x36, 0x35, 0x36, 0x67, 0xad, 0x44,
 };
@@ -185,13 +185,14 @@ struct hob_resource *find_resource_hob_by_uuid(const struct hob_header *hob,
 	return NULL;
 }
 
-void fsp_find_reserved_memory(struct range_entry *re, const void *hob_list)
+void fsp_find_range_hob(struct range_entry *re, const void *hob_list,
+						const uint8_t uuid[16])
 {
 	const struct hob_resource *fsp_mem;
 
 	range_entry_init(re, 0, 0, 0);
 
-	fsp_mem = find_resource_hob_by_uuid(hob_list, uuid_owner_fsp);
+	fsp_mem = find_resource_hob_by_uuid(hob_list, uuid);
 
 	if (!fsp_mem) {
 		return;
@@ -200,6 +201,14 @@ void fsp_find_reserved_memory(struct range_entry *re, const void *hob_list)
 	range_entry_init(re, fsp_mem->addr, fsp_mem->addr + fsp_mem->length, 0);
 }
 
+struct range_entry fsp_find_reserved_memory(const void *hob_list)
+{
+	struct range_entry re;
+
+	fsp_find_range_hob(&re, hob_list, uuid_owner_fsp);
+	return re;
+}
+
 /*
  * Utilities for printing HOB information
  */
diff --git a/src/drivers/intel/fsp2_0/hob_verify.c b/src/drivers/intel/fsp2_0/hob_verify.c
new file mode 100644
index 0000000..b73f7cc
--- /dev/null
+++ b/src/drivers/intel/fsp2_0/hob_verify.c
@@ -0,0 +1,63 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2016 Intel Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <cbmem.h>
+#include <console/console.h>
+#include <fsp/util.h>
+
+struct range_entry fsp_find_bootloader_tolum(const void *hob_list)
+{
+	struct range_entry re;
+
+	fsp_find_range_hob(&re, hob_list, uuid_owner_bootloader_tolum);
+	return re;
+}
+
+void fsp_verify_hobs(const struct hob_header *hob_list_ptr)
+{
+	struct range_entry fsp_mem;
+	struct range_entry tolum;
+
+	/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */
+	tolum = fsp_find_bootloader_tolum(hob_list_ptr);
+	if ((range_entry_base(&tolum) == 0)
+		&& (range_entry_base(&tolum) == range_entry_end(&tolum)))
+		die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
+	if (range_entry_size(&tolum) < cbmem_overhead_size()) {
+		printk(BIOS_ERR,
+			"FSP_BOOTLOADER_TOLUM_SIZE: 0x%08llx < 0x%08lx\n",
+			range_entry_size(&tolum), cbmem_overhead_size());
+		die("ERROR: FSP_BOOTLOADER_TOLUM_HOB too small!\n");
+	}
+
+	/* Locate the FSP reserved memory area */
+	fsp_mem = fsp_find_reserved_memory(hob_list_ptr);
+	if ((range_entry_base(&fsp_mem) == 0)
+		&& (range_entry_base(&fsp_mem) == range_entry_end(&fsp_mem)))
+		die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n");
+
+	/* Verify the the bootloader tolum is above the FSP reserved area */
+	if (range_entry_end(&tolum) <= range_entry_base(&fsp_mem))
+		die("ERROR - FSP reserved region after BIOS TOLUM!\n");
+	if (range_entry_base(&tolum) < range_entry_end(&fsp_mem))
+		die("ERROR - FSP reserved region overlaps BIOS TOLUM!\n");
+
+	/* Verify that the FSP reserved area immediately follows the BIOS
+	 * reserved area
+	 */
+	if (range_entry_base(&tolum) != range_entry_end(&fsp_mem))
+		die("ERROR - Space between FSP reserved region and BIOS TOLUM!\n");
+
+	if (range_entry_end(&tolum) != (uintptr_t)cbmem_top()) {
+		printk(BIOS_CRIT, "0x%p: cbmem_top\n", cbmem_top());
+		die("ERROR - Space between cbmem_top and BIOS TOLUM!\n");
+	}
+}
diff --git a/src/drivers/intel/fsp2_0/include/fsp/debug.h b/src/drivers/intel/fsp2_0/include/fsp/debug.h
index 8965023..6508df5 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/debug.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/debug.h
@@ -14,6 +14,7 @@
 
 #include <fsp/util.h>
 #include <soc/intel/common/util.h>
+#include <memrange.h>
 
 struct hob_header {
 	uint16_t type;
@@ -48,6 +49,7 @@ enum hob_type {
 	HOB_TYPE_END_OF_HOB_LIST		= 0xFFFF,
 };
 
+extern const uint8_t uuid_owner_bootloader_tolum[16];
 extern const struct uuid_name_map  uuid_names[];
 extern const size_t uuid_names_entries;
 
@@ -57,6 +59,13 @@ extern const size_t uuid_names_entries;
  */
 const void *fsp_get_hob_list(void);
 
+/*
+ * Hand-off-block utilities which do not depend on CBMEM, but need to be passed
+ * the HOB list explicitly.
+ */
+void fsp_find_range_hob(struct range_entry *re, const void *hob_list,
+						const uint8_t uuid[16]);
+
 #if IS_ENABLED(CONFIG_DISPLAY_HOBS)
 /* Display HOBs API */
 void fsp_display_hobs(const struct hob_header *hob_list_ptr);
@@ -111,11 +120,31 @@ static inline __attribute__((always_inline)) void fspm_display_upd_values(
 }
 #endif /* CONFIG_DISPLAY_UPD_DATA */
 
+#if IS_ENABLED(CONFIG_VERIFY_HOBS)
+/* Verify HOBs API */
+void fsp_verify_hobs(const struct hob_header *hob_list_ptr);
+
+/* Verify HOBs utility functions */
+struct range_entry fsp_find_bootloader_tolum(const void *hob_list);
+
+#else /* CONFIG_VERIFY_HOBS */
+/* Remove HOB display code from the image */
+static inline __attribute__((always_inline)) void fsp_verify_hobs(
+	const struct hob_header *hob_list_ptr)
+{
+}
+#endif /* CONFIG_VERIFY_HOBS */
+
 static inline __attribute__((always_inline)) void fsp_debug_memory_init(
 	const struct hob_header *hob_list_ptr)
 {
-	/* Display the HOBs */
+	/* Verify that the HOB list pointer was set */
+	if (hob_list_ptr == NULL)
+		die("ERROR - HOB list pointer was not returned!\n");
+
+	/* Display and verify the HOBs */
 	fsp_display_hobs(hob_list_ptr);
+	fsp_verify_hobs(hob_list_ptr);
 
 	/* Display the MTRRs */
 	soc_display_mtrrs();
diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h
index c84a017..0877387 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/util.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/util.h
@@ -18,7 +18,6 @@
 #include <fsp/api.h>
 #include <fsp/debug.h>
 #include <fsp/info_header.h>
-#include <memrange.h>
 
 /*
  * Hand-off-block handling functions that depend on CBMEM, and thus can only
@@ -32,7 +31,7 @@ enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
  * Hand-off-block utilities which do not depend on CBMEM, but need to be passed
  * the HOB list explicitly.
  */
-void fsp_find_reserved_memory(struct range_entry *re, const void *hob_list);
+struct range_entry fsp_find_reserved_memory(const void *hob_list);
 const struct hob_resource *fsp_hob_header_to_resource(
 	const struct hob_header *hob);
 const struct hob_header *fsp_next_hob(const struct hob_header *parent);
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index cfbd3c4..7c27f09 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -64,7 +64,7 @@ static enum fsp_status do_fsp_post_memory_init(void *hob_list_ptr, bool s3wake,
 	struct romstage_handoff *handoff;
 
 	fsp_debug_memory_init(hob_list_ptr);
-	fsp_find_reserved_memory(&fsp_mem, hob_list_ptr);
+	fsp_mem = fsp_find_reserved_memory(hob_list_ptr);
 
 	/* initialize cbmem by adding FSP reserved memory first thing */
 	if (!s3wake) {
@@ -182,7 +182,7 @@ static enum fsp_status do_fsp_memory_init(struct fsp_header *hdr, bool s3wake,
 	enum fsp_status status;
 	fsp_memory_init_fn fsp_raminit;
 	struct FSPM_UPD fspm_upd, *upd;
-	void *hob_list_ptr;
+	void *hob_list_ptr = NULL;
 	struct FSPM_ARCH_UPD *arch_upd;
 
 	post_code(0x34);



More information about the coreboot-gerrit mailing list