coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3293 discussions

[XS] Change in coreboot[main]: arch/arm/include/arch/hlt.h: fix warnings & errors of for loop
by Julius Werner (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Integral. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82073?usp=email ) Change subject: arch/arm/include/arch/hlt.h: fix warnings & errors of for loop ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82073?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: Iae4830a927c8aaf366d49ddd2154f6a180078234 Gerrit-Change-Number: 82073 Gerrit-PatchSet: 1 Gerrit-Owner: Integral <integral(a)member.fsf.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Integral <integral(a)member.fsf.org> Gerrit-Comment-Date: Wed, 24 Apr 2024 21:42:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: arch/arm/eabi_compat.c: fix warnings of macros
by Julius Werner (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Integral. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82072?usp=email ) Change subject: arch/arm/eabi_compat.c: fix warnings of macros ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82072?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: I232f011ba0567af78c69eb8f73366b03d29c2839 Gerrit-Change-Number: 82072 Gerrit-PatchSet: 1 Gerrit-Owner: Integral <integral(a)member.fsf.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Integral <integral(a)member.fsf.org> Gerrit-Comment-Date: Wed, 24 Apr 2024 21:42:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: arch/arm/armv7/exception.c: fix warnings of macros and functions
by Julius Werner (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Integral. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82050?usp=email ) Change subject: arch/arm/armv7/exception.c: fix warnings of macros and functions ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82050?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: I1c5c33fc9210f068ff88c8d981f1a1c739890c9c Gerrit-Change-Number: 82050 Gerrit-PatchSet: 4 Gerrit-Owner: Integral <integral(a)member.fsf.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Integral <integral(a)member.fsf.org> Gerrit-Comment-Date: Wed, 24 Apr 2024 21:42:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/amd/common/amd_pci_util.h: assign 0 to PIN_A in pcie_swizzle_pin
by Felix Held (Code Review) 24 Apr '24

24 Apr '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82049?usp=email ) Change subject: soc/amd/common/amd_pci_util.h: assign 0 to PIN_A in pcie_swizzle_pin ...................................................................... soc/amd/common/amd_pci_util.h: assign 0 to PIN_A in pcie_swizzle_pin Explicitly assign a value of 0 to the first value of the pcie_swizzle_pin enum. This won't change the behavior, but clarifies that the actual values of the enum elements matter. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I21850e21f859f2079f804d4344a1a11856b27d90 Reviewed-on: https://review.coreboot.org/c/coreboot/+/82049 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> --- M src/soc/amd/common/block/pci/pci_routing_info.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, approved diff --git a/src/soc/amd/common/block/pci/pci_routing_info.c b/src/soc/amd/common/block/pci/pci_routing_info.c index ac72553..5d5f355 100644 --- a/src/soc/amd/common/block/pci/pci_routing_info.c +++ b/src/soc/amd/common/block/pci/pci_routing_info.c @@ -7,7 +7,7 @@ #include <types.h> enum pcie_swizzle_pin { - PIN_A, + PIN_A = 0, PIN_B, PIN_C, PIN_D, -- To view, visit https://review.coreboot.org/c/coreboot/+/82049?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: I21850e21f859f2079f804d4344a1a11856b27d90 Gerrit-Change-Number: 82049 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: soc/amd/common/amd_pci_util.h: rename bridge irq in pci_routing_info
by Felix Held (Code Review) 24 Apr '24

