Attention is currently required from: Felix Held, Marshall Dawson, Matt DeVillier.
Hello Marshall Dawson, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84837?usp=email
to look at the new patch set (#10).
The following approvals got outdated and were removed:
Code-Review+1 by Matt DeVillier, Code-Review+2 by Marshall Dawson, Verified+1 by build bot (Jenkins)
Change subject: drivers/spi: add RPMC support
......................................................................
drivers/spi: add RPMC support
Add functions to send the RPMC-related commands to a RPMC-capable SPI
flash to provision the root key, update the HMAC key, increment the
monotonic counter, and request the monotonic counter data. To talk to
the flash chip, the command bytes and polling mechanism described in the
SFDP data structure of the flash are used.
The JESD260 specification was used as a reference.
Some of inspiration was taken from
github.com/teslamotors/coreboot/tree/tesla-4.12-amd
TEST=When used with the later patch that calls some of the functions
added in this patch, sending the RPMC increment monotonic counter
command to the RPMC-capable SPI flash, in this case W74M12JW, is
successful.
Change-Id: Ia9bd69d0105c66bf5ecb6c90e8c782a81912bd40
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
---
M src/drivers/spi/spi_flash_rpmc.c
M src/include/spi_flash.h
2 files changed, 357 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/84837/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/84837?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: Ia9bd69d0105c66bf5ecb6c90e8c782a81912bd40
Gerrit-Change-Number: 84837
Gerrit-PatchSet: 10
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Marshall Dawson has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/85251?usp=email )
Change subject: soc/amd/common/psp/rpmc: fix printk format string
......................................................................
Patch Set 3: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85251/comment/2eabb7a2_06b52ecd?us… :
PS3, Line 14: sting
nit "string"
https://review.coreboot.org/c/coreboot/+/85251/comment/d119a5e1_d4274b41?us… :
PS3, Line 13: The other option would have been to keep using size_t for the
: for loop counter, but using '%zu'
Why would it seem odd? I somewhat prefer the size_t and there a quite a number of other instances of %zu in the source.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85251?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: I32bc584abe835c9c1d732c12311881345b8f0cdf
Gerrit-Change-Number: 85251
Gerrit-PatchSet: 3
Gerrit-Owner: 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: Marshall Dawson <marshalldawson3rd(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-Attention: Jason Glenesk <jason.glenesk(a)gmail.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, 02 Dec 2024 16:46:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Marshall Dawson has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/85238?usp=email )
Change subject: soc/amd/common/psp/psp_smi: report errors in 'handle_psp_command'
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/85238?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: I8c02e8e29ab5619282e5b864a8cea6f0703f6ef2
Gerrit-Change-Number: 85238
Gerrit-PatchSet: 5
Gerrit-Owner: 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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.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, 02 Dec 2024 16:41:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Marshall Dawson has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/84906?usp=email )
Change subject: soc/amd/common/psp_smi_flash: implement SPI flash RPMC command handling
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84906?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: I20e4f60d4e35d33e560fc43212b320e817e13004
Gerrit-Change-Number: 84906
Gerrit-PatchSet: 11
Gerrit-Owner: 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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.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, 02 Dec 2024 16:40:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Marshall Dawson has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/85236?usp=email )
Change subject: soc/amd/common/block/psp/psp_smi_flash.h: fix struct element types
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/85236?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: I6c290535a1896c080b74d892cb289e6e122d4525
Gerrit-Change-Number: 85236
Gerrit-PatchSet: 2
Gerrit-Owner: 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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.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, 02 Dec 2024 16:33:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held.
Marshall Dawson has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/84837?usp=email )
Change subject: drivers/spi: add RPMC support
......................................................................
Patch Set 9: Code-Review+2
(3 comments)
File src/drivers/spi/spi_flash_rpmc.c:
https://review.coreboot.org/c/coreboot/+/84837/comment/3bacf027_f699238b?us… :
PS9, Line 250: memcpy(&buf[8], signature, SPI_RPMC_SIG_LEN);
Could consider using [4+SPI_RPMC_KEY_DATA_LEN], similar to line 218.
https://review.coreboot.org/c/coreboot/+/84837/comment/942f83c8_ef45e6c1?us… :
PS9, Line 281: memcpy(&buf[8], signature, SPI_RPMC_SIG_LEN);
similar
https://review.coreboot.org/c/coreboot/+/84837/comment/eda8f64f_a180dee4?us… :
PS9, Line 314: memcpy(&buf[16], signature, SPI_RPMC_SIG_LEN);
similar
--
To view, visit https://review.coreboot.org/c/coreboot/+/84837?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: Ia9bd69d0105c66bf5ecb6c90e8c782a81912bd40
Gerrit-Change-Number: 84837
Gerrit-PatchSet: 9
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 02 Dec 2024 16:30:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Singer, Jon Murphy, Karthik Ramasubramanian, Martin L Roth.
Raul Rangel has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/85275?usp=email )
Change subject: util/crossgcc: Add libstdcxx target
......................................................................
Patch Set 8:
(7 comments)
Patchset:
PS5:
> My concern is that what we have on upstream blows up too much. […]
Honestly I feel like any modular solution we come up with is going to be more complex than this 48 line diff.
File util/crossgcc/buildgcc:
https://review.coreboot.org/c/coreboot/+/85275/comment/572d1362_90901043?us… :
PS8, Line 1: sh
Ugh POSIX `sh`... why not bash?
https://review.coreboot.org/c/coreboot/+/85275/comment/eea4e8cb_b47a37a3?us… :
PS8, Line 783: declare
`local`?
https://review.coreboot.org/c/coreboot/+/85275/comment/4a86a048_0891770a?us… :
PS8, Line 784: @
POSIX `sh` doesn't actually have arrays AFAIK.
https://review.coreboot.org/c/coreboot/+/85275/comment/afc62b34_c285f9d8?us… :
PS8, Line 787: rm -f "gcc/$TARGETARCH/$GCC_VERSION" && \
: ln -s "$DESTDIR$TARGETDIR/$TARGETARCH/bin" "gcc/$TARGETARCH/$GCC_VERSION" && \
: $MAKE $JOBS CFLAGS_FOR_BUILD="$HOSTCFLAGS" all-gcc && \
: $MAKE install-gcc DESTDIR="$DESTDIR" || touch .failed
:
: if [ ! -f .failed ] && [ "$(echo $TARGETARCH | grep -c -- -mingw32)" -eq 0 ]; then
: # shellcheck disable=SC2086
: $MAKE $JOBS CFLAGS_FOR_BUILD="$HOSTCFLAGS" all-target-libgcc && \
: $MAKE install-target-libgcc DESTDIR=$DESTDIR || touch .failed
: fi
Move this function below `configure_GCC` so that the diff shows that this wasn't changed.
https://review.coreboot.org/c/coreboot/+/85275/comment/6e6c991f_7991344b?us… :
PS8, Line 858: [@]
Not an array
https://review.coreboot.org/c/coreboot/+/85275/comment/8c867bff_50f85096?us… :
PS8, Line 862: LIBSTDCXX_INCLUDE_PATH
Maybe verify that `LIBSTDCXX_INCLUDE_PATH` isn't empty?
--
To view, visit https://review.coreboot.org/c/coreboot/+/85275?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: Ie0c06ffaeab632c27a992dee8abcc403cceabeed
Gerrit-Change-Number: 85275
Gerrit-PatchSet: 8
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 02 Dec 2024 16:27:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com>
Attention is currently required from: Jon Murphy, Karthik Ramasubramanian, Martin L Roth, Raul Rangel.
Felix Singer has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/85275?usp=email )
Change subject: util/crossgcc: Add libstdcxx target
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS5:
> The coreboot toolchain has always been used inside Google as the standard for ChromeOS firmware deve […]
My concern is that what we have on upstream blows up too much. The shell script is already quite big. Now I imagine multiple parties (no matter if companies or individuals) which want to add their use cases, because if we allow one then we also have to allow others. So I would like to limit the coreboot toolchain to what's needed by the project.
Maybe we can figure out a more modular design, so that's easy for you to add things on top and/or extend it. I will think about it. Let me know if you have any ideas.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85275?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: Ie0c06ffaeab632c27a992dee8abcc403cceabeed
Gerrit-Change-Number: 85275
Gerrit-PatchSet: 8
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 02 Dec 2024 15:49:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com>
Attention is currently required from: Karthik Ramasubramanian, Naveen M, Paul Menzel, Pranava Y N, Subrata Banik.
Varun Upadhyay has posted comments on this change by Varun Upadhyay. ( https://review.coreboot.org/c/coreboot/+/85282?usp=email )
Change subject: drivers/soundwire/alc711: Add common Kconfig for ALC7xx soundwire codecs
......................................................................
Patch Set 6:
(2 comments)
File src/drivers/soundwire/alc711/alc711.c:
https://review.coreboot.org/c/coreboot/+/85282/comment/28dee189_9db2ce7e?us… :
PS4, Line 15: SOUNDWIRE_VERSION_1_2
> +1.
Removed the dependancy on patch and maintaining previous config
https://review.coreboot.org/c/coreboot/+/85282/comment/9dcc6326_7a00fff0?us… :
PS4, Line 20: .manufacturer_id = MIPI_MFG_ID_REALTEK,
> > This is never used in the driver. […]
This MIPI_MFG_ID_REALTEK gets included in creating the device address for ACPI table so keeping it as part of code.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85282?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: I5953d0fcb7b15368888901f88c5616757ac42877
Gerrit-Change-Number: 85282
Gerrit-PatchSet: 6
Gerrit-Owner: Varun Upadhyay <varun.upadhyay(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Naveen M <naveen.m(a)intel.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Naveen M <naveen.m(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 02 Dec 2024 15:47:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Varun Upadhyay <varun.upadhyay(a)intel.com>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Naveen M, Paul Menzel, Pranava Y N, Subrata Banik.
Varun Upadhyay has posted comments on this change by Varun Upadhyay. ( https://review.coreboot.org/c/coreboot/+/85440?usp=email )
Change subject: mb/google/fatcat: Update Soundwire codec address based on devicetree
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85440/comment/677c92ab_b5cfe38d?us… :
PS1, Line 8:
> Please elaborate. Maybe: […]
Sure, updated the description.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85440?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: I3322ae2d106d3628dbf627aacf999056d82ee7a9
Gerrit-Change-Number: 85440
Gerrit-PatchSet: 2
Gerrit-Owner: Varun Upadhyay <varun.upadhyay(a)intel.com>
Gerrit-Reviewer: Naveen M <naveen.m(a)intel.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Naveen M <naveen.m(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 02 Dec 2024 15:39:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>