coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3044 discussions

[L] Change in coreboot[main]: mainboard: Drop redundant PciRpEnable from devicetrees
by Nico Huber (Code Review) 14 Jan '24

14 Jan '24

Attention is currently required from: Angel Pons, Christian Walter, Erik van den Bogaert, Frans Hendriks, Jeremy Soller, Jonathon Hall, Michael Niewöhner, Michał Żygowski, Nicholas Sudsgaard, Nick Vaccaro, Nico Huber, Patrick Rudolph, Piotr Król, Sean Rhodes, Subrata Banik, Tim Crawford, Werner Zeh. Nico Huber has uploaded a new patch set (#5) to the change originally created by Nicholas Sudsgaard. ( https://review.coreboot.org/c/coreboot/+/79935?usp=email ) Change subject: mainboard: Drop redundant PciRpEnable from devicetrees ...................................................................... mainboard: Drop redundant PciRpEnable from devicetrees This is the dirty work of the proposed changes: https://review.coreboot.org/c/coreboot/+/79921 (and related changes) This is still not complete, as I was not able to change some configurations. Specifically, in intel/coffeelake_rvp/variants, as I do not know the PCIe addresses of some of the enabled root ports. I tried to be careful, but I cannot guarantee that I did not mess up any configurations. I would appreciate anyone who can put some time into double-checking these changes. Change-Id: I9641a1ed241084f37089094b2392c29e2ac0fd95 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/mainboard/51nb/x210/devicetree.cb M src/mainboard/acer/aspire_vn7_572g/devicetree.cb M src/mainboard/asrock/h110m/devicetree.cb M src/mainboard/clevo/cml-u/variants/l140cu/devicetree.cb M src/mainboard/clevo/kbl-u/variants/n13xwu/devicetree.cb M src/mainboard/clevo/tgl-u/variants/l140mu/devicetree.cb M src/mainboard/facebook/monolith/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/boxy/overridetree.cb M src/mainboard/google/dedede/variants/bugzzy/overridetree.cb M src/mainboard/google/dedede/variants/dexi/overridetree.cb M src/mainboard/google/dedede/variants/dibbi/overridetree.cb M src/mainboard/google/dedede/variants/dita/overridetree.cb M src/mainboard/google/dedede/variants/sasuke/overridetree.cb M src/mainboard/google/dedede/variants/sasukette/overridetree.cb M src/mainboard/google/dedede/variants/storo/overridetree.cb M src/mainboard/google/dedede/variants/taranza/overridetree.cb M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/eve/devicetree.cb M src/mainboard/google/fizz/variants/baseboard/devicetree.cb M src/mainboard/google/fizz/variants/endeavour/overridetree.cb M src/mainboard/google/glados/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/helios/overridetree.cb M src/mainboard/google/hatch/variants/helios_diskswap/overridetree.cb M src/mainboard/google/hatch/variants/kohaku/overridetree.cb M src/mainboard/google/hatch/variants/mushu/overridetree.cb M src/mainboard/google/hatch/variants/nightfury/overridetree.cb M src/mainboard/google/hatch/variants/palkia/overridetree.cb M src/mainboard/google/poppy/variants/atlas/devicetree.cb M src/mainboard/google/poppy/variants/baseboard/devicetree.cb M src/mainboard/google/poppy/variants/nami/devicetree.cb M src/mainboard/google/poppy/variants/nautilus/devicetree.cb M src/mainboard/google/poppy/variants/nocturne/devicetree.cb M src/mainboard/google/poppy/variants/rammus/devicetree.cb M src/mainboard/google/poppy/variants/soraka/devicetree.cb M src/mainboard/google/puff/variants/ambassador/overridetree.cb M src/mainboard/google/puff/variants/baseboard/devicetree.cb M src/mainboard/google/puff/variants/dooly/overridetree.cb M src/mainboard/google/puff/variants/duffy/overridetree.cb M src/mainboard/google/puff/variants/faffy/overridetree.cb M src/mainboard/google/puff/variants/genesis/overridetree.cb M src/mainboard/google/puff/variants/kaisa/overridetree.cb M src/mainboard/google/puff/variants/moonbuggy/overridetree.cb M src/mainboard/google/puff/variants/noibat/overridetree.cb M src/mainboard/google/puff/variants/puff/overridetree.cb M src/mainboard/google/puff/variants/scout/overridetree.cb M src/mainboard/google/puff/variants/wyvern/overridetree.cb M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/chronicler/overridetree.cb M src/mainboard/google/volteer/variants/elemi/overridetree.cb M src/mainboard/google/volteer/variants/voema/overridetree.cb M src/mainboard/hp/280_g2/devicetree.cb M src/mainboard/intel/kunimitsu/devicetree.cb M src/mainboard/intel/saddlebrook/devicetree.cb M src/mainboard/kontron/bsl6/devicetree.cb M src/mainboard/kontron/bsl6/variants/bsl6/overridetree.cb M src/mainboard/libretrend/lt1000/devicetree.cb M src/mainboard/prodrive/hermes/devicetree.cb M src/mainboard/protectli/vault_cml/devicetree.cb M src/mainboard/protectli/vault_ehl/devicetree.cb M src/mainboard/protectli/vault_kbl/devicetree.cb M src/mainboard/purism/librem_cnl/variants/librem_14/overridetree.cb M src/mainboard/purism/librem_cnl/variants/librem_mini/overridetree.cb M src/mainboard/purism/librem_jsl/devicetree.cb M src/mainboard/purism/librem_l1um_v2/devicetree.cb M src/mainboard/purism/librem_skl/devicetree.cb M src/mainboard/razer/blade_stealth_kbl/devicetree.cb M src/mainboard/siemens/chili/variants/base/devicetree.cb M src/mainboard/siemens/chili/variants/chili/devicetree.cb M src/mainboard/siemens/fa_ehl/variants/fa_ehl/devicetree.cb M src/mainboard/siemens/mc_ehl/variants/mc_ehl1/devicetree.cb M src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb M src/mainboard/siemens/mc_ehl/variants/mc_ehl3/devicetree.cb M src/mainboard/siemens/mc_ehl/variants/mc_ehl4/devicetree.cb M src/mainboard/siemens/mc_ehl/variants/mc_ehl5/devicetree.cb M src/mainboard/starlabs/starbook/variants/cml/devicetree.cb M src/mainboard/starlabs/starbook/variants/kbl/devicetree.cb M src/mainboard/starlabs/starbook/variants/tgl/devicetree.cb M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-f/overridetree.cb M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssh-tf/overridetree.cb M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssm-f/overridetree.cb M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssw-f/overridetree.cb M src/mainboard/system76/addw1/devicetree.cb M src/mainboard/system76/bonw14/devicetree.cb M src/mainboard/system76/cml-u/variants/darp6/overridetree.cb M src/mainboard/system76/cml-u/variants/galp4/overridetree.cb M src/mainboard/system76/cml-u/variants/lemp9/overridetree.cb M src/mainboard/system76/gaze15/devicetree.cb M src/mainboard/system76/kbl-u/devicetree.cb M src/mainboard/system76/oryp5/devicetree.cb M src/mainboard/system76/oryp6/devicetree.cb M src/mainboard/system76/tgl-h/variants/gaze16-3050/overridetree.cb M src/mainboard/system76/tgl-h/variants/gaze16-3060/overridetree.cb M src/mainboard/system76/tgl-h/variants/oryp8/overridetree.cb M src/mainboard/system76/tgl-u/variants/darp7/overridetree.cb M src/mainboard/system76/tgl-u/variants/galp5/overridetree.cb M src/mainboard/system76/tgl-u/variants/lemp10/overridetree.cb M src/mainboard/system76/whl-u/devicetree.cb 101 files changed, 65 insertions(+), 464 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/79935/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/79935?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: I9641a1ed241084f37089094b2392c29e2ac0fd95 Gerrit-Change-Number: 79935 Gerrit-PatchSet: 5 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: drivers/uart/sifive.c: Fix divisor calculation
by Maximilian Brune (Code Review) 14 Jan '24

14 Jan '24

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79951?usp=email ) Change subject: drivers/uart/sifive.c: Fix divisor calculation ...................................................................... drivers/uart/sifive.c: Fix divisor calculation The divisor is calculated using the following formula: div = (frequency / baudrate) - 1; The current implementation however essentially cacluates: div = (frequency / baudrate); Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I8a0898ce9016a70c0f91dc8a99fc1cf9e46d20c4 --- M src/drivers/uart/sifive.c 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/79951/1 diff --git a/src/drivers/uart/sifive.c b/src/drivers/uart/sifive.c index d1b89dc..68759c8 100644 --- a/src/drivers/uart/sifive.c +++ b/src/drivers/uart/sifive.c @@ -47,9 +47,14 @@ void uart_init(unsigned int idx) { - unsigned int div; - div = uart_baudrate_divisor(get_uart_baudrate(), - uart_platform_refclk(), uart_input_clock_divider()); + // according to FU740 manual: + // f_baud = f_in / (div + 1) + // <=> + // div = (f_in / f_baud) - 1 + unsigned int div = uart_baudrate_divisor(get_uart_baudrate(), uart_platform_refclk(), + uart_input_clock_divider()); + div -= 1; + sifive_uart_init(uart_platform_baseptr(idx), div); } -- To view, visit https://review.coreboot.org/c/coreboot/+/79951?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: I8a0898ce9016a70c0f91dc8a99fc1cf9e46d20c4 Gerrit-Change-Number: 79951 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: payloads/Kconfig: Make flat-binary configurable
by Maximilian Brune (Code Review) 14 Jan '24

14 Jan '24

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79950?usp=email ) Change subject: payloads/Kconfig: Make flat-binary configurable ...................................................................... payloads/Kconfig: Make flat-binary configurable This adds the ability to add a flat-binary using menuconfig. Test: boot hifive-unmatched mainboard with the following config: CONFIG_PAYLOAD_NONE=n CONFIG_PAYLOAD_ELF=y CONFIG_PAYLOAD_FILE="~/repos/linux-riscv/arch/riscv/boot/Image" CONFIG_PAYLOAD_IS_FLAT_BINARY=y CONFIG_PAYLOAD_OPTIONS="-l 0x82000000 -e 0x82000000" CONFIG_COMPRESSED_PAYLOAD_LZMA=y Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I48c6b53a0c9f5b173c89f1a294a0c37fa1a58f31 --- M payloads/Kconfig 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/79950/1 diff --git a/payloads/Kconfig b/payloads/Kconfig index 2a2773a..9304629 100644 --- a/payloads/Kconfig +++ b/payloads/Kconfig @@ -80,13 +80,14 @@ endchoice config PAYLOAD_OPTIONS - string + string "Additional cbfstool options" default "" help Additional cbfstool options for the payload config PAYLOAD_IS_FLAT_BINARY - def_bool n + bool "Payload is a flat binary" + default n help Add the payload to cbfs as a flat binary type instead of as an elf payload -- To view, visit https://review.coreboot.org/c/coreboot/+/79950?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: I48c6b53a0c9f5b173c89f1a294a0c37fa1a58f31 Gerrit-Change-Number: 79950 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: arch/riscv/boot.c: Add comment for clarity
by Maximilian Brune (Code Review) 14 Jan '24

