Change in coreboot[master]: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform

[Mario Scheithauer (Code Review)]

Mario Scheithauer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

mb/../Kconfig: Correct the maximum number of cpu cores for APL platform

With reference to abandoned patch 43818: [WIP] soc/intel/apollolake:
Drop select SMP | https://review.coreboot.org/c/coreboot/+/43818, the
maximum number of cores for APL platform is herewith fixed.

Change-Id: I555ab72520ab9c174b1656c463a5112eb05a236d
Signed-off-by: Mario Scheithauer 
---
M src/mainboard/google/reef/Kconfig
M src/mainboard/intel/apollolake_rvp/Kconfig
M src/mainboard/intel/leafhill/Kconfig
M src/mainboard/intel/minnow3/Kconfig
M src/mainboard/siemens/mc_apl1/Kconfig
5 files changed, 15 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/45063/1

diff --git a/src/mainboard/google/reef/Kconfig b/src/mainboard/google/reef/Kconfig
index df1d8c4..de4df62 100644
--- a/src/mainboard/google/reef/Kconfig
+++ b/src/mainboard/google/reef/Kconfig
@@ -83,7 +83,7 @@

config MAX_CPUS
    int
-	default 8
+	default 4

config UART_FOR_CONSOLE
    int
diff --git a/src/mainboard/intel/apollolake_rvp/Kconfig b/src/mainboard/intel/apollolake_rvp/Kconfig
index dfd86eb..93f5e94 100644
--- a/src/mainboard/intel/apollolake_rvp/Kconfig
+++ b/src/mainboard/intel/apollolake_rvp/Kconfig
@@ -17,4 +17,9 @@

config UART_FOR_CONSOLE
    default 2
+
+config MAX_CPUS
+	int
+	default 4
+
 endif
diff --git a/src/mainboard/intel/leafhill/Kconfig b/src/mainboard/intel/leafhill/Kconfig
index 652a659..ce8f0da 100644
--- a/src/mainboard/intel/leafhill/Kconfig
+++ b/src/mainboard/intel/leafhill/Kconfig
@@ -69,4 +69,8 @@
    string
    default "fspm.bin"

+config MAX_CPUS
+	int
+	default 4
+
 endif # BOARD_INTEL_LEAFHILL
diff --git a/src/mainboard/intel/minnow3/Kconfig b/src/mainboard/intel/minnow3/Kconfig
index 0c86df6..6c202e0 100644
--- a/src/mainboard/intel/minnow3/Kconfig
+++ b/src/mainboard/intel/minnow3/Kconfig
@@ -69,4 +69,8 @@
    string
    default "fspm.bin"

+config MAX_CPUS
+	int
+	default 4
+
 endif # BOARD_INTEL_MINNOW3
diff --git a/src/mainboard/siemens/mc_apl1/Kconfig b/src/mainboard/siemens/mc_apl1/Kconfig
index 0fe4a25..705351f 100644
--- a/src/mainboard/siemens/mc_apl1/Kconfig
+++ b/src/mainboard/siemens/mc_apl1/Kconfig
@@ -42,6 +42,6 @@

config MAX_CPUS
    int
-	default 8
+	default 4

endif # BOARD_SIEMENS_BASEBOARD_MC_APL1

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG@11 
PS1, Line 11: maximum number of cores for APL platform is herewith fixed.
… from 8 to 4.

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG@12 
PS1, Line 12: 
Could you please also state, why these boards cannot support eight CPUs?

[Werner Zeh (Code Review)]

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG@12 
PS1, Line 12:
> Could you please also state, why these boards cannot support eight CPUs?

It is not that any of the boards can not support 8 CPUs, it is more that there is no Apollo Lake CPU with more than 4 cores.

[Mario Scheithauer (Code Review)]

Hello build bot (Jenkins), Aaron Durbin, Subrata Banik, Angel Pons, Werner Zeh, Kyösti Mälkki,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/45063

