Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33361
Change subject: soc/intel/block/cpu: Add a MP init choice selection ......................................................................
soc/intel/block/cpu: Add a MP init choice selection
Add a choice to select the MP init method for clarity.
Change-Id: I2a7490948f729f1ae9fd2537bf6cbe9ada51f968 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/common/block/cpu/Kconfig 1 file changed, 21 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/33361/1
diff --git a/src/soc/intel/common/block/cpu/Kconfig b/src/soc/intel/common/block/cpu/Kconfig index 8cc572d..6082ce3 100644 --- a/src/soc/intel/common/block/cpu/Kconfig +++ b/src/soc/intel/common/block/cpu/Kconfig @@ -51,18 +51,34 @@ ENHANCED NEM guarantees that modified data is always kept in cache while clean data is replaced.
-config USE_INTEL_FSP_MP_INIT +choice + prompt "Multiple Processor (MP) Initialization Options" + default MP_USE_COREBOOT_NATIVE_MP_INIT if !PLATFORM_USES_FSP2_1 + default MP_USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI if PLATFORM_USES_FSP2_1 + +config MP_USE_COREBOOT_NATIVE_MP_INIT + bool "Perform MP Initialization by coreboot" + +config MP_USE_INTEL_FSP_MP_INIT bool "Perform MP Initialization by FSP" - default n + select USE_INTEL_FSP_MP_INIT + +config MP_USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI + bool "Perform MP Initialization by FSP using coreboot MP PPI service" + depends on FSP_USES_MP_SERVICES_PPI + select USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI + +endchoice + +config USE_INTEL_FSP_MP_INIT + bool depends on !USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI help This option allows FSP to perform multiprocessor initialization.
config USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI - bool "Perform MP Initialization by FSP using coreboot MP PPI service" + bool depends on FSP_USES_MP_SERVICES_PPI - default y if PLATFORM_USES_FSP2_1 - default n help This option allows FSP to make use of MP services PPI published by coreboot to perform multiprocessor initialization.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33361 )
Change subject: soc/intel/block/cpu: Add a MP init choice selection ......................................................................
Patch Set 2: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33361 )
Change subject: soc/intel/block/cpu: Add a MP init choice selection ......................................................................
Patch Set 2:
(4 comments)
Subrata, I just realized that a choice doesn't work well if one wants to select a specific option from the mainboard Kconfig. Currently, intel/saddlebrook does that. I'm not sure if that's necessary.
Do you think we need to be able to select things or should it be always up to the person that configures coreboot? If the latter, the choice makes sense, otherwise it would get too complex, I fear.
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... File src/soc/intel/common/block/cpu/Kconfig:
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 56: default MP_USE_COREBOOT_NATIVE_MP_INIT if !PLATFORM_USES_FSP2_1 NB. First entry is the implicit default.
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 75: depends on !USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI NB. This is redundant now, wouldn't mind to keep it, though.
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 77: This option allows FSP to perform multiprocessor initialization. I guess the help texts should move into the choice.
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 81: depends on FSP_USES_MP_SERVICES_PPI NB. This is redundant now, wouldn't mind to keep it, though.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33361 )
Change subject: soc/intel/block/cpu: Add a MP init choice selection ......................................................................
Patch Set 2:
Patch Set 2:
(4 comments)
Subrata, I just realized that a choice doesn't work well if one wants to select a specific option from the mainboard Kconfig. Currently, intel/saddlebrook does that. I'm not sure if that's necessary.
Do you think we need to be able to select things or should it be always up to the person that configures coreboot? If the latter, the choice makes sense, otherwise it would get too complex, I fear.
[Subrata] I think this is better option "we need to be able to select things" choice might be easy to understand but select makes more sense from SOC
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33361 )
Change subject: soc/intel/block/cpu: Add a MP init choice selection ......................................................................
Patch Set 2:
(3 comments)
As we already have the original, selectable Kconfig options, we could make the choice items depend on them (see inline comments). Might look a little con- fusing, but works: Just checked saddlebrook and icl rvp menus. Saddlebrook is limited to FSP MP init as selected and icl rvp shows all options with PPI default.
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... File src/soc/intel/common/block/cpu/Kconfig:
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 60: bool "Perform MP Initialization by coreboot" depends on !USE_INTEL_FSP_MP_INIT && !USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 64: select USE_INTEL_FSP_MP_INIT depends on !USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI
https://review.coreboot.org/#/c/33361/2/src/soc/intel/common/block/cpu/Kconf... PS2, Line 67: bool "Perform MP Initialization by FSP using coreboot MP PPI service" depends on !USE_INTEL_FSP_MP_INIT
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33361 )
Change subject: soc/intel/block/cpu: Add a MP init choice selection ......................................................................
Abandoned
superseded by CB:63759
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Nico Huber (Code Review)
-
Subrata Banik (Code Review)