coreboot-gerrit August 2024

coreboot-gerrit@coreboot.org

1 participants
3913 discussions

[XS] Change in coreboot[main]: payloads/edk2: set VARIABLE_SUPPORT=SMMSTORE on CONFIG_SMMSTORE_V2
by Sergii Dmytruk (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Matt DeVillier, Sean Rhodes. Sergii Dmytruk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83737?usp=email ) Change subject: payloads/edk2: set VARIABLE_SUPPORT=SMMSTORE on CONFIG_SMMSTORE_V2 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > this is already set for my repo via `payloads/external/edk2/Kconfig` ln 322, I'd just drop that cond […] Thanks, I did not look there, thus decided to move this option to `Makefile` for consistency. Also `CONFIG_SMMSTORE_V2` is the correct option to check given that EDK won't work with version 1. -- To view, visit https://review.coreboot.org/c/coreboot/+/83737?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic59f89c0f708f9b144bd35cd18870d0e1c65677d Gerrit-Change-Number: 83737 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Fri, 02 Aug 2024 17:26:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[S] Change in coreboot[main]: mb/google/hatch: Add FP enable for Dratini
by Jon Murphy (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Nick Vaccaro, Subrata Banik. Jon Murphy has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/83745?usp=email ) Change subject: mb/google/hatch: Add FP enable for Dratini ...................................................................... Patch Set 4: (1 comment) File src/mainboard/google/hatch/variants/dratini/ramstage.c: https://review.coreboot.org/c/coreboot/+/83745/comment/c00fc77b_3dc1fbf7?us… : PS4, Line 29: } > do we need a default case for passing code static analyser check ? […] The behavior change is desired and yes, it's because not all SKUs have a fingerprint sensor. Some devices were manufactured with reused MLBs from devices that had fingerprint, and the FPMCU was populated. Because of this, and because the AP FW indiscriminately enables power to the FPMCU, the FPMCU is present on the i2c bus and causes boid to errantly report FP issues and crashes. Happy to add a default case variant.c. WDYT? -- To view, visit https://review.coreboot.org/c/coreboot/+/83745?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifc450f51b00b9c3b62268ce94884f5749a3e18c0 Gerrit-Change-Number: 83745 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.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-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Aug 2024 17:26:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[XS] Change in coreboot[main]: payloads/edk2: set VARIABLE_SUPPORT=SMMSTORE on CONFIG_SMMSTORE_V2
by Sergii Dmytruk (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Sean Rhodes, Sergii Dmytruk. Hello Benjamin Doron, Lean Sheng Tan, Martin L Roth, Matt DeVillier, Sean Rhodes, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83737?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: payloads/edk2: set VARIABLE_SUPPORT=SMMSTORE on CONFIG_SMMSTORE_V2 ...................................................................... payloads/edk2: set VARIABLE_SUPPORT=SMMSTORE on CONFIG_SMMSTORE_V2 Official EDK2 repository has VARIABLE_SUPPORT defaulting to EMU in UefiPayloadPkg, switch it to SMMSTORE if coreboot is built with SMMSTOREv2. This removes custom default of EDK2_CUSTOM_BUILD_PARAMS for EDK2_REPO_MRCHROMEBOX which is unnecessary now. Change-Id: Ic59f89c0f708f9b144bd35cd18870d0e1c65677d Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M payloads/external/edk2/Kconfig M payloads/external/edk2/Makefile 2 files changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/83737/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83737?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic59f89c0f708f9b144bd35cd18870d0e1c65677d Gerrit-Change-Number: 83737 Gerrit-PatchSet: 2 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[S] Change in coreboot[main]: [WIP] soc/amd/common/psp: add and call PSP SMI SPI access function stubs
by Felix Held (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: ritul guru. Hello ritul guru, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/83756?usp=email to review the following change. Change subject: [WIP] soc/amd/common/psp: add and call PSP SMI SPI access function stubs ...................................................................... [WIP] soc/amd/common/psp: add and call PSP SMI SPI access function stubs Add stub functions for the SPI flash access from the PSP SMI handler and call them for the corresponding P2C mailbox commands. Parts of this patch are taken from CB:65523. Document #55758 Rev. 2.04 was used as a reference. Test=TODO, but when selecting SOC_AMD_COMMON_BLOCK_PSP_SMI, Mandolin still builds Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Signed-off-by: Ritul Guru <ritul.bits(a)gmail.com> Change-Id: Iedbc9d41eb0d4e8d81eeba9c01281161eb839991 --- M src/soc/amd/common/block/psp/Makefile.mk M src/soc/amd/common/block/psp/psp_def.h M src/soc/amd/common/block/psp/psp_smi.c A src/soc/amd/common/block/psp/psp_smi_flash.c 4 files changed, 49 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/83756/1 diff --git a/src/soc/amd/common/block/psp/Makefile.mk b/src/soc/amd/common/block/psp/Makefile.mk index c16a73b..b72343c 100644 --- a/src/soc/amd/common/block/psp/Makefile.mk +++ b/src/soc/amd/common/block/psp/Makefile.mk @@ -5,6 +5,7 @@ ramstage-y += psp.c smm-y += psp.c smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_SMI) += psp_smi.c +smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_SMI) += psp_smi_flash.c smm-y += psp_smm.c bootblock-y += psp_efs.c diff --git a/src/soc/amd/common/block/psp/psp_def.h b/src/soc/amd/common/block/psp/psp_def.h index d58aa0b..c2b2fca 100644 --- a/src/soc/amd/common/block/psp/psp_def.h +++ b/src/soc/amd/common/block/psp/psp_def.h @@ -133,4 +133,9 @@ void enable_psp_smi(void); +enum mbox_p2c_status psp_smi_spi_get_info(struct mbox_default_buffer *buffer); +enum mbox_p2c_status psp_smi_spi_read(struct mbox_default_buffer *buffer); +enum mbox_p2c_status psp_smi_spi_write(struct mbox_default_buffer *buffer); +enum mbox_p2c_status psp_smi_spi_erase(struct mbox_default_buffer *buffer); + #endif /* __AMD_PSP_DEF_H__ */ diff --git a/src/soc/amd/common/block/psp/psp_smi.c b/src/soc/amd/common/block/psp/psp_smi.c index b5a8cf4..43e82d4 100644 --- a/src/soc/amd/common/block/psp/psp_smi.c +++ b/src/soc/amd/common/block/psp/psp_smi.c @@ -8,6 +8,12 @@ #include <types.h> #include "psp_def.h" +/* PSP to x86 commands */ +#define MBOX_PSP_CMD_SPI_INFO 0x83 +#define MBOX_PSP_CMD_SPI_READ 0x84 +#define MBOX_PSP_CMD_SPI_WRITE 0x85 +#define MBOX_PSP_CMD_SPI_ERASE 0x86 + extern struct { u8 buffer[P2C_BUFFER_MAXSIZE]; } __aligned(32) p2c_buffer; @@ -126,6 +132,7 @@ { enum mbox_p2c_status status; u32 cmd; + struct mbox_default_buffer *const buffer = get_psp_command_buffer(); status = check_psp_command(); if (status != MBOX_PSP_SUCCESS) @@ -134,6 +141,18 @@ cmd = get_psp_command(); switch (cmd) { + case MBOX_PSP_CMD_SPI_INFO: + status = psp_smi_spi_get_info(buffer); + break; + case MBOX_PSP_CMD_SPI_READ: + status = psp_smi_spi_read(buffer); + break; + case MBOX_PSP_CMD_SPI_WRITE: + status = psp_smi_spi_write(buffer); + break; + case MBOX_PSP_CMD_SPI_ERASE: + status = psp_smi_spi_erase(buffer); + break; default: printk(BIOS_ERR, "PSP: Unknown command %d\n", cmd); status = MBOX_PSP_UNSUPPORTED; diff --git a/src/soc/amd/common/block/psp/psp_smi_flash.c b/src/soc/amd/common/block/psp/psp_smi_flash.c new file mode 100644 index 0000000..b624b7b --- /dev/null +++ b/src/soc/amd/common/block/psp/psp_smi_flash.c @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <types.h> +#include "psp_def.h" + +enum mbox_p2c_status psp_smi_spi_get_info(struct mbox_default_buffer *buffer) +{ + return MBOX_PSP_UNSUPPORTED; +} + +enum mbox_p2c_status psp_smi_spi_read(struct mbox_default_buffer *buffer) +{ + return MBOX_PSP_UNSUPPORTED; +} + +enum mbox_p2c_status psp_smi_spi_write(struct mbox_default_buffer *buffer) +{ + return MBOX_PSP_UNSUPPORTED; +} + +enum mbox_p2c_status psp_smi_spi_erase(struct mbox_default_buffer *buffer) +{ + return MBOX_PSP_UNSUPPORTED; +} -- To view, visit https://review.coreboot.org/c/coreboot/+/83756?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iedbc9d41eb0d4e8d81eeba9c01281161eb839991 Gerrit-Change-Number: 83756 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>

1 0

[XL] Change in coreboot[main]: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
by Subrata Banik (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Singer, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Saurabh Mishra, Tarun. Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83354?usp=email ) Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock ...................................................................... Patch Set 36: (11 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/83354/comment/b0266911_44897cb9?us… : PS36, Line 16: Did you missed to highlighted that this CL also includes minimal code requires to compile the PTL SoC and mainbaord? https://review.coreboot.org/c/coreboot/+/83354/comment/9c9b4ea2_b4ee9336?us… : PS36, Line 18: PSS specify the acronyms https://review.coreboot.org/c/coreboot/+/83354/comment/6af3c821_bcc1a6c8?us… : PS36, Line 18: v V? File src/soc/intel/pantherlake/Kconfig: https://review.coreboot.org/c/coreboot/+/83354/comment/0c5d196a_fd76a870?us… : PS36, Line 53: 22 I guess we need to revisit this. 22 is too high for PTL-UH SKUs https://review.coreboot.org/c/coreboot/+/83354/comment/590e478b_2f90585b?us… : PS36, Line 133: 10 as per Intel doc (731941), i read as `6 ports USB 2.0` File src/soc/intel/pantherlake/bootblock/pcd.c: https://review.coreboot.org/c/coreboot/+/83354/comment/16dc7c01_b561ae6b?us… : PS36, Line 129: if (pmc_reg_value != 0xffffffff) { thoughts ? ``` if (pmc_reg_value == 0xffffffff) { // show the warning msg as console is already initialised. This will help us incase PCR_PSF8_TO_SHDW_PMC_REG_BASE is not proper or mistake in programming. return; } ``` File src/soc/intel/pantherlake/bootblock/report_platform.c: https://review.coreboot.org/c/coreboot/+/83354/comment/6601d468_8073d50c?us… : PS36, Line 39: { PCI_DID_INTEL_PTL_U_H_ESPI_0, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_1, "Pantherlake SOC-U SuperSKU" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_2, "Pantherlake SOC-U Premium" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_3, "Pantherlake SOC-U Base" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_4, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_5, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_6, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_7, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_8, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_9, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_10, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_11, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_12, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_13, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_14, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_15, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_16, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_17, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_18, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_19, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_20, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_21, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_22, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_23, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_24, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_25, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_26, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_27, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_28, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_29, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_30, "Pantherlake SOC-U" }, : { PCI_DID_INTEL_PTL_U_H_ESPI_31, "Pantherlake SOC-U" }, should be specify these as "Panther Lake SoC-UH" https://review.coreboot.org/c/coreboot/+/83354/comment/c4102a9c_d0ca17d2?us… : PS36, Line 109: GT2 name and macro description are not matching https://review.coreboot.org/c/coreboot/+/83354/comment/d6a0df25_eca656e7?us… : PS36, Line 110: GT2 same https://review.coreboot.org/c/coreboot/+/83354/comment/8ccdbca9_109073d2?us… : PS36, Line 114: static inline uint8_t get_dev_revision(pci_devfn_t dev) : { : return pci_read_config8(dev, PCI_REVISION_ID); : } : : static inline uint16_t get_dev_id(pci_devfn_t dev) : { : return pci_read_config16(dev, PCI_DEVICE_ID); : } : may be good topic for common code File src/soc/intel/pantherlake/include/soc/soc_info.h: https://review.coreboot.org/c/coreboot/+/83354/comment/e960d333_2d20709e?us… : PS36, Line 6: uint8_t get_soctype(void); why we still have this ? -- To view, visit https://review.coreboot.org/c/coreboot/+/83354?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca Gerrit-Change-Number: 83354 Gerrit-PatchSet: 36 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 02 Aug 2024 17:10:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: soc/amd: add PSP SMI handler stub
by Nico Huber (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Varshit Pandya, ritul guru. Nico Huber has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83739?usp=email ) Change subject: soc/amd: add PSP SMI handler stub ...................................................................... Patch Set 3: Code-Review+2 (2 comments) File src/soc/amd/cezanne/smihandler.c: https://review.coreboot.org/c/coreboot/+/83739/comment/48943fa0_f218cbab?us… : PS2, Line 116: psp_smi_handler > oh, looks like i missed the 'in the header part' when reading your comment. […] :) File src/soc/amd/common/block/psp/psp_smm.c: https://review.coreboot.org/c/coreboot/+/83739/comment/851fbbfe_999426e0?us… : PS2, Line 94: configure_psp_smi(); > it's not necessary to do this in smm; however the next patch CB:83740 adds the enable_psp_smi call w […] Ack. Just checking that this wasn't a mistake. -- To view, visit https://review.coreboot.org/c/coreboot/+/83739?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I65989ff529d728cd9d2cd60b384295417bef77ad Gerrit-Change-Number: 83739 Gerrit-PatchSet: 3 Gerrit-Owner: 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: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: ritul guru <ritul.bits(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: Fri, 02 Aug 2024 17:04:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: mb/google/hatch: Add FP enable for Dratini
by Subrata Banik (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Jon Murphy, Nick Vaccaro. Subrata Banik has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/83745?usp=email ) Change subject: mb/google/hatch: Add FP enable for Dratini ...................................................................... Patch Set 4: (1 comment) File src/mainboard/google/hatch/variants/dratini/ramstage.c: https://review.coreboot.org/c/coreboot/+/83745/comment/82fc315a_030a4bd1?us… : PS4, Line 29: } do we need a default case for passing code static analyser check ? Also the behaviour of the platform would now change with this CL as in earlier all SKU might have configured the C11 and A12 (i assume that is not required due to not all SKUs are with FP )? -- To view, visit https://review.coreboot.org/c/coreboot/+/83745?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifc450f51b00b9c3b62268ce94884f5749a3e18c0 Gerrit-Change-Number: 83745 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.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-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Aug 2024 17:00:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: mb/google/hatch: Add FP enable for Dratini
by Jon Murphy (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Nick Vaccaro, Subrata Banik. Hello Nick Vaccaro, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83745?usp=email to look at the new patch set (#4). Change subject: mb/google/hatch: Add FP enable for Dratini ...................................................................... mb/google/hatch: Add FP enable for Dratini Add FP enable/disable based on SKU ID for Dratini. This is meant to resolve a UMA issue with Dratini devices that had the FPMCU populated on non-fp devices. Since the FPMCU is present, and the firmware enables the power GPIO's based on variant, not SKU, the devices were reporting data on fingerprint errantly. BUG=b:354769653 BUG=b:200825114 TEST=Flash to Dratini, test FP. Disable test SKU, flash on Dratini, test FP. Change-Id: Ifc450f51b00b9c3b62268ce94884f5749a3e18c0 Signed-off-by: Jon Murphy <jpmurphy(a)google.com> --- M src/mainboard/google/hatch/variants/dratini/include/variant/sku.h M src/mainboard/google/hatch/variants/dratini/ramstage.c 2 files changed, 29 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/83745/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/83745?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifc450f51b00b9c3b62268ce94884f5749a3e18c0 Gerrit-Change-Number: 83745 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.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-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>

1 0

[L] Change in coreboot[main]: vc/intel/fsp2_0: Add Skeleton FSP header for PTL
by Subrata Banik (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Saurabh Mishra. Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83732?usp=email ) Change subject: vc/intel/fsp2_0: Add Skeleton FSP header for PTL ...................................................................... Patch Set 4: (7 comments) Patchset: PS4: s Commit Message: https://review.coreboot.org/c/coreboot/+/83732/comment/bee804fb_ceb4b48e?us… : PS4, Line 7: vc/intel/fsp2_0 ``` vc/intel/fsp/fsp2_0/ptl: Add placeholder FSP headers to compile ``` Specify that this is dummy header files requires to compile the PTL SoC and mainboard code File src/vendorcode/intel/fsp/fsp2_0/pantherlake/FirmwareVersionInfo.h: https://review.coreboot.org/c/coreboot/+/83732/comment/2c6a0808_f9233843?us… : PS4, Line 1: /** @file : Intel Firmware Version Info (FVI) related definitions. : : @todo update document/spec reference : : Copyright (c) 2016, Intel Corporation. All rights reserved.<BR> : SPDX-License-Identifier: BSD-2-Clause-Patent : : @par Specification Reference: : System Management BIOS (SMBIOS) Reference Specification v3.0.0 dated 2015-Feb-12 : http://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.p… : : **/ please use open source applicable copyright header File src/vendorcode/intel/fsp/fsp2_0/pantherlake/FspProducerDataHeader.h: https://review.coreboot.org/c/coreboot/+/83732/comment/c47cd5a1_74f85937?us… : PS4, Line 13: @copyright : INTEL CONFIDENTIAL : Copyright (C) 2023 Intel Corporation. : : This software and the related documents are Intel copyrighted materials, : and your use of them is governed by the express license under which they : were provided to you ("License"). Unless the License provides otherwise, : you may not use, modify, copy, publish, distribute, disclose or transmit : this software or the related documents without Intel's prior written : permission. : : This software and the related documents are provided as is, with no : express or implied warranties, other than those that are expressly stated : in the License. same File src/vendorcode/intel/fsp/fsp2_0/pantherlake/FspmUpd.h: https://review.coreboot.org/c/coreboot/+/83732/comment/b9bded4b_ea7a4776?us… : PS4, Line 46: why empty lines? File src/vendorcode/intel/fsp/fsp2_0/pantherlake/FspsUpd.h: https://review.coreboot.org/c/coreboot/+/83732/comment/1566030d_766a6f83?us… : PS4, Line 64: same as FSP-M File src/vendorcode/intel/fsp/fsp2_0/pantherlake/MemInfoHob.h: https://review.coreboot.org/c/coreboot/+/83732/comment/7f954436_f93cc33c?us… : PS4, Line 1: /** @file : This file contains definitions required for creation of : Memory S3 Save data, Memory Info data and Memory Platform : data hobs. : : @copyright : INTEL CONFIDENTIAL : Copyright (C) 1999 Intel Corporation. : : This software and the related documents are Intel copyrighted materials, : and your use of them is governed by the express license under which they : were provided to you ("License"). Unless the License provides otherwise, : you may not use, modify, copy, publish, distribute, disclose or transmit : this software or the related documents without Intel's prior written : permission. : : This software and the related documents are provided as is, with no : express or implied warranties, other than those that are expressly stated : in the License. : : @par Specification Reference: : **/ Please follow the correct copyright format for open source code. You can't push Intel Confidential header in GPL -- To view, visit https://review.coreboot.org/c/coreboot/+/83732?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4c069ba64f487259ce746dc52296618d91209602 Gerrit-Change-Number: 83732 Gerrit-PatchSet: 4 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Comment-Date: Fri, 02 Aug 2024 16:40:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: soc/amd: add PSP SMI handler stub
by Felix Held (Code Review) 02 Aug '24

02 Aug '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nico Huber, Varshit Pandya, ritul guru. Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83739?usp=email ) Change subject: soc/amd: add PSP SMI handler stub ...................................................................... Patch Set 3: (1 comment) File src/soc/amd/cezanne/smihandler.c: https://review.coreboot.org/c/coreboot/+/83739/comment/8df06ba3_cd75bb74?us… : PS2, Line 116: psp_smi_handler > while i usually push for avoiding pre-processor usage if possible, but i'm not sure if having a smi […] oh, looks like i missed the 'in the header part' when reading your comment. implemented that to see if i like that one better and i'd say that that is a bit cleaner; pushed a new version -- To view, visit https://review.coreboot.org/c/coreboot/+/83739?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I65989ff529d728cd9d2cd60b384295417bef77ad Gerrit-Change-Number: 83739 Gerrit-PatchSet: 3 Gerrit-Owner: 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: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Aug 2024 16:21:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2024