Change in coreboot[master]: drivers/intel/fsp2_0: move die() calls to the functions

[Michael Niewöhner (Code Review)]

Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

drivers/intel/fsp2_0: move die() calls to the functions

Since there are no calls where we wouldn't die(), move die() calls into
the fsp_find_* functions.

Change-Id: I750a225999688137421bbc560d9d1f5fdf68fd01
Signed-off-by: Michael Niewöhner 
---
M src/drivers/intel/fsp2_0/hand_off_block.c
M src/drivers/intel/fsp2_0/hob_verify.c
M src/drivers/intel/fsp2_0/include/fsp/debug.h
M src/drivers/intel/fsp2_0/include/fsp/util.h
M src/drivers/intel/fsp2_0/memory_init.c
5 files changed, 13 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36314/1

diff --git a/src/drivers/intel/fsp2_0/hand_off_block.c b/src/drivers/intel/fsp2_0/hand_off_block.c
index 89bb6e0..9d031a5 100644
--- a/src/drivers/intel/fsp2_0/hand_off_block.c
+++ b/src/drivers/intel/fsp2_0/hand_off_block.c
@@ -189,9 +189,10 @@
    return 0;
 }

-int fsp_find_reserved_memory(struct range_entry *re)
+void fsp_find_reserved_memory(struct range_entry *re)
 {
-	return fsp_find_range_hob(re, fsp_reserved_memory_guid);
+	if (fsp_find_range_hob(re, fsp_reserved_memory_guid));
+		die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n")
 }

const void *fsp_find_extension_hob_by_guid(const uint8_t *guid, size_t *size)
diff --git a/src/drivers/intel/fsp2_0/hob_verify.c b/src/drivers/intel/fsp2_0/hob_verify.c
index edf3941..e2937d7 100644
--- a/src/drivers/intel/fsp2_0/hob_verify.c
+++ b/src/drivers/intel/fsp2_0/hob_verify.c
@@ -16,9 +16,10 @@
 #include <console/console.h>
 #include <fsp/util.h>

-int fsp_find_bootloader_tolum(struct range_entry *re)
+void fsp_find_bootloader_tolum(struct range_entry *re)
 {
-	return fsp_find_range_hob(re, fsp_bootloader_tolum_guid);
+	if (fsp_find_range_hob(re, fsp_bootloader_tolum_guid))
+		die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
 }

void fsp_verify_memory_init_hobs(void)
@@ -26,9 +27,8 @@
    struct range_entry fsp_mem;
    struct range_entry tolum;

