coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

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

03 Nov '23

Attention is currently required from: Bill XIE, Jérémy Compostella. Nico Huber 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 4: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/78906/comment/a6c0b8ed_869bbcd0 : PS4, Line 11: , I would remove this comma. It separates things too much in an already loooong sentence. Patchset: PS4: I agree to the patch, just want to mention that this will most likely change behavior in unexpected cases. IIRC, we have some `cmos.layout` files that define things outside the checksummed range. Without looking closer, it's hard to say if these should be cleared or not. And then there are also cases when things aren't declared at all. But we could argue that they aren't cleared either with the option-table support disabled. Alas, behavior is vaguely defined in this area of coreboot. -- 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-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 13:36:55 +0000 Gerrit-HasComments: Yes 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 Nico Huber (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Bill XIE, Kyösti Mälkki. Nico Huber 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/03ba4dc7_e2fac200 : PS1, Line 204: (CONFIG(STATIC_OPTION_TABLE) && !acpi_is_wakeup_s3()) > During CB:78288 I Have considered this, and decided to defer CMOS reset on error to the next boot, for CMOS reset during s3 on GM45 platforms will erase their DRAM training data, making s3 resume fail. Yes, but what are the chances that GM45 could continue booting without resetting CMOS? And multiply that with the chance that an error occurs in the first place. I don't think it matters more than returning to a known state, on error. > Do we really need to reset CMOS on error during s3 resume? We could as well ask "Do we really need to reset CMOS ever?". CMOS is read during resume as it is on any boot. So it boils down to the question if we prefer a known default state over a potentially corrupted one. I don't see why we should make an exception for S3 resume. -- 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: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 13:27:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: libpayload/libc/time: Fix possible overflow in multiplication
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Julius Werner, Yu-Ping Wu. Hello Julius Werner, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78888?usp=email to look at the new patch set (#6). Change subject: libpayload/libc/time: Fix possible overflow in multiplication ...................................................................... libpayload/libc/time: Fix possible overflow in multiplication The value from raw_read_cntfrq_el0() could be large enough to cause overflow when multiplied by USECS_PER_SEC. To prevent this, both USECS_PER_SEC and hz can be reduced by dividing them by their GCD. BUG=b:307790895 TEST=boot to kernel and check the timestamps from `cbmem` Change-Id: Ia55532490651fcf47128b83a8554751f050bcc89 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M payloads/libpayload/drivers/timer/arm64_arch_timer.c M payloads/libpayload/drivers/timer/generic.c M payloads/libpayload/drivers/timer/rdtsc.c M payloads/libpayload/include/libpayload.h M payloads/libpayload/libc/time.c 5 files changed, 16 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/78888/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/78888?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: Ia55532490651fcf47128b83a8554751f050bcc89 Gerrit-Change-Number: 78888 Gerrit-PatchSet: 6 Gerrit-Owner: Yidi Lin <yidilin(a)google.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-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newpatchset

1 0

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

03 Nov '23

Attention is currently required from: Bill XIE, Jérémy Compostella. Jonathon Hall 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 4: (2 comments) Patchset: PS4: Thank you Bill! Looks great File src/drivers/pc80/rtc/option.c: https://review.coreboot.org/c/coreboot/+/78906/comment/5841d187_d48de62a : PS4, Line 217: for (i = LB_CKS_RANGE_START; i < MIN(LB_CKS_RANGE_END, length); i++) @Bill It looks like the range end is inclusive, so `LB_CKS_RANGE_END+1` here. Mini v2's checksum only covers one byte and the option_table.h has: ``` #define LB_CKS_RANGE_START 169 #define LB_CKS_RANGE_END 169 ``` Thanks for putting this together! -- 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 12:06:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: libpayload/libc/time: Fix possible overflow in multiplication
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Julius Werner, Yu-Ping Wu. Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78888?usp=email ) Change subject: libpayload/libc/time: Fix possible overflow in multiplication ...................................................................... Patch Set 5: (2 comments) File payloads/libpayload/libc/time.c: https://review.coreboot.org/c/coreboot/+/78888/comment/b4bf0e0b_c4a3a7e9 : PS4, Line 181: 1000000 > There's also `MHz`. Done https://review.coreboot.org/c/coreboot/+/78888/comment/19697f62_38078853 : PS4, Line 189: * hz to uint32_t. > This is architecture-independent code, so this comment doesn't belong here. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78888?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: Ia55532490651fcf47128b83a8554751f050bcc89 Gerrit-Change-Number: 78888 Gerrit-PatchSet: 5 Gerrit-Owner: Yidi Lin <yidilin(a)google.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-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 10:50:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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

