coreboot-gerrit May 2023

coreboot-gerrit@coreboot.org

1 participants
5262 discussions

[S] Change in coreboot[master]: cpu/x86/smm_stub.S: Add a retpoline around the main C handler
by Nico Huber (Code Review) 02 May '23

02 May '23

Attention is currently required from: Martin L Roth, Subrata Banik, Patrick Rudolph, Benjamin Doron, Julius Werner, Maximilian Brune, Arthur Heymans, Jan Samek. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67735 ) Change subject: cpu/x86/smm_stub.S: Add a retpoline around the main C handler ...................................................................... Patch Set 2: (2 comments) File src/cpu/x86/smm/smm_stub.S: https://review.coreboot.org/c/coreboot/+/67735/comment/60aae84f_b21a7b50 PS2, Line 256: 2f > What are these offsets targeting? The `2:` below (f for forward). https://review.coreboot.org/c/coreboot/+/67735/comment/3040493b_1ac0e963 PS2, Line 268: 1b > Ditto. The `1:` above (b for backward). I didn't look into retpolines before, but by the name it's a trampoline (jumping around) using a ret(urn) somehow. It's indeed not easy to follow, here is what happens: * we have a stack frame #0 * jump to 2: * call 1: (creates a new stack frame #1) * call call_c_handler: (creates new stack frame #2) * `movq %rax, (%rsp)` overwrites the return address with c_handler that is still in `rax` since line 253 * `ret` returns to stack frame #1, but execution is in the c_handler now oO * when c_handler returns we return to frame #0, hence continue after the call in line 268 So `trap:` never gets executed and the `lfence` only instructs the speculators (that would assume that the `ret` in line 265 returns to `trap:`) to stop speculating, AIUI. -- To view, visit https://review.coreboot.org/c/coreboot/+/67735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93be3cd6a8c5f1ec29b3bc43195823ac49ec61c5 Gerrit-Change-Number: 67735 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com> Gerrit-Comment-Date: Tue, 02 May 2023 11:50:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jan Samek <jan.samek(a)siemens.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: doc/tutorial/part1.md: fix package name to install qemu on Debian
by Sean Rhodes (Code Review) 02 May '23

02 May '23

Attention is currently required from: Felix Singer, Daniel Rosa Franzini, Paul Menzel. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74894 ) Change subject: doc/tutorial/part1.md: fix package name to install qemu on Debian ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/74894/comment/b819d0a2_c33b1c38 PS2, Line 7: fix Nit: Fix -- To view, visit https://review.coreboot.org/c/coreboot/+/74894 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibae9031a3e397925db95b7283fa8c6573f6d5858 Gerrit-Change-Number: 74894 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Rosa Franzini <danielt3(a)usp.br> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Daniel Rosa Franzini <danielt3(a)usp.br> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 02 May 2023 11:31:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/common: Don't hardcode ramtop offset
by Nico Huber (Code Review) 02 May '23

02 May '23

Attention is currently required from: Sean Rhodes, Subrata Banik, Dinesh Gehlot, Lean Sheng Tan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74511 ) Change subject: soc/intel/common: Don't hardcode ramtop offset ...................................................................... Patch Set 11: (1 comment) File src/soc/intel/common/basecode/ramtop/ramtop.c: https://review.coreboot.org/c/coreboot/+/74511/comment/2f13a2b0_d23a05ea PS10, Line 19: */ > > Runtime error? […] I assume it's already a compile-time error, just telling that it's undefined. But an explicit `#error` would be nice to point people to `cmos.layout`. -- To view, visit https://review.coreboot.org/c/coreboot/+/74511 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5327b5d4e78b715a85072e5d9a62cf8fd2ae92c0 Gerrit-Change-Number: 74511 Gerrit-PatchSet: 11 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 02 May 2023 11:30:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: doc/tutorial/part1.md: fix package name to install qemu on Debian
by Nico Huber (Code Review) 02 May '23

02 May '23