-	/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */
-	if (fsp_find_bootloader_tolum(&tolum))
-		die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
+	/* Verify the size of the TOLUM range */
+	fsp_find_bootloader_tolum(&tolum);
    if (range_entry_size(&tolum) < cbmem_overhead_size()) {
    	printk(BIOS_CRIT,
    		"FSP_BOOTLOADER_TOLUM_SIZE: 0x%08llx < 0x%08zx\n",
@@ -36,11 +36,8 @@
    	die("FSP_BOOTLOADER_TOLUM_HOB too small!\n");
    }

-	/* Locate the FSP reserved memory area */
-	if (fsp_find_reserved_memory(&fsp_mem))
-		die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n");
-
    /* Verify the bootloader tolum is above the FSP reserved area */
+	fsp_find_reserved_memory(&fsp_mem);
    if (range_entry_end(&tolum) <= range_entry_base(&fsp_mem)) {
    	printk(BIOS_CRIT,
    		"TOLUM end: 0x%08llx != 0x%08llx: FSP rsvd base\n",
diff --git a/src/drivers/intel/fsp2_0/include/fsp/debug.h b/src/drivers/intel/fsp2_0/include/fsp/debug.h
index c01ac1c..fa85955 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/debug.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/debug.h
@@ -59,7 +59,6 @@
 const char *fsp_get_hob_type_name(const struct hob_header *hob);
 const char *fsp_get_guid_name(const uint8_t *guid);
 void fsp_print_guid_extension_hob(const struct hob_header *hob);
-int fsp_find_bootloader_tolum(struct range_entry *re);

/*
  * Writes number_of_bytes data bytes from buffer to the console.
diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h
index 282e9e9..303bafe 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/util.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/util.h
@@ -79,11 +79,13 @@
 enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 int fsp_find_range_hob(struct range_entry *re, const uint8_t guid[16]);
 void fsp_display_fvi_version_hob(void);
-int fsp_find_reserved_memory(struct range_entry *re);
+void fsp_find_reserved_memory(struct range_entry *re);
 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);
 bool fsp_guid_compare(const uint8_t guid1[16], const uint8_t guid2[16]);
+void fsp_find_bootloader_tolum(struct range_entry *re);
+

/* Fill in header and validate sanity of component within region device. */
 enum cb_err fsp_validate_component(struct fsp_header *hdr,
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index f31d93c..455dfa5 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -74,8 +74,7 @@
 {
    struct range_entry fsp_mem;

-	if (fsp_find_reserved_memory(&fsp_mem))
-		die("Failed to find FSP_RESERVED_MEMORY_RESOURCE_HOB!\n");
+	fsp_find_reserved_memory(&fsp_mem);

/* initialize cbmem by adding FSP reserved memory first thing */
    if (!s3wake) {

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/36314/1/src/drivers/intel/fsp2_0/ha... 
File src/drivers/intel/fsp2_0/hand_off_block.c:

https://review.coreboot.org/c/coreboot/+/36314/1/src/drivers/intel/fsp2_0/ha... 
PS1, Line 194: 	if (fsp_find_range_hob(re, fsp_reserved_memory_guid));
trailing semicolon indicates no statements, indent implies otherwise

https://review.coreboot.org/c/coreboot/+/36314/1/src/drivers/intel/fsp2_0/ha... 
PS1, Line 194: 	if (fsp_find_range_hob(re, fsp_reserved_memory_guid));
trailing statements should be on next line

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

Uploaded patch set 2.

[Michael Niewöhner (Code Review)]

Hello Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36314

to look at the new patch set (#2).

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

drivers/intel/fsp2_0: move die() calls to the functions

Since there are no calls where we wouldn't die(), move die() calls into
the fsp_find_* functions.

Change-Id: I750a225999688137421bbc560d9d1f5fdf68fd01
Signed-off-by: Michael Niewöhner 
---
M src/drivers/intel/fsp2_0/hand_off_block.c
M src/drivers/intel/fsp2_0/hob_verify.c
M src/drivers/intel/fsp2_0/include/fsp/debug.h
M src/drivers/intel/fsp2_0/include/fsp/util.h
M src/drivers/intel/fsp2_0/memory_init.c
5 files changed, 13 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36314/2

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

Patch Set 2: Code-Review+1

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

Patch Set 2: Code-Review+2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

Patch Set 2: Code-Review+1

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36314 )

Change subject: drivers/intel/fsp2_0: move die() calls to the functions
......................................................................

drivers/intel/fsp2_0: move die() calls to the functions

Since there are no calls where we wouldn't die(), move die() calls into
the fsp_find_* functions.

Change-Id: I750a225999688137421bbc560d9d1f5fdf68fd01
Signed-off-by: Michael Niewöhner 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36314
Tested-by: build bot (Jenkins) 
Reviewed-by: Michael Niewöhner
Reviewed-by: Aaron Durbin 
Reviewed-by: Nico Huber 
---
M src/drivers/intel/fsp2_0/hand_off_block.c
M src/drivers/intel/fsp2_0/hob_verify.c
M src/drivers/intel/fsp2_0/include/fsp/debug.h
M src/drivers/intel/fsp2_0/include/fsp/util.h
M src/drivers/intel/fsp2_0/memory_init.c
5 files changed, 13 insertions(+), 15 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, but someone else must approve
  Aaron Durbin: Looks good to me, approved
  Michael Niewöhner: Looks good to me, but someone else must approve

diff --git a/src/drivers/intel/fsp2_0/hand_off_block.c b/src/drivers/intel/fsp2_0/hand_off_block.c
index 89bb6e0..65ceb20 100644
--- a/src/drivers/intel/fsp2_0/hand_off_block.c
+++ b/src/drivers/intel/fsp2_0/hand_off_block.c
@@ -189,9 +189,10 @@
    return 0;
 }

-int fsp_find_reserved_memory(struct range_entry *re)
+void fsp_find_reserved_memory(struct range_entry *re)
 {
-	return fsp_find_range_hob(re, fsp_reserved_memory_guid);
+	if (fsp_find_range_hob(re, fsp_reserved_memory_guid))
+		die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n");
 }

const void *fsp_find_extension_hob_by_guid(const uint8_t *guid, size_t *size)
diff --git a/src/drivers/intel/fsp2_0/hob_verify.c b/src/drivers/intel/fsp2_0/hob_verify.c
index edf3941..e2937d7 100644
--- a/src/drivers/intel/fsp2_0/hob_verify.c
+++ b/src/drivers/intel/fsp2_0/hob_verify.c
@@ -16,9 +16,10 @@
 #include <console/console.h>
 #include <fsp/util.h>

-int fsp_find_bootloader_tolum(struct range_entry *re)
+void fsp_find_bootloader_tolum(struct range_entry *re)
 {
-	return fsp_find_range_hob(re, fsp_bootloader_tolum_guid);
+	if (fsp_find_range_hob(re, fsp_bootloader_tolum_guid))
+		die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
 }

void fsp_verify_memory_init_hobs(void)
@@ -26,9 +27,8 @@
    struct range_entry fsp_mem;
    struct range_entry tolum;

-	/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */
-	if (fsp_find_bootloader_tolum(&tolum))
-		die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
+	/* Verify the size of the TOLUM range */
+	fsp_find_bootloader_tolum(&tolum);
    if (range_entry_size(&tolum) < cbmem_overhead_size()) {
    	printk(BIOS_CRIT,
    		"FSP_BOOTLOADER_TOLUM_SIZE: 0x%08llx < 0x%08zx\n",
@@ -36,11 +36,8 @@
    	die("FSP_BOOTLOADER_TOLUM_HOB too small!\n");
    }

-	/* Locate the FSP reserved memory area */
-	if (fsp_find_reserved_memory(&fsp_mem))
-		die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n");
-
    /* Verify the bootloader tolum is above the FSP reserved area */
+	fsp_find_reserved_memory(&fsp_mem);
    if (range_entry_end(&tolum) <= range_entry_base(&fsp_mem)) {
    	printk(BIOS_CRIT,
    		"TOLUM end: 0x%08llx != 0x%08llx: FSP rsvd base\n",
diff --git a/src/drivers/intel/fsp2_0/include/fsp/debug.h b/src/drivers/intel/fsp2_0/include/fsp/debug.h
index c01ac1c..fa85955 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/debug.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/debug.h
@@ -59,7 +59,6 @@
 const char *fsp_get_hob_type_name(const struct hob_header *hob);
 const char *fsp_get_guid_name(const uint8_t *guid);
 void fsp_print_guid_extension_hob(const struct hob_header *hob);
-int fsp_find_bootloader_tolum(struct range_entry *re);

/*
  * Writes number_of_bytes data bytes from buffer to the console.
diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h
index 282e9e9..303bafe 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/util.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/util.h
@@ -79,11 +79,13 @@
 enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 int fsp_find_range_hob(struct range_entry *re, const uint8_t guid[16]);
 void fsp_display_fvi_version_hob(void);
-int fsp_find_reserved_memory(struct range_entry *re);
+void fsp_find_reserved_memory(struct range_entry *re);
 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);
 bool fsp_guid_compare(const uint8_t guid1[16], const uint8_t guid2[16]);
+void fsp_find_bootloader_tolum(struct range_entry *re);
+

/* Fill in header and validate sanity of component within region device. */
 enum cb_err fsp_validate_component(struct fsp_header *hdr,
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index f31d93c..455dfa5 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -74,8 +74,7 @@
 {
    struct range_entry fsp_mem;

-	if (fsp_find_reserved_memory(&fsp_mem))
-		die("Failed to find FSP_RESERVED_MEMORY_RESOURCE_HOB!\n");
+	fsp_find_reserved_memory(&fsp_mem);

/* initialize cbmem by adding FSP reserved memory first thing */
    if (!s3wake) {