coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[XS] Change in coreboot[main]: amdfwtool: Remove the dissociated combo BIOS table for recovery A/B
by Bao Zheng (Code Review) 09 Mar '24

09 Mar '24

Attention is currently required from: Felix Held, Fred Reitberger, Zheng Bao. Hello Felix Held, Fred Reitberger, Zheng Bao, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81137?usp=email to look at the new patch set (#2). Change subject: amdfwtool: Remove the dissociated combo BIOS table for recovery A/B ...................................................................... amdfwtool: Remove the dissociated combo BIOS table for recovery A/B For recovery A/B mode, the BIOS tables level 2 are traced by PSP table instead of ROMSIG. There should not be a dedicated BIOS table, nor a combo BIOS table. Change-Id: I8735bd91b32bc9a0e4fc70d293e8d836d5e9c36b Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- M util/amdfwtool/amdfwtool.c 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/81137/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81137?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: I8735bd91b32bc9a0e4fc70d293e8d836d5e9c36b Gerrit-Change-Number: 81137 Gerrit-PatchSet: 2 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: Zheng Bao Gerrit-Attention: Zheng Bao Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: amdfwtool: Do not use combo bios dir for recovery A/B mode
by Bao Zheng (Code Review) 09 Mar '24

09 Mar '24

Attention is currently required from: Zheng Bao. Hello Zheng Bao, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/81137?usp=email to review the following change. Change subject: amdfwtool: Do not use combo bios dir for recovery A/B mode ...................................................................... amdfwtool: Do not use combo bios dir for recovery A/B mode For recovery A/B mode, the BIOS tables level 2 are traced by PSP table instead of ROMSIG. There should not be a dedicated BIOS table, nor a combo BIOS table. Change-Id: I8735bd91b32bc9a0e4fc70d293e8d836d5e9c36b Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- M util/amdfwtool/amdfwtool.c 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/81137/1 diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index e81059d..7e30804 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -1604,7 +1604,8 @@ adjust_current_pointer(&ctx, 0, 0x1000U); - bhd_combo_dir = new_combo_dir(&ctx); + if (!cb_config.recovery_ab) + bhd_combo_dir = new_combo_dir(&ctx); } combo_index = 0; @@ -1728,7 +1729,7 @@ if (!cb_config.use_combo) { fill_bios_directory_to_efs(amd_romsig, biosdir, &ctx, &cb_config); - } else { + } else if (bhd_combo_dir != NULL) { fill_bios_directory_to_efs(amd_romsig, bhd_combo_dir, &ctx, &cb_config); assert_fw_entry(combo_index, MAX_COMBO_ENTRIES, &ctx); -- To view, visit https://review.coreboot.org/c/coreboot/+/81137?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: I8735bd91b32bc9a0e4fc70d293e8d836d5e9c36b Gerrit-Change-Number: 81137 Gerrit-PatchSet: 1 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Attention: Zheng Bao Gerrit-MessageType: newchange

1 0

[XL] Change in coreboot[main]: lib/device_tree: Add some FDT helper functions
by Julius Werner (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Jakub Czapiga, Martin L Roth, Maximilian Brune. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81081?usp=email ) Change subject: lib/device_tree: Add some FDT helper functions ...................................................................... Patch Set 7: (15 comments) Patchset: PS7: I'm not really convinced about the wildcard API tbh, that is way more complicated than what we have on the unflattened side. Hopefully we can find a simpler solution for that, but I'd need to know some more about your use cases first. File src/lib/device_tree.c: https://review.coreboot.org/c/coreboot/+/81081/comment/f4198bb6_93ef33d5 : PS6, Line 27: if (be32toh(ptr[index++]) != FDT_TOKEN_PROPERTY) BTW, upon reading the device tree spec I noticed that in all of these parsing cases we don't take into account that there may be NOP tokens (0x4) mixed in between any other tokens. This code was originally written for ChromeOS where we know we're always only reading device trees fresh out of `dtc` that won't have those, but if you want to apply it more broadly you may want to add support for that. https://review.coreboot.org/c/coreboot/+/81081/comment/dea56b21_1109943f : PS6, Line 53: (char *) This cast becomes unnecessary then too. https://review.coreboot.org/c/coreboot/+/81081/comment/cde4e199_b7517a04 : PS6, Line 81: fdt_read_prop_in_node nit: Would be nice to have some naming consistency between the functions that all read properties, e.g. ``` fdt_read_property() fdt_read_reg_property() fdt_read_alias_property() ``` or ``` fdt_read_prop() fdt_read_reg_prop() fdt_read_alias_prop() ``` https://review.coreboot.org/c/coreboot/+/81081/comment/00e0a469_2ff8ee27 : PS6, Line 99: uintptr_t *addrs, uintptr_t *sizes I feel like it might be cleaner to create a small struct that holds an addr and a size, and then have an array of those passed in here? Then there's less disconnect between which addr belongs to which size. Also you probably want to use uint64_t for those because on some systems physical address space is larger what the processor can logically address in one register. https://review.coreboot.org/c/coreboot/+/81081/comment/3c5cd48d_9c12e49a : PS6, Line 105: printk(BIOS_DEBUG, "no reg property found\n"); nit: maybe print node_offset for all these to provide some context? https://review.coreboot.org/c/coreboot/+/81081/comment/89238fd0_ca96a28b : PS6, Line 118: //TODO replace with something more computational Something like ``` uint8_t *ptr = prop.data; ... if (addr_cells > 2) ...error out... addrs[i] = 0; for (int j = 0; j < addr_cells * 4; j++) addrs[i] = (addrs[i] << 8) | *ptr++; ``` https://review.coreboot.org/c/coreboot/+/81081/comment/f6c07421_e19e917d : PS6, Line 142: are are what? https://review.coreboot.org/c/coreboot/+/81081/comment/b7a2db6e_eaea82a0 : PS6, Line 157: wildcard_idx = path_sub_len; Wildcard globbing in a firmware API feels pretty out of place. Can you show an example where you actually need that? https://review.coreboot.org/c/coreboot/+/81081/comment/3553d95a_d42ba4ad : PS6, Line 227: //TODO more sanity checks Let's not leave TODOs in the code we check in. If you think something more needs to happen here, you should implement it now. (In general I think we should keep all the sanity checking for the FDT blob in `fdt_is_valid()`, and then only call that once per boot, not on every access.) https://review.coreboot.org/c/coreboot/+/81081/comment/55f5c2c4_70ec8cdc : PS6, Line 244: * @param create 1: Create node(s) if not found. 0: Return NULL instead. Documentation doesn't match the function. https://review.coreboot.org/c/coreboot/+/81081/comment/c1e2a2b1_4ea83a18 : PS6, Line 278: static uintptr_t fdt_read_alias_prop(const void *blob, const char *alias_name, Why not just return a `const char *` to the resulting path directly? That should normally be the only thing you care to look up in an alias. https://review.coreboot.org/c/coreboot/+/81081/comment/a638851d_3a544cc6 : PS6, Line 303: alias_prop.name I'm confused, isn't `alias_prop.name` the same as `alias_name`? Aren't you actually interested in `alias_prop.data`? https://review.coreboot.org/c/coreboot/+/81081/comment/374674de_c936c128 : PS6, Line 501: printk(BIOS_ERR, "%s failed\n", __func__); nit: not really necessary to print this if the callee already printed info. (If you're concerned about the right error levels, I think it would be more useful to just bump the messages inside `fdt_is_valid()` to warning or error.) File tests/lib/device_tree-test.c: https://review.coreboot.org/c/coreboot/+/81081/comment/0c3a431e_d1b2d421 : PS7, Line 13: // regular FDT from RISC-V QEMU virt target Note that you can also just have the test executable read in a binary FDT file, that's a bit cleaner. See lib/lzma-test for an example (we should probably factor out that functionality into a common test helper somehow). -- To view, visit https://review.coreboot.org/c/coreboot/+/81081?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: I2fb1d93c5b3e1cb2f7d9584db52bbce3767b63d8 Gerrit-Change-Number: 81081 Gerrit-PatchSet: 7 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Sat, 09 Mar 2024 01:37:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: payload/external/edk2: Explicitly define the build arch
by Benjamin Doron (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Arthur Heymans, Arthur Heymans, Martin L Roth, Matt DeVillier, Nicholas Sudsgaard, Sean Rhodes. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80883?usp=email ) Change subject: payload/external/edk2: Explicitly define the build arch ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > It's just what its built on that matter, not what for. […] The architecture argument - `-a` - needs be `-a IA32 -a X64` so that the entrypoint is built as 32-bit code. UPL's build script might remap `-D BUILD_ARCH=X64` to this for us, I haven't looked recently. In general, yeah, it might be good to make the build string more generic: Unless the program runner can handle mode switches, then we can make `-a IA32` conditional on ARCH_RAMSTAGE_X86_32. -- To view, visit https://review.coreboot.org/c/coreboot/+/80883?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: Icd942d0c15a99231d09f9cbdc5eb48333b6aa6e5 Gerrit-Change-Number: 80883 Gerrit-PatchSet: 3 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 09 Mar 2024 00:24:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: vc/amd/opensil/stub: add stub MPIO driver
by Varshit Pandya (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Felix Held, Martin L Roth. Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81100?usp=email ) Change subject: vc/amd/opensil/stub: add stub MPIO driver ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81100?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: Ib4f5c232859b9abcd10bfa5c21e2f2c3a70b4b0e Gerrit-Change-Number: 81100 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 08 Mar 2024 23:54:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: arch/riscv: use PMP
by ron minnich (Code Review) 08 Mar '24

08 Mar '24

Attention is currently required from: Arthur Heymans, Maximilian Brune, Philipp Hug. ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81153?usp=email ) Change subject: arch/riscv: use PMP ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Confirmed that this works on qemu. It currently creates the right values on the unmatched, but neither linux nor my trivial S-mode code will run. -- To view, visit https://review.coreboot.org/c/coreboot/+/81153?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: I6edce139d340783148cbb446cde004ba96e67944 Gerrit-Change-Number: 81153 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Fri, 08 Mar 2024 20:58:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/asrock: Add Z87E-ITX (Haswell)
by Nicholas Chin (Code Review) 08 Mar '24

