Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46895 )
Change subject: soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T ......................................................................
soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T
The CAR set up by FSP-T is at base 0xfe800000 and has a 0x200000 size. FSP-M seems to have a very large stack usage so it would overflow other car symbols located below the coreboot stack such as timestamps and the pre-ram console, which are now fixed.
TEST: boot with ocp/deltalake.
Change-Id: I886f9391ad79fcfa0724109393e3781a08d954b4 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/xeon_sp/cpx/Kconfig 1 file changed, 12 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/46895/1
diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig index 975afc9..28e7b83 100644 --- a/src/soc/intel/xeon_sp/cpx/Kconfig +++ b/src/soc/intel/xeon_sp/cpx/Kconfig @@ -27,22 +27,25 @@
config DCACHE_RAM_BASE hex - default 0xfe8b0000 + default 0xfe800000
config DCACHE_RAM_SIZE hex - default 0x170000 + default 0x1fff00 help The size of the cache-as-ram region required during bootblock - and/or romstage. + and/or romstage. FSP-T reserves the upper 0x100 for + FspReservedBuffer.
config DCACHE_BSP_STACK_SIZE hex - default 0xA0000 + default 0x140000 help The amount of anticipated stack usage in CAR by bootblock and other stages. It needs to include FSP-M stack requirement and - CB romstage stack requirement. + CB romstage stack requirement. The integration documentation + says this needs to be 256KiB, but practice show this needs to + be a lot more.
config CPU_MICROCODE_CBFS_LOC hex @@ -67,11 +70,13 @@ config FSP_TEMP_RAM_SIZE hex depends on FSP_USES_CB_STACK - default 0xA0000 + default 0x40000 help The amount of anticipated heap usage in CAR by FSP. Refer to Platform FSP integration guide document to know - the exact FSP requirement for Heap setup. + the exact FSP requirement for Heap setup. The FSP integration + documentation says this needs to be at least 128KiB, but practice + show this needs to be 256KiB or more.
config SOC_INTEL_COMMON_BLOCK_P2SB def_bool y
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46895 )
Change subject: soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/46895/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46895/1//COMMIT_MSG@14 PS1, Line 14: TEST: boot with ocp/deltalake. But it was booting before? Maybe mention that stack no longer gets smashed?
TEST=Boot ocp/deltalake, stack no longer gets smashed in romstage.
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46895 )
Change subject: soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T ......................................................................
Patch Set 1: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46895 )
Change subject: soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46895/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46895/1//COMMIT_MSG@14 PS1, Line 14: TEST: boot with ocp/deltalake.
But it was booting before? Maybe mention that stack no longer gets smashed?
TEST=Boot ocp/deltalake, stack no longer gets smashed in romstage.
Yes, so during FSP-M it smashes coreboot things but that's not a problem for FSP-M. The stack is really overflowing it's size. The romstage message about smashed stack does indeed go away.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46895 )
Change subject: soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46895/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46895/1//COMMIT_MSG@14 PS1, Line 14: TEST: boot with ocp/deltalake.
But it was booting before? Maybe mention that stack no longer gets smashed? […]
Done
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46895 )
Change subject: soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T ......................................................................
soc/intel/xeon_sp/cpx: Align coreboot CAR symbols to FSP-T
The CAR set up by FSP-T is at base 0xfe800000 and has a 0x200000 size. FSP-M seems to have a very large stack usage so it would overflow other car symbols located below the coreboot stack such as timestamps and the pre-ram console, which are now fixed.
TEST: boot with ocp/deltalake.
Change-Id: I886f9391ad79fcfa0724109393e3781a08d954b4 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/46895 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Christian Walter christian.walter@9elements.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/cpx/Kconfig 1 file changed, 12 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Christian Walter: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig index 975afc9..28e7b83 100644 --- a/src/soc/intel/xeon_sp/cpx/Kconfig +++ b/src/soc/intel/xeon_sp/cpx/Kconfig @@ -27,22 +27,25 @@
config DCACHE_RAM_BASE hex - default 0xfe8b0000 + default 0xfe800000
config DCACHE_RAM_SIZE hex - default 0x170000 + default 0x1fff00 help The size of the cache-as-ram region required during bootblock - and/or romstage. + and/or romstage. FSP-T reserves the upper 0x100 for + FspReservedBuffer.
config DCACHE_BSP_STACK_SIZE hex - default 0xA0000 + default 0x140000 help The amount of anticipated stack usage in CAR by bootblock and other stages. It needs to include FSP-M stack requirement and - CB romstage stack requirement. + CB romstage stack requirement. The integration documentation + says this needs to be 256KiB, but practice show this needs to + be a lot more.
config CPU_MICROCODE_CBFS_LOC hex @@ -67,11 +70,13 @@ config FSP_TEMP_RAM_SIZE hex depends on FSP_USES_CB_STACK - default 0xA0000 + default 0x40000 help The amount of anticipated heap usage in CAR by FSP. Refer to Platform FSP integration guide document to know - the exact FSP requirement for Heap setup. + the exact FSP requirement for Heap setup. The FSP integration + documentation says this needs to be at least 128KiB, but practice + show this needs to be 256KiB or more.
config SOC_INTEL_COMMON_BLOCK_P2SB def_bool y
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Christian Walter (Code Review)