coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[M] Change in coreboot[main]: cbfs: Remove broken remnants of PAYLOAD_INFO feature
by Julius Werner (Code Review) 19 Mar '24

19 Mar '24

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81087?usp=email ) Change subject: cbfs: Remove broken remnants of PAYLOAD_INFO feature ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > This seems to have broken building FILO: […] Just remove all the `PAYLOAD_INFO()` lines. They haven't worked right in ages anyway. -- To view, visit https://review.coreboot.org/c/coreboot/+/81087?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: I40d8e9b76a171ebcdaa2eae02d54a1ca5e592c85 Gerrit-Change-Number: 81087 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:50:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: libpayload: gdb: Make die_if() format string a literal
by Nick Vaccaro (Code Review) 19 Mar '24

19 Mar '24

Nick Vaccaro has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81361?usp=email ) Change subject: libpayload: gdb: Make die_if() format string a literal ...................................................................... libpayload: gdb: Make die_if() format string a literal CB:77969 made minor changes to the die_if() macro. One of the consequences is that the format string passed to it can no longer be a real `char *` variable, it needs to actually be a string literal. In the vast majority of call sites that is already the case, but there was one instance in the GDB code where we're reusing the same format string many times and for that reason put it into a const variable. Fix that by turning it into a #define macro instead. (Even though this technically duplicates the format string, the linker is able to merge identical string literals together again, so it doesn't really end up taking more space.) Change-Id: I532a04b868f12aa0e3c01422c075ddaade251827 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81361 Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M payloads/libpayload/gdb/transport.c 1 file changed, 6 insertions(+), 6 deletions(-) Approvals: Nick Vaccaro: Looks good to me, approved build bot (Jenkins): Verified diff --git a/payloads/libpayload/gdb/transport.c b/payloads/libpayload/gdb/transport.c index 5b575d0..66a6f20 100644 --- a/payloads/libpayload/gdb/transport.c +++ b/payloads/libpayload/gdb/transport.c @@ -16,12 +16,12 @@ #include <gdb.h> #include <libpayload.h> +#define OUTPUT_OVERRUN_MSG "GDB output buffer overrun (try increasing reply.size)!\n" + /* MMIO word size is not standardized, but *usually* 32 (even on ARM64) */ typedef u32 mmio_word_t; static const int timeout_us = 100 * 1000; -static const char output_overrun[] = "GDB output buffer overrun (try " - "increasing reply.size)!\n"; /* Serial-specific glue code... add more transport layers here when desired. */ @@ -77,7 +77,7 @@ void gdb_message_encode_bytes(struct gdb_message *message, const void *data, int length) { - die_if(message->used + length * 2 > message->size, output_overrun); + die_if(message->used + length * 2 > message->size, OUTPUT_OVERRUN_MSG); const mmio_word_t *aligned = (mmio_word_t *)ALIGN_DOWN((uintptr_t)data, sizeof(*aligned)); mmio_word_t word = be32toh(readl(aligned++)); @@ -114,7 +114,7 @@ void gdb_message_encode_zero_bytes(struct gdb_message *message, int length) { - die_if(message->used + length * 2 > message->size, output_overrun); + die_if(message->used + length * 2 > message->size, OUTPUT_OVERRUN_MSG); memset(message->buf + message->used, '0', length * 2); message->used += length * 2; } @@ -125,13 +125,13 @@ string, message->size - message->used); /* Check >= instead of > to account for strlcpy's trailing '\0'. */ - die_if(message->used >= message->size, output_overrun); + die_if(message->used >= message->size, OUTPUT_OVERRUN_MSG); } void gdb_message_encode_int(struct gdb_message *message, uintptr_t val) { int length = sizeof(uintptr_t) * 2 - __builtin_clz(val) / 4; - die_if(message->used + length > message->size, output_overrun); + die_if(message->used + length > message->size, OUTPUT_OVERRUN_MSG); while (length--) message->buf[message->used++] = to_hex((val >> length * 4) & 0xf); -- To view, visit https://review.coreboot.org/c/coreboot/+/81361?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: I532a04b868f12aa0e3c01422c075ddaade251827 Gerrit-Change-Number: 81361 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: arch/arm64: Add Clang as supported target
by Julius Werner (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Arthur Heymans, Jon Murphy. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74501?usp=email ) Change subject: arch/arm64: Add Clang as supported target ...................................................................... Patch Set 23: (1 comment) File src/arch/arm64/Kconfig: https://review.coreboot.org/c/coreboot/+/74501/comment/424931a3_cd688b1f : PS23, Line 7: default y if ARCH_ARM64 > IIUC the issue is that the option doesn't have a default, it's by default hidden and this would be a […] AFAIK "no" is automatically the default for bool Kconfigs if no other is specified. -- To view, visit https://review.coreboot.org/c/coreboot/+/74501?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: I940a1ccf5cc4ec7bed5b6c8be92fc47922e1e747 Gerrit-Change-Number: 74501 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jon Murphy <jpmurphy(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:16:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: libpayload: gdb: Make die_if() format string a literal
by Nick Vaccaro (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Julius Werner, Maximilian Brune, Nick Vaccaro. Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81361?usp=email ) Change subject: libpayload: gdb: Make die_if() format string a literal ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81361?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: I532a04b868f12aa0e3c01422c075ddaade251827 Gerrit-Change-Number: 81361 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:13:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: amdfwtool: Check sanity before filling the data array
by Martin L Roth (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Bao Zheng, Felix Held, Fred Reitberger, Zheng Bao. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78763?usp=email ) Change subject: amdfwtool: Check sanity before filling the data array ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78763?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: I8284c35a0124ba4588d199024e28d3445c681896 Gerrit-Change-Number: 78763 Gerrit-PatchSet: 7 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:13:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/brox: Select USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS
by Shelley Chen (Code Review) 19 Mar '24