Attention is currently required from: Daniel Rosa Franzini, Paul Menzel. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74894 ) Change subject: doc/tutorial/part1.md: fix package name to install qemu on Debian ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/74894 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibae9031a3e397925db95b7283fa8c6573f6d5858 Gerrit-Change-Number: 74894 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Rosa Franzini <danielt3(a)usp.br> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Daniel Rosa Franzini <danielt3(a)usp.br> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 02 May 2023 11:26:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/hp: Add new port for compaq_8300_elite_usdt
by Riku Viitanen (Code Review) 02 May '23

02 May '23

Attention is currently required from: Kyösti Mälkki. Riku Viitanen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74906 ) Change subject: mb/hp: Add new port for compaq_8300_elite_usdt ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: noticed that reboot doesn't work either. no idea why. File src/mainboard/hp/compaq_8300_elite_usdt/board_info.txt: https://review.coreboot.org/c/coreboot/+/74906/comment/84b811ee_51c974c7 PS1, Line 5: Flashrom support: n > Why not ? flashrom -p internal works on coreboot so i guess this should be a yes then? File src/mainboard/hp/compaq_8300_elite_usdt/gpio.c: https://review.coreboot.org/c/coreboot/+/74906/comment/87ba56d3_cf48defe PS1, Line 5: static const struct pch_gpio_set1 pch_gpio_set1_mode = { > Did you capture these tables with inteltool booting with OEM? Yes. I used OEM BIOS to boot Linux. Then I ran autoport which generated this file. Autoport logs are here: https://viitanen.xyz/8300usdt_autoport_logs.tar.gz -- To view, visit https://review.coreboot.org/c/coreboot/+/74906 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2dc31778c2aa1987d5acdf355973a203dd0bb3a3 Gerrit-Change-Number: 74906 Gerrit-PatchSet: 2 Gerrit-Owner: Riku Viitanen Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Tue, 02 May 2023 11:18:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/hp: Add new port for compaq_8300_elite_usdt
by Riku Viitanen (Code Review) 02 May '23

02 May '23

Attention is currently required from: Riku Viitanen. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74906 to look at the new patch set (#2). Change subject: mb/hp: Add new port for compaq_8300_elite_usdt ...................................................................... mb/hp: Add new port for compaq_8300_elite_usdt New port based on autoport. This board has NPCD379 SuperIO, which seems similar to NPCD378. So I added the SuperIO code from hp8200sff. Seems to work fine. Fan speeds stay low when idling. (Before, they went 100%) The following is tested and is working: * Native raminit with both DIMMs * Libgfxinit textmode and framebuffer on both DisplayPorts and VGA * External USB2 and USB3 ports: they all work * USB 3.0 SuperSpeed on Linux-libre (rear, 4 ports) * Ethernet * Mini-PCIe WLAN * SATA: 2.5" SSD and optical drive bay * Booting Parabola GNU/Linux with SeaBIOS 1.16.1 * PS/2 keyboard * S3 suspend and resume, wake using USB keyboard * Headphone output, line out, internal speaker * Wake on LAN Untested: * mSATA slot. The SATA port needs to be enabled on devicetree too, but I'm unable to test due to lack of hardware * Line in, mic input * MXM graphics card * PS/2 mouse * EHCI debug Not working: * Mini-PCIe USB: I couldn't get it working on vendor BIOS either, so maybe it just isn't present * PS/2 keyboard wake from S3 * Rebooting Change-Id: I2dc31778c2aa1987d5acdf355973a203dd0bb3a3 Signed-off-by: Riku Viitanen <riku.viitanen(a)protonmail.com> --- A Documentation/mainboard/hp/compaq_8300_usdt.md A Documentation/mainboard/hp/compaq_8300_usdt_rom_header.jpg A src/mainboard/hp/compaq_8300_elite_usdt/Kconfig A src/mainboard/hp/compaq_8300_elite_usdt/Kconfig.name A src/mainboard/hp/compaq_8300_elite_usdt/Makefile.inc A src/mainboard/hp/compaq_8300_elite_usdt/acpi/ec.asl A src/mainboard/hp/compaq_8300_elite_usdt/acpi/platform.asl A src/mainboard/hp/compaq_8300_elite_usdt/acpi/superio.asl A src/mainboard/hp/compaq_8300_elite_usdt/acpi_tables.c A src/mainboard/hp/compaq_8300_elite_usdt/board_info.txt A src/mainboard/hp/compaq_8300_elite_usdt/devicetree.cb A src/mainboard/hp/compaq_8300_elite_usdt/dsdt.asl A src/mainboard/hp/compaq_8300_elite_usdt/early_init.c A src/mainboard/hp/compaq_8300_elite_usdt/gma-mainboard.ads A src/mainboard/hp/compaq_8300_elite_usdt/gpio.c A src/mainboard/hp/compaq_8300_elite_usdt/hda_verb.c A src/mainboard/hp/compaq_8300_elite_usdt/mainboard.c 17 files changed, 711 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/74906/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74906 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2dc31778c2aa1987d5acdf355973a203dd0bb3a3 Gerrit-Change-Number: 74906 Gerrit-PatchSet: 2 Gerrit-Owner: Riku Viitanen Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Riku Viitanen Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: vboot: Add vboot_get_locale_id
by Hsuan-ting Chen (Code Review) 02 May '23

02 May '23

Attention is currently required from: Hsuan Ting Chen. Hello Hsuan Ting Chen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/74917 to review the following change. Change subject: vboot: Add vboot_get_locale_id ...................................................................... vboot: Add vboot_get_locale_id Add the support of vboot_get_locale_id which reads the locale id information from vb2 context in misc.h. This will be used in romstage ux for deciding the language to display. BRANCH=brya BUG=b:264666392 TEST=emerge-brya coreboot chromeos-bootimage Change-Id: I34d28c5c7fd4a929537cd07a0d4550f24a43f95e Signed-off-by: Hsuan Ting Chen <roccochen(a)chromium.org> --- M src/security/vboot/misc.h 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/74917/1 diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 8310647..262c329 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -101,4 +101,9 @@ return vb2api_hwcrypto_allowed(vboot_get_context()); } +static inline uint32_t vboot_get_locale_id(struct vb2_context *ctx) +{ + return vb2api_get_locale_id(ctx); +} + #endif /* __VBOOT_MISC_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/74917 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I34d28c5c7fd4a929537cd07a0d4550f24a43f95e Gerrit-Change-Number: 74917 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: soc/intel: Do CSE sync in romstage, unless ramstage chooses otherwise
by Subrata Banik (Code Review) 02 May '23

