Attention is currently required from: Elyes Haouas, Jakub "Kuba" Czapiga, Maximilian Brune.
Julius Werner has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86536?usp=email )
Change subject: libpayload/tests: Remove unused test files
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86536?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: Id2cec6a56ba5ce98832aced6fc2cfd8ebda91f02
Gerrit-Change-Number: 86536
Gerrit-PatchSet: 3
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Tue, 25 Feb 2025 01:33:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Bao Zheng, Felix Held, Julius Werner, Marshall Dawson, Zheng Bao, ritul guru.
Maximilian Brune has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85466?usp=email )
Change subject: amdfwtool: Set address mode of BIOS binary as context defines
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
> Do you use the default setting to build birman-plus. […]
Yes.
The address mode of the bios binary changes from relative to BIOS to relative to table.
That is also what I can see in this patch or am I missing something?
--
To view, visit https://review.coreboot.org/c/coreboot/+/85466?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: I9c2f2983d03c913b28fbd87aa0925a32a4649d62
Gerrit-Change-Number: 85466
Gerrit-PatchSet: 9
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.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: Bao Zheng <fishbaozi(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: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 24 Feb 2025 23:52:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bao Zheng <fishbaozi(a)gmail.com>
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Attention is currently required from: Alok Agarwal, Intel coreboot Reviewers, Jayvik Desai, Jeremy Compostella, Julius Werner, Kapil Porwal, Karthik Ramasubramanian, Paul Menzel, Pranava Y N, Subrata Banik, Vikrant L Jadeja.
Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/85454?usp=email )
Change subject: soc/intel/pantherlake: Display Sign-of-Life during memory training
......................................................................
Patch Set 22:
(1 comment)
File src/soc/intel/pantherlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/85454/comment/f732dc5e_70acc40a?us… :
PS21, Line 387: m_cfg->VgaMessage = (efi_uintn_t)ux_locales_get_text(UX_LOCALE_MSG_MEMORY_TRAINING);
> ``` /* TODO: b/398872610 <A short description of TODO> */ ```
We have an open ticket to track the idea of whether to refactor some code or not. If I add a "TODO" comment in the code and we decide to take no action, I must create a semantically empty ChangeList (CL) just to remove the TODO comment. Including a TODO in the code would only make sense if something were only partially implemented, which is not the case here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85454?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: I993eb0d59cd01fa62f35a77f84e262e389efb367
Gerrit-Change-Number: 85454
Gerrit-PatchSet: 22
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Alok Agarwal <alok.agarwal(a)intel.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-Attention: Alok Agarwal <alok.agarwal(a)intel.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 23:13:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Intel coreboot Reviewers, Name of user not set #1005756, Patrick Rudolph.
Jérémy Compostella has posted comments on this change by Name of user not set #1005756. ( https://review.coreboot.org/c/coreboot/+/86562?usp=email )
Change subject: src/cpu/intel/car/romstage: Refactor stack guard code in romstage
......................................................................
Patch Set 3:
(3 comments)
File src/cpu/intel/car/romstage.c:
https://review.coreboot.org/c/coreboot/+/86562/comment/cd75cffc_5e46b662?us… :
PS3, Line 48: int i;
Shouldn't we use `size_t` for an iteration index variable ?
https://review.coreboot.org/c/coreboot/+/86562/comment/772a5b32_b672bdb7?us… :
PS3, Line 50: if (stack_base == NULL) {
How can `stack_base` be `NULL` ?
https://review.coreboot.org/c/coreboot/+/86562/comment/b12aa9c9_1cc2aee3?us… :
PS3, Line 58: printk(BIOS_DEBUG, "Smashed stack detected in romstage!\n");
Is there a reason to keep executing the loop when an issue has been detected ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/86562?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: I754e422c0023cd7824dd6109f031239756acff4b
Gerrit-Change-Number: 86562
Gerrit-PatchSet: 3
Gerrit-Owner: Name of user not set #1005756
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Patrick Rudolph
Gerrit-Attention: Name of user not set #1005756
Gerrit-Comment-Date: Mon, 24 Feb 2025 23:08:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Johannes Hahn, Mario Scheithauer, Werner Zeh.
Paul Menzel has posted comments on this change by Johannes Hahn. ( https://review.coreboot.org/c/coreboot/+/86424?usp=email )
Change subject: src/mainboard/siemens/fa_ehl: Configure LPDDR4 initialization
......................................................................
Patch Set 6:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/86424/comment/9e2fb0c8_cdb62f24?us… :
PS6, Line 7: src/
This can also be removed.
File src/mainboard/siemens/fa_ehl/variants/fa_ehl/memory.c:
https://review.coreboot.org/c/coreboot/+/86424/comment/e3738d89_e1ab98e5?us… :
PS5, Line 4: #include <gpio.h>
> Should the whole patch be abandoned so I can dissolve remnant dependencies of it before commiting it […]
Sorry, I don’t fully understand. In my opinion the patchset should stay, and you should just amend the commit.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86424?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: If84373dfbc1ecbf916489af6e964f8a7541f5e7b
Gerrit-Change-Number: 86424
Gerrit-PatchSet: 6
Gerrit-Owner: Johannes Hahn <johannes-hahn(a)siemens.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Uwe Poeche <uwe.poeche(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Johannes Hahn <johannes-hahn(a)siemens.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 22:57:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Johannes Hahn <johannes-hahn(a)siemens.com>
Attention is currently required from: Jakub "Kuba" Czapiga, Julius Werner, Maximilian Brune.
Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86536?usp=email )
Change subject: libpayload/tests: Remove unused test files
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> Removing that still makes this test not build because it now has unresolved references. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/86536?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: Id2cec6a56ba5ce98832aced6fc2cfd8ebda91f02
Gerrit-Change-Number: 86536
Gerrit-PatchSet: 3
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 22:22:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Pranava Y N, Subrata Banik, Wonkyu Kim.
Bora Guvendik has posted comments on this change by Bora Guvendik. ( https://review.coreboot.org/c/coreboot/+/84872?usp=email )
Change subject: soc/intel/pantherlake: Inject CSE TS into CBMEM timestamp table
......................................................................
Patch Set 9:
(2 comments)
File src/soc/intel/common/block/cse/Kconfig:
https://review.coreboot.org/c/coreboot/+/84872/comment/b193d0d3_9c689b2f?us… :
PS8, Line 316: _V4
> any reason why we skipped `_v3`?
It wasn't used but I don't know the story behind it.
File src/soc/intel/common/block/include/intelblocks/cse_telemetry_v4.h:
https://review.coreboot.org/c/coreboot/+/84872/comment/fd2b521d_4d0f4321?us… :
PS8, Line 41: SOC_INTEL_COMMON_CSE_TELEMETRY_V4_H
> nit […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/84872?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: Ie7716b8c371b82c13da1b0217dce1a16e7b95cee
Gerrit-Change-Number: 84872
Gerrit-PatchSet: 9
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 24 Feb 2025 22:16:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Bora Guvendik, Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Pranava Y N, Wonkyu Kim.
Hello Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Pranava Y N, Subrata Banik, Wonkyu Kim, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84872?usp=email
to look at the new patch set (#9).
The following approvals got outdated and were removed:
Code-Review+1 by Jayvik Desai, Verified+1 by build bot (Jenkins)
Change subject: soc/intel/pantherlake: Inject CSE TS into CBMEM timestamp table
......................................................................
soc/intel/pantherlake: Inject CSE TS into CBMEM timestamp table
Get boot performance timestamps from CSE and inject them into CBMEM
timestamp table.
990:CSME ROM started execution 0
992:ESE completed AUnit loading 0
944:CSE sent 'Boot Stall Done' to PMC 174,000
945:CSE started to handle ICC configuration 274,000 (100,000)
946:CSE sent 'Host BIOS Prep Done' to PMC 274,000 (0)
947:CSE received 'CPU Reset Done Ack sent' from PMC 448,000 (174,000)
0:1st timestamp 556,874 (108,874)
BUG=b:376218080
TEST=Able to see TS elapse prior to IA reset on Fatcat
Signed-off-by: Bora Guvendik <bora.guvendik(a)intel.com>
Change-Id: Ie7716b8c371b82c13da1b0217dce1a16e7b95cee
---
M src/soc/intel/common/block/cse/Kconfig
M src/soc/intel/common/block/include/intelblocks/cse_telemetry.h
A src/soc/intel/common/block/include/intelblocks/cse_telemetry_v4.h
M src/soc/intel/pantherlake/cse_telemetry.c
4 files changed, 52 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/84872/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/84872?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: Ie7716b8c371b82c13da1b0217dce1a16e7b95cee
Gerrit-Change-Number: 84872
Gerrit-PatchSet: 9
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Patrick Rudolph.
Ana Carolina Cabral has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86583?usp=email )
Change subject: soc/amd/common/block/lpc: Add ROM2 and ROM3 helper functions
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/include/amdblocks/lpc.h:
https://review.coreboot.org/c/coreboot/+/86583/comment/bca1670c_fa57c476?us… :
PS1, Line 106: ROM_ADDRESS_RANGE3_START
Move this up, after ROM_ADDRESS_RANGE2_END.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86583?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: I10d4f0fe8a38e0ba2784a9839270d5dd3398d47a
Gerrit-Change-Number: 86583
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.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: Mon, 24 Feb 2025 21:23:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No