19 Mar '24

Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81224?usp=email ) Change subject: mb/google/brox: Select USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS ...................................................................... mb/google/brox: Select USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS Select USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS in brox Kconfig. This enables a single binary for both SKU1 and SKU2. For SKU2, upon boot from cold reset, it will disable the UFS Controller and then trigger a warm boot. BUG=b:329209576 BRANCH=None TEST=Boot image on SKU1/SKU2 and check S0ix working. Change-Id: Iabd0b3a83aa386e09310b671632368807a4018d4 Signed-off-by: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81224 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub(a)google.com> Reviewed-by: Shelley Chen <shchen(a)google.com> --- M src/mainboard/google/brox/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Shelley Chen: Looks good to me, approved build bot (Jenkins): Verified Karthik Ramasubramanian: Looks good to me, approved diff --git a/src/mainboard/google/brox/Kconfig b/src/mainboard/google/brox/Kconfig index 0c7a112..e7f965a 100644 --- a/src/mainboard/google/brox/Kconfig +++ b/src/mainboard/google/brox/Kconfig @@ -57,6 +57,7 @@ config BOARD_GOOGLE_BROX select BOARD_GOOGLE_BASEBOARD_BROX select CHROMEOS_WIFI_SAR if CHROMEOS + select USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS config BOARD_GOOGLE_BROX_EC_ISH select BOARD_GOOGLE_BASEBOARD_BROX -- To view, visit https://review.coreboot.org/c/coreboot/+/81224?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: Iabd0b3a83aa386e09310b671632368807a4018d4 Gerrit-Change-Number: 81224 Gerrit-PatchSet: 5 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: arch/arm64: Add Clang as supported target
by Jon Murphy (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Arthur Heymans, Julius Werner. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74501?usp=email ) Change subject: arch/arm64: Add Clang as supported target ...................................................................... Patch Set 23: (1 comment) File src/arch/arm64/Kconfig: https://review.coreboot.org/c/coreboot/+/74501/comment/5ca4537f_fac01899 : PS23, Line 7: default y if ARCH_ARM64 > Sorry, I don't really understand... I'm pretty sure that should work. […] IIUC the issue is that the option doesn't have a default, it's by default hidden and this would be adding a default which doesn't make sense if hidden behind another guard: ``` config ARCH_SUPPORTS_CLANG bool help Opt-in flag for architectures that generally work well with CLANG. By default the option would be hidden. ``` You could probably add the default as no and then follow the suggestion above? -- To view, visit https://review.coreboot.org/c/coreboot/+/74501?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: I940a1ccf5cc4ec7bed5b6c8be92fc47922e1e747 Gerrit-Change-Number: 74501 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jon Murphy <jpmurphy(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 19 Mar 2024 21:53:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: libpayload: configs: Add new config.featuretest to broaden CI
by Julius Werner (Code Review) 19 Mar '24

