coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
257 discussions

[S] Change in coreboot[main]: commonlib/include/commonlib: Add volatile qualifier
by Patrick Rudolph (Code Review) 05 Jan '25

05 Jan '25

Attention is currently required from: Angel Pons, Patrick Rudolph. Hello Angel Pons, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85855?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: commonlib/include/commonlib: Add volatile qualifier ...................................................................... commonlib/include/commonlib: Add volatile qualifier With the introduction of the stack canary breakpoint QEMU uncovered a different bug within coreboot. Currently the compiler optimizes over aggressively inline functions and memory stores. That also affects write_at_ble8(), which is supposed to store a single byte at time. The compiler however optimizes multiple byte stores into a single wider (and possibly unaligned) store operation. This can be seen in the emited assembly code of write_le16(), as used to install the EBDA: 401348a: 66 c7 04 25 13 04 00 movw $0x400,0x413 4013491: 00 00 04 Make sure that the compiler does not optimize multiple calls to write_at_ble8() by adding the volatile qualifier. The emitted assembly code of the same function changes to: 401394c: c6 04 25 13 04 00 00 movb $0x0,0x413 4013953: 00 4013954: c6 04 25 14 04 00 00 movb $0x4,0x414 401395b: 04 Fixes a strange bug in QEMU where it triggers the DEBUG breakpoint handler on unaligned 16-bit stores in the first 4KiB of memory. Aligned stores and store outside of the first 4KiB do not dispatch the DEBUG breakpoint handler. Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/commonlib/include/commonlib/endian.h 1 file changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/85855/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/85855?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: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Gerrit-Change-Number: 85855 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: commonlib/include/commonlib: Add volatile qualifier
by Patrick Rudolph (Code Review) 05 Jan '25

05 Jan '25

Attention is currently required from: Angel Pons, Patrick Rudolph. Hello Angel Pons, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85855?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+2 by Angel Pons, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: commonlib/include/commonlib: Add volatile qualifier ...................................................................... commonlib/include/commonlib: Add volatile qualifier With the introduction of the stack canary breakpoint QEMU uncovered a different bug within coreboot. Currently the compiler optimizes over aggressively inline functions and memory stores. That also affects write_at_ble8(), which is supposed to store a single byte at time. The compiler however optimizes multiple byte stores into a single wider (and possibly unaligned) store operation. This can be seen in the emited assembly code of write_le16(), as used to install the EBDA: 401348a: 66 c7 04 25 13 04 00 movw $0x400,0x413 4013491: 00 00 04 Make sure that the compiler does not optimize multiple calls to write_at_ble8() by adding the volatile qualifier. The emitted assembly code of the same function changes to: 401394c: c6 04 25 13 04 00 00 movb $0x0,0x413 4013953: 00 4013954: c6 04 25 14 04 00 00 movb $0x4,0x414 401395b: 04 Fixes a strange bug in QEMU where it triggers the DEBUG breakpoint handler on unaligned 16-bit stores in the first 4KiB of memory. Aligned stores and store outside of the first 4KiB do not dispatch the DEBUG breakpoint handler. Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/commonlib/include/commonlib/endian.h 1 file changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/85855/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/85855?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: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Gerrit-Change-Number: 85855 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>

1 0

[XS] Change in coreboot[main]: commonlib/include/commonlib: Add barrier in write_at_ble8()
by Angel Pons (Code Review) 05 Jan '25

05 Jan '25

Attention is currently required from: Patrick Rudolph. Angel Pons has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85855?usp=email ) Change subject: commonlib/include/commonlib: Add barrier in write_at_ble8() ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85855?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: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Gerrit-Change-Number: 85855 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Sun, 05 Jan 2025 16:25:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: commonlib/include/commonlib: Add barrier in write_at_ble8()
by Patrick Rudolph (Code Review) 05 Jan '25

05 Jan '25

Patrick Rudolph has uploaded a new patch set (#3). ( https://review.coreboot.org/c/coreboot/+/85855?usp=email ) Change subject: commonlib/include/commonlib: Add barrier in write_at_ble8() ...................................................................... commonlib/include/commonlib: Add barrier in write_at_ble8() With the introduction of the stack canary breakpoint QEMU uncovered a different bug within coreboot. Currently the compiler optimizes over aggressively inline functions and memory stores. That also affects write_at_ble8(), which is supposed to store a single byte at time. The compiler however optimizes multiple byte stores into a single wider (and possibly unaligned) store operation. This can be seen in the emited assembly code of write_le16(), as used to install the EBDA: 401348a: 66 c7 04 25 13 04 00 movw $0x400,0x413 4013491: 00 00 04 Make sure that the compiler does not optimize multiple calls to write_at_ble8() by adding a memory barrier. The emitted assembly code of the same function changes to: 401394c: c6 04 25 13 04 00 00 movb $0x0,0x413 4013953: 00 4013954: c6 04 25 14 04 00 00 movb $0x4,0x414 401395b: 04 Fixes a strange bug in QEMU where it triggers the DEBUG breakpoint handler on unaligned 16-bit stores in the first 4KiB of memory. Aligned stores and store outside of the first 4KiB do not dispatch the DEBUG breakpoint handler. Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/commonlib/include/commonlib/endian.h 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/85855/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/85855?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: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Gerrit-Change-Number: 85855 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[XS] Change in coreboot[main]: commonlib/include/commonlib: Add barrier in write_at_ble8()
by Patrick Rudolph (Code Review) 05 Jan '25

05 Jan '25

Patrick Rudolph has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/85855?usp=email ) Change subject: commonlib/include/commonlib: Add barrier in write_at_ble8() ...................................................................... commonlib/include/commonlib: Add barrier in write_at_ble8() With the introduction of the stack canary breakpoint QEMU uncovered a different bug within coreboot. Currently the compiler optimizes over aggressively inline functions and memory stores. That also affects write_at_ble8(), which is supposed to store a single byte at time. The compiler however optimizes multiple byte stores into a single wider (and possibly unaligned) store operation. Make sure that the compiler does not optimize multiple calls to write_at_ble8() by adding a memory barrier. Fixes a strange bug in QEMU where it triggers the DEBUG breakpoint handler on unaligned 16-bit stores in the first 4KiB of memory. Aligned stores and store outside of the first 4KiB do not dispatch the DEBUG breakpoint handler. Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/commonlib/include/commonlib/endian.h 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/85855/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85855?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: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Gerrit-Change-Number: 85855 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[XS] Change in coreboot[main]: commonlib/include/commonlib: Add barrier in write_at_ble8()
by Patrick Rudolph (Code Review) 05 Jan '25

