coreboot-gerrit June 2022

coreboot-gerrit@coreboot.org

1 participants
1234 discussions

Change in coreboot[master]: commonlib: Clean up compiler.h
by Julius Werner (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Philipp Hug, Patrick Georgi, Jakub Czapiga, ron minnich. Hello Philipp Hug, Patrick Georgi, Jakub Czapiga, Yu-Ping Wu, ron minnich, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64737 to look at the new patch set (#4). Change subject: commonlib: Clean up compiler.h ...................................................................... commonlib: Clean up compiler.h This patch contains several minor cleanups related to compiler.h: - Replace __always_unused() (which is a Linux-specific concept that doesn't make sense without also having __maybe_unused(), and had zero uses in the codebase) with __unused() which moves here from helpers.h - Add __underscores__ to the names of all attributes in the compiler attribute shorthand macros. This is necessary to make them work in files where the same name was already used for an identifier (e.g. cbfstool/cbfs.h's `unused` array of file types). - Remove libpayload's own copy of compiler.h and make it directly pull in the commonlib/bsd copy. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I9644da594bb69133843c6b7f12ce50b2e45fd24b --- M payloads/libpayload/Makefile.inc M payloads/libpayload/bin/lpgcc D payloads/libpayload/include/compiler.h M payloads/libpayload/tests/Makefile.inc M src/arch/riscv/opensbi.c M src/commonlib/bsd/include/commonlib/bsd/compiler.h M src/commonlib/include/commonlib/helpers.h 7 files changed, 20 insertions(+), 69 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/64737/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/64737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9644da594bb69133843c6b7f12ce50b2e45fd24b Gerrit-Change-Number: 64737 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: libpayload: Add CONFIG_LP_ELOG and its dependency
by Julius Werner (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Hsuan-ting Chen, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63115 ) Change subject: libpayload: Add CONFIG_LP_ELOG and its dependency ...................................................................... Patch Set 9: (1 comment) File payloads/libpayload/Makefile.inc: https://review.coreboot.org/c/coreboot/+/63115/comment/ba768966_683171bb PS9, Line 51: classes-$(CONFIG_LP_ELOG) += elog "libc" in libpayload isn't really a C library, it's a catchall bucket for all the other stuff that isn't separated into its own library. All the drivers, coreboot table stuff, etc. all go in there too. I don't think it makes sense to draw a line at elog (even if more code will be added, it's not gonna be that much in total... the USB stack alone would be much larger). We can talk about a general reorganization if you want, but this patch is probably not the time for it. And if we do I'm also not sure where the point is in splitting everything out so fine-grainedly. I doubt anyone actually uses any of these individual libraries (depthcharge just links libpayload.a and most other payloads probably do too). I think it may make sense for totally separate code bases (curses, vboot) but for the other stuff that's just a handful of files solely written for and maintained in the libpayload repo each (e.g. libcbfs, compression algos, etc.) I don't really see the point of splitting that out. > Though it is still a small file only, I am not sure if we should prevent including it for the non-ChromeOS platforms since I am not familiar with them. We can change it if someone complains, of course, but by default I wouldn't assume it's necessary. Library code that's not referenced doesn't hurt anyone, the only concerns here are symbol clashes (unlikely) and build times (negligible for so little code). -- To view, visit https://review.coreboot.org/c/coreboot/+/63115 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64d0fdd2a8eff1d89a1ac451d37b61787b5564e7 Gerrit-Change-Number: 63115 Gerrit-PatchSet: 9 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 01 Jun 2022 20:31:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Documentation: Update index.md and add 4.18 release notes
by Julius Werner (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Felix Singer, Martin L Roth. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64843 ) Change subject: Documentation: Update index.md and add 4.18 release notes ...................................................................... Patch Set 2: (1 comment) File Documentation/releases/coreboot-4.18-relnotes.md: https://review.coreboot.org/c/coreboot/+/64843/comment/8a7c5d48_28ac5ec0 PS1, Line 17: > Sure, I can add it. Well, it was announced in April so let's count from there. I thought our release cadence was 6 months anyway so it would end up being the next version, but I guess it has become faster now? Anyway, 4.19 is fine too. -- To view, visit https://review.coreboot.org/c/coreboot/+/64843 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I52814ebbae804ea0ff24a7cec0618054029b8b47 Gerrit-Change-Number: 64843 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Comment-Date: Wed, 01 Jun 2022 20:19:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Martin L Roth <gaumless(a)tutanota.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/adlrvp: x4 slot support (SD card support) for Adl-P RVP
by Cliff Huang (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Bora Guvendik, Anil Kumar K, Jeremy Compostella. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63945 ) Change subject: mb/intel/adlrvp: x4 slot support (SD card support) for Adl-P RVP ...................................................................... Patch Set 6: (1 comment) File src/mainboard/intel/adlrvp/gpio.c: https://review.coreboot.org/c/coreboot/+/63945/comment/d14314cf_dd304efd PS6, Line 189: // PAD_CFG_NF(GPP_A7, NONE, DEEP, NF1), > you should remove this line altogether if unnecessary instead of commenting it out. I would rather keep this as comment for this CL since NF1 is not really for I2S2 SCLK. If this needs to be removed, all these I2S2 related needs to be re-visited and make changes. A separate CL is needed for this. -- To view, visit https://review.coreboot.org/c/coreboot/+/63945 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iba5d83d133b6ae8cd389ddd971db308170094300 Gerrit-Change-Number: 63945 Gerrit-PatchSet: 6 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(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: Cliff Huang <cliff.huang(a)intel.corp-partner.google.com> Gerrit-CC: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com> Gerrit-Comment-Date: Wed, 01 Jun 2022 19:59:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: commonlib: Clean up compiler.h
by build bot (Jenkins) (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Philipp Hug, Patrick Georgi, Jakub Czapiga, ron minnich. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64737 ) Change subject: commonlib: Clean up compiler.h ...................................................................... Patch Set 3: (8 comments) File src/commonlib/bsd/include/commonlib/bsd/compiler.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/79a9a846_ac58f55e PS3, Line 8: #define __packed __attribute__((__gcc_struct__, __packed__)) Prefer __packed over __attribute__((__packed__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/a8f4ef9d_861eed5b PS3, Line 10: #define __packed __attribute__((__packed__)) Prefer __packed over __attribute__((__packed__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/dd1f593a_139af07f PS3, Line 15: #define __aligned(x) __attribute__((__aligned__(x))) Prefer __aligned(x) over __attribute__((__aligned__(x))) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/7b86e152_d08af777 PS3, Line 19: #define __unused __attribute__((__unused__)) __always_unused or __maybe_unused is preferred over __attribute__((__unused__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/8c854678_3c1b73c4 PS3, Line 27: #define __weak __attribute__((__weak__)) Prefer __weak over __attribute__((__weak__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/da4bbd3f_b5b0a49c PS3, Line 31: #define __noreturn __attribute__((__noreturn__)) Prefer __noreturn over __attribute__((__noreturn__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/82815be4_faf5a968 PS3, Line 35: #define __always_inline inline __attribute__((__always_inline__)) Macros with complex values should be enclosed in parentheses Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-150414): https://review.coreboot.org/c/coreboot/+/64737/comment/b3c8cce9_f9c190b4 PS3, Line 35: #define __always_inline inline __attribute__((__always_inline__)) Prefer __always_inline over __attribute__((__always_inline__)) -- To view, visit https://review.coreboot.org/c/coreboot/+/64737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9644da594bb69133843c6b7f12ce50b2e45fd24b Gerrit-Change-Number: 64737 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Wed, 01 Jun 2022 19:58:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: commonlib: Clean up compiler.h
by Julius Werner (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Patrick Georgi, Jakub Czapiga. Hello Patrick Georgi, Jakub Czapiga, Yu-Ping Wu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64737 to look at the new patch set (#3). Change subject: commonlib: Clean up compiler.h ...................................................................... commonlib: Clean up compiler.h This patch contains several minor cleanups related to compiler.h: - Replace __always_unused() (which is a Linux-specific concept that doesn't make sense without also having __maybe_unused(), and had zero uses in the codebase) with __unused() which moves here from helpers.h - Add __underscores__ to the names of all attributes in the compiler attribute shorthand macros. This is necessary to make them work in files where the same name was already used for an identifier (e.g. cbfstool/cbfs.h's `unused` array of file types). - Remove libpayload's own copy of compiler.h and make it directly pull in the commonlib/bsd copy. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I9644da594bb69133843c6b7f12ce50b2e45fd24b --- M payloads/libpayload/Makefile.inc M payloads/libpayload/bin/lpgcc D payloads/libpayload/include/compiler.h M payloads/libpayload/tests/Makefile.inc M src/arch/riscv/opensbi.c M src/commonlib/bsd/include/commonlib/bsd/compiler.h M src/commonlib/include/commonlib/helpers.h 7 files changed, 18 insertions(+), 69 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/64737/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/64737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9644da594bb69133843c6b7f12ce50b2e45fd24b Gerrit-Change-Number: 64737 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/cmn/cse: Implement heci_init() to initialize HECI devices
by Subrata Banik (Code Review) 02 Jun '22

02 Jun '22

Attention is currently required from: Tim Wawrzynczak, Angel Pons, Eric Lai, Lean Sheng Tan. Hello build bot (Jenkins), Sean Rhodes, Tim Wawrzynczak, Angel Pons, Eric Lai, Lean Sheng Tan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64855 to look at the new patch set (#3). Change subject: soc/intel/cmn/cse: Implement heci_init() to initialize HECI devices ...................................................................... soc/intel/cmn/cse: Implement heci_init() to initialize HECI devices This patch implements heci_init() API that perform initialization of all HECI devices as per MAX_HECI_DEVICES config. BUG=none TEST=Able to build and boot google/taeko with this change. No CSE error observed with `heci_init()` called from romstage. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ia25e18a20cc749fc7eee39b0b591d41540fc14c9 --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 47 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/64855/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/64855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia25e18a20cc749fc7eee39b0b591d41540fc14c9 Gerrit-Change-Number: 64855 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/guybrush: Add ACPI _PLD custom values
by Won Chung (Code Review) 02 Jun '22

02 Jun '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64876 to look at the new patch set (#2). Change subject: mb/google/guybrush: Add ACPI _PLD custom values ...................................................................... mb/google/guybrush: Add ACPI _PLD custom values This patch uses ACPI _PLD macros to add custom values for USB ports. +----------------+ | | | Screen | | | +----------------+ C0 | | C1 A0 | MLB DB | A1 | | +----------------+ BUG=b:232298307 TEST=None Signed-off-by: Won Chung <wonchung(a)google.com> Change-Id: Ic9c45aebaf02a16b755f4731e1e3b46cd5dec829 --- M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb 1 file changed, 16 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/64876/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64876 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic9c45aebaf02a16b755f4731e1e3b46cd5dec829 Gerrit-Change-Number: 64876 Gerrit-PatchSet: 2 Gerrit-Owner: Won Chung <wonchung(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/skyrim: Add ACPI _PLD custom values
by Won Chung (Code Review) 02 Jun '22

02 Jun '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64875 to look at the new patch set (#2). Change subject: mb/google/skyrim: Add ACPI _PLD custom values ...................................................................... mb/google/skyrim: Add ACPI _PLD custom values This patch uses ACPI _PLD macros to add custom values for USB ports. +----------------+ | | | Screen | | | +----------------+ C0 | | C1 A0 | MLB DB | | | +----------------+ BUG=b:232298017 TEST=None Signed-off-by: Won Chung <wonchung(a)google.com> Change-Id: Idca3dd468f1b9fde37a1bbf20d65768032c7160b --- M src/mainboard/google/skyrim/variants/baseboard/devicetree.cb M src/mainboard/google/skyrim/variants/skyrim/overridetree.cb 2 files changed, 12 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/64875/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64875 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idca3dd468f1b9fde37a1bbf20d65768032c7160b Gerrit-Change-Number: 64875 Gerrit-PatchSet: 2 Gerrit-Owner: Won Chung <wonchung(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: cbfs: Add CBFS_TYPE_INTEL_FIT and exclude it from CBFS verification
by Julius Werner (Code Review) 02 Jun '22

02 Jun '22

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/64736 ) Change subject: cbfs: Add CBFS_TYPE_INTEL_FIT and exclude it from CBFS verification ...................................................................... cbfs: Add CBFS_TYPE_INTEL_FIT and exclude it from CBFS verification The Intel Firmware Interface Table (FIT) is a bit of an annoying outlier among CBFS files because it gets manipulated by a separate utility (ifittool) after cbfstool has already added it to the image. This will break file hashes created for CBFS verification. This is not actually a problem when booting, since coreboot never actually loads the FIT from CBFS -- instead, it's only in the image for use by platform-specific mechanisms that run before coreboot's bootblock. But having an invalid file hash in the CBFS image is confusing when you want to verify that the image is correctly built for verification. This patch adds a new CBFS file type "intel_fit" which is only used for the intel_fit (and intel_fit_ts, if applicable) file containing the FIT. cbfstool will avoid generating and verifying file hashes for this type, like it already does for the "bootblock" and "cbfs header" types. (Note that this means that any attempt to use the CBFS API to actually access this file from coreboot will result in a verification error when CBFS verification is enabled.) Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I1c1bb6dab0c9ccc6e78529758a42ad3194cd130c Reviewed-on: https://review.coreboot.org/c/coreboot/+/64736 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h M src/cpu/intel/fit/Makefile.inc M util/cbfstool/cbfs.h M util/cbfstool/cbfstool.c 4 files changed, 17 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h index be5c9cd..5b70d1a 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h @@ -30,6 +30,7 @@ CBFS_TYPE_VSA = 0x51, CBFS_TYPE_MBI = 0x52, CBFS_TYPE_MICROCODE = 0x53, + CBFS_TYPE_INTEL_FIT = 0x54, CBFS_TYPE_FSP = 0x60, CBFS_TYPE_MRC = 0x61, CBFS_TYPE_MMA = 0x62, diff --git a/src/cpu/intel/fit/Makefile.inc b/src/cpu/intel/fit/Makefile.inc index ef2090a..d3f12e4 100644 --- a/src/cpu/intel/fit/Makefile.inc +++ b/src/cpu/intel/fit/Makefile.inc @@ -8,7 +8,7 @@ cbfs-files-y += intel_fit intel_fit-file := fit_table.c:struct -intel_fit-type := raw +intel_fit-type := intel_fit intel_fit-align := 16 $(call add_intermediate, set_fit_ptr, $(IFITTOOL)) @@ -41,7 +41,7 @@ cbfs-files-y += intel_fit_ts intel_fit_ts-file := fit_table.c:struct -intel_fit_ts-type := raw +intel_fit_ts-type := intel_fit intel_fit_ts-align := 16 endif # CONFIG_INTEL_ADD_TOP_SWAP_BOOTBLOCK diff --git a/util/cbfstool/cbfs.h b/util/cbfstool/cbfs.h index e1f705e..e905162 100644 --- a/util/cbfstool/cbfs.h +++ b/util/cbfstool/cbfs.h @@ -40,6 +40,7 @@ {CBFS_TYPE_VSA, "vsa"}, {CBFS_TYPE_MBI, "mbi"}, {CBFS_TYPE_MICROCODE, "microcode"}, + {CBFS_TYPE_INTEL_FIT, "intel_fit"}, {CBFS_TYPE_FSP, "fsp"}, {CBFS_TYPE_MRC, "mrc"}, {CBFS_TYPE_CMOS_DEFAULT, "cmos_default"}, diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index b2d5cdb..71c8911 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -279,6 +279,18 @@ return update_anchor(mhc, fmap_hash); } +static bool verification_exclude(enum cbfs_type type) +{ + switch (type) { + case CBFS_TYPE_BOOTBLOCK: + case CBFS_TYPE_CBFSHEADER: + case CBFS_TYPE_INTEL_FIT: + return true; + default: + return false; + } +} + static bool region_is_flashmap(const char *region) { return partitioned_file_region_check_magic(param.image_file, region, @@ -872,7 +884,7 @@ /* Bootblock and CBFS header should never have file hashes. When adding the bootblock it is important that we *don't* look up the metadata hash yet (before it is added) or we'll cache an outdated result. */ - if (param.type != CBFS_TYPE_BOOTBLOCK && param.type != CBFS_TYPE_CBFSHEADER) { + if (!verification_exclude(param.type)) { enum vb2_hash_algorithm mh_algo = get_mh_cache()->cbfs_hash.algo; if (mh_algo != VB2_HASH_INVALID && param.hash != mh_algo) { if (param.hash == VB2_HASH_INVALID) { 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/64736 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1c1bb6dab0c9ccc6e78529758a42ad3194cd130c Gerrit-Change-Number: 64736 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2022