Change in coreboot[master]: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

drivers/intel/fsp2_0: Use coreboot postcar with FSP-T

Allow platforms to use the coreboot postcar code instead of calling
into FSP-M TempRamExit API.

There are a few reasons to do this:
- Tearing down CAR is easy.
- Having control over MTRR's and caching in general.
- The MTRR's set up in postcar be it by coreboot or FSP-M are
  overwritten later on during CPU init so it does not matter.
- This removes the need to find a CBFS file before cbmem is up (this
  causes problems with cbfs_mcache)

Change-Id: I6cf10c7580f3183bfee1cd3c827901cbcf695db7
Signed-off-by: Arthur Heymans 
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/Makefile.inc
M src/soc/intel/common/block/cpu/Makefile.inc
3 files changed, 13 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/48466/1

diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig
index 96ae282..d38a292 100644
--- a/src/drivers/intel/fsp2_0/Kconfig
+++ b/src/drivers/intel/fsp2_0/Kconfig
@@ -117,6 +117,13 @@
    help
      Use FSP APIs to initialize & Tear Down the Cache-As-Ram

+config NO_FSP_TEMP_RAM_EXIT
+	bool
+	depends on FSP_CAR
+	help
+	  Select this on a platform where you want to use FSP-T but use
+	  coreboot code to tear down CAR.
+
 config FSP_M_XIP
    bool
    default n
diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc
index 298198d..3fbfda1 100644
--- a/src/drivers/intel/fsp2_0/Makefile.inc
+++ b/src/drivers/intel/fsp2_0/Makefile.inc
@@ -29,7 +29,9 @@
 ramstage-y += util.c
 ramstage-$(CONFIG_MMA) += mma_core.c

+ifneq ($(CONFIG_NO_FSP_TEMP_RAM_EXIT),y)
 postcar-$(CONFIG_FSP_CAR) += temp_ram_exit.c
+endif
 postcar-$(CONFIG_FSP_CAR) += util.c
 postcar-$(CONFIG_DISPLAY_FSP_HEADER) += header_display.c
 postcar-y += hand_off_block.c
diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc
index 7692076..5131658 100644
--- a/src/soc/intel/common/block/cpu/Makefile.inc
+++ b/src/soc/intel/common/block/cpu/Makefile.inc
@@ -1,6 +1,10 @@
 ifeq ($(CONFIG_FSP_CAR),y)
 bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU)+= car/cache_as_ram_fsp.S
+ifeq ($(CONFIG_NO_FSP_TEMP_RAM_EXIT),y)
+postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car.S
+else
 postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car_fsp.S
+endif
 else
 bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/cache_as_ram.S
 bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += ../../../../../cpu/x86/early_reset.S

[Angel Pons (Code Review)]

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

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 1: Code-Review+1

(3 comments)

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

https://review.coreboot.org/c/coreboot/+/48466/1//COMMIT_MSG@12 
PS1, Line 12: a few
nit: several

https://review.coreboot.org/c/coreboot/+/48466/1//COMMIT_MSG@14 
PS1, Line 14: Having 
Missing a verb: *Allows* having ...

https://review.coreboot.org/c/coreboot/+/48466/1//COMMIT_MSG@17 
PS1, Line 17: This removes the need to find
nit: Avoids/Prevents having to find

