Attention is currently required from: Raul Rangel, Karthik Ramasubramanian.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58954 )
Change subject: lib/cbfs: Use a work queue for cbfs_preload
......................................................................
Patch Set 1:
(1 comment)
File src/lib/cbfs.c:
https://review.coreboot.org/c/coreboot/+/58954/comment/73584417_36fd03b4
PS1, Line 328: DEBUG("%s: cbfs preload thread done\n", __func__);
> Honestly, I share all the same concerns. […]
Woah, wait a second, let's not go overboard. I don't think futures are going to solve these problems any better than threads. With a futures model you still need to "kick" your asynchronous tasks regularly because your DMA hardware can only transfer so many bytes at once before it needs software intervention, and the only real good place to integrate those "kicks" is in udelay(). So you still have exactly the same problem that any udelay() can potentially trigger concurrent accesses to data structures you're currently handling. Maybe not for this one particular function, because this wouldn't run in an extra thread in that case... but for the generic case that is still totally true (e.g. if the driver that does the DMA continue operation also needs to maintain some list that's shared between different contexts for some reason). Just because you're not calling it "multitasking" doesn't mean it's immune to concurrency issues. Using a threading model at least makes these issues more familiar to most programmers.
I think we'll just need to come up with a good model to handle these issues. The classic "secure every data structure with its own mutex" is maybe a bit overkill for our case, although it would work. I think what I suggested at the end of my last comment is probably the best option: Just use disable_coop() as a big hammer around all write operations to the list. It's a very cheap call, and this way you don't need to worry about securing the parts that only read the list.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58954
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I965385e70793557cb14e4b925ae9afbe66a12c0e
Gerrit-Change-Number: 58954
Gerrit-PatchSet: 1
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 20:33:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Julius Werner has submitted this change. ( https://review.coreboot.org/c/qc_blobs/+/59805 )
Change subject: sc7180/boot : Update qclib blobs binaries and release notes
......................................................................
sc7180/boot : Update qclib blobs binaries and release notes
Change-Id: I010bab3f8dc36672b53e08ef5359eef85952cc76
---
M sc7180/boot/QcLib.elf
M sc7180/boot/Release_Notes.txt
2 files changed, 22 insertions(+), 0 deletions(-)
Approvals:
Julius Werner: Verified; Looks good to me, approved
diff --git a/sc7180/boot/QcLib.elf b/sc7180/boot/QcLib.elf
index d71c0e7..571c484 100644
--- a/sc7180/boot/QcLib.elf
+++ b/sc7180/boot/QcLib.elf
Binary files differ
diff --git a/sc7180/boot/Release_Notes.txt b/sc7180/boot/Release_Notes.txt
index cbef393..3cbda88 100644
--- a/sc7180/boot/Release_Notes.txt
+++ b/sc7180/boot/Release_Notes.txt
@@ -1,3 +1,25 @@
+=================== Release 00035 ================================
+This Release Notes file covers these blobs:
+ * QcLib.elf
+
+Version : 00035
+
+Release Date : Dec, 2021
+
+Supported Silicon : SC7180
+
+No special instructions, requirements or dependencies, files must be
+present in this folder to be pulled in during coreboot build
+
+Changes since last version :
+ * LLVM update to 12.0.0
+
+Errata : Nothing to report
+
+Toolchain Version : LLVM 12.0.0
+
+ABI Info : see qclib-interface.txt
+
=================== Release 00030 ================================
This Release Notes file covers these blobs:
* QcLib.elf
--
To view, visit https://review.coreboot.org/c/qc_blobs/+/59805
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: qc_blobs
Gerrit-Branch: master
Gerrit-Change-Id: I010bab3f8dc36672b53e08ef5359eef85952cc76
Gerrit-Change-Number: 59805
Gerrit-PatchSet: 1
Gerrit-Owner: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: merged
Attention is currently required from: Sudheer Amrabadi.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/qc_blobs/+/59805 )
Change subject: sc7180/boot : Update qclib blobs binaries and release notes
......................................................................
Patch Set 1: Verified+1 Code-Review+2
--
To view, visit https://review.coreboot.org/c/qc_blobs/+/59805
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: qc_blobs
Gerrit-Branch: master
Gerrit-Change-Id: I010bab3f8dc36672b53e08ef5359eef85952cc76
Gerrit-Change-Number: 59805
Gerrit-PatchSet: 1
Gerrit-Owner: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Comment-Date: Fri, 10 Dec 2021 20:05:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Reka Norman, Krishna P Bhat D, Usha P, Kangheui Won.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59915 )
Change subject: mb/intel/adlrvp_n: Add initial code for adl-n variant board
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/59915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4abf3bf62ec0398ae75e21575a2fab0d44b5c7ad
Gerrit-Change-Number: 59915
Gerrit-PatchSet: 5
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Kangheui Won <khwon(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Attention: Usha P <usha.p(a)intel.com>
Gerrit-Attention: Kangheui Won <khwon(a)google.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 19:20:33 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Patrick Georgi, Paul Menzel, Julius Werner.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60018 )
Change subject: cbfstool: Do host space address conversion earlier when adding files
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/60018/comment/af217561_f96f7637
PS3, Line 1052: offset
> I agree with Julius, it's probably something that needs fixing wholesale or not, otherwise it may ju […]
Yeah, I was more talking as a followup/cleanup.
--
To view, visit https://review.coreboot.org/c/coreboot/+/60018
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idf4721c5b0700789ddb81c1618d740b3e7f486cb
Gerrit-Change-Number: 60018
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Fri, 10 Dec 2021 19:11:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: YH Lin, Joey Peng.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60022 )
Change subject: mb/google/brya/var/taeko: Add define SOC_INTEL_ALDERLAKE_PCH_P in Kconfig.name
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
i'll have to try to see what happened here, but https://review.coreboot.org/c/coreboot/+/59804 already defined SOC_INTEL_ALDERLAKE_PCH_P for all of the brya boards...
--
To view, visit https://review.coreboot.org/c/coreboot/+/60022
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic58f8fee87559f2d5ad107ca3c9da882e4eb2381
Gerrit-Change-Number: 60022
Gerrit-PatchSet: 4
Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Rasheed Hsueh <rasheed.hsueh(a)lcfc.corp-partner.google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 19:04:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Shelley Chen, Jakub Czapiga, Paul Menzel, Yu-Ping Wu, Jianjun Wang.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59738 )
Change subject: lib: Add __fls() (Find Last Set)
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/59738
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib458abfec7e03b2979569a8440a6e69b0285ac32
Gerrit-Change-Number: 59738
Gerrit-PatchSet: 6
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Shelley Chen <shchen(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: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 18:33:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Michał Żygowski, Frans Hendriks, Arthur Heymans, Wim Vervoorn.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52025 )
Change subject: vc/eltan/security/verified_boot/vboot_check.c: Add cbfs_map_compressed()
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
Sorry, but you're just... breaking every assumption of how CBFS is supposed to be used with this. cbfs_map() does actually work on compressed files if your platform provides a _cbfs_cache, but it then provides you a pointer to the uncompressed data and that's not what you're looking for here, right? You're trying to get at the raw compressed data, and that just doesn't make sense in a system where compression is supposed to be transparent. This whole concept of you trying to stash something as invasive as a verification system away in vendorcode just doesn't really work out.
Haven't we talked about this before (specifically regarding prog_locate_hook())? I thought you guys had already said years ago that you were moving away from that and we could eventually deprecate it? What ever happened to that?
Can you not just use CONFIG_CBFS_VERIFICATION and CONFIG_TPM_MEASURED_BOOT? What high-level use cases are you trying to solve that are not covered by that?
I mean you can submit this patch and call a bunch of internal functions from your code private vendorcode that were not supposed to leave the CBFS core, breaking every abstraction we have... but sooner or later that will break again (in fact it's already broken again and this patch should no longer apply as written on ToT today), and then somebody who was just trying to update supposedly internal parts of the CBFS framework has to poke around here in this stuff that doesn't fit with any of the rest of coreboot and try to do your maintenance work (or they'll just accidentally break you because they don't understand the totally unintended ways in which you're using APIs, like I did in CB:39304). It's just not a long-term viable development approach. Complicated and invasive things like verification and measurement need to be implemented once, in the central framework, in a way that covers everyone's use cases... not a dozen times individually by each vendor. (Same goes for whatever you're doing with the vboot APIs here, btw, that's not how they're supposed to be used and it *will break* sooner or later.)
*Please* move your platforms onto the common frameworks. If you have use cases not covered by them, please come talk to us and we can try to find a solution. If you just keep doing your own thing somewhere that nobody else is looking at, it will keep breaking and eventually it will probably need to be removed for being a maintenance burden.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52025
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idc799f49ee198100deab1ecf66336831d9aed415
Gerrit-Change-Number: 52025
Gerrit-PatchSet: 3
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Wim Vervoorn <wvervoorn(a)eltan.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Wim Vervoorn <wvervoorn(a)eltan.com>
Gerrit-Comment-Date: Fri, 10 Dec 2021 18:28:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment