coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[S] Change in coreboot[main]: soc/intel/tigerlake: Add IRQ mapping for PEG PCI-E ports
by Felix Singer (Code Review) 05 Mar '24

05 Mar '24

Attention is currently required from: Alicja Michalska, Arthur Heymans, Martin L Roth, Nick Vaccaro. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80848?usp=email ) Change subject: soc/intel/tigerlake: Add IRQ mapping for PEG PCI-E ports ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80848?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: If102522efa1a67b362b14d859d9e27a37bad85a4 Gerrit-Change-Number: 80848 Gerrit-PatchSet: 1 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 05 Mar 2024 22:49:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/tigerlake: Add IRQ mapping for PEG PCI-E ports
by Alicja Michalska (Code Review) 05 Mar '24

05 Mar '24

Attention is currently required from: Arthur Heymans, Martin L Roth, Nick Vaccaro. Alicja Michalska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80848?usp=email ) Change subject: soc/intel/tigerlake: Add IRQ mapping for PEG PCI-E ports ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Hi, It looks like you haven't contributed to the coreboot project before. […] Hello Martin! Thank you for warm welcome ❤️ It's my first time submitting patches via gerrit. So far, I've only been submitting them to/telling MrChromebox (Matt) what to change, and those patches eventually made it to upstream in one form or another. I'm *mostly* familiar with coding style, as it's very similar to Linux (and I do contribute to the Linux kernel). I'll read the guidelines you've linked to though. You can find me on OSFW Slack under my usual handle, '@elly' if you need more information. As for merging those patches, I've tested them with TGL-H (port I just submitted), as well as TGL-UP (Google/Volteer/ELDRID, which doesn't have PCIE4.0) for sanity check and I don't see any regressions whatsoever. Currently have the system running with patch applied - Coreboot 24.02, Linux 6.7.7, and 4 days of uptime. PEG0 == 06.0: +-06.0-[04]----00.0 Kingston Technology Company, Inc. KC3000/FURY Renegade NVMe SSD E18 [2646:5013] Subsystem: Kingston Technology Company, Inc. KC3000/FURY Renegade NVMe SSD E18 [2646:5013] Interrupt: pin A routed to IRQ 16 Previous patch in this series is required for Coreboot to recognize the CPU family - otherwise booting the platform with pre-production stepping (TGL-H D0) results in [EMERG] Unknown CPU. Of course Intel doesn't carry microcode for that stepping in their tree, but it can be found publicly. I spoke to Felix during FOSDEM and he assured me merging this shouldn't be a problem since it's just a CPUID definition 😊 -- To view, visit https://review.coreboot.org/c/coreboot/+/80848?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: If102522efa1a67b362b14d859d9e27a37bad85a4 Gerrit-Change-Number: 80848 Gerrit-PatchSet: 1 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 05 Mar 2024 22:45:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support
by Martin L Roth (Code Review) 05 Mar '24

05 Mar '24

Attention is currently required from: Andrey Petrov, Bora Guvendik, Chen, Gang C, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, Ronak Kanabar, Shuo Liu, Subrata Banik, Tarun, Wonkyu Kim. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80275?usp=email ) Change subject: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support ...................................................................... Patch Set 21: Code-Review+1 (1 comment) Patchset: PS21: Kconfig looks good -- 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: 21 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: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 05 Mar 2024 21:44:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Move MEM_ADDR_64MB_SHIFT_BITS to Xeon-SP
by Lean Sheng Tan (Code Review) 05 Mar '24