[Arthur Heymans (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Jonathan Zhang, Christian Walter, Angel Pons, Subrata Banik, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

drivers/intel/fsp2_0: Use coreboot postcar with FSP-T

Allow platforms to use the coreboot postcar code instead of calling
into FSP-M TempRamExit API.

There are several reasons to do this:
- Tearing down CAR is easy.
- Allows having control over MTRR's and caching in general.
- The MTRR's set up in postcar be it by coreboot or FSP-M are
  overwritten later on during CPU init so it does not matter.
- Avoids having to find a CBFS file before cbmem is up (this
  causes problems with cbfs_mcache)

Change-Id: I6cf10c7580f3183bfee1cd3c827901cbcf695db7
Signed-off-by: Arthur Heymans 
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/Makefile.inc
M src/soc/intel/common/block/cpu/Makefile.inc
3 files changed, 13 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/48466/2

[Arthur Heymans (Code Review)]

Attention is currently required from: Angel Pons.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 2:

(3 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/48466/comment/454d5f20_8b58c5ed 
PS1, Line 12: a few
> nit: several

done

https://review.coreboot.org/c/coreboot/+/48466/comment/1d03193b_874144e4 
PS1, Line 14: Having
> Missing a verb: *Allows* having ...

Done

https://review.coreboot.org/c/coreboot/+/48466/comment/f329f4b5_39f50d36 
PS1, Line 17: This removes the need to find
> nit: Avoids/Prevents having to find

Done

[Angel Pons (Code Review)]

Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Attention is currently required from: Daocheng Bu, Arthur Heymans.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 2:

(1 comment)

Patchset:

PS2: 
needs rebase

[Arthur Heymans (Code Review)]

Attention is currently required from: Daocheng Bu, Arthur Heymans.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Jonathan Zhang, Daocheng Bu, Christian Walter, Angel Pons, Subrata Banik, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

drivers/intel/fsp2_0: Use coreboot postcar with FSP-T

Allow platforms to use the coreboot postcar code instead of calling
into FSP-M TempRamExit API.

There are several reasons to do this:
- Tearing down CAR is easy.
- Allows having control over MTRR's and caching in general.
- The MTRR's set up in postcar be it by coreboot or FSP-M are
  overwritten later on during CPU init so it does not matter.
- Avoids having to find a CBFS file before cbmem is up (this
  causes problems with cbfs_mcache)

Change-Id: I6cf10c7580f3183bfee1cd3c827901cbcf695db7
Signed-off-by: Arthur Heymans 
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/Makefile.inc
M src/soc/intel/common/block/cpu/Makefile.inc
3 files changed, 13 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/48466/3

[Arthur Heymans (Code Review)]

Attention is currently required from: Daocheng Bu.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 3:

(1 comment)

Patchset:

PS2:
> needs rebase

Done

[Patrick Georgi (Code Review)]

Attention is currently required from: Daocheng Bu, Arthur Heymans.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

Patchset:

PS3: 
Inheriting earlier CR+2

[Angel Pons (Code Review)]

Attention is currently required from: Daocheng Bu, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48466 )

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: drivers/intel/fsp2_0: Use coreboot postcar with FSP-T
......................................................................

drivers/intel/fsp2_0: Use coreboot postcar with FSP-T

Allow platforms to use the coreboot postcar code instead of calling
into FSP-M TempRamExit API.

There are several reasons to do this:
- Tearing down CAR is easy.
- Allows having control over MTRR's and caching in general.
- The MTRR's set up in postcar be it by coreboot or FSP-M are
  overwritten later on during CPU init so it does not matter.
- Avoids having to find a CBFS file before cbmem is up (this
  causes problems with cbfs_mcache)

Change-Id: I6cf10c7580f3183bfee1cd3c827901cbcf695db7
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48466
Reviewed-by: Patrick Georgi 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/Makefile.inc
M src/soc/intel/common/block/cpu/Makefile.inc
3 files changed, 13 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig
index 056c35c..551b4b9 100644
--- a/src/drivers/intel/fsp2_0/Kconfig
+++ b/src/drivers/intel/fsp2_0/Kconfig
@@ -126,6 +126,13 @@
      defined in the FSP specification but in the SOC integration
      guides.

+config NO_FSP_TEMP_RAM_EXIT
+	bool
+	depends on FSP_CAR
+	help
+	  Select this on a platform where you want to use FSP-T but use
+	  coreboot code to tear down CAR.
+
 config FSP_M_XIP
    bool
    default n
diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc
index 25e2846..b518bec 100644
--- a/src/drivers/intel/fsp2_0/Makefile.inc
+++ b/src/drivers/intel/fsp2_0/Makefile.inc
@@ -29,7 +29,9 @@
 ramstage-y += util.c
 ramstage-$(CONFIG_MMA) += mma_core.c

+ifneq ($(CONFIG_NO_FSP_TEMP_RAM_EXIT),y)
 postcar-$(CONFIG_FSP_CAR) += temp_ram_exit.c
+endif
 postcar-$(CONFIG_FSP_CAR) += util.c
 postcar-$(CONFIG_DISPLAY_FSP_HEADER) += header_display.c
 postcar-y += hand_off_block.c
diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc
index 7692076..5131658 100644
--- a/src/soc/intel/common/block/cpu/Makefile.inc
+++ b/src/soc/intel/common/block/cpu/Makefile.inc
@@ -1,6 +1,10 @@
 ifeq ($(CONFIG_FSP_CAR),y)
 bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU)+= car/cache_as_ram_fsp.S
+ifeq ($(CONFIG_NO_FSP_TEMP_RAM_EXIT),y)
+postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car.S
+else
 postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car_fsp.S
+endif
 else
 bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/cache_as_ram.S
 bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += ../../../../../cpu/x86/early_reset.S