24 Apr '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82048?usp=email ) Change subject: soc/amd/common/amd_pci_util.h: rename bridge irq in pci_routing_info ...................................................................... soc/amd/common/amd_pci_util.h: rename bridge irq in pci_routing_info Rename the 'irq' element of the pci_routing_info struct to 'bridge_irq' to better describe what it's doing. This struct element contains the number of the northbridge IOAPIC IRQ input the bridge IRQ is connected to signal power management or error reporting IRQs. Right now, coreboot doesn't put this information into the ACPI bytecode. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I6410be673d15d6f9b5eb4c80b51fb705fec5b155 Reviewed-on: https://review.coreboot.org/c/coreboot/+/82048 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> --- M src/soc/amd/common/block/include/amdblocks/amd_pci_util.h M src/soc/amd/common/fsp/pci/pci_routing_info.c 2 files changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, approved diff --git a/src/soc/amd/common/block/include/amdblocks/amd_pci_util.h b/src/soc/amd/common/block/include/amdblocks/amd_pci_util.h index f862bf9..db8b704 100644 --- a/src/soc/amd/common/block/include/amdblocks/amd_pci_util.h +++ b/src/soc/amd/common/block/include/amdblocks/amd_pci_util.h @@ -57,7 +57,7 @@ uint8_t devfn; uint8_t group; uint8_t swizzle; - uint8_t irq; + uint8_t bridge_irq; /* also called 'map' */ } __packed; void populate_pirq_data(void); diff --git a/src/soc/amd/common/fsp/pci/pci_routing_info.c b/src/soc/amd/common/fsp/pci/pci_routing_info.c index 5e3c368..96ea1ff 100644 --- a/src/soc/amd/common/fsp/pci/pci_routing_info.c +++ b/src/soc/amd/common/fsp/pci/pci_routing_info.c @@ -34,9 +34,9 @@ routing_table_entries = routing_hob->num_of_entries; for (size_t i = 0; i < routing_table_entries; ++i) { - printk(BIOS_DEBUG, "%02x.%x: group: %u, swizzle: %u, irq: %u\n", + printk(BIOS_DEBUG, "%02x.%x: group: %u, swizzle: %u, bridge irq: %u\n", PCI_SLOT(routing_table[i].devfn), PCI_FUNC(routing_table[i].devfn), - routing_table[i].group, routing_table[i].swizzle, routing_table[i].irq); + routing_table[i].group, routing_table[i].swizzle, routing_table[i].bridge_irq); } *entries = routing_table_entries; -- To view, visit https://review.coreboot.org/c/coreboot/+/82048?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: I6410be673d15d6f9b5eb4c80b51fb705fec5b155 Gerrit-Change-Number: 82048 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: drivers/qemu: Split Cirrus display support from Bochs display support
by Alper Nebi Yasak (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Arthur Heymans, Nico Huber. Alper Nebi Yasak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80375?usp=email ) Change subject: drivers/qemu: Split Cirrus display support from Bochs display support ...................................................................... Patch Set 2: (1 comment) File src/drivers/emulation/qemu/Kconfig: https://review.coreboot.org/c/coreboot/+/80375/comment/3dda641b_e9ac3668 : PS1, Line 26: DRIVERS_EMULATION_QEMU_CIRRUS > Should Cirrus and Bochs be mutually exclusive? We can even use something like `-device cirrus-vga -device bochs-display` on ARM64 to have both, but using multiple displays have its own problems. The drivers can work independently so I also don't think we need to make them exclusive. > I don't think we have to duplicate the resolution Kconfigs. Can we makes those generic DRIVERS_EMULATION_QEMU_X/YRES? I removed the duplication here and added the rename as another change, CB:82060. -- To view, visit https://review.coreboot.org/c/coreboot/+/80375?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: Ic9492b501ed4fdcbda6886db60b1e5348715e667 Gerrit-Change-Number: 80375 Gerrit-PatchSet: 2 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 24 Apr 2024 21:09:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/qemu/cirrus: Allow building for non-x86 architectures
by Alper Nebi Yasak (Code Review) 24 Apr '24

24 Apr '24

