Keith Short has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32773
Change subject: coreboot: add post code for memory error from FSP ......................................................................
coreboot: add post code for memory error from FSP
Add a new post code POST_RAM_FAILURE, used when the Intel FSP code fails to initialize RAM.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ibafefa0fc0b1c525f923929cc91731fbcc1e7533 Signed-off-by: Keith Short keithshort@chromium.org --- M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp2_0/memory_init.c M src/include/console/post_codes.h 3 files changed, 19 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/32773/1
diff --git a/src/drivers/intel/fsp1_1/raminit.c b/src/drivers/intel/fsp1_1/raminit.c index 726cc26..e91e49e 100644 --- a/src/drivers/intel/fsp1_1/raminit.c +++ b/src/drivers/intel/fsp1_1/raminit.c @@ -129,8 +129,10 @@ timestamp_add_now(TS_FSP_MEMORY_INIT_END);
printk(BIOS_DEBUG, "FspMemoryInit returned 0x%08x\n", status); - if (status != EFI_SUCCESS) + if (status != EFI_SUCCESS) { + post_code(POST_RAM_FAILURE); die("ERROR - FspMemoryInit failed to initialize memory!\n"); + }
/* Locate the FSP reserved memory area */ fsp_reserved_bytes = 0; @@ -195,8 +197,10 @@ }
#if CONFIG(DISPLAY_HOBS) - if (hob_list_ptr == NULL) + if (hob_list_ptr == NULL) { + post_code(POST_INVALID_FSP); die("ERROR - HOB pointer is NULL!\n"); + }
/* * Verify that FSP is generating the required HOBs: @@ -274,14 +278,17 @@ printk(BIOS_DEBUG, "0x%08x: Chipset reserved bytes reported by FSP\n", (unsigned int)delta_bytes); + post_code(POST_INVALID_FSP); die("Please verify the chipset reserved size\n"); } #endif }
/* Verify the FSP 1.1 HOB interface */ - if (fsp_verification_failure) + if (fsp_verification_failure) { + post_code(POST_INVALID_FSP); die("ERROR - coreboot's requirements not met by FSP binary!\n"); + }
/* Display the memory configuration */ report_memory_config(); diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 879b477..bc0e538 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -317,6 +317,7 @@ /* Handle any errors returned by FspMemoryInit */ fsp_handle_reset(status); if (status != FSP_SUCCESS) { + post_code(POST_RAM_FAILURE); printk(BIOS_CRIT, "FspMemoryInit returned 0x%08x\n", status); die("FspMemoryInit returned an error!\n"); } diff --git a/src/include/console/post_codes.h b/src/include/console/post_codes.h index ce26aac..4b4226e 100644 --- a/src/include/console/post_codes.h +++ b/src/include/console/post_codes.h @@ -341,6 +341,14 @@ #define POST_INVALID_FSP 0xE2
/** + * \brief RAM failure + * + * Set if RAM could not be initialized. This includes RAM is missing, + * unsupported RAM configuration, or RAM failure. + */ +#define POST_RAM_FAILURE 0xE3 + +/** * \brief TPM failure * * An error with the TPM, either unexepcted state or communications failure.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32773 )
Change subject: coreboot: add post code for memory error from FSP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32773/2/src/drivers/intel/fsp1_1/raminit.c File src/drivers/intel/fsp1_1/raminit.c:
https://review.coreboot.org/#/c/32773/2/src/drivers/intel/fsp1_1/raminit.c@2... PS2, Line 201: post_code(POST_INVALID_FSP); : die("ERROR - HOB pointer is NULL!\n"); Unrelated to your change, but shouldn't this check be outside of the config check? We use hob_list_ptr again on line 297.
If it were only used in the section displaying the hobs, we shouldn't be killing the system just because we can't print out the information. Yeah, it's probably a symptom of a larger problem, but we should run as long as we can.
Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32773 )
Change subject: coreboot: add post code for memory error from FSP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32773/2/src/drivers/intel/fsp1_1/raminit.c File src/drivers/intel/fsp1_1/raminit.c:
https://review.coreboot.org/#/c/32773/2/src/drivers/intel/fsp1_1/raminit.c@2... PS2, Line 201: post_code(POST_INVALID_FSP); : die("ERROR - HOB pointer is NULL!\n");
Unrelated to your change, but shouldn't this check be outside of the config check? We use hob_list_ […]
Looks like the uses of hob_list_ptr outside of this block all have NULL checks. However, the print_hob_type_structure() doesn't appear to have proper NULL checks. I'll move the NULL check around that call only and not call die().
Hello Patrick Rudolph, Huang Jin, Lee Leahy, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32773
to look at the new patch set (#3).
Change subject: coreboot: add post code for memory error ......................................................................
coreboot: add post code for memory error
Add a new post code POST_RAM_FAILURE, used when the Intel FSP code fails to initialize RAM.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ibafefa0fc0b1c525f923929cc91731fbcc1e7533 Signed-off-by: Keith Short keithshort@chromium.org --- M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp2_0/memory_init.c M src/include/console/post_codes.h 3 files changed, 12 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/32773/3
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32773 )
Change subject: post_code: add post code for memory error ......................................................................
Patch Set 4: Code-Review+2
Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32773 )
Change subject: post_code: add post code for memory error ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32773/2/src/drivers/intel/fsp1_1/raminit.c File src/drivers/intel/fsp1_1/raminit.c:
https://review.coreboot.org/#/c/32773/2/src/drivers/intel/fsp1_1/raminit.c@2... PS2, Line 201: post_code(POST_INVALID_FSP); : die("ERROR - HOB pointer is NULL!\n");
Looks like the uses of hob_list_ptr outside of this block all have NULL checks. […]
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32773 )
Change subject: post_code: add post code for memory error ......................................................................
Patch Set 5: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32773 )
Change subject: post_code: add post code for memory error ......................................................................
post_code: add post code for memory error
Add a new post code POST_RAM_FAILURE, used when the Intel FSP code fails to initialize RAM.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ibafefa0fc0b1c525f923929cc91731fbcc1e7533 Signed-off-by: Keith Short keithshort@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32773 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M Documentation/POSTCODES M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp2_0/memory_init.c M src/include/console/post_codes.h 4 files changed, 13 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/Documentation/POSTCODES b/Documentation/POSTCODES index 855940f..2a8285b 100644 --- a/Documentation/POSTCODES +++ b/Documentation/POSTCODES @@ -19,6 +19,7 @@ 0xe0 Boot media (e.g. SPI ROM) is corrupt 0xe1 Resource stored within CBFS is corrupt 0xe2 Vendor binary (e.g. FSP) generated a fatal error +0xe3 RAM could not be initialized 0xf8 Entry into elf boot 0xf3 Jumping to payload
diff --git a/src/drivers/intel/fsp1_1/raminit.c b/src/drivers/intel/fsp1_1/raminit.c index fc6f848..eff011a 100644 --- a/src/drivers/intel/fsp1_1/raminit.c +++ b/src/drivers/intel/fsp1_1/raminit.c @@ -130,7 +130,8 @@
printk(BIOS_DEBUG, "FspMemoryInit returned 0x%08x\n", status); if (status != EFI_SUCCESS) - die("ERROR - FspMemoryInit failed to initialize memory!\n"); + die_with_post_code(POST_RAM_FAILURE, + "ERROR - FspMemoryInit failed to initialize memory!\n");
/* Locate the FSP reserved memory area */ fsp_reserved_bytes = 0; diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 449b57d..60e3310 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -316,7 +316,8 @@ fsp_handle_reset(status); if (status != FSP_SUCCESS) { printk(BIOS_CRIT, "FspMemoryInit returned 0x%08x\n", status); - die("FspMemoryInit returned an error!\n"); + die_with_post_code(POST_RAM_FAILURE, + "FspMemoryInit returned an error!\n"); }
do_fsp_post_memory_init(s3wake, fsp_version); diff --git a/src/include/console/post_codes.h b/src/include/console/post_codes.h index 478c811..07927ec 100644 --- a/src/include/console/post_codes.h +++ b/src/include/console/post_codes.h @@ -341,6 +341,14 @@ #define POST_INVALID_VENDOR_BINARY 0xe2
/** + * \brief RAM failure + * + * Set if RAM could not be initialized. This includes RAM is missing, + * unsupported RAM configuration, or RAM failure. + */ +#define POST_RAM_FAILURE 0xe3 + +/** * \brief TPM failure * * An error with the TPM, either unexepcted state or communications failure.
-
Jett Rink (Code Review) -
Keith Short (Code Review)
-
Martin Roth (Code Review)
-
Patrick Rudolph (Code Review)