02 May '23

Attention is currently required from: Tarun Tuli, Jérémy Compostella, Kapil Porwal, Sridhar Siricilla. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74805 ) Change subject: soc/intel: Do CSE sync in romstage, unless ramstage chooses otherwise ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/cannonlake/Kconfig: https://review.coreboot.org/c/coreboot/+/74805/comment/554c823c_de0faa7c PS2, Line 115: if SOC_INTEL_CSE_LITE_SKU > > > When CSE Lite FW update should happen is board choice, i don't think this should part of SoC Kconfig list. This was discussed earlier with Googlers (Furquan) so we pushed it to board configuration. > > > > Are you referring below change which selects CSE lite from mainboard ? as I mentioned/showed there instances already in SoC Kconfig where SOC_INTEL_CSE_LITE_SKU is used as decision point to select other config. For low maintenance it's better to keep it inside SoC (knowing there are 100+ boards to move the romstage section). > > > > ``` > > select SOC_INTEL_CSE_LITE_SKU > > ``` > > > > let me think if there are any better ways. I believe there are still some way. > > check the new patchset. Ping! -- To view, visit https://review.coreboot.org/c/coreboot/+/74805 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3f5017fbcf917201eaf8233089050bd31c3d1917 Gerrit-Change-Number: 74805 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Tue, 02 May 2023 10:52:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/alderlake: Select FSP MULTIPHASE SI INIT RETURN BROKEN config
by Subrata Banik (Code Review) 02 May '23