Alper Nebi Yasak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82065?usp=email ) Change subject: drivers/qemu/cirrus: Allow building for non-x86 architectures ...................................................................... drivers/qemu/cirrus: Allow building for non-x86 architectures The Cirrus display driver uses port I/O functions and VGA support code for initialization, so it could only have been built on x86 so far, but the device can be used just fine on other architectures on the QEMU side as long as the emulated platform supports PCI. Previous commits add port I/O functions and enable building VGA code for more architectures including ARM* and RISC-V, which should allow this driver to be successfully built and used on these as well. Allow the Cirrus display driver to be built for non-x86 QEMU boards by changing the Kconfig dependencies. Make VGA text framebuffer support depend on x86, because it isn't usable at the standard 0xB8000 address on other architectrures. Add a dependency on PCI since this is a PCI device and vexpress-a9 (qemu-armv7) doesn't have the (emulated) hardware for PCI. Change-Id: I168d3bf9b27d5eefa1d4844b861d73ec8437ee16 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/emulation/qemu/Kconfig 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/82065/1 diff --git a/src/drivers/emulation/qemu/Kconfig b/src/drivers/emulation/qemu/Kconfig index f176563..2c49eaa 100644 --- a/src/drivers/emulation/qemu/Kconfig +++ b/src/drivers/emulation/qemu/Kconfig @@ -17,9 +17,10 @@ config DRIVERS_EMULATION_QEMU_CIRRUS bool "cirrus svga driver" default y - depends on CPU_QEMU_X86 + depends on PCI + depends on VENDOR_EMULATION depends on MAINBOARD_DO_NATIVE_VGA_INIT - select HAVE_VGA_TEXT_FRAMEBUFFER + select HAVE_VGA_TEXT_FRAMEBUFFER if CPU_QEMU_X86 select HAVE_LINEAR_FRAMEBUFFER select VGA help -- To view, visit https://review.coreboot.org/c/coreboot/+/82065?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: I168d3bf9b27d5eefa1d4844b861d73ec8437ee16 Gerrit-Change-Number: 82065 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: drivers/pc80/vga: Allow building for non-x86 architectures
by Alper Nebi Yasak (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Jérémy Compostella. Alper Nebi Yasak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82064?usp=email ) Change subject: drivers/pc80/vga: Allow building for non-x86 architectures ...................................................................... drivers/pc80/vga: Allow building for non-x86 architectures The Cirrus driver uses the VGA support code for its initialization, which in turn depends on port I/O functions to talk to the PCI device. Previous commits implement these and fix build errors with VGA code on more architectures, so it's possible to have things also work there to an extent. However, building VGA support code is restricted to x86 architectures in its Makefile, so trying to build the Cirrus driver for e.g. ARM64 results in linker errors. Remove the Makefile restriction. Change-Id: Ic3199fda500c51aa3261cd9eff9a5b0b052fd742 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/pc80/vga/Makefile.mk 1 file changed, 0 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/82064/1 diff --git a/src/drivers/pc80/vga/Makefile.mk b/src/drivers/pc80/vga/Makefile.mk index 63ec6ba..db103d8 100644 --- a/src/drivers/pc80/vga/Makefile.mk +++ b/src/drivers/pc80/vga/Makefile.mk @@ -1,7 +1,5 @@ ## SPDX-License-Identifier: GPL-2.0-only -ifeq ($(CONFIG_ARCH_X86),y) - romstage-$(CONFIG_ROMSTAGE_VGA) += vga_io.c romstage-$(CONFIG_ROMSTAGE_VGA) += vga_palette.c romstage-$(CONFIG_ROMSTAGE_VGA) += vga_font_8x16.c @@ -11,5 +9,3 @@ ramstage-$(CONFIG_VGA) += vga_palette.c ramstage-$(CONFIG_VGA) += vga_font_8x16.c ramstage-$(CONFIG_VGA) += vga.c - -endif -- To view, visit https://review.coreboot.org/c/coreboot/+/82064?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: Ic3199fda500c51aa3261cd9eff9a5b0b052fd742 Gerrit-Change-Number: 82064 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: drivers/pc80/vga/vga: Move big buffer for vga_write_text out of stack
by Alper Nebi Yasak (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Jérémy Compostella. Alper Nebi Yasak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82063?usp=email ) Change subject: drivers/pc80/vga/vga: Move big buffer for vga_write_text out of stack ...................................................................... drivers/pc80/vga/vga: Move big buffer for vga_write_text out of stack The vga_write_text() uses a buffer for text large enough to fill the entire of VGA text mode screen, in order to split its input into lines and print each line. When trying to build it for ARM64 we get the following error: src/drivers/pc80/vga/vga.c: In function 'vga_write_text': src/drivers/pc80/vga/vga.c:286:1: error: stack usage is 2064 bytes [-Werror=stack-usage=] 286 | vga_write_text(enum VGA_TEXT_ALIGNMENT alignment, unsigned int line, | ^~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:419: build/qemu_arm64/ramstage/drivers/pc80/vga/vga.o] Error 1 This is an error because toolchain.mk adds `-Wstack-usage=1536` to CFLAGS for non-x86 architectures, which is accompanied with a note that some boards only have 2K stacks, so ignoring the warning with #pragma is likely unsafe. The toolchain.mk comment advises us to "Store larger buffers in BSS, use static to share data in cache-as-ram on x86". Mark the big text buffer as static and clear it each time it's called. Change-Id: Icbc6da144988060532d686d199afd7f26cbc6679 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/pc80/vga/vga.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/82063/1 diff --git a/src/drivers/pc80/vga/vga.c b/src/drivers/pc80/vga/vga.c index 7f8ce69..9fbf5bc 100644 --- a/src/drivers/pc80/vga/vga.c +++ b/src/drivers/pc80/vga/vga.c @@ -287,7 +287,8 @@ const unsigned char *ustring) { const char *string = (const char *)ustring; - char str[VGA_COLUMNS * VGA_LINES] = {0}; + static char str[VGA_COLUMNS * VGA_LINES] = {0}; + memset(str, 0, sizeof(str) - 1); memcpy(str, string, strnlen(string, sizeof(str) - 1)); char *token = strtok(str, "\n"); -- To view, visit https://review.coreboot.org/c/coreboot/+/82063?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: Icbc6da144988060532d686d199afd7f26cbc6679 Gerrit-Change-Number: 82063 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: drivers/qemu/cirrus: Add missing arch/io.h include
by Alper Nebi Yasak (Code Review) 24 Apr '24

24 Apr '24

Alper Nebi Yasak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82062?usp=email ) Change subject: drivers/qemu/cirrus: Add missing arch/io.h include ...................................................................... drivers/qemu/cirrus: Add missing arch/io.h include The Cirrus display driver uses port I/O functions without including the arch/io.h header that defines them. The header is included through arch/pci_ops.h and arch/pci_io_cfg.h on x86, so we don't normally get a build error. This is not the case on other architectures. Include the necessary arch/io.h header directly in the Cirrus driver to help make it possible to build it on other architectures. Change-Id: I84c76e6e723c776f45d40d98029e33adb57005c0 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/emulation/qemu/cirrus.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/82062/1 diff --git a/src/drivers/emulation/qemu/cirrus.c b/src/drivers/emulation/qemu/cirrus.c index 6fa9ac2..a98fabb 100644 --- a/src/drivers/emulation/qemu/cirrus.c +++ b/src/drivers/emulation/qemu/cirrus.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #include <stdint.h> +#include <arch/io.h> #include <console/console.h> #include <device/device.h> #include <device/pci.h> -- To view, visit https://review.coreboot.org/c/coreboot/+/82062?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: I84c76e6e723c776f45d40d98029e33adb57005c0 Gerrit-Change-Number: 82062 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024