coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[XS] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: use device status for port_present
by Felix Held (Code Review) 21 Mar '24

21 Mar '24

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81342?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: use device status for port_present ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/81342?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: Ieaa2af6c5ff3fc7e25992e7fdf14d37ee4a57d62 Gerrit-Change-Number: 81342 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 21 Mar 2024 21:47:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: simplify per_device_config arguments
by Felix Held (Code Review) 21 Mar '24

21 Mar '24

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81341?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: simplify per_device_config arguments ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/81341?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: I0466f7ad2f5c9874d45712fa9f89b978bd2a09bc Gerrit-Change-Number: 81341 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 21 Mar 2024 21:47:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: move PCIe port function below mpio chip
by Felix Held (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Arthur Heymans, Varshit Pandya. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81340?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: move PCIe port function below mpio chip ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/81340?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: I7e39bf68d30d7d00b16f943953e8207d6fe9ef41 Gerrit-Change-Number: 81340 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 21 Mar 2024 21:47:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: arch/riscv: add Kconfig variable RISCV_SOC_HAS_MENVCFG
by ron minnich (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Martin L Roth, Maximilian Brune, Patrick Georgi, Philipp Hug. ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81410?usp=email ) Change subject: arch/riscv: add Kconfig variable RISCV_SOC_HAS_MENVCFG ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81410/comment/716f9f0e_0cd7e482 : PS1, Line 10: Provide a Kconfig variable, default y, to enable it. > that's why it's not interactive. […] So I think this is what you get if you misconfigure your image, but we make it harder to do so (see the conditional in the Kconfig -- there are VERY few SoC any moere that don't to menvcfg). Patchset: PS1: > yeah, you're right. […] code removed, statement is now true -- To view, visit https://review.coreboot.org/c/coreboot/+/81410?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: I627dde377708cde2491d4abe625c482d64f7aa87 Gerrit-Change-Number: 81410 Gerrit-PatchSet: 5 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Thu, 21 Mar 2024 21:47:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Comment-In-Reply-To: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: add debug output for unused chip
by Felix Held (Code Review) 21 Mar '24

21 Mar '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81384?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: add debug output for unused chip ...................................................................... vc/amd/opensil/genoa_poc/mpio: add debug output for unused chip Print that the MPIO chip of one of the MPIO-related PCI device functions is unused and is skipped, if the type is IFTYPE_UNUSED and the corresponding PCI device function isn't enabled. This allows to differentiate between this case and the case where the type isn't IFTYPE_UNUSED. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I4fc28d39a229494b487b300b28f92bf3adad66f5 --- M src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/81384/1 diff --git a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c index d79e917..089096f 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c +++ b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c @@ -138,6 +138,8 @@ if (is_dev_enabled(dev)) { printk(BIOS_WARNING, "Unused MPIO chip, disabling PCI device.\n"); dev->enabled = false; + } else { + printk(BIOS_DEBUG, "Unused MPIO chip, skipping.\n"); } return; } -- To view, visit https://review.coreboot.org/c/coreboot/+/81384?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: I4fc28d39a229494b487b300b28f92bf3adad66f5 Gerrit-Change-Number: 81384 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: fix unused MPIO chip warning
by Felix Held (Code Review) 21 Mar '24

21 Mar '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81383?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: fix unused MPIO chip warning ...................................................................... vc/amd/opensil/genoa_poc/mpio: fix unused MPIO chip warning When the chip of one of the MPIO-related PCI device functions has the type IFTYPE_UNUSED, there is no corresponding MPIO engine, so replace 'engine' with 'chip' in the warning. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I0f55a3f8e1d220d4eb7b0287d03b7af2e5d2889f --- M src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/81383/1 diff --git a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c index 2ad10af..d79e917 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c +++ b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c @@ -136,7 +136,7 @@ if (config->type == IFTYPE_UNUSED) { if (is_dev_enabled(dev)) { - printk(BIOS_WARNING, "Unused MPIO engine, disabling PCI device.\n"); + printk(BIOS_WARNING, "Unused MPIO chip, disabling PCI device.\n"); dev->enabled = false; } return; -- To view, visit https://review.coreboot.org/c/coreboot/+/81383?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: I0f55a3f8e1d220d4eb7b0287d03b7af2e5d2889f Gerrit-Change-Number: 81383 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: arch/riscv: add Kconfig variable RISCV_SOC_HAS_MENVCFG
by ron minnich (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Martin L Roth, Maximilian Brune, Patrick Georgi, Philipp Hug. Hello Martin L Roth, Maximilian Brune, Patrick Georgi, Philipp Hug, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81410?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: arch/riscv: add Kconfig variable RISCV_SOC_HAS_MENVCFG ...................................................................... arch/riscv: add Kconfig variable RISCV_SOC_HAS_MENVCFG Older parts do not have the menvcfg csr. Provide a Kconfig variable, default y, to enable it. Check the variable in the payload code, when coreboot SBI is used, and print out if it is enabled. Add constants for this new CSR. Signed-off-by: Ronald G Minnich <rminnich(a)gmail.com> Change-Id: I627dde377708cde2491d4abe625c482d64f7aa87 --- M src/arch/riscv/Kconfig M src/arch/riscv/include/arch/encoding.h 2 files changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/81410/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/81410?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: I627dde377708cde2491d4abe625c482d64f7aa87 Gerrit-Change-Number: 81410 Gerrit-PatchSet: 5 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: mb/google/brox: Configure ISH device based on FW_CONFIG
by Karthik Ramasubramanian (Code Review) 21 Mar '24

21 Mar '24

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81418?usp=email ) Change subject: mb/google/brox: Configure ISH device based on FW_CONFIG ...................................................................... mb/google/brox: Configure ISH device based on FW_CONFIG ISH Firmware name needs to be configured only when full sensing capabilities are enabled through ISH_ENABLE FW_CONFIG. Similarly dma property needs to be added only when UFS is enabled through STORAGE_UFS FW_CONFIG. Hence configure the ISH device at run-time based on FW_CONFIG. BUG=None TEST=Build Brox BIOS image and boot to OS. Change-Id: I678416acd48e03ab77ae299beae6e295a688b8df Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/brox/variants/brox/fw_config.c M src/mainboard/google/brox/variants/brox/overridetree.cb 2 files changed, 14 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/81418/1 diff --git a/src/mainboard/google/brox/variants/brox/fw_config.c b/src/mainboard/google/brox/variants/brox/fw_config.c index 162f4a1..d8fd44d 100644 --- a/src/mainboard/google/brox/variants/brox/fw_config.c +++ b/src/mainboard/google/brox/variants/brox/fw_config.c @@ -1,10 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <bootstate.h> +#include <drivers/intel/ish/chip.h> #include <fw_config.h> #include <gpio.h> #define GPIO_PADBASED_OVERRIDE(b, a) gpio_padbased_override(b, a, ARRAY_SIZE(a)) +#define ISH_FIRMWARE_NAME "brox_ish.bin" static const struct pad_config ish_enable_pads[] = { /* GPP_B5 : ISH I2C0_SDA */ @@ -27,9 +29,19 @@ static void fw_config_handle(void *unused) { + struct device *ish_config_device = DEV_PTR(ish_conf); + struct drivers_intel_ish_config *config = config_of(ish_config_device); + if (fw_config_probe(FW_CONFIG(ISH, ISH_ENABLE))) { - printk(BIOS_INFO, "Configure GPIOs for ISH.\n"); + printk(BIOS_INFO, "Configure GPIOs, device config for ISH.\n"); gpio_configure_pads(ish_enable_pads, ARRAY_SIZE(ish_enable_pads)); + + config->firmware_name = ISH_FIRMWARE_NAME; + } + + if (fw_config_probe(FW_CONFIG(STORAGE, STORAGE_UFS))) { + printk(BIOS_INFO, "Configure GPIOs, device config for UFS.\n"); + config->add_acpi_dma_property = true; } } BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, fw_config_handle, NULL); diff --git a/src/mainboard/google/brox/variants/brox/overridetree.cb b/src/mainboard/google/brox/variants/brox/overridetree.cb index 2c4335b..7b00098 100644 --- a/src/mainboard/google/brox/variants/brox/overridetree.cb +++ b/src/mainboard/google/brox/variants/brox/overridetree.cb @@ -300,9 +300,7 @@ end device ref ish on chip drivers/intel/ish - register "firmware_name" = ""brox_ish.bin"" - register "add_acpi_dma_property" = "true" - device generic 0 on end + device generic 0 alias ish_conf on end end probe ISH ISH_ENABLE probe STORAGE STORAGE_UFS -- To view, visit https://review.coreboot.org/c/coreboot/+/81418?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: I678416acd48e03ab77ae299beae6e295a688b8df Gerrit-Change-Number: 81418 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: drivers/intel/ish: Include stdbool.h to identify bool type
by Karthik Ramasubramanian (Code Review) 21 Mar '24

21 Mar '24

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81417?usp=email ) Change subject: drivers/intel/ish: Include stdbool.h to identify bool type ...................................................................... drivers/intel/ish: Include stdbool.h to identify bool type When the concerned chip.h file is included in a source file, it causes compilation error saying unknown type name bool. Fix it by including the stdbool.h file in the chip.h file. BUG=None TEST=Build Brox by including the chip.h file in one of the source files. Change-Id: I4159e2c281c3e89dc45555ce38ad8637a3bf8587 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/drivers/intel/ish/chip.h 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/81417/1 diff --git a/src/drivers/intel/ish/chip.h b/src/drivers/intel/ish/chip.h index 69f3253..3bcf65a 100644 --- a/src/drivers/intel/ish/chip.h +++ b/src/drivers/intel/ish/chip.h @@ -3,6 +3,8 @@ #ifndef __DRIVERS_INTEL_ISH_CHIP_H__ #define __DRIVERS_INTEL_ISH_CHIP_H__ +#include <stdbool.h> + /* * Intel Integrated Sensor Hub (ISH) */ -- To view, visit https://review.coreboot.org/c/coreboot/+/81417?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: I4159e2c281c3e89dc45555ce38ad8637a3bf8587 Gerrit-Change-Number: 81417 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[main]: arch/riscv: remove misaligned load/store/fetch handling
by ron minnich (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Philipp Hug. Hello Philipp Hug, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81416?usp=email to look at the new patch set (#2). Change subject: arch/riscv: remove misaligned load/store/fetch handling ...................................................................... arch/riscv: remove misaligned load/store/fetch handling Testing on the unmatched shows the code no longer works completely correctly; Linux has taken over the handling of misalignment anyway, because handling it in firmware, with the growing complexity of the ISA and the awkward way in which it has to be handled, is more trouble than its worth. Plus, we don't WANT misalignment handled, magically, in firmware: the cost of getting it wrong is high (as I've spent a month learning); the performance is terrible (350x slowdown; and most toolchains now know to avoid unaligned load/store on RISC-V anyway. But, mostly, if alignment problems exist, *we need to know*, and if they're handled invisibly in firmware, we don't. The problem with invisble handling was shown a while back in the Go toolchain: runtime had a small error, such that many misaligned load/store were happening, and it was not discovered for some time. Had a trap been directed to kernel or user on misalignment, the problem would have been known immediately, not after many months. (The error, btw, was masking the address with 3, not 7, to detect misalignment; an easy mistake!). But, the coreboot code does not work any more any way, and it's not worth fixing. Remove it. Tested by booting Linux to runlevel 1; before, it would hang on an alignment fault, as the alignment code was failing (somewhere). This takes the coreboot SBI code much closer to revival. Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Signed-off-by: Ronald G Minnich <rminnich(a)gmail.com> --- M src/arch/riscv/Makefile.mk D src/arch/riscv/misaligned.c M src/arch/riscv/trap_handler.c M src/arch/riscv/virtual_memory.c 4 files changed, 6 insertions(+), 265 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/81416/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81416?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: I84a8d433ed2f50745686a8c109d101e8718f2a46 Gerrit-Change-Number: 81416 Gerrit-PatchSet: 2 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024