[S] Change in coreboot[main]: arch/arm64/arch_timer: Fix possible overflow in multiplication
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Arthur Heymans, Julius Werner, Paul Menzel, Yu-Ping Wu. Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78800?usp=email ) Change subject: arch/arm64/arch_timer: Fix possible overflow in multiplication ...................................................................... Patch Set 8: (2 comments) Patchset: PS7: > Hmm... if you shuffle the memlayout (src/soc/qualcomm/sc7280/memlayout. […] CB:78893 File src/arch/arm64/arch_timer.c: https://review.coreboot.org/c/coreboot/+/78800/comment/80345fa7_264351c8 : PS7, Line 21: 1000000 > Could use USECS_PER_SEC here as well. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78800?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: I366667de05392913150414f0fa9058725be71c52 Gerrit-Change-Number: 78800 Gerrit-PatchSet: 8 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 10:49:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/qualcomm/sc7280: Enlarge BOOTBLOCK to 44K
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Yidi Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78893?usp=email ) Change subject: soc/qualcomm/sc7280: Enlarge BOOTBLOCK to 44K ...................................................................... soc/qualcomm/sc7280: Enlarge BOOTBLOCK to 44K After CB:78800 applied, the bootblock increases 2128 bytes and exceeded its allotted size (40K). Therefore, we enlarge BOOTBLOCK to 44K to solve the compilation error. This patch also increases PRERAM_CBFS_CACHE to 103K to fill the empty space (1K) between TIMESTAMP and TTB. BRANCH=none BUG=none TEST=./util/abuild/abuild -p none -t GOOGLE_HEROBRINE -x -a -B Change-Id: Iae9d44939b29098e823508dd3965a1bae7a69041 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M src/soc/qualcomm/sc7280/memlayout.ld 1 file changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/78893/1 diff --git a/src/soc/qualcomm/sc7280/memlayout.ld b/src/soc/qualcomm/sc7280/memlayout.ld index 95824c7..231b9fe 100644 --- a/src/soc/qualcomm/sc7280/memlayout.ld +++ b/src/soc/qualcomm/sc7280/memlayout.ld @@ -33,12 +33,12 @@ BSRAM_START(0x14800000) REGION(pbl_timestamps, 0x14800000, 84K, 4K) WATCHDOG_TOMBSTONE(0x14818FFC, 4) - BOOTBLOCK(0x14819000, 40K) - PRERAM_CBFS_CACHE(0x14823000, 102K) - TIMESTAMP(0x1483C800, 1K) - TTB(0x1483D000, 56K) - STACK(0x1484B000, 16K) - VBOOT2_WORK(0x1484F000, 12K) + BOOTBLOCK(0x14819000, 44K) + PRERAM_CBFS_CACHE(0x14824000, 103K) + TIMESTAMP(0x1483DC00, 1K) + TTB(0x1483E000, 56K) + STACK(0x1484C000, 16K) + VBOOT2_WORK(0x14850000, 12K) DMA_COHERENT(0x14853000, 8K) REGION(qclib_serial_log, 0x14857000, 4K, 4K) CBFS_MCACHE(0x14858000,16K) -- To view, visit https://review.coreboot.org/c/coreboot/+/78893?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: Iae9d44939b29098e823508dd3965a1bae7a69041 Gerrit-Change-Number: 78893 Gerrit-PatchSet: 1 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: libpayload/libc/time: Fix possible overflow in multiplication
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Julius Werner, Yidi Lin, Yu-Ping Wu. Hello Julius Werner, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78888?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+2 by Yu-Ping Wu Change subject: libpayload/libc/time: Fix possible overflow in multiplication ...................................................................... libpayload/libc/time: Fix possible overflow in multiplication The value from raw_read_cntfrq_el0() could be large enough to cause overflow when multiplied by USECS_PER_SEC. To prevent this, both USECS_PER_SEC and hz can be reduced by dividing them by their GCD. BUG=b:307790895 TEST=boot to kernel and check the timestamps from `cbmem` Change-Id: Ia55532490651fcf47128b83a8554751f050bcc89 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M payloads/libpayload/drivers/timer/arm64_arch_timer.c M payloads/libpayload/drivers/timer/rdtsc.c M payloads/libpayload/include/libpayload.h M payloads/libpayload/libc/time.c 4 files changed, 14 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/78888/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/78888?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: Ia55532490651fcf47128b83a8554751f050bcc89 Gerrit-Change-Number: 78888 Gerrit-PatchSet: 5 Gerrit-Owner: Yidi Lin <yidilin(a)google.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-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: arch/arm64/arch_timer: Fix possible overflow in multiplication
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Arthur Heymans, Paul Menzel, Yidi Lin, Yu-Ping Wu. Hello Arthur Heymans, Julius Werner, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78800?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Code-Review+2 by Yu-Ping Wu Change subject: arch/arm64/arch_timer: Fix possible overflow in multiplication ...................................................................... arch/arm64/arch_timer: Fix possible overflow in multiplication The value from raw_read_cntfrq_el0() could be large enough to cause overflow when multiplied by USECS_PER_SEC. To prevent this, both USECS_PER_SEC and tfreq can be reduced by dividing them by their GCD. BUG=b:307790895 TEST=emerge-geralt coreboot TEST=boot to kernel and check the timestamps from `cbmem` Change-Id: I366667de05392913150414f0fa9058725be71c52 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M src/arch/arm64/arch_timer.c 1 file changed, 20 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/78800/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/78800?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: I366667de05392913150414f0fa9058725be71c52 Gerrit-Change-Number: 78800 Gerrit-PatchSet: 8 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: mb/google/brya/var/marasov: Enable wifi sar table for Intel module
by Daniel Peng (Code Review) 03 Nov '23

03 Nov '23

Daniel Peng has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/78310?usp=email ) Change subject: mb/google/brya/var/marasov: Enable wifi sar table for Intel module ...................................................................... Abandoned Move the changed about branch from master to main. -- To view, visit https://review.coreboot.org/c/coreboot/+/78310?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5b5c6bea6c2c916fb682044218ec7b3a5d2659f6 Gerrit-Change-Number: 78310 Gerrit-PatchSet: 3 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: 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-MessageType: abandon

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023