05 Jan '25

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85855?usp=email ) Change subject: commonlib/include/commonlib: Add barrier in write_at_ble8() ...................................................................... commonlib/include/commonlib: Add barrier in write_at_ble8() With the introduction of the stack canary breakpoint QEMU uncovered a different bug within coreboot. Currently the compiler optimizes over aggressively inline functions and memory stores. That also affects write_at_ble8(), which is supposed to store a single byte at time. The compiler however optimizes multiple byte stores into a single wider (and possibly unaligned) store operation. Make sure that the compiler does not optimize multiple calls to write_at_ble8() by adding a memory barrier. Fixes a strange bug in QEMU where it triggers the DEBUG INT handler on unaligned 16-bit stores in the first 4KiB of memory. Aligned stores and store outside of the first 4KiB do not dispatch the DEBUG INT handler. Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/commonlib/include/commonlib/endian.h 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/85855/1 diff --git a/src/commonlib/include/commonlib/endian.h b/src/commonlib/include/commonlib/endian.h index 84c90b4..d0280b1 100644 --- a/src/commonlib/include/commonlib/endian.h +++ b/src/commonlib/include/commonlib/endian.h @@ -25,6 +25,11 @@ static inline void write_ble8(void *dest, uint8_t val) { *(uint8_t *)dest = val; + /* + * Prevent compilers from optimizing multiple write_ble8() into + * a single unaligned store operation by adding a memory barrier. + */ + __asm__ __volatile__("" : : : "memory"); } static inline void write_at_ble8(void *dest, uint8_t val, size_t offset) -- To view, visit https://review.coreboot.org/c/coreboot/+/85855?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ibbc661235a38c7f7540b656a67f067c3e51105d1 Gerrit-Change-Number: 85855 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Correct MT6363 buck5 enable API
by Yidi Lin (Code Review) 05 Jan '25

05 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yu-Ping Wu. Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85838?usp=email ) Change subject: soc/mediatek/mt8196: Correct MT6363 buck5 enable API ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/85838/comment/6bfcc8c4_7f257c05?us… : PS1, Line 12: build pass Please elaborate how you verify this patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/85838?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: I0af1e0582ae8fc1e219f3cce536aed9985108be5 Gerrit-Change-Number: 85838 Gerrit-PatchSet: 1 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hope Wang <hope.wang(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(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: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Sun, 05 Jan 2025 13:44:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Correct MT6363 buck5 enable API
by Yidi Lin (Code Review) 05 Jan '25

05 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yu-Ping Wu. Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85838?usp=email ) Change subject: soc/mediatek/mt8196: Correct MT6363 buck5 enable API ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85838/comment/ee7dc351_2d70199a?us… : PS1, Line 7: Correct MT6363 buck5 enable API Fix MT6363 buck5 enablement API https://review.coreboot.org/c/coreboot/+/85838/comment/4afbdcc9_5f57ead4?us… : PS1, Line 9: MT6363 buck5 is not used in rauru/navi. Fixed the incorrect mask and : offset in the enable API. The MT6363 buck5 API's mask and offset settings were incorrect, preventing the buck from being disabled. This issue is resolved by correcting these two values. -- To view, visit https://review.coreboot.org/c/coreboot/+/85838?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: I0af1e0582ae8fc1e219f3cce536aed9985108be5 Gerrit-Change-Number: 85838 Gerrit-PatchSet: 1 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hope Wang <hope.wang(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(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: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Sun, 05 Jan 2025 13:16:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: testetsttestsetsete
by Name of user not set (Code Review) 05 Jan '25

05 Jan '25

Name of user not set #1005756 has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/85854?usp=email ) Change subject: testetsttestsetsete ...................................................................... Abandoned It's just test. Very Sorry... -- To view, visit https://review.coreboot.org/c/coreboot/+/85854?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I652bc4916f5c623be352d476b083098b8240d03d Gerrit-Change-Number: 85854 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1005756 Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>

1 0

[XS] Change in coreboot[main]: testetsttestsetsete
by Name of user not set (Code Review) 05 Jan '25

05 Jan '25

Name of user not set #1005756 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85854?usp=email ) Change subject: testetsttestsetsete ...................................................................... testetsttestsetsete Change-Id: I652bc4916f5c623be352d476b083098b8240d03d Signed-off-by: test <test(a)test.com> --- M Makefile 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/85854/1 diff --git a/Makefile b/Makefile index 491e702..c49f8c6 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ ## SPDX-License-Identifier: BSD-3-Clause +#testtest ifneq ($(words $(CURDIR)),1) $(error Error: Path to the main directory cannot contain spaces) -- To view, visit https://review.coreboot.org/c/coreboot/+/85854?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I652bc4916f5c623be352d476b083098b8240d03d Gerrit-Change-Number: 85854 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1005756

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025