05 Mar '24

Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80634?usp=email ) Change subject: soc/intel/xeon_sp: Move MEM_ADDR_64MB_SHIFT_BITS to Xeon-SP ...................................................................... soc/intel/xeon_sp: Move MEM_ADDR_64MB_SHIFT_BITS to Xeon-SP Move MEM_ADDR_64MB_SHIFT_BITS from FSP headers to Xeon-SP common layer to reduce the dependency. TEST=intel/archercity CRB Change-Id: I4e1a652ad58233f7514cb9b23813d75144b8d435 Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80634 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> --- M src/soc/intel/xeon_sp/include/soc/util.h M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h M src/vendorcode/intel/fsp/fsp2_0/sapphirerapids_sp/defs_memmap.h M src/vendorcode/intel/fsp/fsp2_0/skylake_sp/hob_memmap.h 4 files changed, 2 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved diff --git a/src/soc/intel/xeon_sp/include/soc/util.h b/src/soc/intel/xeon_sp/include/soc/util.h index 43bf9d6..dd84f0c 100644 --- a/src/soc/intel/xeon_sp/include/soc/util.h +++ b/src/soc/intel/xeon_sp/include/soc/util.h @@ -6,6 +6,8 @@ #include <cpu/x86/msr.h> #include <hob_iiouds.h> +#define MEM_ADDR_64MB_SHIFT_BITS 26 + void lock_pam0123(void); void unlock_pam_regions(void); diff --git a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h index 573b5c3..4a74710 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h +++ b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h @@ -57,7 +57,6 @@ #define MAX_IMC_PER_SOCKET 2 #define MEM_TYPE_RESERVED (1 << 8) -#define MEM_ADDR_64MB_SHIFT_BITS 26 #define NGN_MAX_SERIALNUMBER_STRLEN 4 #define NGN_MAX_PARTNUMBER_STRLEN 20 diff --git a/src/vendorcode/intel/fsp/fsp2_0/sapphirerapids_sp/defs_memmap.h b/src/vendorcode/intel/fsp/fsp2_0/sapphirerapids_sp/defs_memmap.h index 89897bc..0b8ac73 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/sapphirerapids_sp/defs_memmap.h +++ b/src/vendorcode/intel/fsp/fsp2_0/sapphirerapids_sp/defs_memmap.h @@ -46,7 +46,6 @@ #define MEMTYPE_VOLATILE_MASK (MEMTYPE_1LM_MASK | MEMTYPE_2LM_MASK) #define MEM_TYPE_RESERVED (1 << 8) -#define MEM_ADDR_64MB_SHIFT_BITS 26 //------------------------------------------------------------------------------------ // Uncomment line(s) below to override macro definitions in FSP MemoryMapDataHob.h diff --git a/src/vendorcode/intel/fsp/fsp2_0/skylake_sp/hob_memmap.h b/src/vendorcode/intel/fsp/fsp2_0/skylake_sp/hob_memmap.h index c35b2f6..f97056a 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/skylake_sp/hob_memmap.h +++ b/src/vendorcode/intel/fsp/fsp2_0/skylake_sp/hob_memmap.h @@ -43,7 +43,6 @@ #define MAX_IMC_PER_SOCKET 2 #define MEM_TYPE_RESERVED (1 << 8) -#define MEM_ADDR_64MB_SHIFT_BITS 26 // // System Memory Map HOB information -- To view, visit https://review.coreboot.org/c/coreboot/+/80634?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: I4e1a652ad58233f7514cb9b23813d75144b8d435 Gerrit-Change-Number: 80634 Gerrit-PatchSet: 5 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[master]: mb/google/rex/var/screebo: Prevent camera LED blinking during boot
by Martin L Roth (Code Review) 05 Mar '24

05 Mar '24

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/79767?usp=email ) Change subject: mb/google/rex/var/screebo: Prevent camera LED blinking during boot ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/79767?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I43044e64c2c3a645ec0cad2ac903cc19ac89c9af Gerrit-Change-Number: 79767 Gerrit-PatchSet: 1 Gerrit-Owner: Jason Z Chen <jason.z.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[S] Change in coreboot[master]: mb/google/nipperkin: change WLAN's PSPP setting to default
by Martin L Roth (Code Review) 05 Mar '24

05 Mar '24

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/79125?usp=email ) Change subject: mb/google/nipperkin: change WLAN's PSPP setting to default ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/79125?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1bd1e30cf73130b1a7fefa13a420a584c3beb7a6 Gerrit-Change-Number: 79125 Gerrit-PatchSet: 1 Gerrit-Owner: Jason Nein <finaljason(a)gmail.com> Gerrit-MessageType: abandon

1 0

[XS] Change in coreboot[main]: mb/google/oak: Don't build the ChromeEC codebase by default
by Felix Singer (Code Review) 05 Mar '24

