Sindhoor Tilak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
post_code: add missing postcode calls
The change adds postcode calls wherever necessary on top of the updated set of postcode defines
Change-Id: Ia75cd863bf6ffac2c91ff78aefabc5327b1c138b Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/postcar_loader.c M src/console/init.c M src/cpu/intel/car/p3/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/intel/haswell/romstage.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/cache_as_ram.S M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S M src/soc/intel/xeon_sp/romstage.c 10 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/43000/1
diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index f79c90c..7b3088f 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -104,6 +104,7 @@
postcar_frame_common_mtrrs(pcf);
+ post_code(POST_ENTRY_POST_CAR); run_postcar_phase(pcf); /* We do not return here. */ } diff --git a/src/console/init.c b/src/console/init.c index 1dba9ad..cfe2e2e 100644 --- a/src/console/init.c +++ b/src/console/init.c @@ -62,6 +62,7 @@
asmlinkage void console_init(void) { + post_code(POST_CONSOLE_READY); init_log_level();
if (CONFIG(DEBUG_CONSOLE_INIT)) diff --git a/src/cpu/intel/car/p3/cache_as_ram.S b/src/cpu/intel/car/p3/cache_as_ram.S index 7a96441..e0cebc1 100644 --- a/src/cpu/intel/car/p3/cache_as_ram.S +++ b/src/cpu/intel/car/p3/cache_as_ram.S @@ -20,6 +20,7 @@ xor %edx, %edx
clear_fixed_mtrr: + post_code(POST_CAR_FIXED_MTRR) add $-2, %ebx movzwl fixed_mtrr_list(%ebx), %ecx wrmsr diff --git a/src/cpu/intel/car/p4-netburst/cache_as_ram.S b/src/cpu/intel/car/p4-netburst/cache_as_ram.S index 6481404..6ddcf19 100644 --- a/src/cpu/intel/car/p4-netburst/cache_as_ram.S +++ b/src/cpu/intel/car/p4-netburst/cache_as_ram.S @@ -23,6 +23,7 @@ andl $LAPIC_BASE_MSR_BOOTSTRAP_PROCESSOR, %eax jz ap_init
+ post_code(POST_CAR_FIXED_MTRR) /* Clear/disable fixed MTRRs */ mov $fixed_mtrr_list_size, %ebx xor %eax, %eax diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c index 35566c4..f3090e7 100644 --- a/src/cpu/intel/haswell/romstage.c +++ b/src/cpu/intel/haswell/romstage.c @@ -18,6 +18,7 @@ int boot_mode; int wake_from_s3;
+ post_code(POST_ENTRY_ROMSTAGE); enable_lapic();
wake_from_s3 = early_pch_init(params->gpio_map, params->rcba_config); diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c index 617416a..4a65cab 100644 --- a/src/drivers/amd/agesa/romstage.c +++ b/src/drivers/amd/agesa/romstage.c @@ -84,6 +84,7 @@ postcar_frame_init(&pcf, HIGH_ROMSTAGE_STACK_SIZE); recover_postcar_frame(&pcf, cb->s3resume);
+ post_code(POST_ENTRY_POST_CAR); run_postcar_phase(&pcf); /* We do not return. */ } diff --git a/src/drivers/intel/fsp1_1/cache_as_ram.S b/src/drivers/intel/fsp1_1/cache_as_ram.S index 2a7678e..8ffcbcc 100644 --- a/src/drivers/intel/fsp1_1/cache_as_ram.S +++ b/src/drivers/intel/fsp1_1/cache_as_ram.S @@ -51,12 +51,14 @@ movl $(~(CACHE_ROM_SIZE - 1) | MTRR_PHYS_MASK_VALID), %eax wrmsr
+ post_code(POST_CAR_INIT_CACHE) /* Enable cache */ movl %cr0, %eax andl $(~(CR0_CacheDisable | CR0_NoWriteThrough)), %eax invd movl %eax, %cr0
+ post_code(POST_CAR_MTRR_ENABLE) /* Enable MTRR. */ movl $MTRR_DEF_TYPE_MSR, %ecx rdmsr diff --git a/src/northbridge/intel/sandybridge/raminit_mrc.c b/src/northbridge/intel/sandybridge/raminit_mrc.c index aa7d677..aaa7f81 100644 --- a/src/northbridge/intel/sandybridge/raminit_mrc.c +++ b/src/northbridge/intel/sandybridge/raminit_mrc.c @@ -363,6 +363,7 @@
pei_data.boot_mode = s3resume ? 2 : 0; timestamp_add_now(TS_BEFORE_INITRAM); + post_code(POST_ROM_SDRAM_INIT); sdram_initialize(&pei_data);
/* Sanity check mrc_var location by verifying a known field */ diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S index 5932fe6..69633ae 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S @@ -87,6 +87,7 @@ movd %mm1, %eax push %eax
+ post_code(POST_ENTRY_C_BOOTBLOCK) /* We can call into C functions now */ call bootblock_c_entry
diff --git a/src/soc/intel/xeon_sp/romstage.c b/src/soc/intel/xeon_sp/romstage.c index 02ed7eb..86e121e 100644 --- a/src/soc/intel/xeon_sp/romstage.c +++ b/src/soc/intel/xeon_sp/romstage.c @@ -41,6 +41,7 @@ /* Cache the memory-mapped boot media. */ postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT);
+ post_code(POST_ENTRY_POST_CAR); run_postcar_phase(&pcf); }
Hello build bot (Jenkins), Lee Leahy, Angel Pons, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43000
to look at the new patch set (#2).
Change subject: post_code: add missing postcode calls ......................................................................
post_code: add missing postcode calls
The change adds postcode calls wherever necessary on top of the updated set of postcode defines
Change-Id: Ia75cd863bf6ffac2c91ff78aefabc5327b1c138b Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/bootblock_crt0.S M src/arch/x86/postcar_loader.c M src/console/init.c M src/cpu/intel/car/p3/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/intel/haswell/romstage.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/cache_as_ram.S M src/northbridge/intel/pineview/romstage.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S M src/soc/intel/xeon_sp/romstage.c 13 files changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/43000/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... File src/arch/x86/bootblock_crt0.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... PS2, Line 26: mov %eax, %ebx After post_code, %eax no longer has BIST.
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... PS2, Line 48: movd %eax, %mm0 After post_code, %eax no longer has BIST.
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p3/cache_... File src/cpu/intel/car/p3/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p3/cache_... PS2, Line 23: post_code(POST_CAR_FIXED_MTRR) We need %eax == 0 inside this loop. Move after the loop.
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p4-netbur... File src/cpu/intel/car/p4-netburst/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p4-netbur... PS2, Line 26: post_code(POST_CAR_FIXED_MTRR) Move after the loop clear_fixed_mtrr.
Hello build bot (Jenkins), Damien Zammit, Lee Leahy, Angel Pons, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43000
to look at the new patch set (#3).
Change subject: post_code: add missing postcode calls ......................................................................
post_code: add missing postcode calls
The change adds postcode calls wherever necessary on top of the updated set of postcode defines
Change-Id: Ia75cd863bf6ffac2c91ff78aefabc5327b1c138b Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/bootblock_crt0.S M src/arch/x86/postcar_loader.c M src/console/init.c M src/cpu/intel/car/p3/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/intel/haswell/romstage.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/cache_as_ram.S M src/northbridge/intel/pineview/romstage.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S M src/soc/intel/xeon_sp/romstage.c M src/southbridge/intel/common/finalize.c 14 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/43000/3
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... File src/arch/x86/bootblock_crt0.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... PS2, Line 26: mov %eax, %ebx
After post_code, %eax no longer has BIST.
Ack
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... PS2, Line 48: movd %eax, %mm0
After post_code, %eax no longer has BIST.
Ack
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p3/cache_... File src/cpu/intel/car/p3/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p3/cache_... PS2, Line 23: post_code(POST_CAR_FIXED_MTRR)
We need %eax == 0 inside this loop. Move after the loop.
Ack
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p4-netbur... File src/cpu/intel/car/p4-netburst/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p4-netbur... PS2, Line 26: post_code(POST_CAR_FIXED_MTRR)
Move after the loop clear_fixed_mtrr.
Ack
Hello build bot (Jenkins), Damien Zammit, Lee Leahy, Angel Pons, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43000
to look at the new patch set (#4).
Change subject: post_code: add missing postcode calls ......................................................................
post_code: add missing postcode calls
The change adds postcode calls wherever necessary on top of the updated set of postcode defines
Change-Id: Ia75cd863bf6ffac2c91ff78aefabc5327b1c138b Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/bootblock_crt0.S M src/arch/x86/postcar_loader.c M src/console/init.c M src/cpu/intel/car/p3/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/intel/haswell/romstage.c M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/cache_as_ram.S M src/northbridge/intel/pineview/romstage.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S M src/soc/intel/xeon_sp/romstage.c M src/southbridge/intel/common/finalize.c 14 files changed, 21 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/43000/4
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... File src/arch/x86/bootblock_crt0.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... PS2, Line 26: mov %eax, %ebx
Ack
Placed it below the mov instruction to prevent changes to %eax
https://review.coreboot.org/c/coreboot/+/43000/2/src/arch/x86/bootblock_crt0... PS2, Line 48: movd %eax, %mm0
Ack
Added extra mov instructions to leave the value in %eax unchanged
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p3/cache_... File src/cpu/intel/car/p3/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p3/cache_... PS2, Line 23: post_code(POST_CAR_FIXED_MTRR)
Ack
Done
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p4-netbur... File src/cpu/intel/car/p4-netburst/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/43000/2/src/cpu/intel/car/p4-netbur... PS2, Line 26: post_code(POST_CAR_FIXED_MTRR)
Ack
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43000/6/src/drivers/amd/agesa/romst... File src/drivers/amd/agesa/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/drivers/amd/agesa/romst... PS6, Line 87: post_code Move into run_postcar_phase
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_mrc.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... PS6, Line 366: POST_ROM_SDRAM_INIT Where's the difference to POST_RAM_INIT ?
https://review.coreboot.org/c/coreboot/+/43000/6/src/soc/intel/broadwell/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/soc/intel/broadwell/rom... PS6, Line 79: post_code Move into romstage_handoff_init
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43000/6/src/drivers/amd/agesa/romst... File src/drivers/amd/agesa/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/drivers/amd/agesa/romst... PS6, Line 87: post_code
Move into run_postcar_phase
Ack
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_mrc.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... PS6, Line 366: POST_ROM_SDRAM_INIT
Where's the difference to POST_RAM_INIT ?
RAM_INIT is before the beginning of raminit, while this is for beginning of UEFI PEI raminit
https://review.coreboot.org/c/coreboot/+/43000/6/src/soc/intel/broadwell/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/soc/intel/broadwell/rom... PS6, Line 79: post_code
Move into romstage_handoff_init
Ack
Hello build bot (Jenkins), Damien Zammit, Lee Leahy, Angel Pons, Huang Jin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43000
to look at the new patch set (#7).
Change subject: post_code: add missing postcode calls ......................................................................
post_code: add missing postcode calls
The change adds postcode calls wherever necessary on top of the updated set of postcode defines
Change-Id: Ia75cd863bf6ffac2c91ff78aefabc5327b1c138b Signed-off-by: Sindhoor Tilak sindhoor@sin9yt.net --- M src/arch/x86/bootblock_crt0.S M src/arch/x86/postcar_loader.c M src/console/init.c M src/cpu/intel/car/non-evict/exit_car.S M src/cpu/intel/car/p3/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/intel/car/p4-netburst/exit_car.S M src/drivers/amd/agesa/exit_car.S M src/drivers/intel/fsp1_1/cache_as_ram.S M src/lib/romstage_handoff.c M src/northbridge/intel/haswell/romstage.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S 14 files changed, 22 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/43000/7
Sindhoor Tilak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43000/6/src/drivers/amd/agesa/romst... File src/drivers/amd/agesa/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/drivers/amd/agesa/romst... PS6, Line 87: post_code
Ack
Done
https://review.coreboot.org/c/coreboot/+/43000/6/src/soc/intel/broadwell/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/soc/intel/broadwell/rom... PS6, Line 79: post_code
Ack
Done
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 7: Code-Review+2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_mrc.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... PS6, Line 366: POST_ROM_SDRAM_INIT
RAM_INIT is before the beginning of raminit, while this is for beginning of UEFI PEI raminit
ok, makes sense. But why is there no post code when returning from sdram_initialize?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_mrc.c:
https://review.coreboot.org/c/coreboot/+/43000/6/src/northbridge/intel/sandy... PS6, Line 366: POST_ROM_SDRAM_INIT
ok, makes sense. […]
Um, why is there a need to differentiate between native raminit and MRC raminit? They are mutually exclusive and one knows at compile time which one will run.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43000/8/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/8/src/northbridge/intel/haswe... PS8, Line 21: enable_lapic(); that's not the right spot, romstage_common is called late on haswell. Also add post_code(POST_ENTRY_ROMSTAGE); to more than 2 platforms?
I'd suggest one commit that adds the same post code to multiple files.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43000/8/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/romstage.c:
https://review.coreboot.org/c/coreboot/+/43000/8/src/northbridge/intel/haswe... PS8, Line 21: enable_lapic();
that's not the right spot, romstage_common is called late on haswell. […]
That would make more sense. Plus, don't we have a common place where romstage starts?
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43000 )
Change subject: post_code: add missing postcode calls ......................................................................
Patch Set 8: Code-Review-1
(1 comment)
This looks ok, but it needs to be tested on a large number of devices before it can go in (see comment)
https://review.coreboot.org/c/coreboot/+/43000/8/src/arch/x86/bootblock_crt0... File src/arch/x86/bootblock_crt0.S:
https://review.coreboot.org/c/coreboot/+/43000/8/src/arch/x86/bootblock_crt0... PS8, Line 26: post_code(POST_RESET_VECTOR_CORRECT) IIRC some early post codes were explicitly removed because they crashed some Intel platforms (BDW-DE)
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43000?usp=email )
Change subject: post_code: add missing postcode calls ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Angel Pons (Code Review) -
Frans Hendriks (Code Review)
-
Kyösti Mälkki (Code Review)
-
Martin L Roth (Code Review)
-
Patrick Rudolph (Code Review)
-
Sindhoor Tilak (Code Review)
-
Stefan Reinauer (Code Review)