coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
745 discussions

[XS] Change in coreboot[main]: Documentation: Fix make rule for sphinx-autobuild
by Nicholas Chin (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Václav Straka. Nicholas Chin has posted comments on this change by Václav Straka. ( https://review.coreboot.org/c/coreboot/+/85833?usp=email ) Change subject: Documentation: Fix make rule for sphinx-autobuild ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Also, considering that sphinx seems to want to move away from the generated makefile (Makefile.sphinx) https://github.com/sphinx-doc/sphinx/issues/5618#issuecomment-502415633 I wonder if it would make more sense to just drop Makefile.sphinx and directly call sphinx-build/sphinx-autobuild in `Documentation/Makefile`. The targets in Makefile.sphinx are just calling sphinx-(auto)build directly, so it's essentially just adding an extra step for no good reason. -- To view, visit https://review.coreboot.org/c/coreboot/+/85833?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: I3d3b0547acd7b070925d9bee818ee1ef230f5f46 Gerrit-Change-Number: 85833 Gerrit-PatchSet: 1 Gerrit-Owner: Václav Straka <venda.straka(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Václav Straka <venda.straka(a)gmail.com> Gerrit-Comment-Date: Thu, 02 Jan 2025 22:19:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: Documentation: Fix make rule for sphinx-autobuild
by Nicholas Chin (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Václav Straka. Nicholas Chin has posted comments on this change by Václav Straka. ( https://review.coreboot.org/c/coreboot/+/85833?usp=email ) Change subject: Documentation: Fix make rule for sphinx-autobuild ...................................................................... Patch Set 1: (1 comment) File Documentation/Makefile.sphinx: https://review.coreboot.org/c/coreboot/+/85833/comment/256b3ac7_c7ec97e7?us… : PS1, Line 24: $(ALLSPHINXOPTS) ALLSPHINXOPTS seems to be outdated; it was removed when this file was regenerated in commit 35599f9a6671. The documentation does have a recommendation for how to add a livehtml target: https://pypi.org/project/sphinx-autobuild/ ``` livehtml: sphinx-autobuild "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/85833?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: I3d3b0547acd7b070925d9bee818ee1ef230f5f46 Gerrit-Change-Number: 85833 Gerrit-PatchSet: 1 Gerrit-Owner: Václav Straka <venda.straka(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Václav Straka <venda.straka(a)gmail.com> Gerrit-Comment-Date: Thu, 02 Jan 2025 22:10:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: lib/{fit,fit_payload}.c: Enhance support for FIT images
by Benjamin Doron (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Felix Held, Julius Werner, Lean Sheng Tan, Matt DeVillier, Maximilian Brune, Philipp Hug, ron minnich. Benjamin Doron has posted comments on this change by Benjamin Doron. ( https://review.coreboot.org/c/coreboot/+/84796?usp=email ) Change subject: lib/{fit,fit_payload}.c: Enhance support for FIT images ...................................................................... Patch Set 10: (9 comments) File src/lib/fit.c: https://review.coreboot.org/c/coreboot/+/84796/comment/209a886b_15797b65?us… : PS4, Line 121: be32_to_cpu(*(size_t *) > > I'm surprised there are two ways to convert endianness in coreboot. […] Done https://review.coreboot.org/c/coreboot/+/84796/comment/f2a1dea2_bee13cdd?us… : PS4, Line 523: } > Simon Glass told me elsewhere that `loadables` in the config node is the supported way to do this. […] https://github.com/tianocore/edk2/pull/10570/commits/50583c8509539d943575b1… is not ready. I'll mark this as a TODO comment in the code, tag myself, and mark this as done for now. File src/lib/fit_payload.c: https://review.coreboot.org/c/coreboot/+/84796/comment/bc522ddd_27b37854?us… : PS6, Line 18: __weak void platform_fdt_add_secondaries(struct device_tree *tree, const char *name, struct region *secondary) > Ah, that's fair. […] We discussed this on another comment. Done https://review.coreboot.org/c/coreboot/+/84796/comment/af8f91f4_8a6bab0a?us… : PS6, Line 116: secondaries.size = kernel->size - primary_image->size; > You could underflow in the size if `region_size(kernel)` is smaller than `primary_image->size` for w […] Since splitting firmware/kernel handling, I believe this doesn't apply anymore. https://review.coreboot.org/c/coreboot/+/84796/comment/b89376e2_c1700164?us… : PS6, Line 118: /* We may want to check if relocation is needed and then at least do something: > As it happens, I've just realised that our secondary images never need relocation: they're UEFI FVs, […] Done https://review.coreboot.org/c/coreboot/+/84796/comment/cc47b02b_bafc82e0?us… : PS6, Line 131: continue; > As it happens, I'm not sure how much meaning `require-fit` has anymore, now that I've made EDK2 no l […] Done https://review.coreboot.org/c/coreboot/+/84796/comment/786e138c_7e1ee9ac?us… : PS6, Line 274: if (!fit_payload_arch(payload, config, &code, &fdt, &initrd)) { > This makes sense. […] I think this is valid for now: I don't enable building Linux kernel payloads as FITs for x86, which is the one missing the support File src/lib/fit_payload.c: https://review.coreboot.org/c/coreboot/+/84796/comment/30bdf87c_9842b034?us… : PS9, Line 29: printk(BIOS_ERR, "WEAK: %s:%s() called to add %s to FDT\n", __FILE__, __func__, name); Right, I see your point. I've occasionally made this argument myself, that device trees are containers for data, so it doesn't and can't specify every kind of configuration parameter that exists. Since there's no spec to contain this idea, I'll just drop the code from here and have the follow-up add a callout in. > You mean the one that only goes up to section 3.4 here: https://fitspec.osfw.foundation/ ? ;) > > It sounds like I need some extra context that's not yet officially published(?) and maybe then I'll have an easier time following what all this is supposed to do. Oh, I see what happened. I was looking at the PDF, where the introduction is "chapter 4." I'm referring to the introduction, subsection .1, the second paragraph. https://review.coreboot.org/c/coreboot/+/84796/comment/ebfa1d3d_32eab895?us… : PS9, Line 403: &fdt, &initrd > Oh, okay, I see how the FDT thing gets added in the later patches. […] Ah, yes, that's true. I didn't think of that, thanks. I'm going to hope that we didn't discover a ramdisk property previously, since a FIT with a firmware, rather than a kernel, should never be indicating a formal ramdisk. That's a Linux concept, and even if Linux is the internal environment of a firmware, the detail of its initrd should probably be hidden. I'm going to add an `assert(!config->ramdisk)` statement into fit_extract_firmware, and move the ramdisk code in with fit_extract_kernel. That should do it, and draw attention in case someone specs or implements differently. -- To view, visit https://review.coreboot.org/c/coreboot/+/84796?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: I6a21954c0dc5fd820d135e8cd0599ce87903a1c0 Gerrit-Change-Number: 84796 Gerrit-PatchSet: 10 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.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-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 02 Jan 2025 21:56:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: commonlib/device_tree.c: Add a function that reads FDT ints
by Benjamin Doron (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Benjamin Doron, Julius Werner, Lean Sheng Tan. Hello Julius Werner, Lean Sheng Tan, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85643?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Code-Review+2 by Julius Werner, Verified+1 by build bot (Jenkins) Change subject: commonlib/device_tree.c: Add a function that reads FDT ints ...................................................................... commonlib/device_tree.c: Add a function that reads FDT ints Follow the recommendation at https://review.coreboot.org/c/coreboot/+/84796/comment/21f615a2_99a41147/ and implement support for reading integer properties generically, using their size to determine how much to read. This will be used for reading `load`, `entry` and perhaps others. Change-Id: I02d27eb5e23dfbfc1404d209ee8d60968e22bb80 Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> --- M src/commonlib/device_tree.c M src/commonlib/include/commonlib/device_tree.h 2 files changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/85643/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/85643?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: I02d27eb5e23dfbfc1404d209ee8d60968e22bb80 Gerrit-Change-Number: 85643 Gerrit-PatchSet: 6 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <tanleansheng(a)outlook.com>

1 0

[L] Change in coreboot[main]: Add UPL FDT handoff
by Benjamin Doron (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Arthur Heymans, Jakub Czapiga, Jérémy Compostella, Lean Sheng Tan, Matt DeVillier, Philipp Hug, Simon Glass, ron minnich. Benjamin Doron has uploaded a new patch set (#33) to the change originally created by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/76591?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Add UPL FDT handoff ...................................................................... Add UPL FDT handoff This adds another handoff that is basically the same as coreboot tables. The only real difference is that it uses the devicetree format to transfer the information to payload. This handoff is inspired by the UPL (universal payload) specification. Tested: start qemu q35 with coreinfo as payload and see that console still works and the devicetree is printed by coreboot. Tested: start qemu q35 with EDK2 UPL as the payload (pull the entire patch train and https://github.com/tianocore/edk2/pull/6382) and boot to the UEFI shell. Change-Id: I36148e9de6ee992a67ec853ef5cbf1b5f83b44ae Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> --- M payloads/Kconfig M src/arch/arm/include/arch/cbconfig.h M src/arch/arm64/include/arch/cbconfig.h M src/arch/ppc64/include/arch/cbconfig.h M src/arch/riscv/include/arch/cbconfig.h M src/arch/x86/include/arch/cbconfig.h M src/arch/x86/tables.c M src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h M src/drivers/uart/pl011.c M src/drivers/uart/uart8250io.c M src/drivers/uart/uart8250mem.c M src/include/boot/coreboot_tables.h M src/include/boot/tables.h A src/include/boot/upl_fdt_table.h M src/lib/Makefile.mk M src/lib/bootmem.c M src/lib/coreboot_table.c M src/lib/fit.c M src/lib/fit_payload.c A src/lib/tables.c A src/lib/upl_fdt_table.c M tests/lib/Makefile.mk 22 files changed, 504 insertions(+), 65 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/76591/33 -- To view, visit https://review.coreboot.org/c/coreboot/+/76591?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: I36148e9de6ee992a67ec853ef5cbf1b5f83b44ae Gerrit-Change-Number: 76591 Gerrit-PatchSet: 33 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Shuo Liu <shuo.liu(a)intel.com> Gerrit-CC: coreboot org <coreboot.org(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: Simon Glass <sjg(a)chromium.org> Gerrit-Attention: ron minnich <rminnich(a)gmail.com>

1 0

[L] Change in coreboot[main]: lib/{fit,fit_payload}.c: Enhance support for FIT images
by Benjamin Doron (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Benjamin Doron, Felix Held, Lean Sheng Tan, Matt DeVillier, Maximilian Brune, Philipp Hug, ron minnich. Hello Felix Held, Julius Werner, Lean Sheng Tan, Matt DeVillier, Maximilian Brune, Philipp Hug, build bot (Jenkins), ron minnich, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84796?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: lib/{fit,fit_payload}.c: Enhance support for FIT images ...................................................................... lib/{fit,fit_payload}.c: Enhance support for FIT images Implement support for loading images from FIT "firmware" props. These do not require quirks, so the implementation is common across architectures. This is done with the intention of supporting Universal Payload. We implement support for the "load" and "entry" props. Here, these are implemented as fdt64_t. A follow-up will read prop-sized ints. The selected configuration no longer requires an FDT. In this case, we'll attempt a config matching the board ID, then fallback to the FIT default compat. Change-Id: I6a21954c0dc5fd820d135e8cd0599ce87903a1c0 Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> --- M payloads/Kconfig M src/arch/arm/fit_payload.c M src/arch/arm64/fit_payload.c M src/arch/riscv/fit_payload.c M src/include/fit.h M src/lib/fit.c M src/lib/fit_payload.c 7 files changed, 299 insertions(+), 90 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/84796/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/84796?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: I6a21954c0dc5fd820d135e8cd0599ce87903a1c0 Gerrit-Change-Number: 84796 Gerrit-PatchSet: 11 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.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-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[XS] Change in coreboot[main]: Documentation: Fix make rule for sphinx-autobuild
by Václav Straka (Code Review) 02 Jan '25

02 Jan '25

Václav Straka has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85833?usp=email ) Change subject: Documentation: Fix make rule for sphinx-autobuild ...................................................................... Documentation: Fix make rule for sphinx-autobuild Add source directory to arguments of sphinx-autobuild Tested in docker and natively Change-Id: I3d3b0547acd7b070925d9bee818ee1ef230f5f46 Signed-off-by: Vesek <venda.straka(a)gmail.com> --- M Documentation/Makefile.sphinx 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/85833/1 diff --git a/Documentation/Makefile.sphinx b/Documentation/Makefile.sphinx index c4984ec..164f90e 100644 --- a/Documentation/Makefile.sphinx +++ b/Documentation/Makefile.sphinx @@ -21,7 +21,7 @@ @echo "Starting sphinx-autobuild. The HTML pages are in $(BUILDDIR)." @echo "Press Ctrl-C to stop." @echo - $(SPHINXAUTOBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR) + $(SPHINXAUTOBUILD) -b html "$(SOURCEDIR)" $(ALLSPHINXOPTS) $(BUILDDIR) # Catch-all target: route all unknown targets to Sphinx using the new # "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS). -- To view, visit https://review.coreboot.org/c/coreboot/+/85833?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: I3d3b0547acd7b070925d9bee818ee1ef230f5f46 Gerrit-Change-Number: 85833 Gerrit-PatchSet: 1 Gerrit-Owner: Václav Straka <venda.straka(a)gmail.com>

1 0

[M] Change in coreboot[main]: cpu/x86/64bit: Install extended page tables in BSS
by Jérémy Compostella (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Patrick Rudolph, Shuo Liu, Tim Chu. Jérémy Compostella has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85806?usp=email ) Change subject: cpu/x86/64bit: Install extended page tables in BSS ...................................................................... Patch Set 3: (4 comments) File src/cpu/x86/64bit/mmu.c: https://review.coreboot.org/c/coreboot/+/85806/comment/6773cc75_93c50cab?us… : PS3, Line 12: #define _PRES (1ULL << 0) Can't you use the BIT macro? https://review.coreboot.org/c/coreboot/+/85806/comment/a5df0763_15d5152b?us… : PS3, Line 64: int size_t https://review.coreboot.org/c/coreboot/+/85806/comment/1bafc4aa_8da36ea0?us… : PS3, Line 59: if (cpu_supports_1gb_hugepage()) { : const uintptr_t max_addr = 1ULL * GiB * 512ULL * 512ULL; : const uint64_t incr = 1ULL * GiB; : : /* Build P4MLE */ : for (int i = 0; i < 512; i++) : p4mle[i] = _GEN_DIR(&pdpt[i * 512]); : : /* Build PDPT */ : uint64_t *page_table_ptr = pdpt; : for (uintptr_t addr = 0; addr < max_addr; addr += incr, page_table_ptr++) : *page_table_ptr = _GEN_PAGE(addr); : } else { : const uintptr_t max_addr = 2ULL * MiB * 512ULL * 512ULL; : const uint64_t incr = 2ULL * MiB; : : /* Build P4MLE */ : p4mle[0] = _GEN_DIR(&pdpt[0]); : : /* Build PDPT */ : for (int i = 0; i < 512; i++) : pdpt[i] = _GEN_DIR(&pdt[i * 512]); : : /* Build PDT */ : uint64_t *page_table_ptr = pdt; : for (uintptr_t addr = 0; addr < max_addr; addr += incr, page_table_ptr++) : *page_table_ptr = _GEN_PAGE(addr); : } : There are a lot in common between these two branches, isn't it a way to share more code ? https://review.coreboot.org/c/coreboot/+/85806/comment/60ed878c_1b7db564?us… : PS3, Line 150: else unnecessary `else` keyword. -- To view, visit https://review.coreboot.org/c/coreboot/+/85806?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: Ifab50975e0382a1f5c27b55bca1dbbb66b37ba3a Gerrit-Change-Number: 85806 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Thu, 02 Jan 2025 18:57:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: lib/{fit,fit_payload}.c: Enhance support for FIT images
by Julius Werner (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Benjamin Doron, Felix Held, Lean Sheng Tan, Matt DeVillier, Maximilian Brune, Philipp Hug, ron minnich. Julius Werner has posted comments on this change by Benjamin Doron. ( https://review.coreboot.org/c/coreboot/+/84796?usp=email ) Change subject: lib/{fit,fit_payload}.c: Enhance support for FIT images ...................................................................... Patch Set 10: (3 comments) File payloads/Kconfig: https://review.coreboot.org/c/coreboot/+/84796/comment/89b7803b_cfe6c752?us… : PS9, Line 186: default y if PAYLOAD_LINUX && (ARCH_ARM64 || ARCH_RISCV || ARCH_ARM) > Oh, I think that made it align with other instances of this dependency in the file, which allowed me […] Acknowledged File src/lib/fit_payload.c: https://review.coreboot.org/c/coreboot/+/84796/comment/bb528311_24a2157d?us… : PS9, Line 29: printk(BIOS_ERR, "WEAK: %s:%s() called to add %s to FDT\n", __FILE__, __func__, name); > Do you have any ideas for paths? Possibly, `/firmware/secondary-images`. If we decide on some path and a PR for the spec is up, I'll change this code to reflect that and drop the callout. You mean in the FDT? That's the problem, the FDT is really specific to the payload you're loading, there are no "universal" rules on paths. `/firmware/coreboot` and all that stuff is specific to the Linux kernel. UPL has its own spec on how the FDT is supposed to look like, which tries to vaguely match the kernel in style where that works but is really not the same. If you say that there are other firmware payloads that are loaded by this but are not UPL, then I don't know how the FDT is supposed to look like for them, that's up to them. The entire point of UPL was to standardize that FDT, after all. > No, see section 4.1 of the FIT spec. You mean the one that only goes up to section 3.4 here: https://fitspec.osfw.foundation/ ? ;) It sounds like I need some extra context that's not yet officially published(?) and maybe then I'll have an easier time following what all this is supposed to do. https://review.coreboot.org/c/coreboot/+/84796/comment/0611fae8_89a52d48?us… : PS9, Line 403: &fdt, &initrd > Yes. […] Oh, okay, I see how the FDT thing gets added in the later patches. But for initrd, if one specific UPL payload had an initrd, wouldn't that just be just another secondary loadable for it? Wouldn't `load_secondaries()` take care of it? -- To view, visit https://review.coreboot.org/c/coreboot/+/84796?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: I6a21954c0dc5fd820d135e8cd0599ce87903a1c0 Gerrit-Change-Number: 84796 Gerrit-PatchSet: 10 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.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-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 02 Jan 2025 18:54:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: mb/hp/pro_3500_series: Move usb configuration
by Keith Hui (Code Review) 02 Jan '25

02 Jan '25

Attention is currently required from: Keith Hui, Nicholas Chin, Václav Straka. Keith Hui has posted comments on this change by Václav Straka. ( https://review.coreboot.org/c/coreboot/+/85831?usp=email ) Change subject: mb/hp/pro_3500_series: Move usb configuration ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: This reminds me to sweep again for any leftovers. -- To view, visit https://review.coreboot.org/c/coreboot/+/85831?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: I69f1f800e4e2de5367795400d619bf9c000ff184 Gerrit-Change-Number: 85831 Gerrit-PatchSet: 2 Gerrit-Owner: Václav Straka <venda.straka(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Václav Straka <venda.straka(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Thu, 02 Jan 2025 18:35:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

← Newer
1
...
61
62
63
64
65
66
67
...
75
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025