Change in coreboot[master]: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake

[Shreesh Chhabbi (Code Review)]

Hello Shreesh Chhabbi,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake

Signed-off-by: Shreesh Chhabbi 
Change-Id: I602d93eb4f8243ec49993b00691140d9a6cf5733
---
M src/soc/intel/tigerlake/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/45094/1

diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig
index 8718f97..5911479 100644
--- a/src/soc/intel/tigerlake/Kconfig
+++ b/src/soc/intel/tigerlake/Kconfig
@@ -24,7 +24,7 @@
    select INTEL_DESCRIPTOR_MODE_CAPABLE
    select HAVE_SMI_HANDLER
    select IDT_IN_EVERY_STAGE
-	select INTEL_CAR_NEM #TODO - Enable INTEL_CAR_NEM_ENHANCED
+	select USE_CAR_NEM_ENHANCED_V2
    select INTEL_GMA_ACPI
    select INTEL_GMA_ADD_VBT if RUN_FSP_GOP
    select IOAPIC

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has uploaded a new patch set (#2) to the change originally created by Shreesh Chhabbi. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake

Selects Cache QoS mask MSR programming flow for Tigerlake SoC.

Cq-Depend: 43494
Signed-off-by: Shreesh Chhabbi 
Change-Id: I602d93eb4f8243ec49993b00691140d9a6cf5733
---
M src/soc/intel/tigerlake/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/45094/2

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 2:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45094/2//COMMIT_MSG@11 
PS2, Line 11: Cq-Depend: 43494
This isn't necessary, this is only used for Chrome OS, when we have changes from > 1 repo that have to land together to avoid breaking a tree.

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has uploaded a new patch set (#3) to the change originally created by Shreesh Chhabbi. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake

Selects Cache QoS mask MSR programming flow for Tigerlake SoC.

BUG=b:145958015
TEST= Build and boot to Chrome OS on TGL-UP3 RVP.
Recipe used:
1. Patch https://review.coreboot.org/c/coreboot/+/43494 which implements calculation of
CQOS mask dynamically based on stack size usage & incorporates Tigerlake SoC
specific programming flow.
2. QS Engineering Microcode based on 0x56 Official Microcode with LLC CQOS change.
3. QS SoC Part.

Signed-off-by: Shreesh Chhabbi 
Change-Id: I602d93eb4f8243ec49993b00691140d9a6cf5733
---
M src/soc/intel/tigerlake/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/45094/3

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 3:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45094/2//COMMIT_MSG@11 
PS2, Line 11: Cq-Depend: 43494
> This isn't necessary, this is only used for Chrome OS, when we have changes from > 1 repo that have  […]

Ok. Thanks Tim. Should we manually ensure that patch 43494 gets merged first?

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has uploaded a new patch set (#4) to the change originally created by Shreesh Chhabbi. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake

Selects Cache QoS mask MSR programming flow for Tigerlake SoC.

BUG=b:145958015
TEST= Build and boot to Chrome OS on TGL-UP3 RVP.
Recipe used:
1. Patch https://review.coreboot.org/c/coreboot/+/43494 that
implements calculation of CQOS mask dynamically based on stack
size usage & incorporates Tigerlake SoC specific programming flow.
2. QS Engineering Microcode based on 0x56 Official Microcode with
LLC CQOS change.
3. QS SoC Part

Signed-off-by: Shreesh Chhabbi 
Change-Id: I602d93eb4f8243ec49993b00691140d9a6cf5733
---
M src/soc/intel/tigerlake/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/45094/4

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 4:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45094/2//COMMIT_MSG@11 
PS2, Line 11: Cq-Depend: 43494
> Ok. Thanks Tim. […]

As long as they're in a relation chain in Gerrit (which they currently are), then they will land in the correct order in the chromium tree.

[Shreesh Chhabbi (Code Review)]

Shreesh Chhabbi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 4:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45094/2//COMMIT_MSG@11 
PS2, Line 11: Cq-Depend: 43494
> As long as they're in a relation chain in Gerrit (which they currently are), then they will land in  […]

Ok. Thanks.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 4: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake

Selects Cache QoS mask MSR programming flow for Tigerlake SoC.

BUG=b:145958015
TEST= Build and boot to Chrome OS on TGL-UP3 RVP.
Recipe used:
1. Patch https://review.coreboot.org/c/coreboot/+/43494 that
implements calculation of CQOS mask dynamically based on stack
size usage & incorporates Tigerlake SoC specific programming flow.
2. QS Engineering Microcode based on 0x56 Official Microcode with
LLC CQOS change.
3. QS SoC Part

Signed-off-by: Shreesh Chhabbi 
Change-Id: I602d93eb4f8243ec49993b00691140d9a6cf5733
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45094
Tested-by: build bot (Jenkins) 
Reviewed-by: Tim Wawrzynczak 
---
M src/soc/intel/tigerlake/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig
index cdec3ef..826f8af 100644
--- a/src/soc/intel/tigerlake/Kconfig
+++ b/src/soc/intel/tigerlake/Kconfig
@@ -25,7 +25,7 @@
    select INTEL_DESCRIPTOR_MODE_CAPABLE
    select HAVE_SMI_HANDLER
    select IDT_IN_EVERY_STAGE
-	select INTEL_CAR_NEM #TODO - Enable INTEL_CAR_NEM_ENHANCED
+	select USE_CAR_NEM_ENHANCED_V2
    select INTEL_GMA_ACPI
    select INTEL_GMA_ADD_VBT if RUN_FSP_GOP
    select IOAPIC

[Caveh Jalali (Code Review)]

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 5:

this has caused a regression on ES2: CPU does not boot - seems to crash before
any serial console output.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45094 )

Change subject: soc/intel/tigerlake: Select USE_CAR_NEM_ENHANCED_V2 for tigerlake
......................................................................

Patch Set 5:
> Patch Set 5:
>
> this has caused a regression on ES2: CPU does not boot - seems to crash before
> any serial console output.

What uCode do you have?
There wouldn't be any serial output, entering NEM is one of the first things bootblock does, after setting up protected mode