coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3041 discussions

[S] Change in coreboot[main]: amdfwtool: Remove the function's dependency to ctx
by Felix Held (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Bao Zheng, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78304?usp=email ) Change subject: amdfwtool: Remove the function's dependency to ctx ...................................................................... Patch Set 4: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/78304/comment/ad6fd89b_0620ed72 : PS4, Line 9: writebody write_body function https://review.coreboot.org/c/coreboot/+/78304/comment/757e2d6f_c37fcb7c : PS4, Line 11: would be good to also mention that the amdfwtool_cleanup calls are removed from write_body, but since write_body now returns to the main function, amdfwtool_cleanup still ends up getting called -- To view, visit https://review.coreboot.org/c/coreboot/+/78304?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: I639828498fa45911f430500735e90ddc198b6af5 Gerrit-Change-Number: 78304 Gerrit-PatchSet: 4 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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-Comment-Date: Thu, 29 Feb 2024 21:38:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: amdfwtool: Move the functions to handle_file.c
by Felix Held (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Bao Zheng, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78305?usp=email ) Change subject: amdfwtool: Move the functions to handle_file.c ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78305?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: I4cfec13cbc2a86dc352758541cce915a838e0d0f Gerrit-Change-Number: 78305 Gerrit-PatchSet: 5 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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-Comment-Date: Thu, 29 Feb 2024 21:30:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: lib: Declare heap in assembly
by Julius Werner (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Arthur Heymans, Maximilian Brune, Nico Huber. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80776?usp=email ) Change subject: lib: Declare heap in assembly ...................................................................... Patch Set 3: (1 comment) File src/lib/program.ld: https://review.coreboot.org/c/coreboot/+/80776/comment/025e7559_ab9a2cb4 : PS3, Line 141: } > > > I think a simpler solution is to just add a `: to_load` here. (We already have that for the `. […] `to_load` is just how we call the program segment, it could be any name. I'm not exactly sure what's happening here but there seems to be some heuristic that sections which are explicitly marked as NOLOAD and don't contain any actual objects aren't automatically added to a suitable program segment like all sections normally are. Adding the segment assignment explicitly works around that. (In general linker scripts seem to rely on a lot of these "magic defaults" for backwards compatibility with how things were "commonly done" in the past, it's probably better to rely less on that and write more stuff explicitly where possible.) -- To view, visit https://review.coreboot.org/c/coreboot/+/80776?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: I67cb5ce886fda313e0720b0bc7c6e66e4aae45fa Gerrit-Change-Number: 80776 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 29 Feb 2024 21:08:17 +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]: lib/program.ld: Make (NOLOAD) and to_load more explicit
by Julius Werner (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Arthur Heymans. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80803?usp=email ) Change subject: lib/program.ld: Make (NOLOAD) and to_load more explicit ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80803?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: Ic14543ba580abe7a34c69bba714eae8cce504977 Gerrit-Change-Number: 80803 Gerrit-PatchSet: 2 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-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 29 Feb 2024 21:05:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/vpd: Return NULL for missing "feature_device_info"
by Subrata Banik (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Gwendal Grignou, Nick Vaccaro, Shelley Chen. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80805?usp=email ) Change subject: drivers/vpd: Return NULL for missing "feature_device_info" ...................................................................... Patch Set 1: (1 comment) File src/drivers/vpd/vpd_device_feature.c: https://review.coreboot.org/c/coreboot/+/80805/comment/45356a99_98508781 : PS1, Line 10: static char device_info[VPD_FEATURE_DEVICE_INFO_LEN]; > > Could |device_info| be in the stack and give as an argument to the function? The function is called only once and there is no logic to cache the data. > > in that case the caller (outside drivers/vpd) of the vpd_get_feature_device_info() has to pass the `device_info` buffer and also needs to know the possible size (VPD_FEATURE_DEVICE_INFO_LEN) which may not required to be exposed outside of this file. > > I don't think there is any problem of using static variable as we are in ramstage (and there is no memory specific restriction) more reference here, https://github.com/coreboot/coreboot/blob/main/src/drivers/vpd/vpd_serial.c… I don't see any caller of VPD API are actually passing the argument over stack. -- To view, visit https://review.coreboot.org/c/coreboot/+/80805?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: I76fc220ed792abdfefb0b1a37873b5b828bfdda8 Gerrit-Change-Number: 80805 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 20:10:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: mb/starlabs/byte_adl: Add Alder Lake N Byte Mk II
by Sean Rhodes (Code Review) 29 Feb '24

