Jonathan Zhang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2
CPX-SP FSP is of FSP 2.2, so select PLATFORM_USES_FSP2_2. The corresponding workaround of hardcoding StackBase and StackSize FSP-M UPD parameters is removed.
Add CPX-SP soc implmentation of soc_fsp_multi_phase_init_is_enable() to indicate that FSP-S multi phase init is not enabled, since it is not supported by CPX-SP FSP.
Increase FSP_TEMP_RAM_SIZE, DCACHE_BSP_STACK_SIZE, and adjust DCACHE_RAM_SIZE accordingly.
Update DCACHE_RAM_BASE.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I25e39083df1ebfe78871561b0a0e230b66524ea9 --- M src/soc/intel/xeon_sp/cpx/Kconfig M src/soc/intel/xeon_sp/cpx/Makefile.inc A src/soc/intel/xeon_sp/cpx/ramstage.c M src/soc/intel/xeon_sp/cpx/romstage.c 4 files changed, 22 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/44049/1
diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig index 8139930..031d10b 100644 --- a/src/soc/intel/xeon_sp/cpx/Kconfig +++ b/src/soc/intel/xeon_sp/cpx/Kconfig @@ -6,6 +6,8 @@ bool default y
+select PLATFORM_USES_FSP2_2 + config FSP_HEADER_PATH string "Location of FSP headers" depends on MAINBOARD_USES_FSP2_0 @@ -25,18 +27,24 @@ help This option allows you to select MMIO Base Address of sideband bus.
-# currently FSP hardcodes [0fe800000;fe930000] for its heap config DCACHE_RAM_BASE hex - default 0xfe9a0000 + default 0xfe8b0000
config DCACHE_RAM_SIZE hex - default 0x60000 + default 0x170000 + help + The size of the cache-as-ram region required during bootblock + and/or romstage.
config DCACHE_BSP_STACK_SIZE hex - default 0x10000 + default 0xA0000 + 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.
config CPU_MICROCODE_CBFS_LOC hex @@ -57,7 +65,7 @@ config FSP_TEMP_RAM_SIZE hex depends on FSP_USES_CB_STACK - default 0x70000 + default 0xA0000 help The amount of anticipated heap usage in CAR by FSP. Refer to Platform FSP integration guide document to know diff --git a/src/soc/intel/xeon_sp/cpx/Makefile.inc b/src/soc/intel/xeon_sp/cpx/Makefile.inc index 969fe25..89f18d1 100644 --- a/src/soc/intel/xeon_sp/cpx/Makefile.inc +++ b/src/soc/intel/xeon_sp/cpx/Makefile.inc @@ -12,7 +12,7 @@ romstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c romstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c
-ramstage-y += chip.c acpi.c cpu.c soc_util.c +ramstage-y += chip.c acpi.c cpu.c soc_util.c ramstage.c ramstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c
diff --git a/src/soc/intel/xeon_sp/cpx/ramstage.c b/src/soc/intel/xeon_sp/cpx/ramstage.c new file mode 100644 index 0000000..deb9030 --- /dev/null +++ b/src/soc/intel/xeon_sp/cpx/ramstage.c @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <fsp/api.h> + +int soc_fsp_multi_phase_init_is_enable(void) +{ + return 0; +} diff --git a/src/soc/intel/xeon_sp/cpx/romstage.c b/src/soc/intel/xeon_sp/cpx/romstage.c index 9952d62..7093ec9 100644 --- a/src/soc/intel/xeon_sp/cpx/romstage.c +++ b/src/soc/intel/xeon_sp/cpx/romstage.c @@ -13,16 +13,6 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) { FSPM_CONFIG *m_cfg = &mupd->FspmConfig; - FSPM_ARCH_UPD *arch_upd = &mupd->FspmArchUpd; - - /* - * Currently FSP for CPX does not implement user-provided StackBase/Size - * properly. When KTI link needs to be trained, inter-socket communication - * library needs quite a bit of memory for its heap usage. However, location - * is hardcoded so this workaround is needed. - */ - arch_upd->StackBase = (void *) 0xfe930000; - arch_upd->StackSize = 0x70000;
/* ErrorLevel - 0 (disable) to 8 (verbose) */ m_cfg->DebugPrintLevel = 8;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 1: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG@9 PS1, Line 9: of remove 'of'
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG@13 PS1, Line 13: implmentation impl*e*mentation
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG@17 PS1, Line 17: DCACHE_RAM_SIZE Put on the next line
https://review.coreboot.org/c/coreboot/+/44049/1/src/soc/intel/xeon_sp/cpx/K... File src/soc/intel/xeon_sp/cpx/Kconfig:
https://review.coreboot.org/c/coreboot/+/44049/1/src/soc/intel/xeon_sp/cpx/K... PS1, Line 9: select PLATFORM_USES_FSP2_2 This should be selected from `config SOC_INTEL_COOPERLAKE_SP`
Hello Philipp Deppenwiese, build bot (Jenkins), Anjaneya "Reddy" Chagam, Patrick Georgi, Martin Roth, nateman1352, Angel Pons, Subrata Banik, Patrick Rudolph, Nate DeSimone,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44049
to look at the new patch set (#2).
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2
CPX-SP FSP is FSP 2.2, so select PLATFORM_USES_FSP2_2.
The workaround of hardcoding StackBase and StackSize FSP-M UPD parameters is removed.
Add CPX-SP soc implementation of soc_fsp_multi_phase_init_is_enable() to indicate that FSP-S multi phase init is not enabled, since it is not supported by CPX-SP FSP.
Increase FSP_TEMP_RAM_SIZE, DCACHE_BSP_STACK_SIZE, and adjust DCACHE_RAM_SIZE accordingly.
Correct DCACHE_RAM_BASE.
Remove duplicate entries in soc/intel/xeon_sp/cpx/Kconfig which have same values as in soc/intel/xeon_sp/Kconfig.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I25e39083df1ebfe78871561b0a0e230b66524ea9 --- M src/soc/intel/xeon_sp/Kconfig M src/soc/intel/xeon_sp/cpx/Kconfig M src/soc/intel/xeon_sp/cpx/Makefile.inc A src/soc/intel/xeon_sp/cpx/ramstage.c M src/soc/intel/xeon_sp/cpx/romstage.c 5 files changed, 21 insertions(+), 34 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/44049/2
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG@9 PS1, Line 9: of
remove 'of'
Done
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG@13 PS1, Line 13: implmentation
impl*e*mentation
Done
https://review.coreboot.org/c/coreboot/+/44049/1//COMMIT_MSG@17 PS1, Line 17: DCACHE_RAM_SIZE
Put on the next line
Done
https://review.coreboot.org/c/coreboot/+/44049/1/src/soc/intel/xeon_sp/cpx/K... File src/soc/intel/xeon_sp/cpx/Kconfig:
https://review.coreboot.org/c/coreboot/+/44049/1/src/soc/intel/xeon_sp/cpx/K... PS1, Line 9: select PLATFORM_USES_FSP2_2
This should be selected from `config SOC_INTEL_COOPERLAKE_SP`
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/44049/2/src/soc/intel/xeon_sp/Kconf... File src/soc/intel/xeon_sp/Kconfig:
https://review.coreboot.org/c/coreboot/+/44049/2/src/soc/intel/xeon_sp/Kconf... PS2, Line 35: PLATFORM_USES_FSP2_0 This needs to be moved to SOC_INTEL_SKYLAKE_SP
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 2:
(1 comment)
Thanks, Angel!
https://review.coreboot.org/c/coreboot/+/44049/2/src/soc/intel/xeon_sp/Kconf... File src/soc/intel/xeon_sp/Kconfig:
https://review.coreboot.org/c/coreboot/+/44049/2/src/soc/intel/xeon_sp/Kconf... PS2, Line 35: PLATFORM_USES_FSP2_0
This needs to be moved to SOC_INTEL_SKYLAKE_SP
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44049/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44049/2//COMMIT_MSG@23 PS2, Line 23: Remove duplicate entries in soc/intel/xeon_sp/cpx/Kconfig which : have same values as in soc/intel/xeon_sp/Kconfig. Personally, I'd do this on a separate change. Clean-up shouldn't be mixed in with behavior changes
Hello Philipp Deppenwiese, build bot (Jenkins), Anjaneya "Reddy" Chagam, Patrick Georgi, Martin Roth, nateman1352, Angel Pons, Subrata Banik, Patrick Rudolph, Nate DeSimone,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44049
to look at the new patch set (#3).
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2
CPX-SP FSP is FSP 2.2, so select PLATFORM_USES_FSP2_2. SKX-SP continues to select PLATFORM_USES_FSP2_0, as SKX-SP FSP is FSP 2.0.
Correct DCACHE_RAM_BASE. Increase FSP_TEMP_RAM_SIZE, DCACHE_BSP_STACK_SIZE, and adjust DCACHE_RAM_SIZE accordingly. Thus the workaround of hardcoding StackBase and StackSize FSP-M UPD parameters is removed.
Add CPX-SP soc implementation of soc_fsp_multi_phase_init_is_enable() to indicate that FSP-S multi phase init is not enabled, since it is not supported by CPX-SP FSP.
Remove duplicate entries in soc/intel/xeon_sp/cpx/Kconfig which have same values as in soc/intel/xeon_sp/Kconfig.
TESTED=booted YV3 config A to target OS.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I25e39083df1ebfe78871561b0a0e230b66524ea9 --- M src/soc/intel/xeon_sp/Kconfig M src/soc/intel/xeon_sp/cpx/Kconfig M src/soc/intel/xeon_sp/cpx/Makefile.inc A src/soc/intel/xeon_sp/cpx/ramstage.c M src/soc/intel/xeon_sp/cpx/romstage.c 5 files changed, 22 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/44049/3
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44049/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44049/2//COMMIT_MSG@23 PS2, Line 23: Remove duplicate entries in soc/intel/xeon_sp/cpx/Kconfig which : have same values as in soc/intel/xeon_sp/Kconfig.
Personally, I'd do this on a separate change. […]
Done
Hello Philipp Deppenwiese, build bot (Jenkins), Anjaneya "Reddy" Chagam, Patrick Georgi, Martin Roth, nateman1352, Angel Pons, Subrata Banik, Patrick Rudolph, Nate DeSimone,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44049
to look at the new patch set (#4).
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2
CPX-SP FSP is FSP 2.2, so select PLATFORM_USES_FSP2_2. SKX-SP continues to select PLATFORM_USES_FSP2_0, as SKX-SP FSP is FSP 2.0.
Correct DCACHE_RAM_BASE. Increase FSP_TEMP_RAM_SIZE, DCACHE_BSP_STACK_SIZE, and adjust DCACHE_RAM_SIZE accordingly. Thus the workaround of hardcoding StackBase and StackSize FSP-M UPD parameters is removed.
Add CPX-SP soc implementation of soc_fsp_multi_phase_init_is_enable() to indicate that FSP-S multi phase init is not enabled, since it is not supported by CPX-SP FSP.
TESTED=booted YV3 config A to target OS.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I25e39083df1ebfe78871561b0a0e230b66524ea9 --- M src/soc/intel/xeon_sp/Kconfig M src/soc/intel/xeon_sp/cpx/Kconfig M src/soc/intel/xeon_sp/cpx/Makefile.inc A src/soc/intel/xeon_sp/cpx/ramstage.c M src/soc/intel/xeon_sp/cpx/romstage.c 5 files changed, 22 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/44049/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 4: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44049 )
Change subject: soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2 ......................................................................
soc/intel/xeon_sp/cpx: enable PLATFORM_USES_FSP2_2
CPX-SP FSP is FSP 2.2, so select PLATFORM_USES_FSP2_2. SKX-SP continues to select PLATFORM_USES_FSP2_0, as SKX-SP FSP is FSP 2.0.
Correct DCACHE_RAM_BASE. Increase FSP_TEMP_RAM_SIZE, DCACHE_BSP_STACK_SIZE, and adjust DCACHE_RAM_SIZE accordingly. Thus the workaround of hardcoding StackBase and StackSize FSP-M UPD parameters is removed.
Add CPX-SP soc implementation of soc_fsp_multi_phase_init_is_enable() to indicate that FSP-S multi phase init is not enabled, since it is not supported by CPX-SP FSP.
TESTED=booted YV3 config A to target OS.
Signed-off-by: Jonathan Zhang jonzhang@fb.com Change-Id: I25e39083df1ebfe78871561b0a0e230b66524ea9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44049 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Subrata Banik subrata.banik@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/Kconfig M src/soc/intel/xeon_sp/cpx/Kconfig M src/soc/intel/xeon_sp/cpx/Makefile.inc A src/soc/intel/xeon_sp/cpx/ramstage.c M src/soc/intel/xeon_sp/cpx/romstage.c 5 files changed, 22 insertions(+), 21 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/Kconfig b/src/soc/intel/xeon_sp/Kconfig index b410dec..cf9ba94 100644 --- a/src/soc/intel/xeon_sp/Kconfig +++ b/src/soc/intel/xeon_sp/Kconfig @@ -9,12 +9,14 @@ config SOC_INTEL_SKYLAKE_SP bool select XEON_SP_COMMON_BASE + select PLATFORM_USES_FSP2_0 help Intel Skylake-SP support
config SOC_INTEL_COOPERLAKE_SP bool select XEON_SP_COMMON_BASE + select PLATFORM_USES_FSP2_2 help Intel Cooperlake-SP support
@@ -31,7 +33,6 @@ select POSTCAR_CONSOLE select SOC_INTEL_COMMON select SOC_INTEL_COMMON_RESET - select PLATFORM_USES_FSP2_0 select FSP_PLATFORM_MEMORY_SETTINGS_VERSIONS select FSP_T_XIP select FSP_M_XIP diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig index bd1fa97..93098e8 100644 --- a/src/soc/intel/xeon_sp/cpx/Kconfig +++ b/src/soc/intel/xeon_sp/cpx/Kconfig @@ -2,10 +2,6 @@
if SOC_INTEL_COOPERLAKE_SP
-config MAINBOARD_USES_FSP2_0 - bool - default y - config FSP_HEADER_PATH string "Location of FSP headers" depends on MAINBOARD_USES_FSP2_0 @@ -25,18 +21,24 @@ help This option allows you to select MMIO Base Address of sideband bus.
-# currently FSP hardcodes [0fe800000;fe930000] for its heap config DCACHE_RAM_BASE hex - default 0xfe9a0000 + default 0xfe8b0000
config DCACHE_RAM_SIZE hex - default 0x60000 + default 0x170000 + help + The size of the cache-as-ram region required during bootblock + and/or romstage.
config DCACHE_BSP_STACK_SIZE hex - default 0x10000 + default 0xA0000 + 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.
config CPU_MICROCODE_CBFS_LOC hex @@ -57,7 +59,7 @@ config FSP_TEMP_RAM_SIZE hex depends on FSP_USES_CB_STACK - default 0x70000 + default 0xA0000 help The amount of anticipated heap usage in CAR by FSP. Refer to Platform FSP integration guide document to know diff --git a/src/soc/intel/xeon_sp/cpx/Makefile.inc b/src/soc/intel/xeon_sp/cpx/Makefile.inc index 969fe25..89f18d1 100644 --- a/src/soc/intel/xeon_sp/cpx/Makefile.inc +++ b/src/soc/intel/xeon_sp/cpx/Makefile.inc @@ -12,7 +12,7 @@ romstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c romstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c
-ramstage-y += chip.c acpi.c cpu.c soc_util.c +ramstage-y += chip.c acpi.c cpu.c soc_util.c ramstage.c ramstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c
diff --git a/src/soc/intel/xeon_sp/cpx/ramstage.c b/src/soc/intel/xeon_sp/cpx/ramstage.c new file mode 100644 index 0000000..deb9030 --- /dev/null +++ b/src/soc/intel/xeon_sp/cpx/ramstage.c @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <fsp/api.h> + +int soc_fsp_multi_phase_init_is_enable(void) +{ + return 0; +} diff --git a/src/soc/intel/xeon_sp/cpx/romstage.c b/src/soc/intel/xeon_sp/cpx/romstage.c index 9952d62..7093ec9 100644 --- a/src/soc/intel/xeon_sp/cpx/romstage.c +++ b/src/soc/intel/xeon_sp/cpx/romstage.c @@ -13,16 +13,6 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) { FSPM_CONFIG *m_cfg = &mupd->FspmConfig; - FSPM_ARCH_UPD *arch_upd = &mupd->FspmArchUpd; - - /* - * Currently FSP for CPX does not implement user-provided StackBase/Size - * properly. When KTI link needs to be trained, inter-socket communication - * library needs quite a bit of memory for its heap usage. However, location - * is hardcoded so this workaround is needed. - */ - arch_upd->StackBase = (void *) 0xfe930000; - arch_upd->StackSize = 0x70000;
/* ErrorLevel - 0 (disable) to 8 (verbose) */ m_cfg->DebugPrintLevel = 8;
-
Angel Pons (Code Review) -
Jonathan Zhang (Code Review)
-
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)