08 Mar '24

Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81022?usp=email ) Change subject: mb/asrock: Add Z87E-ITX (Haswell) ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/81022?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: I56c22d8f5505f9a4da25f8b4406b00978af1a586 Gerrit-Change-Number: 81022 Gerrit-PatchSet: 2 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 08 Mar 2024 19:44:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: WIP: arch/riscv: Enable FPU, vector extensions, timer and counters
by ron minnich (Code Review) 08 Mar '24

08 Mar '24

ron minnich has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/81128?usp=email ) Change subject: WIP: arch/riscv: Enable FPU, vector extensions, timer and counters ...................................................................... Abandoned a -- To view, visit https://review.coreboot.org/c/coreboot/+/81128?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: I54be0467654ee7a445d61e4724dc58ee86b62d6f Gerrit-Change-Number: 81128 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Philipp Hug <philipp(a)hug.cx> Gerrit-MessageType: abandon

1 0

[M] Change in coreboot[master]: WIP: Working on enabling coreboot SBI again
by ron minnich (Code Review) 08 Mar '24

08 Mar '24

ron minnich has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/81129?usp=email ) Change subject: WIP: Working on enabling coreboot SBI again ...................................................................... Abandoned a -- To view, visit https://review.coreboot.org/c/coreboot/+/81129?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: I5a9b648f9ad1a4b01ec69178f770419b37bb1377 Gerrit-Change-Number: 81129 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[M] Change in coreboot[master]: arch/riscv: Add SMP support for exception handler
by ron minnich (Code Review) 08 Mar '24

08 Mar '24

ron minnich has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/81130?usp=email ) Change subject: arch/riscv: Add SMP support for exception handler ...................................................................... Abandoned gerrit hell -- To view, visit https://review.coreboot.org/c/coreboot/+/81130?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: Ia1f97b82e329f6358061072f98278cf56b503618 Gerrit-Change-Number: 81130 Gerrit-PatchSet: 1 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Xiang W <wxjstz(a)126.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024