14 Jan '24

Attention is currently required from: Philipp Hug, ron minnich. Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79949?usp=email ) Change subject: arch/riscv/boot.c: Add comment for clarity ...................................................................... arch/riscv/boot.c: Add comment for clarity It simply adds a comment to indicate to the reader that the RISCV_PAYLOAD_MODE_S parameter causes OpenSBI to switch to Supervisor mode. Otherwise it could be interpreted that coreboot switches to Supervisor mode before starting OpenSBI (which is not the case) Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: Ib62be0c2ff59361200df4c65f9aca5f7456a0ada --- M src/arch/riscv/boot.c 1 file changed, 4 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/79949/1 diff --git a/src/arch/riscv/boot.c b/src/arch/riscv/boot.c index a59f9d1..6f744d3 100644 --- a/src/arch/riscv/boot.c +++ b/src/arch/riscv/boot.c @@ -31,10 +31,12 @@ fdt = prog_entry_arg(prog); if (ENV_RAMSTAGE && prog_type(prog) == PROG_PAYLOAD) { - if (CONFIG(RISCV_OPENSBI)) + if (CONFIG(RISCV_OPENSBI)) { + // tell OpenSBI to switch to Supervisor mode before jumping to payload run_payload_opensbi(prog, fdt, args->opensbi, RISCV_PAYLOAD_MODE_S); - else + } else { run_payload(prog, fdt, RISCV_PAYLOAD_MODE_S); + } } else { void (*doit)(int hart_id, void *fdt, void *arg) = prog_entry(prog); doit(hart_id, fdt, prog_entry_arg(prog)); -- To view, visit https://review.coreboot.org/c/coreboot/+/79949?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: Ib62be0c2ff59361200df4c65f9aca5f7456a0ada Gerrit-Change-Number: 79949 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: arch/riscv: Add OPENSBI_FW_DYNAMIC_BOOT_HART option
by Maximilian Brune (Code Review) 14 Jan '24

14 Jan '24

Attention is currently required from: Philipp Hug, ron minnich. Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79948?usp=email ) Change subject: arch/riscv: Add OPENSBI_FW_DYNAMIC_BOOT_HART option ...................................................................... arch/riscv: Add OPENSBI_FW_DYNAMIC_BOOT_HART option This adds another option to tell OpenSBI which hart to use for booting. Test: Start hifive-unmatched board and see that Hart 1 (instead of 0) is used for running OpenSBI. Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: Id58bd6ae3b55a5ef3f1a5c97dfa07c79aa4c78d0 --- M src/arch/riscv/Kconfig M src/arch/riscv/opensbi.c 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/79948/1 diff --git a/src/arch/riscv/Kconfig b/src/arch/riscv/Kconfig index 2578b12..d3f27c7 100644 --- a/src/arch/riscv/Kconfig +++ b/src/arch/riscv/Kconfig @@ -66,6 +66,16 @@ help The linking address used to build opensbi. +config OPENSBI_FW_DYNAMIC_BOOT_HART + int + default RISCV_WORKING_HARTID + help + This specifies the hart that OpenSBI will use to boot the payload. If set to + value -1, OpenSBI will use a 'lottery' scheme to figure it out itself. + This hart must be able to execute in the Supervisor priviledged mode, since + that is the mode usually used for the payload. It is not OpenSBI will again + figure it out itself. + config ARCH_RISCV_U # U (user) mode is for programs. bool diff --git a/src/arch/riscv/opensbi.c b/src/arch/riscv/opensbi.c index 48f0f2a..bf26b22 100644 --- a/src/arch/riscv/opensbi.c +++ b/src/arch/riscv/opensbi.c @@ -20,6 +20,8 @@ .version = FW_DYNAMIC_INFO_VERSION_MAX, .next_mode = payload_mode, .next_addr = (uintptr_t)payload, + .options = 0, + .boot_hart = CONFIG_OPENSBI_FW_DYNAMIC_BOOT_HART, }; csr_write(mepc, opensbi); -- To view, visit https://review.coreboot.org/c/coreboot/+/79948?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: Id58bd6ae3b55a5ef3f1a5c97dfa07c79aa4c78d0 Gerrit-Change-Number: 79948 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: include/bootmem.h: Add comment
by Maximilian Brune (Code Review) 14 Jan '24

14 Jan '24

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79947?usp=email ) Change subject: include/bootmem.h: Add comment ...................................................................... include/bootmem.h: Add comment Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I8f6752e887af8c1ceba56153e3da864abd040ffa --- M src/include/bootmem.h 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/79947/1 diff --git a/src/include/bootmem.h b/src/include/bootmem.h index 0478a12..5796765 100644 --- a/src/include/bootmem.h +++ b/src/include/bootmem.h @@ -46,6 +46,7 @@ * BM_MEM_RAMSTAGE : Translates to LB_MEM_RAM. * BM_MEM_PAYLOAD : Translates to LB_MEM_RAM. * BM_MEM_BL31 : Translates to LB_MEM_RESERVED. + * BM_MEM_OPENSBI : Translates to LB_MEM_RESERVED. */ void bootmem_write_memory_table(struct lb_memory *mem); -- To view, visit https://review.coreboot.org/c/coreboot/+/79947?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: I8f6752e887af8c1ceba56153e3da864abd040ffa Gerrit-Change-Number: 79947 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: [RFC] region: Turn region_end() into an inclusive region_last()
by Nico Huber (Code Review) 14 Jan '24

