coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
3342 discussions

[XS] Change in coreboot[main]: FIXME remove asserts for mt8186 code
by Yu-Ping Wu (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Arthur Heymans, Hung-Te Lin, Yidi Lin. Yu-Ping Wu has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84208?usp=email ) Change subject: FIXME remove asserts for mt8186 code ...................................................................... Patch Set 3: (1 comment) File src/soc/mediatek/mt8186/mt6366.c: https://review.coreboot.org/c/coreboot/+/84208/comment/82e328bd_e3832b0b?us… : PS3, Line 594: assert(vddq_uv <= 680000); Actually, after looking into this, all the callers are passing valid values here. The minimum upper bound to successfully build GOOGLE_STEELIX with LTO is `assert(vddq_uv <= 1031250)`. It seems that the value 1031250 comes from the `mainboard_set_regulator_voltage(MTK_REGULATOR_VPROC12, 1031250)` call. However, that function call isn't related to the code here at all, because the regulator id is different (`VPROC12` vs `VDDQ`). Therefore, I think this is a false positive reported by the linker. -- To view, visit https://review.coreboot.org/c/coreboot/+/84208?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: I6185e87a374f8722dba545d6bbce1c3a8de53e7e Gerrit-Change-Number: 84208 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 07:55:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[NoOp] Change in blobs[main]: 3rdparty/blobs/mb/google/guybrush: Update signed PSP verstage binaries
by Jon Murphy (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Karthik Ramasubramanian, Martin Roth, Raul Rangel. Jon Murphy has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/blobs/+/84181?usp=email ) Change subject: 3rdparty/blobs/mb/google/guybrush: Update signed PSP verstage binaries ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Is the signing token for Guybrush already present? If not, please add that too in this CL. https://review.coreboot.org/plugins/gitiles/blobs/+/refs/heads/main/mainboa… -- To view, visit https://review.coreboot.org/c/blobs/+/84181?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: blobs Gerrit-Branch: main Gerrit-Change-Id: Ida220410a4b896b746dda74258fa45a51524dbc2 Gerrit-Change-Number: 84181 Gerrit-PatchSet: 2 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-CC: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 07:43:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/riven: Update GPIO pins for 3rd dmic support
by Kapil Porwal (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: David Wu, Dinesh Gehlot, Jayvik Desai, Nick Vaccaro, Rishika Raj. Kapil Porwal has posted comments on this change by David Wu. ( https://review.coreboot.org/c/coreboot/+/84141?usp=email ) Change subject: mb/google/nissa/var/riven: Update GPIO pins for 3rd dmic support ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84141?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: I6c8780ce37b5d3987f5cdf6e6e6d0b4896b33230 Gerrit-Change-Number: 84141 Gerrit-PatchSet: 2 Gerrit-Owner: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Wed, 04 Sep 2024 07:36:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: FIXME remove asserts for mt8186 code
by Yu-Ping Wu (Code Review) 04 Sep '24

04 Sep '24

1 0

[M] Change in coreboot[main]: include/spd_bin.h: Add SPD IO layer
by Shuo Liu (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: yuchi.chen(a)intel.com. Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/84201?usp=email ) Change subject: include/spd_bin.h: Add SPD IO layer ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84201/comment/61dc0f59_a1d69449?us… : PS3, Line 9: By default, PCH SMBus code will be called to retrieve SPD data. This codes https://review.coreboot.org/c/coreboot/+/84201/comment/e9c099fc_aca4c354?us… : PS3, Line 10: patch add a SPD IO layer so that SoC could implement its specific SPD adds -- To view, visit https://review.coreboot.org/c/coreboot/+/84201?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: I656298aeda409fca3c85266b5b8727fac9bfc917 Gerrit-Change-Number: 84201 Gerrit-PatchSet: 3 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Comment-Date: Wed, 04 Sep 2024 07:23:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: soc/intel/common/systemagent: Fixup systemagent address
by Shuo Liu (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Arthur Heymans, Felix Held, Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com. Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83318?usp=email ) Change subject: soc/intel/common/systemagent: Fixup systemagent address ...................................................................... Patch Set 17: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/83318?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: If32c2a6524c9d55ce7f9c3dd203bcf85cab76c2c Gerrit-Change-Number: 83318 Gerrit-PatchSet: 17 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Vasiliy Khoruzhick <vasilykh(a)arista.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 04 Sep 2024 07:20:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: soc/intel/common/systemagent: Add Kconfig item HAVE_TSEG_LIMIT_REGISTER
by Shuo Liu (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: yuchi.chen(a)intel.com. Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/84200?usp=email ) Change subject: soc/intel/common/systemagent: Add Kconfig item HAVE_TSEG_LIMIT_REGISTER ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/84200?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: I6cb4fbecc1dbafc770d3809a75d05917a141a9af Gerrit-Change-Number: 84200 Gerrit-PatchSet: 2 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Comment-Date: Wed, 04 Sep 2024 07:20:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: src/intel/cmm/block: Update sa_get_tseg_size function
by Wonkyu Kim (Code Review) 04 Sep '24

04 Sep '24

Wonkyu Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84210?usp=email ) Change subject: src/intel/cmm/block: Update sa_get_tseg_size function ...................................................................... src/intel/cmm/block: Update sa_get_tseg_size function Use CONFIG_SMM_TSEG_SIZE rather than calulating based on gsm base and tseg base as gsm base may not be configured when internal graphic IP is disabled and CONFIG_SMM_TSEG_SIZE is passed by FSP UPD. Signed-off-by: Wonkyu Kim <wonkyu.kim(a)intel.com> Change-Id: Ic3d5175add1f7dc6e2c9f1c38133b36ffc59e789 --- M src/soc/intel/common/block/systemagent/systemagent_early.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/84210/1 diff --git a/src/soc/intel/common/block/systemagent/systemagent_early.c b/src/soc/intel/common/block/systemagent/systemagent_early.c index aecdfbb..773e74c 100644 --- a/src/soc/intel/common/block/systemagent/systemagent_early.c +++ b/src/soc/intel/common/block/systemagent/systemagent_early.c @@ -148,6 +148,10 @@ size_t sa_get_tseg_size(void) { + if (CONFIG_SMM_TSEG_SIZE) { + return CONFIG_SMM_TSEG_SIZE; + } + return sa_get_gsm_base() - sa_get_tseg_base(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/84210?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: Ic3d5175add1f7dc6e2c9f1c38133b36ffc59e789 Gerrit-Change-Number: 84210 Gerrit-PatchSet: 1 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com>

1 0

[XS] Change in coreboot[main]: amdfwtool: Set L2 table size as 0x400
by Bao Zheng (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Anand Vaikar, Felix Held, Marshall Dawson, Zheng Bao, ritul guru. Bao Zheng has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/84194?usp=email ) Change subject: amdfwtool: Set L2 table size as 0x400 ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84194/comment/f4b1c629_d18e983b?us… : PS1, Line 10: Cezann > nit: Cezanne Done https://review.coreboot.org/c/coreboot/+/84194/comment/3ef65e6a_15b91100?us… : PS1, Line 10: Cezann > nit: Cezanne Done https://review.coreboot.org/c/coreboot/+/84194/comment/1b8022dd_39f81ee1?us… : PS1, Line 12: birman, Chausie, Skyrim, mayan > another nit, capitalize or lower case all the names consistently. Done https://review.coreboot.org/c/coreboot/+/84194/comment/b65795c9_890a0514?us… : PS1, Line 12: birman, Chausie, Skyrim, mayan > another nit, capitalize or lower case all the names consistently. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84194?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: I2c0af6579dbe2a3a61e1fe9c79d69491fd45a5bb Gerrit-Change-Number: 84194 Gerrit-PatchSet: 2 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Anand Vaikar <a.vaikar2021(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Anand Vaikar <a.vaikar2021(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 04 Sep 2024 07:01:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com>

1 0

[S] Change in coreboot[main]: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx
by Arthur Heymans (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik, Tarun. Arthur Heymans has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84205?usp=email ) Change subject: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/meteorlake/meminit.c: https://review.coreboot.org/c/coreboot/+/84205/comment/f754a116_a83a7336?us… : PS2, Line 209: has_spd = true; > > > > > Can you help me understand the purpose of `has_spd`? Does it mean the mainboard has an SPD chip and we'll read the hex file using SMBUS? > > > > > > > > would need_spd be a better name? LPDDRx DRAM technology does not use SPD and the code for SPD in soc/intel/common is doing doing undefined behavior like overflowing an array. > > > > > > Yes, that name sounds better. I'd like to suggest adding a comment that explains the purpose of this variable. People often confuse the SPD chip with the SPD hex data file. The code should make it clear that the SPD chip isn't required to read the SPD hex file (need_spd). > > > > No actually it's about SPD data and this code is wrong. The soc/intel/common code is simply wrong for LPDDRx. The loop over channels and dimms per channel is overflowing. There are no DIMMs in LPDDRx which is what need to be addressed. > > okay, so when you set `has_spd` or `need_spd`, are you saying that we will read SPD data over SMBUS and for lpddrx, we shall pass SPD hex file ? No that's not the issue. CONFIG_DIMM_MAX is 4 which is how large the array is for containing SPD pointers. LPDDRx has 16 bit per channel, so the code thinks there are 8 channels in total. CONFIG_DIMM_PER_CHANNEL is set to 2. The common code loops over channels and dimms per channel so it's overflowing. What is the appropriate fix? do you think. https://qa.coreboot.org/job/coreboot-gerrit/263746/testReport/junit/(root)/… is the compilation error with LTO which can detect the buffer overflow. -- To view, visit https://review.coreboot.org/c/coreboot/+/84205?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: Ib3d4ed8032bb06b6d08fbc2dc4b697df88745243 Gerrit-Change-Number: 84205 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(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-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:59:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024