19 Mar '24

Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81362?usp=email ) Change subject: libpayload: configs: Add new config.featuretest to broaden CI ...................................................................... libpayload: configs: Add new config.featuretest to broaden CI This patch adds a new config to libpayload whose sole purpose it is to be as different as possible from the defconfig, in order to try to get the CI to exercise more code paths and thus catch more issues. Change-Id: Ia6bd7572056b7a02acb686542810e661e015cc69 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- A payloads/libpayload/configs/config.featuretest 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/81362/1 diff --git a/payloads/libpayload/configs/config.featuretest b/payloads/libpayload/configs/config.featuretest new file mode 100644 index 0000000..7044d59 --- /dev/null +++ b/payloads/libpayload/configs/config.featuretest @@ -0,0 +1,32 @@ +# The goal of this config is to be as different as possible from the defaults, +# to exercise more code paths in the CI. + +CONFIG_LP_GPL=y +CONFIG_LP_DEVELOPER=y +CONFIG_LP_LTO=y +CONFIG_LP_REMOTEGDB=y +CONFIG_LP_MEMMAP_RAM_ONLY=y +CONFIG_LP_MULTIBOOT=y +CONFIG_LP_TINYCURSES=y +# CONFIG_LP_LZMA is not set +# CONFIG_LP_LZ4 is not set +CONFIG_LP_SKIP_CONSOLE_INIT=y +# CONFIG_LP_CBMEM_CONSOLE is not set +# CONFIG_LP_SERIAL_CONSOLE is not set +# CONFIG_LP_VGA_VIDEO_CONSOLE is not set +CONFIG_LP_COREBOOT_VIDEO_CONSOLE=y +CONFIG_LP_COREBOOT_VIDEO_CENTERED=y +CONFIG_LP_CBGFX_FAST_RESAMPLE=y +# CONFIG_LP_PC_I8042 is not set +# CONFIG_LP_PC_MOUSE is not set +# CONFIG_LP_PC_KEYBOARD is not set +# CONFIG_LP_PCI is not set +# CONFIG_LP_NVRAM is not set +CONFIG_LP_RTC_PORT_EXTENDED_VIA=y +# CONFIG_LP_SPEAKER is not set +# CONFIG_LP_STORAGE is not set +# CONFIG_LP_USB is not set +CONFIG_LP_UDC_CI=y +CONFIG_LP_UDC_DWC2=y +CONFIG_LP_DEBUG_MALLOC=y +CONFIG_LP_ENABLE_APIC=y -- To view, visit https://review.coreboot.org/c/coreboot/+/81362?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: Ia6bd7572056b7a02acb686542810e661e015cc69 Gerrit-Change-Number: 81362 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: add IFTYPE_ prefix to mpio_type values
by Martin L Roth (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Arthur Heymans, Felix Held. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81338?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: add IFTYPE_ prefix to mpio_type values ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81338?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: I229a3402c36941ee5347e3704fcf8d8a1bbc78a6 Gerrit-Change-Number: 81338 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 19 Mar 2024 21:34:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/stub/mpio: change mpio_engine_type prefix to IFTYPE
by Martin L Roth (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Felix Held. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81346?usp=email ) Change subject: vc/amd/opensil/stub/mpio: change mpio_engine_type prefix to IFTYPE ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81346?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: If81c5ea01ba147b71b423004a2199b348ffac99a Gerrit-Change-Number: 81346 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin L Roth <gaumless(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: Tue, 19 Mar 2024 21:34:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024