05 Mar '24

Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81023?usp=email ) Change subject: mb/google/oak: Don't build the ChromeEC codebase by default ...................................................................... mb/google/oak: Don't build the ChromeEC codebase by default Currently, the oak boards are the only boards that build the ChromeEC by default as a part of the coreboot build. As a part of replacing the chromeec submodule with a different build mechanism, disable this default. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: Idd4fe45e52dbdd1c8dccf0d2c09d5cf6d61aa839 Reviewed-on: https://review.coreboot.org/c/coreboot/+/81023 Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Julius Werner <jwerner(a)chromium.org> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Reviewed-by: Yidi Lin <yidilin(a)google.com> --- M src/mainboard/google/oak/Kconfig 1 file changed, 0 insertions(+), 8 deletions(-) Approvals: Yu-Ping Wu: Looks good to me, approved Matt DeVillier: Looks good to me, approved Hung-Te Lin: Looks good to me, but someone else must approve build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Yidi Lin: Looks good to me, approved diff --git a/src/mainboard/google/oak/Kconfig b/src/mainboard/google/oak/Kconfig index 3eba1f0..07c22d4 100644 --- a/src/mainboard/google/oak/Kconfig +++ b/src/mainboard/google/oak/Kconfig @@ -46,14 +46,6 @@ int default 9 -config EC_GOOGLE_CHROMEEC_BOARDNAME - string - default "oak" - -config EC_GOOGLE_CHROMEEC_PD_BOARDNAME - string - default "oak_pd" - ########################################################## #### Update below when adding a new derivative board. #### ########################################################## -- To view, visit https://review.coreboot.org/c/coreboot/+/81023?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: Idd4fe45e52dbdd1c8dccf0d2c09d5cf6d61aa839 Gerrit-Change-Number: 81023 Gerrit-PatchSet: 3 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: mb/google/brox: Enable Wake on WLAN for SKU1
by Shelley Chen (Code Review) 05 Mar '24

05 Mar '24

Attention is currently required from: Ashish Kumar Mishra, Subrata Banik. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80780?usp=email ) Change subject: mb/google/brox: Enable Wake on WLAN for SKU1 ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80780?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: I04c35da2c9ac57cafdf7f7a35d83ab2e7a05fe4a Gerrit-Change-Number: 80780 Gerrit-PatchSet: 1 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.corp-partner.google.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Tue, 05 Mar 2024 21:33:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp: Work around multi-socket Xeon-SP pipe init bug
by Jérémy Compostella (Code Review) 05 Mar '24

05 Mar '24

