coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support
by Jérémy Compostella (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Andrey Petrov, Bora Guvendik, Ronak Kanabar, Subrata Banik, Wonkyu Kim. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80275?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP 2.4 support ...................................................................... Patch Set 3: (6 comments) Patchset: PS3: > quick feedback > 1. This CL doesn't mentioned the test vehicle used to verify Test vehicle is not public. Considering the 2.4 specification is public, we can provide the implementation. > 2. specify the FSP 2.4 spec link I gave the document number but I did not add the link as links can change. File src/drivers/intel/fsp2_0/Kconfig: https://review.coreboot.org/c/coreboot/+/80275/comment/c9b78efb_ee6bc4c9 : PS3, Line 58: default n if PLATFORM_USES_FSP2_4 > ideally this config can be only select if `depends on !PLATFORM_USES_FSP2_4` 2.4 also supports 32-bits. File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80275/comment/a30afcba_e976d340 : PS3, Line 32: void __weak platform_fsp_multi_phase_init_cb(uint32_t phase_index) > nit: […] The idea was to reuse the same hook. The risk is that if we have a converged romstage and ramstage image then this would break... I'll separate those. https://review.coreboot.org/c/coreboot/+/80275/comment/bbbc2147_ff2092c6 : PS3, Line 282: error_handler > nit: […] Considering we are in memory_init.c and this is static function I don't see the benefit of surcharging the function name with unnecessary prefix. https://review.coreboot.org/c/coreboot/+/80275/comment/e5ed38f3_310e5125 : PS3, Line 297: multi_phase_init > nit: multi_phase_mem_init Considering we are in memory_init.c and this is static function I don't see the benefit of surcharging the function name with unnecessary prefix. https://review.coreboot.org/c/coreboot/+/80275/comment/6794e6f0_8d9fe00f : PS3, Line 437: if (hdr->fsp_multi_phase_mem_init_entry_offset) > why not like this ? […] Wouldn't it be dangerous to look outside after the limit of the data structure ? These fields are added with 2.4. -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 3 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Wed, 31 Jan 2024 19:44:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support
by Subrata Banik (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Andrey Petrov, Bora Guvendik, Jérémy Compostella, Ronak Kanabar, Wonkyu Kim. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80275?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP 2.4 support ...................................................................... Patch Set 3: (7 comments) Patchset: PS3: some more generic feedback, FSPM_ARCH2_UPD should need a EDK2 uprev. Please submit that as well as base CL. Until things are not in place, I suggest to mark this CL as WIP File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80275/comment/2168fd69_3e20357e : PS3, Line 32: void __weak platform_fsp_multi_phase_init_cb(uint32_t phase_index) nit: platform_fsp_multi_phase_mem_init_cb https://review.coreboot.org/c/coreboot/+/80275/comment/01d3d332_2dba7f2e : PS3, Line 282: error_handler nit: fsp_memory_init_error_handler https://review.coreboot.org/c/coreboot/+/80275/comment/e9908f37_501195f9 : PS3, Line 295: : #if CONFIG(PLATFORM_USES_FSP2_4) i don't believe u need this. we don't see this inside silicon_init.c https://review.coreboot.org/c/coreboot/+/80275/comment/ff0cbc1d_e7f970df : PS3, Line 297: multi_phase_init nit: multi_phase_mem_init https://review.coreboot.org/c/coreboot/+/80275/comment/42273947_dc46d6fb : PS3, Line 437: if (hdr->fsp_multi_phase_mem_init_entry_offset) why not like this ? ``` .... /* Implementing multi_phase_mem_init() is optional till < FSP 2.4 spec */ if (hdr->fsp_multi_phase_mem_init_entry_offset == NULL) return; multi_phase_mem_init(hdr); .... ``` File src/drivers/intel/fsp2_0/silicon_init.c: https://review.coreboot.org/c/coreboot/+/80275/comment/d30e3cfe_533c148d : PS3, Line 25: platform_fsp_multi_phase_init_cb nit: platform_fsp_multi_phase_si_init_cb -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 3 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Wed, 31 Jan 2024 18:32:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support
by Subrata Banik (Code Review) 31 Jan '24

31 Jan '24

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support
by Jérémy Compostella (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Andrey Petrov, Bora Guvendik, Ronak Kanabar, Wonkyu Kim. Hello Andrey Petrov, Bora Guvendik, Ronak Kanabar, Wonkyu Kim, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80275?usp=email to look at the new patch set (#3). Change subject: drivers/intel/fsp2_0: Add FSP 2.4 support ...................................................................... drivers/intel/fsp2_0: Add FSP 2.4 support Intel Firmware Support Package 2.4 specification brings some significant changes compared to version 2.3 (cf. documents 736809 and 644852 respectively): 1. It supports FSP-M multi-phase init. Some fields have been added to the FSP header data structure for this purpose. 2. The `FSPM_ARCH2_UPD' and `FSPS_ARCH2_UPD' data structures must be used in place of `FSPM_ARCH_UPD' and `FSPS_ARCH_UPD' respectively. 3. It supports 64-bits FSP: - FSP functions must be called with the stack 16-bytes aligned. This is already setup properly with the default value of the `mpreferred-stack-boundary' compiler option (4). - The FSP stack buffer supplied by coreboot through the `StackBase' UPD must be 16-bytes aligned. A few notes: - `PLATFORM_USES_FSP2_X86_32' is set to `n' by default if FSP 2.4 is enabled as 64-bits FSP should be norm moving forward. - Similarly to what is done for silicon initialization, timestamps and post-codes are used during the memory initialization multi-phase. However, since post-codes are in short supply, memory and silicon multi-phase init share the same post-codes. Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/commonlib/include/commonlib/console/post_codes.h M src/commonlib/include/commonlib/timestamp_serialized.h M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M util/cbfstool/eventlog.c 8 files changed, 126 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/80275/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 3 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support
by Jérémy Compostella (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Andrey Petrov, Ronak Kanabar. Hello Andrey Petrov, Ronak Kanabar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80275?usp=email to look at the new patch set (#2). Change subject: drivers/intel/fsp2_0: Add FSP 2.4 support ...................................................................... drivers/intel/fsp2_0: Add FSP 2.4 support Intel Firmware Support Package 2.4 specification brings some significant changes compared to version 2.3 (cf. documents 736809 and 644852 respectively): 1. It supports FSP-M multi-phase init. Some fields have been added to the FSP header data structure for this purpose. 2. The `FSPM_ARCH2_UPD' and `FSPS_ARCH2_UPD' data structures must be used in place of `FSPM_ARCH_UPD' and `FSPS_ARCH_UPD' respectively. 3. It supports 64-bits FSP: - FSP functions must be called with the stack 16-bytes aligned. This is already setup properly with the default value of the `mpreferred-stack-boundary' compiler option (4). - The FSP stack buffer supplied by coreboot through the `StackBase' UPD must be 16-bytes aligned. A few notes: - `PLATFORM_USES_FSP2_X86_32' is set to `n' by default if FSP 2.4 is enabled as 64-bits FSP should be norm moving forward. - Similarly to what is done for silicon initialization, timestamps and post-codes are used during the memory initialization multi-phase. However, since post-codes are in short supply, memory and silicon multi-phase init share the same post-codes. Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/commonlib/include/commonlib/console/post_codes.h M src/commonlib/include/commonlib/timestamp_serialized.h M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M util/cbfstool/eventlog.c 8 files changed, 126 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/80275/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 2 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: Documentation: Describe how SMMSTORE can be used safely
by Benjamin Doron (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Nico Huber, Patrick Georgi. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79095?usp=email ) Change subject: Documentation: Describe how SMMSTORE can be used safely ...................................................................... Patch Set 2: (1 comment) File Documentation/drivers/smmstore.md: https://review.coreboot.org/c/coreboot/+/79095/comment/b7c43285_0ab4b6a4 : PS2, Line 177: As a remedy, CLEAR could be disabled after the initial repacking, : within the boot process, so that SMMSTORE becomes an append-only : store. In this case, the attacker could fill up the buffer, leading : to a DoS of the variable store until it's repacked. As described : earlier, once there's an attacker on the system, the variable store : lost its function until the attacker has been evicted, anyway. > Raw write from outside SMM is supposed to be disabled in the SMMSTORE model, yes. […] I don't understand. SMMSTOREv2 is a raw read/write capability, likely for greater flexibility with payloads. It seems we've been considering different versions of SMMSTORE. Can you add a note that SMMSTOREv2 requires more consideration? There are no keys to use as indices, and a block-based LOCK command might not be granular enough, but... something would be necessary. -- To view, visit https://review.coreboot.org/c/coreboot/+/79095?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I48f44d3416d210e1e6b19d18cad787e380ffeebc Gerrit-Change-Number: 79095 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Comment-Date: Wed, 31 Jan 2024 17:45:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org> Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support
by Jérémy Compostella (Code Review) 31 Jan '24

31 Jan '24

Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80275?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP 2.4 support ...................................................................... drivers/intel/fsp2_0: Add FSP 2.4 support Intel Firmware Support Package 2.4 specification brings some significant changes compared to version 2.3 (cf. documents 736809 and 644852 respectively): 1. It supports FSP-M multi-phase init. Some fields have been added to the FSP header data structure for this purpose. 2. The `FSPM_ARCH2_UPD' and `FSPS_ARCH2_UPD' data structures must be used in place of `FSPM_ARCH_UPD' and `FSPS_ARCH_UPD' respectively. 3. It supports 64-bits FSP: - FSP functions must be called with the stack 16-bytes aligned. This is already setup properly with the default value of the `mpreferred-stack-boundary' compiler option (4). - The FSP stack buffer supplied by coreboot through the `StackBase' UPD must be 16-bytes aligned. A few notes: - `PLATFORM_USES_FSP2_X86_32' is set to `n' by default if FSP 2.4 is enabled as 64-bits FSP should be norm moving forward. - Similarly to what is done for silicon initialization, timestamps and post-codes are used during the memory initialization multi-phase. However, since post-codes are in short supply, memory and silicon multi-phase init share the same post-codes. Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/commonlib/include/commonlib/console/post_codes.h M src/commonlib/include/commonlib/timestamp_serialized.h M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M util/cbfstool/eventlog.c 9 files changed, 126 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/80275/1 diff --git a/src/commonlib/include/commonlib/console/post_codes.h b/src/commonlib/include/commonlib/console/post_codes.h index c0b15c5..f117b96 100644 --- a/src/commonlib/include/commonlib/console/post_codes.h +++ b/src/commonlib/include/commonlib/console/post_codes.h @@ -334,18 +334,18 @@ #define POSTCODE_X86_WRITE_ACPITABLE 0x9c /** - * \brief Before calling FSP Multiphase SiliconInit + * \brief Before calling FSP Multiphase MemoryInit or SiliconInit * - * Going to call into FSP binary for Multiple phase SI Init + * Going to call into FSP binary for Multiple phase Init */ -#define POSTCODE_FSP_MULTI_PHASE_SI_INIT_ENTRY 0xa0 +#define POSTCODE_FSP_MULTI_PHASE_INIT_ENTRY 0xa0 /** - * \brief After calling FSP Multiphase SiliconInit + * \brief After calling FSP Multiphase MemoryInit or SiliconInit * - * FSP binary returned from Multiple phase SI Init + * FSP binary returned from Multiple phase Init */ -#define POSTCODE_FSP_MULTI_PHASE_SI_INIT_EXIT 0xa1 +#define POSTCODE_FSP_MULTI_PHASE_INIT_EXIT 0xa1 /** * \brief After calling FSP Notify (after PCI enumeration) diff --git a/src/commonlib/include/commonlib/timestamp_serialized.h b/src/commonlib/include/commonlib/timestamp_serialized.h index e4439cd..cd44cd1 100644 --- a/src/commonlib/include/commonlib/timestamp_serialized.h +++ b/src/commonlib/include/commonlib/timestamp_serialized.h @@ -140,6 +140,8 @@ TS_FSP_MULTI_PHASE_SI_INIT_END = 963, TS_FSP_MEMORY_INIT_LOAD = 970, TS_FSP_SILICON_INIT_LOAD = 971, + TS_FSP_MULTI_PHASE_MEM_INIT_START = 972, + TS_FSP_MULTI_PHASE_MEM_INIT_END = 973, /* 990+ reserved for vendorcode extensions (990-999: Intel ME continued) */ TS_ME_ROM_START = 990, @@ -319,6 +321,9 @@ TS_NAME_DEF(TS_FSP_MULTI_PHASE_SI_INIT_START, TS_FSP_MULTI_PHASE_SI_INIT_END, "calling FspMultiPhaseSiInit"), TS_NAME_DEF(TS_FSP_MULTI_PHASE_SI_INIT_END, 0, "returning from FspMultiPhaseSiInit"), + TS_NAME_DEF(TS_FSP_MULTI_PHASE_MEM_INIT_START, TS_FSP_MULTI_PHASE_MEM_INIT_END, + "calling FspMultiPhaseMemInit"), + TS_NAME_DEF(TS_FSP_MULTI_PHASE_MEM_INIT_END, 0, "returning from FspMultiPhaseMemInit"), TS_NAME_DEF(TS_FSP_ENUMERATE_START, TS_FSP_ENUMERATE_END, "calling FspNotify(AfterPciEnumeration)"), TS_NAME_DEF(TS_FSP_ENUMERATE_END, 0, "returning from FspNotify(AfterPciEnumeration)"), diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index e27249f..61d68f6 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -39,14 +39,27 @@ 1. Added ExtendedImageRevision field in FSP_INFO_HEADER 2. Added FSP_NON_VOLATILE_STORAGE_HOB2 +config PLATFORM_USES_FSP2_4 + bool + default n + select PLATFORM_USES_FSP2_3 + help + Include FSP 2.4 wrappers and functionality. + Features added into FSP 2.4 specification that impact coreboot are: + 1. FSP-M multi phase init support + 2. FSPM_ARCH2_UPD and FSPS_ARCH2_UPD data structures must be + used in place of FSPM_ARCH_UPD and FSPS_ARCH_UPD respectively + 3. 64-bits support + if PLATFORM_USES_FSP2_0 config PLATFORM_USES_FSP2_X86_32 bool + default n if PLATFORM_USES_FSP2_4 default y help - The FSP 2.0 runs in x86_32 protected mode. - Once there's a x86_64 FSP this needs to default to n. + Specify if the FSP binaries are 32-bits (yes) or 64-bits + (no). Starting with FSP specification 2.4 they can be 64-bits. config HAVE_INTEL_FSP_REPO bool diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h index c035452..72c2ce9 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/api.h +++ b/src/drivers/intel/fsp2_0/include/fsp/api.h @@ -8,6 +8,14 @@ #include <fsp/soc_binding.h> #include <soc/intel/common/mma.h> +#if CONFIG(PLATFORM_USES_FSP2_4) +#define FSPM_ARCHx_UPD FSPM_ARCH2_UPD +#define FSPS_ARCHx_UPD FSPS_ARCH2_UPD +#else +#define FSPM_ARCHx_UPD FSPM_ARCH_UPD +#define FSPS_ARCHx_UPD FSPS_ARCH_UPD +#endif + #define FSP_SUCCESS EFI_SUCCESS #define FSP_INVALID_PARAMETER EFI_INVALID_PARAMETER #define FSP_DEVICE_ERROR EFI_DEVICE_ERROR @@ -31,6 +39,11 @@ END_OF_FIRMWARE = 0xF0 }; +struct fsp_multi_phase_get_number_of_phases_params { + uint32_t number_of_phases; + uint32_t phases_executed; +}; + /* Main FSP stages */ void preload_fspm(void); void fsp_memory_init(bool s3wake); diff --git a/src/drivers/intel/fsp2_0/include/fsp/info_header.h b/src/drivers/intel/fsp2_0/include/fsp/info_header.h index fceebec7e..e8d853b 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/info_header.h +++ b/src/drivers/intel/fsp2_0/include/fsp/info_header.h @@ -12,7 +12,6 @@ #define FSP_HDR_ATTRIB_FSPS 3 #define FSP_IMAGE_ID_LENGTH 8 -#if CONFIG(PLATFORM_USES_FSP2_X86_32) struct fsp_header { uint32_t signature; //FSPH uint32_t header_length; @@ -37,10 +36,11 @@ uint32_t fsp_multi_phase_si_init_entry_offset; uint16_t extended_image_revision; uint16_t res4; -} __packed; -#else -#error You need to implement this struct for x86_64 FSP +#if CONFIG(PLATFORM_USES_FSP2_4) + uint32_t fsp_multi_phase_mem_init_entry_offset; + uint32_t fsp_smm_init_entry_offset; #endif +} __packed; enum cb_err fsp_identify(struct fsp_header *hdr, const void *fsp_blob); diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h index acf337f..65888ef6 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/util.h +++ b/src/drivers/intel/fsp2_0/include/fsp/util.h @@ -198,7 +198,7 @@ typedef asmlinkage uint32_t (*fsp_memory_init_fn) (void *raminit_upd, void **hob_list); typedef asmlinkage uint32_t (*fsp_silicon_init_fn)(void *silicon_upd); -typedef asmlinkage uint32_t (*fsp_multi_phase_si_init_fn)(struct fsp_multi_phase_params *); +typedef asmlinkage uint32_t (*fsp_multi_phase_init_fn)(struct fsp_multi_phase_params *); typedef asmlinkage uint32_t (*fsp_notify_fn)(struct fsp_notify_params *); #include <fsp/debug.h> diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index f5de5c3..9bf7e65 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -28,7 +28,17 @@ #include <intelbasecode/ramtop.h> #endif +/* Callbacks for SoC/Mainboard specific overrides */ +void __weak platform_fsp_multi_phase_init_cb(uint32_t phase_index) +{ + /* Leave for the SoC/Mainboard to implement if necessary. */ +} + +#if CONFIG(PLATFORM_USES_FSP2_X86_32) static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(sizeof(uint64_t)); +#else +static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(16); +#endif /* * Helper function to store the MRC cache version into CBMEM @@ -84,7 +94,7 @@ romstage_handoff_init(s3wake); } -static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t version) +static void fsp_fill_mrc_cache(FSPM_ARCHx_UPD *arch_upd, uint32_t version) { void *data; size_t mrc_size; @@ -127,7 +137,7 @@ return CB_SUCCESS; } -static enum cb_err setup_fsp_stack_frame(FSPM_ARCH_UPD *arch_upd, +static enum cb_err setup_fsp_stack_frame(FSPM_ARCHx_UPD *arch_upd, const struct memranges *memmap) { uintptr_t stack_begin; @@ -148,7 +158,7 @@ return CB_SUCCESS; } -static enum cb_err fsp_fill_common_arch_params(FSPM_ARCH_UPD *arch_upd, +static enum cb_err fsp_fill_common_arch_params(FSPM_ARCHx_UPD *arch_upd, bool s3wake, uint32_t version, const struct memranges *memmap) { @@ -269,12 +279,68 @@ return ver; } +static void error_handler(const char *context, uint32_t status, bool die_on_error) +{ + if (status == FSP_SUCCESS) + return; + + fsp_handle_reset(status); + if (die_on_error) + die_with_post_code(POSTCODE_RAM_FAILURE, + "%s returned with error 0x%zx!\n", + context, (size_t)status); + + printk(BIOS_SPEW, "%s returned 0x%zx\n", context, (size_t)status); +} + +#if CONFIG(PLATFORM_USES_FSP2_4) +static void multi_phase_init(const struct fsp_header *hdr) +{ + uint32_t status; + fsp_multi_phase_init_fn fsp_multi_phase_init; + struct fsp_multi_phase_params multi_phase_params; + struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; + + fsp_multi_phase_init = (fsp_multi_phase_init_fn)(uintptr_t) + (hdr->image_base + hdr->fsp_multi_phase_mem_init_entry_offset); + + post_code(POSTCODE_FSP_MULTI_PHASE_INIT_ENTRY); + timestamp_add_now(TS_FSP_MULTI_PHASE_MEM_INIT_START); + + /* Get number of phases */ + multi_phase_params.multi_phase_action = GET_NUMBER_OF_PHASES; + multi_phase_params.phase_index = 0; + multi_phase_params.multi_phase_param_ptr = &multi_phase_get_number; + status = fsp_multi_phase_init(&multi_phase_params); + error_handler("FspMultiPhaseMemInit NumberOfPhases", status, false); + + /* Execute all phases */ + for (uint32_t i = 1; i <= multi_phase_get_number.number_of_phases; i++) { + printk(BIOS_SPEW, "Executing Phase %u of FspMultiPhaseMemInit\n", i); + /* + * Give SoC/mainboard a chance to perform any operation before + * Multi Phase Execution + */ + platform_fsp_multi_phase_init_cb(i); + + multi_phase_params.multi_phase_action = EXECUTE_PHASE; + multi_phase_params.phase_index = i; + multi_phase_params.multi_phase_param_ptr = NULL; + status = fsp_multi_phase_init(&multi_phase_params); + error_handler("FspMultiPhaseMemInit Execute", status, false); + } + + post_code(POSTCODE_FSP_MULTI_PHASE_INIT_EXIT); + timestamp_add_now(TS_FSP_MULTI_PHASE_MEM_INIT_END); +} +#endif + static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake) { uint32_t status; fsp_memory_init_fn fsp_raminit; FSPM_UPD fspm_upd, *upd; - FSPM_ARCH_UPD *arch_upd; + FSPM_ARCHx_UPD *arch_upd; uint32_t version; const struct fsp_header *hdr = &context->header; const struct memranges *memmap = &context->memmap; @@ -365,11 +431,12 @@ timestamp_add_now(TS_FSP_MEMORY_INIT_END); /* Handle any errors returned by FspMemoryInit */ - fsp_handle_reset(status); - if (status != FSP_SUCCESS) { - die_with_post_code(POSTCODE_RAM_FAILURE, - "FspMemoryInit returned with error 0x%08x!\n", status); - } + error_handler("FspMemoryInit", status, true); + +#if CONFIG(PLATFORM_USES_FSP2_4) + if (hdr->fsp_multi_phase_mem_init_entry_offset) + multi_phase_init(hdr); +#endif do_fsp_post_memory_init(s3wake, version); diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index e543628..e2b4d9c 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -21,11 +21,6 @@ struct fsp_header fsps_hdr; -struct fsp_multi_phase_get_number_of_phases_params { - uint32_t number_of_phases; - uint32_t phases_executed; -}; - /* Callbacks for SoC/Mainboard specific overrides */ void __weak platform_fsp_multi_phase_init_cb(uint32_t phase_index) { @@ -84,7 +79,7 @@ static void fsp_fill_common_arch_params(FSPS_UPD *supd) { #if CONFIG(FSPS_HAS_ARCH_UPD) - FSPS_ARCH_UPD *s_arch_cfg = &supd->FspsArchUpd; + FSPS_ARCHx_UPD *s_arch_cfg = &supd->FspsArchUpd; s_arch_cfg->EnableMultiPhaseSiliconInit = fsp_is_multi_phase_init_enabled(); #endif } @@ -94,7 +89,7 @@ FSPS_UPD *upd, *supd; fsp_silicon_init_fn silicon_init; uint32_t status; - fsp_multi_phase_si_init_fn multi_phase_si_init; + fsp_multi_phase_init_fn multi_phase_si_init; struct fsp_multi_phase_params multi_phase_params; struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; @@ -174,7 +169,7 @@ if (multi_phase_si_init == NULL) return; - post_code(POSTCODE_FSP_MULTI_PHASE_SI_INIT_ENTRY); + post_code(POSTCODE_FSP_MULTI_PHASE_INIT_ENTRY); timestamp_add_now(TS_FSP_MULTI_PHASE_SI_INIT_START); /* Get NumberOfPhases Value */ multi_phase_params.multi_phase_action = GET_NUMBER_OF_PHASES; @@ -201,7 +196,7 @@ fsps_return_value_handler(FSP_MULTI_PHASE_SI_INIT_EXECUTE_PHASE_API, status); } timestamp_add_now(TS_FSP_MULTI_PHASE_SI_INIT_END); - post_code(POSTCODE_FSP_MULTI_PHASE_SI_INIT_EXIT); + post_code(POSTCODE_FSP_MULTI_PHASE_INIT_EXIT); } static void *fsps_allocator(void *arg_unused, size_t size, const union cbfs_mdata *mdata_unused) diff --git a/util/cbfstool/eventlog.c b/util/cbfstool/eventlog.c index a87ca98..04f8a4f 100644 --- a/util/cbfstool/eventlog.c +++ b/util/cbfstool/eventlog.c @@ -407,8 +407,8 @@ {POSTCODE_OS_ENTER_WAKE, "ACPI _WAK Method"}, {POSTCODE_FSP_MEMORY_EXIT, "FSP-M Exit"}, {POSTCODE_FSP_SILICON_EXIT, "FSP-S Exit"}, - {POSTCODE_FSP_MULTI_PHASE_SI_INIT_ENTRY, "FSP-S Init Enter"}, - {POSTCODE_FSP_MULTI_PHASE_SI_INIT_EXIT, "FPS-S Init Exit"}, + {POSTCODE_FSP_MULTI_PHASE_INIT_ENTRY, "FSP-M/S Multi Phase Init Enter"}, + {POSTCODE_FSP_MULTI_PHASE_INIT_EXIT, "FPS-M/S Multi Phase Init Exit"}, {POSTCODE_FSP_NOTIFY_AFTER_ENUMERATE, "FSP Notify After Enumerate"}, {POSTCODE_FSP_NOTIFY_AFTER_FINALIZE, "FSP Notify After Finalize"}, {POSTCODE_INVALID_ROM, "Invalid ROM"}, -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: mb/up/squared: Make mini PCIe port mode configurable
by Reto Buerki (Code Review) 31 Jan '24

31 Jan '24

Reto Buerki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80261?usp=email ) Change subject: mb/up/squared: Make mini PCIe port mode configurable ...................................................................... mb/up/squared: Make mini PCIe port mode configurable Add ENABLE_MSATA config knob and pad configuration to put Mini PCIe port into mSATA mode. Tested with Kingston SUV500MS120G mSATA SSD. The pad configuration has been deduced via inteltool/intelp2m by toggling the stock BIOS setting. Change-Id: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f Signed-off-by: Reto Buerki <reet(a)codelabs.ch> --- M src/mainboard/up/squared/Kconfig M src/mainboard/up/squared/gpio.h M src/mainboard/up/squared/romstage.c 3 files changed, 16 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/80261/1 diff --git a/src/mainboard/up/squared/Kconfig b/src/mainboard/up/squared/Kconfig index 91b1cbb..ee2dd12 100644 --- a/src/mainboard/up/squared/Kconfig +++ b/src/mainboard/up/squared/Kconfig @@ -74,4 +74,8 @@ bool default y +config ENABLE_MSATA + bool "Use mini-PCIe port for mSATA" + default n + endif diff --git a/src/mainboard/up/squared/gpio.h b/src/mainboard/up/squared/gpio.h index 1aecc18..876b326 100644 --- a/src/mainboard/up/squared/gpio.h +++ b/src/mainboard/up/squared/gpio.h @@ -777,4 +777,14 @@ PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_43, UP_20K, DEEP, NF1, HIZCRx0, DISPUPD), }; +/* + * Pad configuration to put MiniPCIe port into mSATA mode. + */ +static const struct pad_config msata_mode_gpio_table[] = { + /* ------- GPIO Group North-West ------- */ + + /* GPIO_213 - GPIO */ + PAD_CFG_TERM_GPO(GPIO_213, 0, UP_20K, DEEP), +}; + #endif /* CFG_GPIO_H */ diff --git a/src/mainboard/up/squared/romstage.c b/src/mainboard/up/squared/romstage.c index 12e4135..bc7a13d 100644 --- a/src/mainboard/up/squared/romstage.c +++ b/src/mainboard/up/squared/romstage.c @@ -103,6 +103,8 @@ FSP_M_CONFIG *config = &memupd->FspmConfig; gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); + if (CONFIG(ENABLE_MSATA)) + gpio_configure_pads(msata_mode_gpio_table, ARRAY_SIZE(msata_mode_gpio_table)); uint8_t memory_skuid = get_memory_skuid(); printk(BIOS_DEBUG, "MAINBOARD: Found memory SKU ID: 0x%02x\n", memory_skuid); -- To view, visit https://review.coreboot.org/c/coreboot/+/80261?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f Gerrit-Change-Number: 80261 Gerrit-PatchSet: 1 Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: soc/amd/common/data_fabric/domain: use unsigned long for resource index
by Paul Menzel (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80266?usp=email ) Change subject: soc/amd/common/data_fabric/domain: use unsigned long for resource index ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/80266?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I60ac0e30627001698565b7256421780f9a94bf65 Gerrit-Change-Number: 80266 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 31 Jan 2024 17:19:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/amd/common/block/noncar: Merge romstage into bootblock
by Paul Menzel (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Arthur Heymans, Felix Held, Fred Reitberger, Jason Glenesk, Marshall Dawson, Marvin Drees, Matt DeVillier, Raul Rangel. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64950?usp=email ) Change subject: soc/amd/common/block/noncar: Merge romstage into bootblock ...................................................................... Patch Set 29: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/64950?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I7bd5affdb7f40cbd3bd4e599c3a5cb79984fea68 Gerrit-Change-Number: 64950 Gerrit-PatchSet: 29 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Marvin Drees <marvin.drees(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Marvin Drees <marvin.drees(a)9elements.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 31 Jan 2024 17:19:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

← Newer
1
...
5
6
7
8
9
10
11
...
305
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024