coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[XS] Change in coreboot[main]: fmap: Die immediately on verification failure
by Yu-Ping Wu (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Jakub Czapiga, Julius Werner. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78903?usp=email ) Change subject: fmap: Die immediately on verification failure ...................................................................... Patch Set 1: Code-Review+1 (2 comments) File src/lib/fmap.c: https://review.coreboot.org/c/coreboot/+/78903/comment/ffbb35be_b775c039 : PS1, Line 42: filled with a tampered FMAP but the later fallback path is fed a valid one. */ Also explain why we want to die even if `!CONFIG(TOCTOU_SAFETY)`? BTW there's a TODO comment which I think now can be fixed. ``` config TOCTOU_SAFETY depends on !VBOOT # TODO: can only allow this once vboot fully integrated ``` https://review.coreboot.org/c/coreboot/+/78903/comment/d36ac012_ae2dcb85 : PS1, Line 127: sizeof(struct fmap) Not related to this patch, but this is supposed to be `FMAP_SIZE`, right? (at least for `CONFIG(CBFS_VERIFICATION) && ENV_INITIAL_STAGE`) -- To view, visit https://review.coreboot.org/c/coreboot/+/78903?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I59ec91c3e5a59fdd960b0ba54ae5f15ddb850480 Gerrit-Change-Number: 78903 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Nov 2023 06:19:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Jonathon Hall, Jérémy Compostella. Hello Jonathon Hall, Jérémy Compostella, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78906?usp=email to look at the new patch set (#4). Change subject: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum ...................................................................... drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum Proposed in the comment of commit 29030d0f3dad ("drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume"), during sanitize_cmos(), only reset CMOS range covered by checksum, and the checksum itself from the file cmos.default in CBFS, in order to prevent other runtime data in CMOS (e.g. the DRAM training data on GM45 platforms for s3 resume) being erased. Tested: cherry-pick this commit before commit 44a48ce7a46c ("Kconfig: Bring HEAP_SIZE to a common, large value"), which is already before my commit 29030d0f3dad , Thinkpad X200 with CONFIG(STATIC_OPTION_TABLE) can resume from s3 again, indicating that DRAM training data are no longer erased. Signed-off-by: Bill XIE <persmule(a)hardenedlinux.org> Co-authored-by: Jonathon Hall <jonathon.hall(a)puri.sm> Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a --- M src/drivers/pc80/rtc/option.c 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/78906/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/78906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a Gerrit-Change-Number: 78906 Gerrit-PatchSet: 4 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Jonathon Hall, Jérémy Compostella. Hello Jonathon Hall, Jérémy Compostella, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78906?usp=email to look at the new patch set (#3). Change subject: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum ...................................................................... drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum Proposed in the comment of commit 29030d0f3dad ("drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume"), during sanitize_cmos(), only reset CMOS range covered by checksum, and the checksum itself from the file cmos.default in CBFS, in order to prevent other runtime data in CMOS (e.g. the DRAM training data on GM45 platforms for s3 resume) being erased. Tested: cherry-pick this commit before commit 44a48ce7a46c ("Kconfig: Bring HEAP_SIZE to a common, large value"), which is already before my commit 29030d0f3dad , Thinkpad X200 with CONFIG(STATIC_OPTION_TABLE) can resume from s3 again, indicating that DRAM training data are no longer erased. Signed-off-by: Bill XIE <persmule(a)hardenedlinux.org> Co-authored-by: Jonathon Hall <jonathon.hall(a)puri.sm> Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a --- M src/drivers/pc80/rtc/option.c 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/78906/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/78906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a Gerrit-Change-Number: 78906 Gerrit-PatchSet: 3 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Jonathon Hall, Jérémy Compostella. Hello Jonathon Hall, Jérémy Compostella, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78906?usp=email to look at the new patch set (#2). Change subject: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum ...................................................................... drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum Proposed in the comment of commit 29030d0f3dad ("drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume"), during sanitize_cmos(), only reset CMOS range covered by checksum, and the checksum itself from the file cmos.default in CBFS, in order to prevent other runtime data in CMOS (e.g. the DRAM training data on GM45 platforms for s3 resume) being erased. Tested: cherry-pick this commit before commit 44a48ce7a46c ("Kconfig: Bring HEAP_SIZE to a common, large value"), which is already before my commit 29030d0f3dad , Thinkpad X200 with CONFIG(STATIC_OPTION_TABLE) can resume from s3 again, indicating that DRAM training data are no longer erased. Signed-off-by: Bill XIE <persmule(a)hardenedlinux.org> Co-authored-by: Jonathon Hall <jonathon.hall(a)puri.sm> Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a --- M 3rdparty/amd_blobs M src/drivers/pc80/rtc/option.c 2 files changed, 4 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/78906/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a Gerrit-Change-Number: 78906 Gerrit-PatchSet: 2 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: pc80/rtc: Clear CMOS on errors on S3 resume too
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Kyösti Mälkki, Nico Huber. Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78881?usp=email ) Change subject: pc80/rtc: Clear CMOS on errors on S3 resume too ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/78881?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1533875466f6eb7b409bb19ef98f6dcaf2d115c4 Gerrit-Change-Number: 78881 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 05:59:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: pc80/rtc: Clear CMOS on errors on S3 resume too
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Kyösti Mälkki, Nico Huber. Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78881?usp=email ) Change subject: pc80/rtc: Clear CMOS on errors on S3 resume too ...................................................................... Patch Set 1: (1 comment) File src/drivers/pc80/rtc/option.c: https://review.coreboot.org/c/coreboot/+/78881/comment/c8d761c9_fa032a97 : PS1, Line 204: (CONFIG(STATIC_OPTION_TABLE) && !acpi_is_wakeup_s3()) > @Jonathon Hall The ultimate solution fur this may be your proposal in the comment of CB:78288 to exclude the range of memory training data from being reset. Implemented in CB:78906. -- To view, visit https://review.coreboot.org/c/coreboot/+/78881?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1533875466f6eb7b409bb19ef98f6dcaf2d115c4 Gerrit-Change-Number: 78881 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 05:58:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Jonathon Hall, Jérémy Compostella. Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78906?usp=email ) Change subject: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I implemented your proposal in CB:78288 here. -- To view, visit https://review.coreboot.org/c/coreboot/+/78906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a Gerrit-Change-Number: 78906 Gerrit-PatchSet: 1 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Comment-Date: Fri, 03 Nov 2023 05:55:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Bill XIE has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78906?usp=email ) Change subject: drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum ...................................................................... drivers/pc80/rtc/option.c: Reset only CMOS range covered by checksum Proposed in the comment of commit 29030d0f3dad (drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume), during sanitize_cmos(), only reset CMOS range covered by checksum, and the checksum itself from the file cmos.default in CBFS, in order to prevent other runtime data in CMOS (e.g. the DRAM training data on GM45 platforms for s3 resume) being erased. Tested: cherry-pick this commit before commit 44a48ce7a46c ("Kconfig: Bring HEAP_SIZE to a common, large value"), which is already before my commit 29030d0f3dad , Thinkpad X200 with CONFIG(STATIC_OPTION_TABLE) can resume from s3 again, indicating that DRAM training data are no longer erased. Signed-off-by: Bill XIE <persmule(a)hardenedlinux.org> Co-authored-by: Jonathon Hall <jonathon.hall(a)puri.sm> Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a --- M src/drivers/pc80/rtc/option.c 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/78906/1 diff --git a/src/drivers/pc80/rtc/option.c b/src/drivers/pc80/rtc/option.c index 0954335..5ca44e8 100644 --- a/src/drivers/pc80/rtc/option.c +++ b/src/drivers/pc80/rtc/option.c @@ -213,9 +213,10 @@ return; u8 control_state = cmos_disable_rtc(); - /* Max length of 256 spans bank 0 and bank 1 */ - for (i = 14; i < MIN(256, length); i++) + /* Copy checked range and the checksum from the default */ + for (i = LB_CKS_RANGE_START; i < MIN(LB_CKS_RANGE_END, length); i++) cmos_write_inner(cmos_default[i], i); + cmos_write_inner(cmos_default[LB_CKS_LOC], LB_CKS_LOC); cmos_restore_rtc(control_state); } } -- To view, visit https://review.coreboot.org/c/coreboot/+/78906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I872bf5f41422bc3424cd8631e932aaae2ae82f7a Gerrit-Change-Number: 78906 Gerrit-PatchSet: 1 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: mb/google/brya/var/marasov: Enable Wi-Fi sar table for Intel module
by Eric Lai (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Daniel Peng, Karthik Ramasubramanian, Kyle Lin, Nick Vaccaro, Paul Menzel. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77789?usp=email ) Change subject: mb/google/brya/var/marasov: Enable Wi-Fi sar table for Intel module ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/77789?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5b5c6bea6c2c916fb682044218ec7b3a5d2659f6 Gerrit-Change-Number: 77789 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Kyle Lin <kylelinck(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 05:46:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: util/docker/coreboot-sdk: Add tmux to package list
by Felix Singer (Code Review) 03 Nov '23

03 Nov '23

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78890?usp=email ) Change subject: util/docker/coreboot-sdk: Add tmux to package list ...................................................................... util/docker/coreboot-sdk: Add tmux to package list tmux is a very useful tool. So add it to the package list. Change-Id: I1c389863cc40750012f971aae943d14ce82dcb3c Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M util/docker/coreboot-sdk/Dockerfile 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/78890/1 diff --git a/util/docker/coreboot-sdk/Dockerfile b/util/docker/coreboot-sdk/Dockerfile index daced1b..b4b0322 100644 --- a/util/docker/coreboot-sdk/Dockerfile +++ b/util/docker/coreboot-sdk/Dockerfile @@ -78,6 +78,7 @@ rsync \ sharutils \ shellcheck \ + tmux \ unifont \ unzip \ uuid-dev \ -- To view, visit https://review.coreboot.org/c/coreboot/+/78890?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c389863cc40750012f971aae943d14ce82dcb3c Gerrit-Change-Number: 78890 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023