02 May '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74785 ) Change subject: soc/intel/alderlake: Select FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN config ...................................................................... soc/intel/alderlake: Select FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN config At present the problem has only been reported with Alder Lake and Raptor Lake FSP where MultiPhaseSiInit API is unable to return any ERROR status. Hence, this patch ensures to select applicable W/A config to read FSP return status from the FSP Reset HOB. BUG=b:278665768 TEST=Able to select FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN for ADL/RPL SoC code and call into this API to know the return status from MultiPhaseSiInit FSP API. Without this patch: IshInit() Start IshDisable() Start IshPerformGlobalReset() .... .... FSP returning control to Bootloader with reset required return status 40000003 FspMultiPhaseSiInit Index-1 returned 0 <-- after control returns into coreboot, the `status` from the FSP API is reset to `0` instead 0x40000003. Hence, coreboot avoid hitting the reset. With this patch: IshInit() Start IshDisable() Start IshPerformGlobalReset() .... .... FSP returning control to Bootloader with reset required return status 40000003 FSP: handling reset type 40000003 <-- coreboot is able to understand the reset request in proper. GLOBAL RESET! global_reset() called! HECI: Global Reset(Type:1) Command Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I18a918cca7e19e03ed6020c55c86c64a94212963 Reviewed-on: https://review.coreboot.org/c/coreboot/+/74785 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> --- M src/soc/intel/alderlake/Kconfig 1 file changed, 55 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Eric Lai: Looks good to me, approved Tarun Tuli: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index cce2052..0e5a671 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -61,6 +61,7 @@ select FSP_COMPRESS_FSP_S_LZ4 select FAST_SPI_SUPPORTS_EXT_BIOS_WINDOW select FSP_M_XIP + select FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select FSP_USES_CB_DEBUG_EVENT_HANDLER select FSPS_HAS_ARCH_UPD -- To view, visit https://review.coreboot.org/c/coreboot/+/74785 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I18a918cca7e19e03ed6020c55c86c64a94212963 Gerrit-Change-Number: 74785 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Apply FSP Reset Status W/A for MultiPhaseSiInit
by Subrata Banik (Code Review) 02 May '23

02 May '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74784 ) Change subject: drivers/intel/fsp2_0: Apply FSP Reset Status W/A for MultiPhaseSiInit ...................................................................... drivers/intel/fsp2_0: Apply FSP Reset Status W/A for MultiPhaseSiInit This patch calls into fsp_get_pch_reset_status() to get the MultiPhaseSiInit API return status if FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN is enabled. Ideally FSP API should be able to return the status (both success and error code) upon exiting the FSP API but unfortunately there are some scenarios in ADL/RPL FSP where MultiPhaseSiInit API is unable to return any ERROR status. Hence, this function can be considered as an additional hook to read the FSP reset status by reading the dedicated HOB without relying on the FSP API exit status code. Any SoC platform that selects the FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN config will call into this newly added API to get the FSP return status from MultiPhaseSiInit. BUG=b:278665768 TEST=Able to select FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN for ADL/RPL SoC code and call into this API to know the return status from MultiPhaseSiInit FSP API. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I749c9986e17e4cbab333b29425c9a4a4ba4128fa Reviewed-on: https://review.coreboot.org/c/coreboot/+/74784 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Himanshu Sahdev <himanshu.sahdev(a)intel.com> --- M src/drivers/intel/fsp2_0/silicon_init.c 1 file changed, 40 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Eric Lai: Looks good to me, approved Tarun Tuli: Looks good to me, approved Kapil Porwal: Looks good to me, approved Himanshu Sahdev: Looks good to me, but someone else must approve diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 678892a..9505e1d 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -10,6 +10,7 @@ #include <fsp/api.h> #include <fsp/util.h> #include <program_loading.h> +#include <soc/intel/common/reset.h> #include <soc/intel/common/vbt.h> #include <stage_cache.h> #include <string.h> @@ -194,6 +195,8 @@ multi_phase_params.phase_index = i; multi_phase_params.multi_phase_param_ptr = NULL; status = multi_phase_si_init(&multi_phase_params); + if (CONFIG(FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN)) + status = fsp_get_pch_reset_status(); fsps_return_value_handler(FSP_MULTI_PHASE_SI_INIT_EXECUTE_PHASE_API, status); } timestamp_add_now(TS_FSP_MULTI_PHASE_SI_INIT_END); -- To view, visit https://review.coreboot.org/c/coreboot/+/74784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I749c9986e17e4cbab333b29425c9a4a4ba4128fa Gerrit-Change-Number: 74784 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.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 May 2023