14 Jan '24

Attention is currently required from: Jakub Czapiga, Jérémy Compostella. Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79946?usp=email ) Change subject: [RFC] region: Turn region_end() into an inclusive region_last() ...................................................................... [RFC] region: Turn region_end() into an inclusive region_last() The current region_end() implementation is susceptible to overflow if the region is at the end of the addressable space. A common case with the memory-mapped flash of x86 directly below the 32-bit limit. Change-Id: Ic4bd6eced638745b7e845504da74542e4220554a Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/commonlib/include/commonlib/region.h M src/commonlib/region.c M src/cpu/x86/smm/smm_module_loader.c M src/drivers/spi/winbond.c M tests/commonlib/region-test.c M tests/lib/region_file-test.c M util/cbfstool/cbfstool.c M util/cbfstool/cse_serger.c 8 files changed, 26 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/79946/1 diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h index 8de3c5d..cf31f8e 100644 --- a/src/commonlib/include/commonlib/region.h +++ b/src/commonlib/include/commonlib/region.h @@ -122,15 +122,15 @@ return r->size; } -static inline size_t region_end(const struct region *r) +static inline size_t region_last(const struct region *r) { - return region_offset(r) + region_sz(r); + return region_offset(r) + region_sz(r) - 1; } static inline bool region_overlap(const struct region *r1, const struct region *r2) { - return (region_end(r1) > region_offset(r2)) && - (region_offset(r1) < region_end(r2)); + return (region_last(r1) >= region_offset(r2)) && + (region_offset(r1) <= region_last(r2)); } /* Helper to dynamically initialize region device. */ @@ -154,9 +154,9 @@ return region_offset(region_device_region(rdev)); } -static inline size_t region_device_end(const struct region_device *rdev) +static inline size_t region_device_last(const struct region_device *rdev) { - return region_end(region_device_region(rdev)); + return region_last(region_device_region(rdev)); } /* Memory map entire region device. Same semantics as rdev_mmap() above. */ diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 9dc6637..1a4a236 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -10,10 +10,10 @@ if (region_offset(c) < region_offset(p)) return 0; - if (region_end(c) > region_end(p)) + if (region_last(c) > region_last(p)) return 0; - if (region_end(c) < region_offset(c)) + if (region_last(c) < region_offset(p)) return 0; return 1; @@ -22,7 +22,7 @@ static int normalize_and_ok(const struct region *outer, struct region *inner) { inner->offset += region_offset(outer); - return region_end(inner) - 1 >= region_offset(inner) && + return region_last(inner) >= region_offset(inner) && region_is_subregion(outer, inner); } diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 39b36e61..1f1127d 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -335,7 +335,7 @@ } for (int i = 0; i < loader_params->num_cpus; i++) - mod_params->save_state_top[i] = region_end(&cpus[i].ss); + mod_params->save_state_top[i] = region_last(&cpus[i].ss) + 1; if (CONFIG(RUNTIME_CONFIGURABLE_SMM_LOGLEVEL)) mod_params->smm_log_level = mainboard_set_smm_log_level(); @@ -349,7 +349,7 @@ static void print_region(const char *name, const struct region region) { printk(BIOS_DEBUG, "%-12s [0x%zx-0x%zx]\n", name, region_offset(&region), - region_end(&region)); + region_last(&region)); } /* STM + Handler + (Stub + Save state) * CONFIG_MAX_CPUS + stacks */ @@ -425,7 +425,7 @@ return -1; const struct region smram = region_create(smram_base, smram_size); - const uintptr_t smram_top = region_end(&smram); + const uintptr_t smram_top = region_last(&smram) + 1; const size_t stm_size = CONFIG(STM) ? CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE : 0; diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 3137bfe..0f8c170 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -375,7 +375,7 @@ } printk(BIOS_DEBUG, "WINBOND: flash protected range 0x%08zx-0x%08zx\n", - region_offset(&wp_region), region_end(&wp_region)); + region_offset(&wp_region), region_last(&wp_region)); return region_is_subregion(&wp_region, region); } @@ -503,7 +503,7 @@ int ret; /* Need to touch TOP or BOTTOM */ - if (region_offset(region) != 0 && region_end(region) != flash->size) + if (region_offset(region) != 0 && region_last(region) + 1 != flash->size) return -1; params = flash->part; @@ -601,7 +601,7 @@ return ret; printk(BIOS_DEBUG, "WINBOND: write-protection set to range " - "0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + "0x%08zx-0x%08zx\n", region_offset(region), region_last(region)); return ret; } diff --git a/tests/commonlib/region-test.c b/tests/commonlib/region-test.c index 3280482..6a7ce9e 100644 --- a/tests/commonlib/region-test.c +++ b/tests/commonlib/region-test.c @@ -17,7 +17,7 @@ struct region outer = {.offset = VAL(2), .size = VAL(4)}; assert_int_equal(region_offset(&outer), VAL(2)); assert_int_equal(region_sz(&outer), VAL(4)); - assert_int_equal(region_end(&outer), VAL(6)); + assert_int_equal(region_last(&outer) + 1, VAL(6)); struct region inner = {.offset = VAL(3), .size = VAL(2)}; assert_true(region_is_subregion(&outer, &inner)); @@ -118,7 +118,7 @@ { assert_int_equal(region_device_offset(&mock_rdev), 0); assert_int_equal(region_device_sz(&mock_rdev), ~(size_t)0); - assert_int_equal(region_device_end(&mock_rdev), ~(size_t)0); + assert_int_equal(region_device_last(&mock_rdev), ~(size_t)0 - 1); } /* @@ -254,7 +254,7 @@ assert_int_equal(rdev_chain(&child, &mock_rdev, child_offs, child_size), 0); assert_int_equal(region_device_sz(&child), child_size); assert_int_equal(region_device_offset(&child), child_offs); - assert_int_equal(region_device_end(&child), child_offs + child_size); + assert_int_equal(region_device_last(&child), child_offs + child_size - 1); assert_int_equal(rdev_relative_offset(&mock_rdev, &child), child_offs); assert_int_equal(rdev_relative_offset(&child, &mock_rdev), -1); @@ -334,7 +334,9 @@ u8 scratch[size]; int i; struct region_device mem; - rdev_chain_mem_rw(&mem, backing, size); + + assert_true((uintptr_t)backing <= SIZE_MAX); + assert_int_equal(rdev_chain_mem_rw(&mem, backing, size), 0); /* Test writing to and reading from full mapping. */ memset(backing, 0xa5, size); diff --git a/tests/lib/region_file-test.c b/tests/lib/region_file-test.c index 56bb8e4f..fee3b84 100644 --- a/tests/lib/region_file-test.c +++ b/tests/lib/region_file-test.c @@ -142,17 +142,6 @@ assert_int_equal(3, regf.slot); } -static void test_region_file_init_invalid_region_device(void **state) -{ - struct region_device bad_dev; - struct region_file regf; - - rdev_chain_mem_rw(&bad_dev, NULL, 0); - - /* Expect fail when passing invalid region_device. */ - assert_int_equal(-1, region_file_init(&regf, &bad_dev)); -} - static void test_region_file_data(void **state) { /* region_device with empty data buffer */ @@ -310,9 +299,6 @@ cmocka_unit_test_setup_teardown(test_region_file_init_real_data, setup_teardown_region_file_test, setup_teardown_region_file_test), - cmocka_unit_test_setup_teardown(test_region_file_init_invalid_region_device, - setup_teardown_region_file_test, - setup_teardown_region_file_test), cmocka_unit_test_setup_teardown(test_region_file_data, setup_teardown_region_file_test, setup_teardown_region_file_test), diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 10d6501..0bb6e5d 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -423,9 +423,9 @@ &mmap_window_table[j].flash_space)) { ERROR("Flash space windows (base=0x%zx, limit=0x%zx) and (base=0x%zx, limit=0x%zx) overlap!\n", region_offset(&mmap_window_table[i].flash_space), - region_end(&mmap_window_table[i].flash_space), + region_last(&mmap_window_table[i].flash_space), region_offset(&mmap_window_table[j].flash_space), - region_end(&mmap_window_table[j].flash_space)); + region_last(&mmap_window_table[j].flash_space)); return false; } @@ -433,9 +433,9 @@ &mmap_window_table[j].host_space)) { ERROR("Host space windows (base=0x%zx, limit=0x%zx) and (base=0x%zx, limit=0x%zx) overlap!\n", region_offset(&mmap_window_table[i].flash_space), - region_end(&mmap_window_table[i].flash_space), + region_last(&mmap_window_table[i].flash_space), region_offset(&mmap_window_table[j].flash_space), - region_end(&mmap_window_table[j].flash_space)); + region_last(&mmap_window_table[j].flash_space)); return false; } } diff --git a/util/cbfstool/cse_serger.c b/util/cbfstool/cse_serger.c index 9b776e6..d16e1af 100644 --- a/util/cbfstool/cse_serger.c +++ b/util/cbfstool/cse_serger.c @@ -647,7 +647,7 @@ size_t end_offset; for (size_t i = 0; i < BP_TOTAL; i++) { - end_offset = region_end(&params.layout_regions[i]); + end_offset = region_last(&params.layout_regions[i]) + 1; if (end_offset > offset) offset = end_offset; } -- To view, visit https://review.coreboot.org/c/coreboot/+/79946?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: Ic4bd6eced638745b7e845504da74542e4220554a Gerrit-Change-Number: 79946 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: [NEEDS TEST] region: Check for overflows after offset calculation
by Nico Huber (Code Review) 14 Jan '24

14 Jan '24

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79945?usp=email ) Change subject: [NEEDS TEST] region: Check for overflows after offset calculation ...................................................................... [NEEDS TEST] region: Check for overflows after offset calculation As we currently do not know where overflows actually happen, this needs testing to avoid boot failures due to potential bugs somewhere else. Change-Id: Ia6054e6e9b6f13502351a2d94c8a0af15266e43e Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/commonlib/region.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/79945/1 diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 4153f0a..9dc6637 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -22,7 +22,8 @@ static int normalize_and_ok(const struct region *outer, struct region *inner) { inner->offset += region_offset(outer); - return region_is_subregion(outer, inner); + return region_end(inner) - 1 >= region_offset(inner) && + region_is_subregion(outer, inner); } static const struct region_device *rdev_root(const struct region_device *rdev) -- To view, visit https://review.coreboot.org/c/coreboot/+/79945?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: Ia6054e6e9b6f13502351a2d94c8a0af15266e43e Gerrit-Change-Number: 79945 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: [RFC] region: Hide struct region members
by Nico Huber (Code Review) 14 Jan '24