Attention is currently required from: Arthur Heymans. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80579?usp=email ) Change subject: drivers/intel/fsp: Work around multi-socket Xeon-SP pipe init bug ...................................................................... Patch Set 7: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80579/comment/f6423ddd_461130aa : PS7, Line 174: & CONFIG(FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND)) { : extern char _fspm_heap[]; : extern char _efspm_heap[]; Shouldn't they be declared in `src/include/symbols.h` ? At the very minimum I suggest that you use the `DECLARE_REGION` macro. -- To view, visit https://review.coreboot.org/c/coreboot/+/80579?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: I38a4f4b7470556e528a1672044c31f8bd92887d4 Gerrit-Change-Number: 80579 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Ray Ni <ray.ni(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 05 Mar 2024 21:31:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp: Work around multi-socket Xeon-SP pipe init bug
by Nico Huber (Code Review) 05 Mar '24

05 Mar '24

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80579?usp=email ) Change subject: drivers/intel/fsp: Work around multi-socket Xeon-SP pipe init bug ...................................................................... drivers/intel/fsp: Work around multi-socket Xeon-SP pipe init bug Starting with Intel CPX there is a bug in the reference code during the Pipe init. This code synchronises the CAR between sockets in FSP-M. This code implicitly assumes that the FSP heap is right above the RC heap, where both of them are located at the bottom part of CAR. Work around this issue by making that implicit assumption done in FSP explicit in the coreboot linker script and allocation. TEST=intel/archercity CRB Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Change-Id: I38a4f4b7470556e528a1672044c31f8bd92887d4 Reviewed-on: https://review.coreboot.org/c/coreboot/+/80579 Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Shuo Liu <shuo.liu(a)intel.com> --- M src/arch/x86/car.ld M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/memory_init.c M src/soc/intel/xeon_sp/cpx/Kconfig M src/soc/intel/xeon_sp/spr/Kconfig 5 files changed, 25 insertions(+), 2 deletions(-) Approvals: Shuo Liu: Looks good to me, but someone else must approve build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved Nico Huber: Looks good to me, approved diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld index 17e6eea..8e1af15 100644 --- a/src/arch/x86/car.ld +++ b/src/arch/x86/car.ld @@ -9,6 +9,10 @@ .car.data . (NOLOAD) : { _car_region_start = . ; . += CONFIG_FSP_M_RC_HEAP_SIZE; +#if CONFIG(FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND) + REGION(fspm_heap, ., CONFIG_FSP_TEMP_RAM_SIZE, 16) +#endif + #if CONFIG(PAGING_IN_CACHE_AS_RAM) /* Page table pre-allocation. CONFIG_DCACHE_RAM_BASE should be 4KiB * aligned when using this option. */ @@ -107,7 +111,7 @@ . = _car_region_start; .car.fspm_rc_heap . (NOLOAD) : { -. += CONFIG_FSP_M_RC_HEAP_SIZE; + . += CONFIG_FSP_M_RC_HEAP_SIZE; } . = _car_region_end; diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index e27249f..274c3e5 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -221,6 +221,17 @@ without reinitializing stack pointer. This feature is supported Icelake onwards. +config FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND + bool + help + Starting with Intel CPX there is a bug in there reference code during + the pipe init. This code synchronises the CAR between sockets in FSP-M. + This code implicitly assumes that the FSP heap is right above the + RC heap, where both of them are located at the bottom part of CAR. + Select this to have an explicit handling of the FSP StackBase to work + around this issue. This is needed on multi-socket Xeon-SP systems. + This will place the FSP heap right above the FSP-M RC heap. + config FSP_TEMP_RAM_SIZE hex help diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index f5de5c3..c096e86 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -170,7 +170,13 @@ [FSP_BOOT_IN_RECOVERY_MODE] = "boot in recovery mode", }; - if (CONFIG(FSP_USES_CB_STACK) || !ENV_CACHE_AS_RAM) { + if (CONFIG(FSP_USES_CB_STACK) && ENV_RAMINIT + && CONFIG(FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND)) { + extern char _fspm_heap[]; + extern char _efspm_heap[]; + arch_upd->StackBase = (uintptr_t)_fspm_heap; + arch_upd->StackSize = (size_t)(_efspm_heap - _fspm_heap); + } else if (CONFIG(FSP_USES_CB_STACK) || !ENV_CACHE_AS_RAM) { arch_upd->StackBase = (uintptr_t)temp_ram; arch_upd->StackSize = sizeof(temp_ram); } else if (setup_fsp_stack_frame(arch_upd, memmap)) { diff --git a/src/soc/intel/xeon_sp/cpx/Kconfig b/src/soc/intel/xeon_sp/cpx/Kconfig index ac166c3..b9d63d3 100644 --- a/src/soc/intel/xeon_sp/cpx/Kconfig +++ b/src/soc/intel/xeon_sp/cpx/Kconfig @@ -7,6 +7,7 @@ select CACHE_MRC_SETTINGS select NO_FSP_TEMP_RAM_EXIT select HAVE_INTEL_FSP_REPO + select FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND help Intel Cooper Lake-SP support diff --git a/src/soc/intel/xeon_sp/spr/Kconfig b/src/soc/intel/xeon_sp/spr/Kconfig index bb88bec..ace5c07 100644 --- a/src/soc/intel/xeon_sp/spr/Kconfig +++ b/src/soc/intel/xeon_sp/spr/Kconfig @@ -13,6 +13,7 @@ select SOC_INTEL_CSE_SERVER_SKU select XEON_SP_COMMON_BASE select HAVE_IOAT_DOMAINS + select FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND help Intel Sapphire Rapids-SP support -- To view, visit https://review.coreboot.org/c/coreboot/+/80579?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: I38a4f4b7470556e528a1672044c31f8bd92887d4 Gerrit-Change-Number: 80579 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Ray Ni <ray.ni(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024