29 Feb '24

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80705?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/starlabs/byte_adl: Add Alder Lake N Byte Mk II ...................................................................... mb/starlabs/byte_adl: Add Alder Lake N Byte Mk II Tested using `edk2` from `github.com/starlabsltd/edk2/tree/uefipayload_vs`: * Windows 11 * Ubuntu 22.04 * Manjaro 22 No known issues. https://starlabs.systems/pages/byte-specification Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: Idff2d883a8c29f0fee9d633708aac92061a45132 --- M Documentation/mainboard/index.md A Documentation/mainboard/starlabs/byte_adl.md M src/ec/starlabs/merlin/acpi/battery.asl A src/mainboard/starlabs/byte_adl/Kconfig A src/mainboard/starlabs/byte_adl/Kconfig.name A src/mainboard/starlabs/byte_adl/Makefile.inc A src/mainboard/starlabs/byte_adl/acpi/ec.asl A src/mainboard/starlabs/byte_adl/acpi/mainboard.asl A src/mainboard/starlabs/byte_adl/acpi/sleep.asl A src/mainboard/starlabs/byte_adl/acpi/superio.asl A src/mainboard/starlabs/byte_adl/board_info.txt A src/mainboard/starlabs/byte_adl/bootblock.c A src/mainboard/starlabs/byte_adl/cmos.default A src/mainboard/starlabs/byte_adl/cmos.layout A src/mainboard/starlabs/byte_adl/dsdt.asl A src/mainboard/starlabs/byte_adl/hda_verb.c A src/mainboard/starlabs/byte_adl/include/variants.h A src/mainboard/starlabs/byte_adl/mainboard.c A src/mainboard/starlabs/byte_adl/smbios.c A src/mainboard/starlabs/byte_adl/variants/mk_ii/Makefile.inc A src/mainboard/starlabs/byte_adl/variants/mk_ii/board.fmd A src/mainboard/starlabs/byte_adl/variants/mk_ii/data.vbt A src/mainboard/starlabs/byte_adl/variants/mk_ii/devicetree.cb A src/mainboard/starlabs/byte_adl/variants/mk_ii/devtree.c A src/mainboard/starlabs/byte_adl/variants/mk_ii/gpio.c A src/mainboard/starlabs/byte_adl/variants/mk_ii/hda_verb.c A src/mainboard/starlabs/byte_adl/variants/mk_ii/romstage.c A src/mainboard/starlabs/byte_adl/variants/mk_ii/vboot.fmd A src/mainboard/starlabs/byte_adl/vboot.c 29 files changed, 1,366 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/80705/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/80705?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: Idff2d883a8c29f0fee9d633708aac92061a45132 Gerrit-Change-Number: 80705 Gerrit-PatchSet: 6 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: drivers/vpd: Return NULL for missing "feature_device_info"
by Subrata Banik (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Gwendal Grignou, Nick Vaccaro, Shelley Chen. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80805?usp=email ) Change subject: drivers/vpd: Return NULL for missing "feature_device_info" ...................................................................... Patch Set 1: (1 comment) File src/drivers/vpd/vpd_device_feature.c: https://review.coreboot.org/c/coreboot/+/80805/comment/03d8beea_5a8b8778 : PS1, Line 10: static char device_info[VPD_FEATURE_DEVICE_INFO_LEN]; > Could |device_info| be in the stack and give as an argument to the function? The function is called only once and there is no logic to cache the data. in that case the caller (outside drivers/vpd) of the vpd_get_feature_device_info() has to pass the `device_info` buffer and also needs to know the possible size (VPD_FEATURE_DEVICE_INFO_LEN) which may not required to be exposed outside of this file. I don't think there is any problem of using static variable as we are in ramstage (and there is no memory specific restriction) -- To view, visit https://review.coreboot.org/c/coreboot/+/80805?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: I76fc220ed792abdfefb0b1a37873b5b828bfdda8 Gerrit-Change-Number: 80805 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 19:55:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection
by Gwendal Grignou (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Kapil Porwal, Paul Menzel, Paz Zcharya, Subrata Banik. Gwendal Grignou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email ) Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ...................................................................... Patch Set 9: Code-Review-1 (2 comments) File src/vendorcode/google/chromeos/tpm_factory_config.c: https://review.coreboot.org/c/coreboot/+/80738/comment/8b13d103_39dce3e4 : PS9, Line 61: return (factory_config & CHROMEBOOK_PLUS_DEVICE) == CHROMEBOOK_PLUS_DEVICE; That won't work for future device, since feature_level could be 2. Beside, hard branded device is enough to display the hard branded logo, so ``` return (factory_config & CHROMEBOOK_PLUS_HARD_BRANDED) == CHROMEBOOK_PLUS_HARD_BRANDED; ``` is enough. https://review.coreboot.org/c/coreboot/+/80738/comment/6546a377_3db07331 : PS9, Line 80: */ Let document further to explain the `CAE` magic since we are not going to put protobuf library in the near future: ``` /* * |feature_device_info| contains a base64-encoded protobuf and the first entry is * |feature_level|, greater than 1 for chromebook X device. * (see https://chromium.googlesource.com/chromiumos/platform/feature-management/+/…) * * The first byte currently hold the feature level. / const char decoded_feature_device_info[B64_DECODED_SIZE(VPD_FEATURE_DEVICE_INFO_LEN)]; if (b64_decode(feature_device_info, strlen(feature_device_info), decoded_feature_device_info)) > 0) return decoded_feature_device_info[0] > 1; else return false; ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/80738?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: I9bb1e868764738333977bd8c990bea4253c9d37b Gerrit-Change-Number: 80738 Gerrit-PatchSet: 9 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paz Zcharya <pazz(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 19:52:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/vpd: Return NULL for missing "feature_device_info"
by Gwendal Grignou (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Nick Vaccaro, Shelley Chen, Subrata Banik. Gwendal Grignou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80805?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: drivers/vpd: Return NULL for missing "feature_device_info" ...................................................................... Patch Set 1: Code-Review-1 (1 comment) File src/drivers/vpd/vpd_device_feature.c: https://review.coreboot.org/c/coreboot/+/80805/comment/68975aab_36a30248 : PS1, Line 10: static char device_info[VPD_FEATURE_DEVICE_INFO_LEN]; Could |device_info| be in the stack and give as an argument to the function? The function is called only once and there is no logic to cache the data. -- To view, visit https://review.coreboot.org/c/coreboot/+/80805?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: I76fc220ed792abdfefb0b1a37873b5b828bfdda8 Gerrit-Change-Number: 80805 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Thu, 29 Feb 2024 19:44:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: doc/releases: Add 24.02.1 release section
by Felix Singer (Code Review) 29 Feb '24

29 Feb '24

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80798?usp=email ) Change subject: doc/releases: Add 24.02.1 release section ...................................................................... doc/releases: Add 24.02.1 release section Change-Id: I4d217c3dba4aa3ec30732b914009a6e9d53371c7 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M Documentation/releases/coreboot-24.02-relnotes.md 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/80798/1 diff --git a/Documentation/releases/coreboot-24.02-relnotes.md b/Documentation/releases/coreboot-24.02-relnotes.md index eb5349e..e4e7892 100644 --- a/Documentation/releases/coreboot-24.02-relnotes.md +++ b/Documentation/releases/coreboot-24.02-relnotes.md @@ -31,6 +31,15 @@ 'master' branch have been switched to 'main'. +Release 24.02.1 +--------------- + +### lib/rtc: Fix off-by-one error in February day count in leap year + +The month argument passed to rtc\_month\_days is 0-based, not 1-based. +This results in the RTC being reverted to the build date constantly +on 29th February 2024. + Significant or interesting changes ---------------------------------- -- To view, visit https://review.coreboot.org/c/coreboot/+/80798?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: I4d217c3dba4aa3ec30732b914009a6e9d53371c7 Gerrit-Change-Number: 80798 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-MessageType: newchange

1 0

← Newer
1
2
3
4
5
6
7
...
305
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024