Sindhoor Tilak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
post_code: replace die postcodes with die_with_post_code
This change replaces failure postcode calls with die_with_post_code calls
Change-Id: I6188da11df046131eed1e77c41ae229852c2b5ac Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/postcar_loader.c M src/drivers/intel/fsp1_1/fsp_util.c M src/lib/ramtest.c M src/soc/amd/stoneyridge/romstage.c M src/soc/intel/xeon_sp/romstage.c 5 files changed, 12 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/43131/1
diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index fd1c172..7b3088f 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -97,7 +97,8 @@ void prepare_and_run_postcar(struct postcar_frame *pcf) { if (postcar_frame_init(pcf, 0)) - die("Unable to initialize postcar frame.\n"); + die_with_post_code(POST_EXIT_CAR_INIT_FAIL, + "Unable to initialize postcar frame.\n");
fill_postcar_frame(pcf);
diff --git a/src/drivers/intel/fsp1_1/fsp_util.c b/src/drivers/intel/fsp1_1/fsp_util.c index 9e7865d..a5ed5b2 100644 --- a/src/drivers/intel/fsp1_1/fsp_util.c +++ b/src/drivers/intel/fsp1_1/fsp_util.c @@ -123,8 +123,8 @@ fsp_header_ptr = (void *)find_fsp(CONFIG_FSP_LOC); if ((u32)fsp_header_ptr < 0xff) { /* output something in case there is no serial */ - post_code(0x4F); - die("Can't find the FSP!\n"); + die_with_post_code(POST_FSP_FAILURE, + "Can't find the FSP!\n"); } }
diff --git a/src/lib/ramtest.c b/src/lib/ramtest.c index c6cd7a4..9b381f7 100644 --- a/src/lib/ramtest.c +++ b/src/lib/ramtest.c @@ -108,7 +108,8 @@ } } if (failures) { - post_code(0xea); + die_with_post_code(POST_RAM_TEST_FAIL, + "\nDRAM did _NOT_ verify!\n"); printk(BIOS_DEBUG, "\nDRAM did _NOT_ verify!\n"); return 1; } @@ -126,7 +127,7 @@ */ printk(BIOS_DEBUG, "Testing DRAM at: %08lx\n", start); if (ram_bitset_nodie(start)) - die("DRAM ERROR"); + die_with_post_code(POST_RAM_FAILURE, "DRAM ERROR"); printk(BIOS_DEBUG, "Done.\n"); }
@@ -198,8 +199,7 @@
write_phys(dst, backup); if (fail) { - post_code(0xea); - die("RAM INIT FAILURE!\n"); + die_with_post_code(POST_RAM_FAILURE, "RAM INIT FAILURE!\n"); } phys_memory_barrier(); } diff --git a/src/soc/amd/stoneyridge/romstage.c b/src/soc/amd/stoneyridge/romstage.c index 9fb80f8..991afc0 100644 --- a/src/soc/amd/stoneyridge/romstage.c +++ b/src/soc/amd/stoneyridge/romstage.c @@ -120,9 +120,9 @@ if (CONFIG(SMM_TSEG)) smm_list_regions();
- post_code(0x44); if (postcar_frame_init(&pcf, 0)) - die("Unable to initialize postcar frame.\n"); + die_with_post_code(POST_EXIT_CAR_INIT_FAIL, + "Unable to initialize postcar frame.\n");
/* * We need to make sure ramstage will be run cached. At this point exact diff --git a/src/soc/intel/xeon_sp/romstage.c b/src/soc/intel/xeon_sp/romstage.c index 9f4d78d..2d1b02e 100644 --- a/src/soc/intel/xeon_sp/romstage.c +++ b/src/soc/intel/xeon_sp/romstage.c @@ -25,7 +25,8 @@ unlock_pam_regions();
if (postcar_frame_init(&pcf, 1 * KiB)) - die("Unable to initialize postcar frame.\n"); + die_with_post_code(POST_EXIT_CAR_INIT_FAIL, + "Unable to initialize postcar frame.\n");
/* * We need to make sure ramstage will be run cached. At this point exact
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43131/1/src/lib/ramtest.c File src/lib/ramtest.c:
https://review.coreboot.org/c/coreboot/+/43131/1/src/lib/ramtest.c@113 PS1, Line 113: printk(BIOS_DEBUG, "\nDRAM did _NOT_ verify!\n"); This changes behavior, doesn’t it? Should we die, if the test fails? If yes, the print line is not reached.
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43131/1/src/lib/ramtest.c File src/lib/ramtest.c:
https://review.coreboot.org/c/coreboot/+/43131/1/src/lib/ramtest.c@113 PS1, Line 113: printk(BIOS_DEBUG, "\nDRAM did _NOT_ verify!\n");
This changes behavior, doesn’t it? Should we die, if the test fails? If yes, the print line is not r […]
My bad, in this case it shouldn't have been replaced with a die. Ack
Hello build bot (Jenkins), Lee Leahy, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43131
to look at the new patch set (#3).
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
post_code: replace die postcodes with die_with_post_code
This change replaces failure postcode calls with die_with_post_code calls
Change-Id: I6188da11df046131eed1e77c41ae229852c2b5ac Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/postcar_loader.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/fsp_util.c M src/lib/ramtest.c M src/soc/amd/stoneyridge/romstage.c M src/soc/intel/xeon_sp/romstage.c 6 files changed, 14 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/43131/3
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43131/1/src/lib/ramtest.c File src/lib/ramtest.c:
https://review.coreboot.org/c/coreboot/+/43131/1/src/lib/ramtest.c@113 PS1, Line 113: printk(BIOS_DEBUG, "\nDRAM did _NOT_ verify!\n");
My bad, in this case it shouldn't have been replaced with a die. […]
Done
Hello build bot (Jenkins), Lee Leahy, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43131
to look at the new patch set (#5).
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
post_code: replace die postcodes with die_with_post_code
This change replaces failure postcode calls with die_with_post_code calls
Change-Id: I6188da11df046131eed1e77c41ae229852c2b5ac Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/postcar_loader.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/fsp_util.c M src/lib/ramtest.c M src/soc/amd/stoneyridge/romstage.c M src/soc/intel/xeon_sp/romstage.c 6 files changed, 14 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/43131/5
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 6:
(1 comment)
Is 'Unable' aligned with POST_* in files?
https://review.coreboot.org/c/coreboot/+/43131/6/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/43131/6/src/arch/x86/postcar_loader... PS6, Line 101: "Unable to initialize postcar frame.\n"); Is 'Unable' aligned with POST_?
Hello build bot (Jenkins), Lee Leahy, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43131
to look at the new patch set (#7).
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
post_code: replace die postcodes with die_with_post_code
This change replaces failure postcode calls with die_with_post_code calls
Change-Id: I6188da11df046131eed1e77c41ae229852c2b5ac Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/postcar_loader.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/fsp_util.c M src/lib/ramtest.c M src/soc/amd/stoneyridge/romstage.c M src/soc/intel/xeon_sp/romstage.c 6 files changed, 14 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/43131/7
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43131/6/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/43131/6/src/arch/x86/postcar_loader... PS6, Line 101: "Unable to initialize postcar frame.\n");
Is 'Unable' aligned with POST_?
Done
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 7: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43131/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43131/7//COMMIT_MSG@7 PS7, Line 7: post_code: replace die postcodes with die_with_post_code This is not true. This change mainly adds postcode calls in places there were none beforehand. Sure, it might make sense to emit postcodes in most of these situations, but please update the commit summary and message accordingly.
https://review.coreboot.org/c/coreboot/+/43131/7/src/soc/amd/stoneyridge/rom... File src/soc/amd/stoneyridge/romstage.c:
https://review.coreboot.org/c/coreboot/+/43131/7/src/soc/amd/stoneyridge/rom... PS7, Line 114: post_code(0x43); Why is this gone?
https://review.coreboot.org/c/coreboot/+/43131/7/src/soc/amd/stoneyridge/rom... PS7, Line 123: post_code(0x44); Why is this gone?
https://review.coreboot.org/c/coreboot/+/43131/7/src/soc/amd/stoneyridge/rom... PS7, Line 115: die_with_post_code(POST_CBMEM_RECOVER_FAIL, This wasn't dying before
Attention is currently required from: Sindhoor Tilak. Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43131 )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS7: Sindhoor, Any updates to this patch?
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43131?usp=email )
Change subject: post_code: replace die postcodes with die_with_post_code ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Frans Hendriks (Code Review)
-
Paul Menzel (Code Review)
-
Rocky Phagura (Code Review)
-
Sindhoor Tilak (Code Review)
-
Stefan Reinauer (Code Review)