Keith Short has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32772
Change subject: coreboot: add post code for invalid FSP ......................................................................
coreboot: add post code for invalid FSP
Add a new post code POST_INVALID_FSP, used when coreboot fails to locate or validate Intel FSP.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ib1e359d4e8772c37922b1b779135e58c73bff6b4 Signed-off-by: Keith Short keithshort@chromium.org --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/include/console/post_codes.h M src/soc/intel/fsp_baytrail/romstage/romstage.c M src/soc/intel/fsp_broadwell_de/romstage/romstage.c 5 files changed, 19 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/32772/1
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index b3afb98..879b477 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -276,8 +276,10 @@
upd = (FSPM_UPD *)(hdr->cfg_region_offset + hdr->image_base);
- if (upd->FspUpdHeader.Signature != FSPM_UPD_SIGNATURE) + if (upd->FspUpdHeader.Signature != FSPM_UPD_SIGNATURE) { + post_code(POST_INVALID_FSP); die("Invalid FSPM signature!\n"); + }
/* Copy the default values from the UPD area */ memcpy(&fspm_upd, upd, sizeof(fspm_upd)); @@ -289,8 +291,10 @@
/* Fill common settings on behalf of chipset. */ if (fsp_fill_common_arch_params(arch_upd, s3wake, fsp_version, - memmap) != CB_SUCCESS) + memmap) != CB_SUCCESS) { + post_code(POST_INVALID_FSP); die("FSPM_ARCH_UPD not found!\n"); + }
/* Give SoC and mainboard a chance to update the UPD */ platform_fsp_memory_init_params_cb(&fspm_upd, fsp_version); diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 402b05d..dd4d730 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -32,8 +32,10 @@
supd = (FSPS_UPD *) (hdr->cfg_region_offset + hdr->image_base);
- if (supd->FspUpdHeader.Signature != FSPS_UPD_SIGNATURE) + if (supd->FspUpdHeader.Signature != FSPS_UPD_SIGNATURE) { + post_code(POST_INVALID_FSP); die("Invalid FSPS signature\n"); + }
upd = xmalloc(sizeof(FSPS_UPD));
diff --git a/src/include/console/post_codes.h b/src/include/console/post_codes.h index 637083a..ce26aac 100644 --- a/src/include/console/post_codes.h +++ b/src/include/console/post_codes.h @@ -333,6 +333,14 @@ #define POST_INVALID_CBFS 0xE1
/** + * \brief FSP error + * + * Set if firmware failed to find or validate a FSP resource, or another + * FSP error occurred. + */ +#define POST_INVALID_FSP 0xE2 + +/** * \brief TPM failure * * An error with the TPM, either unexepcted state or communications failure. diff --git a/src/soc/intel/fsp_baytrail/romstage/romstage.c b/src/soc/intel/fsp_baytrail/romstage/romstage.c index c46b09e..93900ef 100644 --- a/src/soc/intel/fsp_baytrail/romstage/romstage.c +++ b/src/soc/intel/fsp_baytrail/romstage/romstage.c @@ -208,6 +208,7 @@ post_code(0x48); printk(BIOS_DEBUG, "Starting the Intel FSP (early_init)\n"); fsp_early_init(fsp_info_header); + post_code(POST_INVALID_FSP); die("Uh Oh! fsp_early_init should not return here.\n"); }
diff --git a/src/soc/intel/fsp_broadwell_de/romstage/romstage.c b/src/soc/intel/fsp_broadwell_de/romstage/romstage.c index a75dabd..12a1702 100644 --- a/src/soc/intel/fsp_broadwell_de/romstage/romstage.c +++ b/src/soc/intel/fsp_broadwell_de/romstage/romstage.c @@ -84,6 +84,7 @@ post_code(0x48); printk(BIOS_DEBUG, "Starting the Intel FSP (early_init)\n"); fsp_early_init(fsp_info_header); + post_code(POST_FSP_FAILURE); die("Uh Oh! fsp_early_init should not return here.\n"); }
Hello Patrick Rudolph, Huang Jin, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32772
to look at the new patch set (#2).
Change subject: coreboot: add post code for invalid FSP ......................................................................
coreboot: add post code for invalid FSP
Add a new post code POST_INVALID_FSP, used when coreboot fails to locate or validate Intel FSP.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ib1e359d4e8772c37922b1b779135e58c73bff6b4 Signed-off-by: Keith Short keithshort@chromium.org --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/include/console/post_codes.h M src/soc/intel/fsp_baytrail/romstage/romstage.c M src/soc/intel/fsp_broadwell_de/romstage/romstage.c 5 files changed, 19 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/32772/2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: coreboot: add post code for invalid FSP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32772/2/src/include/console/post_codes.h File src/include/console/post_codes.h:
https://review.coreboot.org/#/c/32772/2/src/include/console/post_codes.h@341 PS2, Line 341: POST_INVALID_FSP Is there any reason to make this FSP specific?
Maybe use: POST_INVALID_VENDOR_BINARY
Or use POST_INVALID_ROM here?
Hello Patrick Rudolph, Huang Jin, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32772
to look at the new patch set (#3).
Change subject: coreboot: add post code for invalid vendor binary ......................................................................
coreboot: add post code for invalid vendor binary
Add a new post code POST_INVALID_VENDOR_BINARY, used when coreboot fails to locate validate a vendor supplied binary.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ib1e359d4e8772c37922b1b779135e58c73bff6b4 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/drivers/intel/fsp2_0/silicon_init.c M src/include/console/post_codes.h M src/soc/intel/fsp_baytrail/romstage/romstage.c M src/soc/intel/fsp_broadwell_de/romstage/romstage.c 6 files changed, 26 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/32772/3
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: coreboot: add post code for invalid vendor binary ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: coreboot: add post code for invalid vendor binary ......................................................................
Patch Set 3: -Code-Review
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 4: Code-Review+2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 4:
checks on mrc.bin are missing
Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 4:
(1 comment)
Patch Set 4:
checks on mrc.bin are missing
Can you elaborate a little more please? Is there a specific file you are seeing checks on mrc.bin missing in?
https://review.coreboot.org/#/c/32772/2/src/include/console/post_codes.h File src/include/console/post_codes.h:
https://review.coreboot.org/#/c/32772/2/src/include/console/post_codes.h@341 PS2, Line 341: POST_INVALID_FSP
Is there any reason to make this FSP specific? […]
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 4:
Patch Set 4:
(1 comment)
Patch Set 4:
checks on mrc.bin are missing
Can you elaborate a little more please? Is there a specific file you are seeing checks on mrc.bin missing in?
void sdram_initialize(struct pei_data *pei_data) in src/northbridge/intel/sandybridge/raminit_mrc.c src/northbridge/intel/haswell/raminit.c
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
(1 comment)
Patch Set 4:
checks on mrc.bin are missing
Can you elaborate a little more please? Is there a specific file you are seeing checks on mrc.bin missing in?
void sdram_initialize(struct pei_data *pei_data) in src/northbridge/intel/sandybridge/raminit_mrc.c src/northbridge/intel/haswell/raminit.c
Sure, if those are the same issue, it doesn't hurt to update them.
Jett Rink has uploaded a new patch set (#5) to the change originally created by Keith Short. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
post_code: add post code for invalid vendor binary
Add a new post code POST_INVALID_VENDOR_BINARY, used when coreboot fails to locate or validate a vendor supplied binary.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ib1e359d4e8772c37922b1b779135e58c73bff6b4 Signed-off-by: Keith Short keithshort@chromium.org --- M Documentation/POSTCODES M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/include/console/post_codes.h M src/northbridge/intel/haswell/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/fsp_baytrail/romstage/romstage.c M src/soc/intel/fsp_broadwell_de/romstage/romstage.c 9 files changed, 31 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/32772/5
Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 5:
Patch Set 4:
Patch Set 4:
Patch Set 4:
(1 comment)
Patch Set 4:
checks on mrc.bin are missing
Can you elaborate a little more please? Is there a specific file you are seeing checks on mrc.bin missing in?
void sdram_initialize(struct pei_data *pei_data) in src/northbridge/intel/sandybridge/raminit_mrc.c src/northbridge/intel/haswell/raminit.c
Sure, if those are the same issue, it doesn't hurt to update them.
Done. (updated /raminit(_mrc)?.c/ files)
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
Patch Set 11: Code-Review+2
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32772 )
Change subject: post_code: add post code for invalid vendor binary ......................................................................
post_code: add post code for invalid vendor binary
Add a new post code POST_INVALID_VENDOR_BINARY, used when coreboot fails to locate or validate a vendor supplied binary.
BUG=b:124401932 BRANCH=sarien TEST=build coreboot for sarien and arcada platforms
Change-Id: Ib1e359d4e8772c37922b1b779135e58c73bff6b4 Signed-off-by: Keith Short keithshort@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32772 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Duncan Laurie dlaurie@chromium.org --- M Documentation/POSTCODES M src/drivers/intel/fsp1_1/raminit.c M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/include/console/post_codes.h M src/northbridge/intel/haswell/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/fsp_baytrail/romstage/romstage.c M src/soc/intel/fsp_broadwell_de/romstage/romstage.c 9 files changed, 31 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved
diff --git a/Documentation/POSTCODES b/Documentation/POSTCODES index 162e863..855940f 100644 --- a/Documentation/POSTCODES +++ b/Documentation/POSTCODES @@ -18,6 +18,7 @@ 0x89 Devices have been enabled 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 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 726cc26..fc6f848 100644 --- a/src/drivers/intel/fsp1_1/raminit.c +++ b/src/drivers/intel/fsp1_1/raminit.c @@ -195,9 +195,6 @@ }
#if CONFIG(DISPLAY_HOBS) - if (hob_list_ptr == NULL) - die("ERROR - HOB pointer is NULL!\n"); - /* * Verify that FSP is generating the required HOBs: * 7.1: FSP_BOOTLOADER_TEMP_MEMORY_HOB only produced for FSP 1.0 @@ -244,7 +241,10 @@ "ERROR - Missing one or more required FSP HOBs!\n");
/* Display the HOBs */ - print_hob_type_structure(0, hob_list_ptr); + if (hob_list_ptr != NULL) + print_hob_type_structure(0, hob_list_ptr); + else + printk(BIOS_ERR, "ERROR - HOB pointer is NULL!\n"); #endif
/* Get the address of the CBMEM region for the FSP reserved memory */ @@ -274,14 +274,16 @@ printk(BIOS_DEBUG, "0x%08x: Chipset reserved bytes reported by FSP\n", (unsigned int)delta_bytes); - die("Please verify the chipset reserved size\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Please verify the chipset reserved size\n"); } #endif }
/* Verify the FSP 1.1 HOB interface */ if (fsp_verification_failure) - die("ERROR - coreboot's requirements not met by FSP binary!\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "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 b3afb98..449b57d 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -277,7 +277,8 @@ upd = (FSPM_UPD *)(hdr->cfg_region_offset + hdr->image_base);
if (upd->FspUpdHeader.Signature != FSPM_UPD_SIGNATURE) - die("Invalid FSPM signature!\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Invalid FSPM signature!\n");
/* Copy the default values from the UPD area */ memcpy(&fspm_upd, upd, sizeof(fspm_upd)); @@ -290,7 +291,8 @@ /* Fill common settings on behalf of chipset. */ if (fsp_fill_common_arch_params(arch_upd, s3wake, fsp_version, memmap) != CB_SUCCESS) - die("FSPM_ARCH_UPD not found!\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "FSPM_ARCH_UPD not found!\n");
/* Give SoC and mainboard a chance to update the UPD */ platform_fsp_memory_init_params_cb(&fspm_upd, fsp_version); diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 402b05d..b0a697d 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -33,7 +33,8 @@ supd = (FSPS_UPD *) (hdr->cfg_region_offset + hdr->image_base);
if (supd->FspUpdHeader.Signature != FSPS_UPD_SIGNATURE) - die("Invalid FSPS signature\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Invalid FSPS signature\n");
upd = xmalloc(sizeof(FSPS_UPD));
diff --git a/src/include/console/post_codes.h b/src/include/console/post_codes.h index 7bd1ee0..478c811 100644 --- a/src/include/console/post_codes.h +++ b/src/include/console/post_codes.h @@ -333,6 +333,14 @@ #define POST_INVALID_CBFS 0xe1
/** + * \brief Vendor binary error + * + * Set if firmware failed to find or validate a vendor binary, or the binary + * generated a fatal error. + */ +#define POST_INVALID_VENDOR_BINARY 0xe2 + +/** * \brief TPM failure * * An error with the TPM, either unexepcted state or communications failure. diff --git a/src/northbridge/intel/haswell/raminit.c b/src/northbridge/intel/haswell/raminit.c index 050dbd1..c24bb67 100644 --- a/src/northbridge/intel/haswell/raminit.c +++ b/src/northbridge/intel/haswell/raminit.c @@ -166,7 +166,8 @@ default: printk(BIOS_ERR, "MRC returned %x.\n", rv); } - die("Nonzero MRC return value.\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Nonzero MRC return value.\n"); } } else { die("UEFI PEI System Agent not found.\n"); diff --git a/src/northbridge/intel/sandybridge/raminit_mrc.c b/src/northbridge/intel/sandybridge/raminit_mrc.c index f032b8a..e88d356 100644 --- a/src/northbridge/intel/sandybridge/raminit_mrc.c +++ b/src/northbridge/intel/sandybridge/raminit_mrc.c @@ -235,7 +235,8 @@ default: printk(BIOS_ERR, "MRC returned %x.\n", rv); } - die("Nonzero MRC return value.\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Nonzero MRC return value.\n"); } } else { die("UEFI PEI System Agent not found.\n"); diff --git a/src/soc/intel/fsp_baytrail/romstage/romstage.c b/src/soc/intel/fsp_baytrail/romstage/romstage.c index c46b09e..030b5df 100644 --- a/src/soc/intel/fsp_baytrail/romstage/romstage.c +++ b/src/soc/intel/fsp_baytrail/romstage/romstage.c @@ -208,7 +208,8 @@ post_code(0x48); printk(BIOS_DEBUG, "Starting the Intel FSP (early_init)\n"); fsp_early_init(fsp_info_header); - die("Uh Oh! fsp_early_init should not return here.\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Uh Oh! fsp_early_init should not return here.\n"); }
/******************************************************************************* diff --git a/src/soc/intel/fsp_broadwell_de/romstage/romstage.c b/src/soc/intel/fsp_broadwell_de/romstage/romstage.c index a75dabd..121cb25 100644 --- a/src/soc/intel/fsp_broadwell_de/romstage/romstage.c +++ b/src/soc/intel/fsp_broadwell_de/romstage/romstage.c @@ -84,7 +84,8 @@ post_code(0x48); printk(BIOS_DEBUG, "Starting the Intel FSP (early_init)\n"); fsp_early_init(fsp_info_header); - die("Uh Oh! fsp_early_init should not return here.\n"); + die_with_post_code(POST_INVALID_VENDOR_BINARY, + "Uh Oh! fsp_early_init should not return here.\n"); }
/*******************************************************************************
-
Duncan Laurie (Code Review) -
Jett Rink (Code Review)
-
Keith Short (Code Review)
-
Martin Roth (Code Review)
-
Patrick Rudolph (Code Review)