coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[M] Change in coreboot[main]: util/ifdtool: Support the DCI enable bit on Skylake/Kaby Lake
by Máté Kukri (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Angel Pons, Paul Menzel. Máté Kukri has posted comments on this change by Máté Kukri. ( https://review.coreboot.org/c/coreboot/+/82272?usp=email ) Change subject: util/ifdtool: Support the DCI enable bit on Skylake/Kaby Lake ...................................................................... Patch Set 4: (10 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/82272/comment/d53962e3_01ee322b?us… : PS3, Line 14: CCT > CCA Done https://review.coreboot.org/c/coreboot/+/82272/comment/ed50bb61_59d8b410?us… : PS3, Line 19: to be honored > unneeded words Done https://review.coreboot.org/c/coreboot/+/82272/comment/ceab156a_ec62cde6?us… : PS3, Line 21: do > does ("the *state* of FPFs" is singular) Done https://review.coreboot.org/c/coreboot/+/82272/comment/f74c19c9_07798e24?us… : PS3, Line 22: > Maybe paste the lines on the system you tested this with? Done File util/ifdtool/ifdtool.c: https://review.coreboot.org/c/coreboot/+/82272/comment/bdc58549_9da8ba24?us… : PS3, Line 1891: "DCI enable not supported for chipset! (Might be because the bit location" : "is unknown or because platform is too old)\n" > nit: Try to reflow these lines so that they fit in 96 characters (move "location" to second line?). Done https://review.coreboot.org/c/coreboot/+/82272/comment/ec6b101a_3a2ddd6d?us… : PS3, Line 2182: for skylake or newer platform > well it's just skylake and kabylake for now (but newer ones can be added) Done https://review.coreboot.org/c/coreboot/+/82272/comment/0629e934_78374b17?us… : PS3, Line 2237: /*[11]=*/ > I like that idea Done https://review.coreboot.org/c/coreboot/+/82272/comment/820a76c4_61897e6f?us… : PS3, Line 2511: dcienable > 1 > What if `dcienable` is less than 0? […] this isn't the only problem: - strtol could reporti an internal under/overflow? (at least that results in LONG_{MIN,MAX}) - without giving it an endp, and checking *endp != '\0', strol could silently ignore trailing garbage at the end of the optarg string - and i guess the truncation from long to int can also hide invalid arguments (and trigger "implmentation defined behaviour") I've changed it to strtoul to catch the obvious case, but this program is so full of the usual C issues, I'd probably be rewriting the entire thing if wanted to fix all such problems. https://review.coreboot.org/c/coreboot/+/82272/comment/7fabc2be_80eb3d5e?us… : PS3, Line 2512: fprintf(stderr, "error: Illegal value\n"); > Please be more descriptive, e.g. […] Done https://review.coreboot.org/c/coreboot/+/82272/comment/e53fbef8_b5db2a09?us… : PS3, Line 2518: // fallthrough > Actually, this should exit with failure... […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/82272?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: I242484e7008bb453cb9a0dc186c8a75e6c96e3d3 Gerrit-Change-Number: 82272 Gerrit-PatchSet: 4 Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 11:26:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Máté Kukri <kukri.mate(a)gmail.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[XS] Change in coreboot[main]: cpu/x86: Add IA32_DEBUG_INTERFACE definitions to msr.h
by Máté Kukri (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Jérémy Compostella. Máté Kukri has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83268?usp=email ) Change subject: cpu/x86: Add IA32_DEBUG_INTERFACE definitions to msr.h ...................................................................... cpu/x86: Add IA32_DEBUG_INTERFACE definitions to msr.h - Useful for hacking in a quick JTAG enable in early boot - It seems like the FSP sets this up on SKL/KBL - Maybe there should be native code and a Kconfig option for it? Signed-off-by: Mate Kukri <kukri.mate(a)gmail.com> Change-Id: Ib4cb50b1f3e97bf37a8e44b25b909fa1581f7b06 --- M src/include/cpu/x86/msr.h 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/83268/1 diff --git a/src/include/cpu/x86/msr.h b/src/include/cpu/x86/msr.h index d369972..ceef095 100644 --- a/src/include/cpu/x86/msr.h +++ b/src/include/cpu/x86/msr.h @@ -109,6 +109,11 @@ #define IA32_CR_SF_QOS_MASK_1 0x1891 #define IA32_CR_SF_QOS_MASK_2 0x1892 +#define IA32_DEBUG_INTERFACE 0xc80 +#define DEBUG_ENABLE 1 +#define DEBUG_LOCK (1<<30) +#define DEBUG_OCCURED (1<<31) + #ifndef __ASSEMBLER__ typedef struct msrinit_struct { -- To view, visit https://review.coreboot.org/c/coreboot/+/83268?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: Ib4cb50b1f3e97bf37a8e44b25b909fa1581f7b06 Gerrit-Change-Number: 83268 Gerrit-PatchSet: 1 Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[M] Change in coreboot[main]: util/ifdtool: Support the DCI enable bit on Skylake/Kaby Lake
by Máté Kukri (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Angel Pons, Máté Kukri. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82272?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: util/ifdtool: Support the DCI enable bit on Skylake/Kaby Lake ...................................................................... util/ifdtool: Support the DCI enable bit on Skylake/Kaby Lake - DCI can also be enabled from firmware via IOSF-SB (port 0xb8 byte 4 bit 4). - Firmware has to set bit 0 of the IA32_DEBUG_INTERFACE (0xc80) MSR to consent to the CPU being debugged. - DCI can be used to debug coreboot either using: - SVT CCA adapter (aka blue box) - Regular USB3 A-to-A cable with the VCC pin removed - Setting this bit will cause the ME firmware to enable DCI, before the big CPU comes out of reset. - The file "/home/mca/eom" in the ME's MFS file system has to contain b"\x00" for the ME firmware to honor this bit. (For DCI enable changing the filesystem is enough, the state of FPFs does not matter.) - Tested on Dell OptiPlex 3050 and Lenovo ThinkPad T480 Change-Id: I242484e7008bb453cb9a0dc186c8a75e6c96e3d3 Signed-off-by: Mate Kukri <kukri.mate(a)gmail.com> --- M util/ifdtool/ifdtool.c 1 file changed, 99 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/82272/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/82272?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: I242484e7008bb453cb9a0dc186c8a75e6c96e3d3 Gerrit-Change-Number: 82272 Gerrit-PatchSet: 4 Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>

1 0

[XS] Change in coreboot[main]: Makefile.mk: Use Wcast-function-type GCC option
by Elyes Haouas (Code Review) 30 Jun '24

30 Jun '24

Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82736?usp=email to look at the new patch set (#3). Change subject: Makefile.mk: Use Wcast-function-type GCC option ...................................................................... Makefile.mk: Use Wcast-function-type GCC option Change-Id: I25415d7fd82879889ffaa1bb534ad5d0b174854e Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M Makefile.mk 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/82736/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/82736?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: I25415d7fd82879889ffaa1bb534ad5d0b174854e Gerrit-Change-Number: 82736 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: Makefile.mk: Use Wcast-function-type command option
by Elyes Haouas (Code Review) 30 Jun '24

30 Jun '24

Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/82736?usp=email ) Change subject: Makefile.mk: Use Wcast-function-type command option ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/82736/comment/05926a18_5ee64adc?us… : PS2, Line 7: command > Maybe say GCC? Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/82736?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: I25415d7fd82879889ffaa1bb534ad5d0b174854e Gerrit-Change-Number: 82736 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 30 Jun 2024 11:04:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>

1 0

[XS] Change in coreboot[main]: Makefile.mk: Use Walloc-size GCC option
by Elyes Haouas (Code Review) 30 Jun '24

30 Jun '24

Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82734?usp=email to look at the new patch set (#3). Change subject: Makefile.mk: Use Walloc-size GCC option ...................................................................... Makefile.mk: Use Walloc-size GCC option Change-Id: Ia26dcf097db125a5a734660d08d875459179241b Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M Makefile.mk 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/82734/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/82734?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: Ia26dcf097db125a5a734660d08d875459179241b Gerrit-Change-Number: 82734 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: Makefile.mk: Use Walloc-size command option
by Elyes Haouas (Code Review) 30 Jun '24

30 Jun '24

Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/82734?usp=email ) Change subject: Makefile.mk: Use Walloc-size command option ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/82734/comment/4dc8152e_95c1272c?us… : PS2, Line 7: command > Maybe say GCC? to be exact, it's more like "GCC command option", but I have no objection to your suggestion ;) https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/#toc-GCC-Command-Options -- To view, visit https://review.coreboot.org/c/coreboot/+/82734?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: Ia26dcf097db125a5a734660d08d875459179241b Gerrit-Change-Number: 82734 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 30 Jun 2024 11:04:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>

1 0

[S] Change in coreboot[main]: commonlib/bsd/lz4_wrapper.c: Fix misaligned access
by Nico Huber (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Angel Pons, Arthur Heymans, Julius Werner, Maximilian Brune, Philipp Hug, Ron Minnich. Nico Huber has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/81910?usp=email ) Change subject: commonlib/bsd/lz4_wrapper.c: Fix misaligned access ...................................................................... Patch Set 6: Code-Review+2 (1 comment) File src/commonlib/bsd/lz4_wrapper.c: https://review.coreboot.org/c/coreboot/+/81910/comment/564fbf68_4f4ee0e9?us… : PS5, Line 10: /* LZ4 comes with its own supposedly portable memory access functions, but they : * seem to be very inefficient in practice (at least on ARM64). Since coreboot : * knows about endianness and allows some basic assumptions (such as unaligned : * access support), we can easily write the ones we need ourselves. */ > They now reflect the discussions in the comments. I have a feeling that the orginal comment meant to include LZ4_copy8(). I don't mind the change, though. Seems less complicated than having to guess what a comment applies to exactly. -- To view, visit https://review.coreboot.org/c/coreboot/+/81910?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: Id165829bfd35be2bce2bbb019c208a304f627add Gerrit-Change-Number: 81910 Gerrit-PatchSet: 6 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 09:05:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: mb/msi/ms7d25,ms7e06: Enable discrete TPM module support
by Angel Pons (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Michał Kopeć, Michał Żygowski, Sergii Dmytruk. Angel Pons has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/80456?usp=email ) Change subject: mb/msi/ms7d25,ms7e06: Enable discrete TPM module support ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80456?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: Ie6e7026b6f1cec842bce4ef40b6db7feb75200e3 Gerrit-Change-Number: 80456 Gerrit-PatchSet: 5 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 08:46:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: lib: Add Kconfig variable for fw_config default value
by Subrata Banik (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Julius Werner, Wonkyu Kim. Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83153?usp=email ) Change subject: lib: Add Kconfig variable for fw_config default value ...................................................................... Patch Set 6: Code-Review-1 (1 comment) Patchset: PS6: Please file a bug to have more specific discussion which I'm unable to follow here. I sense you are trying to boot windows SKU with same binary as Chrome SKU hence, you would like to unify things just for it to be unified. CBI is specific to chrome design and ideally the same shouldn't exist unless implemented now I would like to ensure FW_CBI config is not at all enabled for all windows SKU. The downside would be, all probe function should fail. to fix that problem, you need a dedicated devicetree file for booting to Windows SKU. There may be more req from Intel which I'm missing here, hence, please don't rush for the review/merge unless we have good understanding of the requirement. -- To view, visit https://review.coreboot.org/c/coreboot/+/83153?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: I9694433e01bdcd9ff1e550585c14ea7ccee020a3 Gerrit-Change-Number: 83153 Gerrit-PatchSet: 6 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Sun, 30 Jun 2024 08:23:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

← Newer
1
...
6
7
8
9
10
11
12
...
353
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024