Change in coreboot[master]: soc/intel/skylake: Add option to disabble HyperTreading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to disabble HyperTreading
......................................................................

soc/intel/skylake: Add option to disabble HyperTreading

Needs test on KBL.

Change-Id: I3ebab68ff868c78105bb4b35abffb92f3ccf1705
Signed-off-by: Patrick Rudolph 
---
M src/soc/intel/skylake/Kconfig
M src/soc/intel/skylake/romstage/romstage_fsp20.c
2 files changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/35208/1

diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig
index 9cb8d45..8bf66f3 100644
--- a/src/soc/intel/skylake/Kconfig
+++ b/src/soc/intel/skylake/Kconfig
@@ -78,6 +78,11 @@
    select TSC_SYNC_MFENCE
    select UDELAY_TSC

+config FSP_HYPERTHREADING
+	bool "Enable HyperThreading"
+	depends on SOC_INTEL_KABYLAKE
+	default y
+
 config CPU_INTEL_NUM_FIT_ENTRIES
    int
    default 10
diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c
index ecd1428..f3f8063 100644
--- a/src/soc/intel/skylake/romstage/romstage_fsp20.c
+++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c
@@ -251,6 +251,7 @@
    	m_cfg->PchHpetDeviceNumber = 15;
    	m_cfg->PchHpetFunctionNumber = 0;
    }
+	m_cfg->HyperThreading = CONFIG(FSP_HYPERTHREADING);
 }

static void soc_primary_gfx_config_params(FSP_M_CONFIG *m_cfg,

[Angel Pons (Code Review)]

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

Change subject: soc/intel/skylake: Add option to disabble HyperTreading
......................................................................

Patch Set 1: Code-Review+2

If it's the FSP, I guess it should work...

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to disabble HyperTreading
......................................................................

Patch Set 1:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/35208/1//COMMIT_MSG@7 
PS1, Line 7: disabble
disable

https://review.coreboot.org/c/coreboot/+/35208/1//COMMIT_MSG@7 
PS1, Line 7: HyperTreading
HyperThreading

[Paul Menzel (Code Review)]

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

Change subject: soc/intel/skylake: Add option to disabble HyperTreading
......................................................................

Patch Set 1:

(2 comments)

Could you also hook up a CMOS option to make it run-time configurable?

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

https://review.coreboot.org/c/coreboot/+/35208/1//COMMIT_MSG@8 
PS1, Line 8: 
Let coreboot configure the FSP to enable Hyper-threading.

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 82: 	bool "Enable HyperThreading"
Hyper-threading

https://en.wikipedia.org/wiki/Hyper-threading

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to disabble HyperTreading
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 83: SOC_INTEL_KABYLAKE
why dependent on KBL (vs FSP 2.0)?

[Angel Pons (Code Review)]

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

Change subject: soc/intel/skylake: Add option to disabble HyperTreading
......................................................................

Patch Set 1: -Code-Review

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 83: SOC_INTEL_KABYLAKE
> why dependent on KBL (vs FSP 2. […]

Good question

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),

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

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

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

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

soc/intel/skylake: Add option to toggle Hyper-Threading

Tested on Supermicro X11SSH-TF.

Change-Id: I3ebab68ff868c78105bb4b35abffb92f3ccf1705
Signed-off-by: Patrick Rudolph 
---
M src/soc/intel/skylake/Kconfig
M src/soc/intel/skylake/romstage/romstage_fsp20.c
2 files changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/35208/2

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 2:

(3 comments)
> Patch Set 1:
>
> (2 comments)
>
> Could you also hook up a CMOS option to make it run-time configurable?

No.

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

https://review.coreboot.org/c/coreboot/+/35208/1//COMMIT_MSG@7 
PS1, Line 7: disabble
> disable

Done

https://review.coreboot.org/c/coreboot/+/35208/1//COMMIT_MSG@7 
PS1, Line 7: HyperTreading
> HyperThreading

Done

https://review.coreboot.org/c/coreboot/+/35208/1//COMMIT_MSG@8 
PS1, Line 8:
> Let coreboot configure the FSP to enable Hyper-threading.

Done

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 82: 	bool "Enable HyperThreading"
> Hyper-threading […]

Done

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 83: SOC_INTEL_KABYLAKE
> Good question

SKL don't have this option.

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Angel Pons (Code Review)]

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

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 2: Code-Review-1

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 83: SOC_INTEL_KABYLAKE
> SKL don't have this option.

Why not? FSP 2.0 runs on both Skylake and Kabylake, if it provides that option it should work for both...

If you mean the Skylake-only FSP 1.1 doesn't have that option, then this should depend on FSP 2.0 instead.

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 83: SOC_INTEL_KABYLAKE
> Why not? FSP 2. […]

good point.

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),

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

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

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

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

soc/intel/skylake: Add option to toggle Hyper-Threading

Tested on Supermicro X11SSH-TF.

Change-Id: I3ebab68ff868c78105bb4b35abffb92f3ccf1705
Signed-off-by: Patrick Rudolph 
---
M src/soc/intel/skylake/Kconfig
M src/soc/intel/skylake/romstage/romstage_fsp20.c
2 files changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/35208/3

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/1/src/soc/intel/skylake/Kconf... 
PS1, Line 83: SOC_INTEL_KABYLAKE
> good point.

Done

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Name of user not set (Code Review)]

Name of user not set #1002476 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/3/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/3/src/soc/intel/skylake/Kconf... 
PS3, Line 85: 
add help for the config?

help
  Enable Intel(r) Hyper-Threading Technology for the Skylake SoC.

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35208/3/src/soc/intel/skylake/Kconf... 
File src/soc/intel/skylake/Kconfig:

https://review.coreboot.org/c/coreboot/+/35208/3/src/soc/intel/skylake/Kconf... 
PS3, Line 85:
> add help for the config? […]

It's already clear what this config does.

Change in coreboot[master]: soc/intel/skylake: Add option to toggle Hyper-Threading

[Patrick Rudolph (Code Review)]

Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35208 )

Change subject: soc/intel/skylake: Add option to toggle Hyper-Threading
......................................................................

soc/intel/skylake: Add option to toggle Hyper-Threading

Tested on Supermicro X11SSH-TF.

Change-Id: I3ebab68ff868c78105bb4b35abffb92f3ccf1705
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35208
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
---
M src/soc/intel/skylake/Kconfig
M src/soc/intel/skylake/romstage/romstage_fsp20.c
2 files changed, 6 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Held: Looks good to me, approved
  Name of user not set #1002476: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig
index 9cb8d45..901e5f9 100644
--- a/src/soc/intel/skylake/Kconfig
+++ b/src/soc/intel/skylake/Kconfig
@@ -78,6 +78,11 @@
    select TSC_SYNC_MFENCE
    select UDELAY_TSC

+config FSP_HYPERTHREADING
+	bool "Enable Hyper-Threading"
+	depends on MAINBOARD_USES_FSP2_0
+	default y
+
 config CPU_INTEL_NUM_FIT_ENTRIES
    int
    default 10
diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c
index 002a284..deda533 100644
--- a/src/soc/intel/skylake/romstage/romstage_fsp20.c
+++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c
@@ -252,6 +252,7 @@
    	m_cfg->PchHpetDeviceNumber = V_P2SB_HBDF_DEV;
    	m_cfg->PchHpetFunctionNumber = V_P2SB_HBDF_FUN;
    }
+	m_cfg->HyperThreading = CONFIG(FSP_HYPERTHREADING);
 }

static void soc_primary_gfx_config_params(FSP_M_CONFIG *m_cfg,