Subrata Banik submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved David Guckian: Looks good to me, but someone else must approve
soc/intel/{apl,cnl,dnv,skl}: Skip ucode loading by FSP-T

It is a requirement for Firmware to have Firmware Interface Table (FIT),
which contains pointers to each microcode update.
The microcode update is loaded for all logical processors before reset vector.

FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength are
input parameters to TempRamInit API.
If these values are 0, FSP will not attempt to update microcode.

Since Gen-4 all IA-SoC has FIT loading ucode even before cpu reset in place
hence skipping FSP-T loading ucode after CPU reset options.

Also removed unused kconfig CONFIG_CPU_MICROCODE_CBFS_LOC and
CONFIG_CPU_MICROCODE_CBFS_LEN

Change-Id: I3a406fa0e2e62e3363c2960e173dc5f5f5ca0455
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37187
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: David Guckian
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/cpu/Makefile.inc
M src/drivers/intel/fsp2_0/Kconfig
M src/soc/intel/apollolake/fspcar.c
M src/soc/intel/cannonlake/bootblock/bootblock.c
M src/soc/intel/denverton_ns/Kconfig
M src/soc/intel/denverton_ns/bootblock/bootblock.c
M src/soc/intel/skylake/fspcar.c
7 files changed, 50 insertions(+), 40 deletions(-)

diff --git a/src/cpu/Makefile.inc b/src/cpu/Makefile.inc
index b80c30d..4b5d67b 100644
--- a/src/cpu/Makefile.inc
+++ b/src/cpu/Makefile.inc
@@ -59,9 +59,4 @@

cpu_microcode_blob.bin-file ?= $(obj)/cpu_microcode_blob.bin
cpu_microcode_blob.bin-type := microcode
-
-ifneq ($(CONFIG_CPU_MICROCODE_CBFS_LOC),)
-cpu_microcode_blob.bin-COREBOOT-position := $(CONFIG_CPU_MICROCODE_CBFS_LOC)
-else
cpu_microcode_blob.bin-align := 16
-endif
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig
index 1fd4b0c..77382d3 100644
--- a/src/drivers/intel/fsp2_0/Kconfig
+++ b/src/drivers/intel/fsp2_0/Kconfig
@@ -37,21 +37,6 @@
Add the FSP-M and FSP-S binaries to CBFS. Currently coreboot does not
use the FSP-T binary and it is not added.