to look at the new patch set (#2).

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

mb/../Kconfig: Correct the maximum number of cpu cores for APL platform

With reference to abandoned patch 43818: [WIP] soc/intel/apollolake:
Drop select SMP | https://review.coreboot.org/c/coreboot/+/43818, the
maximum number of cores for APL platform is herewith fixed from 8 to 4.
A list of all Apollo Lake CPUs can be found under the following link
https://ark.intel.com/content/www/us/en/ark/products/codename/80644/apollo-l....

Change-Id: I555ab72520ab9c174b1656c463a5112eb05a236d
Signed-off-by: Mario Scheithauer 
---
M src/mainboard/google/reef/Kconfig
M src/mainboard/intel/apollolake_rvp/Kconfig
M src/mainboard/intel/leafhill/Kconfig
M src/mainboard/intel/minnow3/Kconfig
M src/mainboard/siemens/mc_apl1/Kconfig
5 files changed, 15 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/45063/2

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG@11 
PS1, Line 11: maximum number of cores for APL platform is herewith fixed.
> … from 8 to 4.

Done

https://review.coreboot.org/c/coreboot/+/45063/1//COMMIT_MSG@12 
PS1, Line 12:
> It is not that any of the boards can not support 8 CPUs, it is more that there is no Apollo Lake CPU […]

I added a link in the commit message.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45063/2//COMMIT_MSG@11 
PS2, Line 11: maximum number of cores for APL platform is herewith fixed from 8 to 4.
Several boards change from MAX_CPUS=1 to 4? Does that actually fix something on MP init (like impartial MTRR or SMM setup) or are those non-socketed single-core SKUs on reference boards that do not require the change?

The odd case of MAX_CPUS=1 and SMP=Y is a separate issue, should not be hidden by blindly incrementing MAX_CPUS.

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45063/2//COMMIT_MSG@11 
PS2, Line 11: maximum number of cores for APL platform is herewith fixed from 8 to 4.
> Several boards change from MAX_CPUS=1 to 4? Does that actually fix something on MP init (like impart […]

Then I reduce the number from 8 to 4 only for reef and mc_apl1. Is that okay?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
File src/mainboard/google/reef/Kconfig:

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
PS2, Line 84: config MAX_CPUS
            : 	int
            : 	default 4
Please drop redundant values (if SoC defaults to 4, this is not needed)

[Mario Scheithauer (Code Review)]

Hello build bot (Jenkins), Aaron Durbin, Angel Pons, Subrata Banik, Werner Zeh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/45063

to look at the new patch set (#3).

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

mb/../Kconfig: Correct the maximum number of cpu cores for APL platform

With reference to abandoned patch 43818: [WIP] soc/intel/apollolake:
Drop select SMP | https://review.coreboot.org/c/coreboot/+/43818, the
maximum number of cores for APL platform is herewith fixed from 8 to 4.
A list of all Apollo Lake CPUs can be found under the following link
https://ark.intel.com/content/www/us/en/ark/products/codename/80644/apollo-l....

Change-Id: I555ab72520ab9c174b1656c463a5112eb05a236d
Signed-off-by: Mario Scheithauer 
---
M src/mainboard/google/reef/Kconfig
M src/mainboard/siemens/mc_apl1/Kconfig
2 files changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/45063/3

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
File src/mainboard/google/reef/Kconfig:

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
PS2, Line 84: config MAX_CPUS
            : 	int
            : 	default 4
> Please drop redundant values (if SoC defaults to 4, this is not needed)

I cannot find the default setting in the APL SOC directory.
Can you please tell me where you think the default setting for APL is made?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
File src/mainboard/google/reef/Kconfig:

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
PS2, Line 84: config MAX_CPUS
            : 	int
            : 	default 4
> I cannot find the default setting in the APL SOC directory. […]

Uh, I mistook `apollolake_rvp` for `soc/intel/apollolake`. Sorry.

Still, it would be good to set a default for MAX_CPUS in there, so that only boards with MAX_CPUS != 4 need to explicitly override the value.

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
File src/mainboard/google/reef/Kconfig:

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
PS2, Line 84: config MAX_CPUS
            : 	int
            : 	default 4
> Uh, I mistook `apollolake_rvp` for `soc/intel/apollolake`. Sorry. […]

But if I do that, then I have to set the number for apollolake_rvp, minnow3 and leafhill to 1 over Kconfig. Maybe this should be done in a separate patch.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
File src/mainboard/google/reef/Kconfig:

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
PS2, Line 84: config MAX_CPUS
            : 	int
            : 	default 4
> But if I do that, then I have to set the number for apollolake_rvp, minnow3 and leafhill to 1 over K […]

I've made some commits to unify MAX_CPUS for APL and GLK: https://review.coreboot.org/q/topic:%22apl-glk-max-cpus"

Please let me know if this approach would work for you.

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
File src/mainboard/google/reef/Kconfig:

https://review.coreboot.org/c/coreboot/+/45063/2/src/mainboard/google/reef/K... 
PS2, Line 84: config MAX_CPUS
            : 	int
            : 	default 4
> I've made some commits to unify MAX_CPUS for APL and GLK: https://review.coreboot. […]

Sure, I will check it out.

[Mario Scheithauer (Code Review)]

Mario Scheithauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45063 )

Change subject: mb/../Kconfig: Correct the maximum number of cpu cores for APL platform
......................................................................

Abandoned

There is another patch which fixes the bug (45143: apollolake: Limit MAX_CPUS to 4 | https://review.coreboot.org/c/coreboot/+/45143).