14 Jan '24

Attention is currently required from: Jakub Czapiga, Julius Werner, Nico Huber, Philipp Hug, ron minnich. Hello Jakub Czapiga, Julius Werner, Philipp Hug, build bot (Jenkins), ron minnich, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79907?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: [RFC] region: Hide struct region members ...................................................................... [RFC] region: Hide struct region members We cannot make `struct region` opaque for several reasons (size needs to be known for local objects, a lot of inline API functions, ...). However, we still want to encourage using the high-level region API instead of directly accessing and manipulating the struct members. This patch tries to achieve this by renaming the struct members. Only if REGION_INTERNAL_STRUCTURES is defined before including <region.h>, the usual member names are used. Change-Id: I713be9cf0bab4c2e21113b55e7229ab50f06c6cf Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/arch/arm/fit_payload.c M src/arch/arm64/fit_payload.c M src/arch/riscv/fit_payload.c M src/commonlib/include/commonlib/region.h M src/commonlib/region.c M src/lib/fit_payload.c M tests/commonlib/region-test.c M tests/lib/cbfs-lookup-test.c M tests/lib/fmap-test.c 9 files changed, 26 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/79907/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79907?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: I713be9cf0bab4c2e21113b55e7229ab50f06c6cf Gerrit-Change-Number: 79907 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> 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-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: mb/arm/rdn2: Add ACPI support for GIC table
by Naresh Solanki (Code Review) 14 Jan '24

14 Jan '24

Naresh Solanki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/79526?usp=email ) Change subject: mb/arm/rdn2: Add ACPI support for GIC table ...................................................................... Abandoned taken care in another patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/79526?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: I55ba7c380e0986d9f146ef4c90c9034dd610f750 Gerrit-Change-Number: 79526 Gerrit-PatchSet: 2 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.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 January 2024