-config CPU_MICROCODE_CBFS_LEN
- hex "Microcode update region length in bytes"
- depends on FSP_CAR
- default 0x0
- help
- The length in bytes of the microcode update region.
-
-config CPU_MICROCODE_CBFS_LOC
- hex "Microcode update base address in CBFS"
- depends on FSP_CAR
- default 0x0
- help
- The location (base address) in CBFS that contains the
- microcode update binary.
-
config FSP_T_CBFS
string "Name of FSP-T in CBFS"
depends on FSP_CAR
diff --git a/src/soc/intel/apollolake/fspcar.c b/src/soc/intel/apollolake/fspcar.c
index 8b1089f..a284116 100644
--- a/src/soc/intel/apollolake/fspcar.c
+++ b/src/soc/intel/apollolake/fspcar.c
@@ -25,6 +25,17 @@
.FsptCommonUpd = {
.Revision = 0,
.Reserved = {0},
+ /*
+ * It is a requirement for firmware to have Firmware Interface Table
+ * (FIT), which contains pointers to each microcode update.
+ * The microcode update is loaded for all logical processors before
+ * cpu reset vector.
+ *
+ * All SoC since Gen-4 has above mechanism in place to load microcode
+ * even before hitting CPU reset vector. Hence skipping FSP-T loading
+ * microcode after CPU reset by passing '0' value to
+ * FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength.
+ */
.MicrocodeRegionBase = 0,
.MicrocodeRegionLength = 0,
.CodeRegionBase =
diff --git a/src/soc/intel/cannonlake/bootblock/bootblock.c b/src/soc/intel/cannonlake/bootblock/bootblock.c
index 653ba30..9f85397 100644
--- a/src/soc/intel/cannonlake/bootblock/bootblock.c
+++ b/src/soc/intel/cannonlake/bootblock/bootblock.c
@@ -30,10 +30,19 @@
.Reserved = {0},
},
.FsptCoreUpd = {
- .MicrocodeRegionBase =
- (uint32_t)CONFIG_CPU_MICROCODE_CBFS_LOC,
- .MicrocodeRegionSize =
- (uint32_t)CONFIG_CPU_MICROCODE_CBFS_LEN,
+ /*
+ * It is a requirement for firmware to have Firmware Interface Table
+ * (FIT), which contains pointers to each microcode update.
+ * The microcode update is loaded for all logical processors before
+ * cpu reset vector.
+ *
+ * All SoC since Gen-4 has above mechanism in place to load microcode
+ * even before hitting CPU reset vector. Hence skipping FSP-T loading
+ * microcode after CPU reset by passing '0' value to
+ * FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength.
+ */
+ .MicrocodeRegionBase = 0,
+ .MicrocodeRegionLength = 0,
.CodeRegionBase =
(uint32_t)(0x100000000ULL - CONFIG_ROM_SIZE),
.CodeRegionSize = (uint32_t)CONFIG_ROM_SIZE,
diff --git a/src/soc/intel/denverton_ns/Kconfig b/src/soc/intel/denverton_ns/Kconfig
index 0ce0d5b..713aae6 100644
--- a/src/soc/intel/denverton_ns/Kconfig
+++ b/src/soc/intel/denverton_ns/Kconfig
@@ -108,14 +108,6 @@
hex
default 0x10000

-config CPU_MICROCODE_CBFS_LOC
- hex
- default 0xfff20040
-
-config CPU_MICROCODE_CBFS_LEN
- hex
- default 0x0ff80
-
config CPU_BCLK_MHZ
int
default 100
diff --git a/src/soc/intel/denverton_ns/bootblock/bootblock.c b/src/soc/intel/denverton_ns/bootblock/bootblock.c
index f75de1f..47c76b5 100644
--- a/src/soc/intel/denverton_ns/bootblock/bootblock.c
+++ b/src/soc/intel/denverton_ns/bootblock/bootblock.c
@@ -31,10 +31,19 @@
.Reserved = {0},
},
.FsptCoreUpd = {
- .MicrocodeRegionBase =
- (UINT32)CONFIG_CPU_MICROCODE_CBFS_LOC,
- .MicrocodeRegionLength =
- (UINT32)CONFIG_CPU_MICROCODE_CBFS_LEN,
+ /*
+ * It is a requirement for firmware to have Firmware Interface Table
+ * (FIT), which contains pointers to each microcode update.
+ * The microcode update is loaded for all logical processors before
+ * cpu reset vector.
+ *
+ * All SoC since Gen-4 has above mechanism in place to load microcode
+ * even before hitting CPU reset vector. Hence skipping FSP-T loading
+ * microcode after CPU reset by passing '0' value to
+ * FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength.
+ */
+ .MicrocodeRegionBase = 0,
+ .MicrocodeRegionLength = 0,
.CodeRegionBase =
(UINT32)(0x100000000ULL - CONFIG_ROM_SIZE),
.CodeRegionLength = (UINT32)CONFIG_ROM_SIZE,
diff --git a/src/soc/intel/skylake/fspcar.c b/src/soc/intel/skylake/fspcar.c
index a4c3726..0d27f57 100644
--- a/src/soc/intel/skylake/fspcar.c
+++ b/src/soc/intel/skylake/fspcar.c
@@ -23,10 +23,19 @@
.Reserved = {0},
},
.FsptCoreUpd = {
- .MicrocodeRegionBase =
- (uint32_t)CONFIG_CPU_MICROCODE_CBFS_LOC,
- .MicrocodeRegionSize =
- (uint32_t)CONFIG_CPU_MICROCODE_CBFS_LEN,
+ /*
+ * It is a requirement for firmware to have Firmware Interface Table
+ * (FIT), which contains pointers to each microcode update.
+ * The microcode update is loaded for all logical processors before
+ * cpu reset vector.
+ *
+ * All SoC since Gen-4 has above mechanism in place to load microcode
+ * even before hitting CPU reset vector. Hence skipping FSP-T loading
+ * microcode after CPU reset by passing '0' value to
+ * FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength.
+ */
+ .MicrocodeRegionBase = 0,
+ .MicrocodeRegionLength = 0,
.CodeRegionBase =
(uint32_t)(0x100000000ULL - CONFIG_ROM_SIZE),
.CodeRegionSize = (uint32_t)CONFIG_ROM_SIZE,

To view, visit change 37187. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3a406fa0e2e62e3363c2960e173dc5f5f5ca0455
Gerrit-Change-Number: 37187
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: David Guckian
Gerrit-Reviewer: David Guckian <david.guckian@intel.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan@intel.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com>
Gerrit-Reviewer: Vincent Zimmer <vincent.zimmer@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged