coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3293 discussions

[XS] Change in coreboot[main]: drivers/qemu/bochs: Use arch-independent data port
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/+/82061?usp=email ) Change subject: drivers/qemu/bochs: Use arch-independent data port ...................................................................... drivers/qemu/bochs: Use arch-independent data port The QEMU Bochs display driver uses 0x1CF as its VBE DISPI data port, but this is only present on x86. Instead, use the port at 0x1D0 [1] which is available on both x86 and non-x86 architectures. The data port is also calculated inline based on the VBE DISPI index port while reading and writing, update those expressions as well. [1] https://web.archive.org/web/20240404032816/https://www.qemu.org/docs/master… Change-Id: I899beb742d42c26f3e57023f05ff459094fce5f1 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/emulation/qemu/bochs.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/82061/1 diff --git a/src/drivers/emulation/qemu/bochs.c b/src/drivers/emulation/qemu/bochs.c index 2e0526f..70d17cb 100644 --- a/src/drivers/emulation/qemu/bochs.c +++ b/src/drivers/emulation/qemu/bochs.c @@ -14,7 +14,7 @@ /* VGA init. We use the Bochs VESA VBE extensions */ #define VBE_DISPI_IOPORT_INDEX 0x01CE -#define VBE_DISPI_IOPORT_DATA 0x01CF +#define VBE_DISPI_IOPORT_DATA 0x01D0 #define VBE_DISPI_INDEX_ID 0x0 #define VBE_DISPI_INDEX_XRES 0x1 @@ -46,7 +46,7 @@ { if (res->flags & IORESOURCE_IO) { outw(index, res->base); - outw(val, res->base + 1); + outw(val, res->base + 2); } else { write16(res2mmio(res, 0x500 + index * 2, 0), val); } @@ -56,7 +56,7 @@ { if (res->flags & IORESOURCE_IO) { outw(index, res->base); - return inw(res->base + 1); + return inw(res->base + 2); } else { return read16(res2mmio(res, 0x500 + index * 2, 0)); } -- To view, visit https://review.coreboot.org/c/coreboot/+/82061?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: I899beb742d42c26f3e57023f05ff459094fce5f1 Gerrit-Change-Number: 82061 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: drivers/qemu: Clarify config option name for QEMU display resolution
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/+/82060?usp=email ) Change subject: drivers/qemu: Clarify config option name for QEMU display resolution ...................................................................... drivers/qemu: Clarify config option name for QEMU display resolution A previous commit splits out Cirrus display support from Bochs display support, with both using the pre-existing Bochs config options for the requested display resolution. Rename these config names to clarify they are not only specific to the Bochs display driver. Change-Id: Ie0a5e75731231bb768d7728867196c9ab5c53a00 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/emulation/qemu/Kconfig M src/drivers/emulation/qemu/bochs.c M src/drivers/emulation/qemu/cirrus.c 3 files changed, 8 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/82060/1 diff --git a/src/drivers/emulation/qemu/Kconfig b/src/drivers/emulation/qemu/Kconfig index 0a1a078..5b01ae7 100644 --- a/src/drivers/emulation/qemu/Kconfig +++ b/src/drivers/emulation/qemu/Kconfig @@ -24,14 +24,14 @@ help VGA driver for qemu emulated cirrus svga card. -config DRIVERS_EMULATION_QEMU_BOCHS_XRES - int "bochs vga xres" +config DRIVERS_EMULATION_QEMU_XRES + int "qemu vga xres" default 800 depends on LINEAR_FRAMEBUFFER depends on DRIVERS_EMULATION_QEMU_BOCHS || DRIVERS_EMULATION_QEMU_CIRRUS -config DRIVERS_EMULATION_QEMU_BOCHS_YRES - int "bochs vga yres" +config DRIVERS_EMULATION_QEMU_YRES + int "qemu vga yres" default 600 depends on LINEAR_FRAMEBUFFER depends on DRIVERS_EMULATION_QEMU_BOCHS || DRIVERS_EMULATION_QEMU_CIRRUS diff --git a/src/drivers/emulation/qemu/bochs.c b/src/drivers/emulation/qemu/bochs.c index 0650430..2e0526f 100644 --- a/src/drivers/emulation/qemu/bochs.c +++ b/src/drivers/emulation/qemu/bochs.c @@ -39,8 +39,8 @@ #define VBE_DISPI_LFB_ENABLED 0x40 #define VBE_DISPI_NOCLEARMEM 0x80 -static int width = CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_XRES; -static int height = CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_YRES; +static int width = CONFIG_DRIVERS_EMULATION_QEMU_XRES; +static int height = CONFIG_DRIVERS_EMULATION_QEMU_YRES; static void bochs_write(struct resource *res, int index, int val) { diff --git a/src/drivers/emulation/qemu/cirrus.c b/src/drivers/emulation/qemu/cirrus.c index 1dc8ac9..6fa9ac2 100644 --- a/src/drivers/emulation/qemu/cirrus.c +++ b/src/drivers/emulation/qemu/cirrus.c @@ -9,8 +9,8 @@ #include <pc80/vga_io.h> #include <framebuffer_info.h> -static int width = CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_XRES; -static int height = CONFIG_DRIVERS_EMULATION_QEMU_BOCHS_YRES; +static int width = CONFIG_DRIVERS_EMULATION_QEMU_XRES; +static int height = CONFIG_DRIVERS_EMULATION_QEMU_YRES; static u32 addr = 0; enum -- To view, visit https://review.coreboot.org/c/coreboot/+/82060?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: Ie0a5e75731231bb768d7728867196c9ab5c53a00 Gerrit-Change-Number: 82060 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: mainboard/qemu-aarch64: Set CONFIG_PCI_IOBASE to 0x3eff0000
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/+/82059?usp=email ) Change subject: mainboard/qemu-aarch64: Set CONFIG_PCI_IOBASE to 0x3eff0000 ...................................................................... mainboard/qemu-aarch64: Set CONFIG_PCI_IOBASE to 0x3eff0000 Define the PCI I/O base address necessary to use port I/O functions on the qemu-aarch64 mainboard, so that we can get the VGA display devices working. The config value is from hw/arm/virt.c [1]: [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, [1] https://gitlab.com/qemu-project/qemu/-/blob/v8.2.3/hw/arm/virt.c#L164 Change-Id: I85439ba68740d64f789983b37d9c95f849ce4f72 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/mainboard/emulation/qemu-aarch64/Kconfig 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/82059/1 diff --git a/src/mainboard/emulation/qemu-aarch64/Kconfig b/src/mainboard/emulation/qemu-aarch64/Kconfig index dcb4f23..af689ba 100644 --- a/src/mainboard/emulation/qemu-aarch64/Kconfig +++ b/src/mainboard/emulation/qemu-aarch64/Kconfig @@ -29,6 +29,9 @@ config ECAM_MMCONF_BUS_NUMBER default 256 +config PCI_IOBASE + default 0x3eff0000 + config MEMLAYOUT_LD_FILE string default "src/mainboard/emulation/qemu-aarch64/memlayout.ld" -- To view, visit https://review.coreboot.org/c/coreboot/+/82059?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: I85439ba68740d64f789983b37d9c95f849ce4f72 Gerrit-Change-Number: 82059 Gerrit-PatchSet: 1 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newchange

1 0

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

24 Apr '24

Attention is currently required from: Alper Nebi Yasak. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80376?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: drivers/qemu/bochs: Allow building for non-x86 architectures ...................................................................... drivers/qemu/bochs: Allow building for non-x86 architectures The Bochs display driver uses port I/O functions to initialize the VGA device, so it could only have been built on x86 architectures so far, but its supported devices can be used just fine on others on the QEMU side as long as the emulated platform supports PCI. A previous commit adds port I/O functions for more including ARM* and RISC-V, which should enable this driver to be successfully built and used on these as well. Allow the Bochs 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 architectures. 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: I7f72d7ea13e54ecf89d067394c02b572c5f92d24 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/76/80376/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80376?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: I7f72d7ea13e54ecf89d067394c02b572c5f92d24 Gerrit-Change-Number: 80376 Gerrit-PatchSet: 2 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newpatchset

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: Alper Nebi Yasak, Arthur Heymans, Nico Huber. Hello Nico Huber, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80375?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Nico Huber, Verified+1 by build bot (Jenkins) Change subject: drivers/qemu: Split Cirrus display support from Bochs display support ...................................................................... drivers/qemu: Split Cirrus display support from Bochs display support QEMU's Cirrus display device is supported along with the Bochs driver since commit 7905f9254ebc ("qemu: cirrus native video init"). It is no longer the default since QEMU 2.2. The code supporting it can work independently of the Bochs display driver and depends more heavily on port I/O and VGA support code, so split it from that code to make it easier to support the Bochs driver in other architectures. Change-Id: Ic9492b501ed4fdcbda6886db60b1e5348715e667 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- M src/drivers/emulation/qemu/Kconfig M src/drivers/emulation/qemu/Makefile.mk 2 files changed, 15 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/80375/2 -- 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-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: arch/io.h: Add port I/O functions to other architectures
by Alper Nebi Yasak (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Arthur Heymans, Julius Werner, ron minnich. Hello Arthur Heymans, Julius Werner, Nico Huber, Philipp Hug, build bot (Jenkins), ron minnich, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80372?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/io.h: Add port I/O functions to other architectures ...................................................................... arch/io.h: Add port I/O functions to other architectures The QEMU Bochs display driver and the QEMU Firmware Configuration interface code (in the qemu-i440fx mainboard dir) were written for x86. These devices are available in QEMU VMs of other architectures as well, so we want to port them to be independent from x86. The main problem is that the drivers use x86 port I/O functions to communicate with devices over PCI I/O space. These are currently not available for ARM* and RISC-V, although it is often still possible to access PCI I/O ports over MMIO through a translator. Add implementations of port I/O functions that work with PCI I/O space on these architectures as well, assuming there is such a translator at a known address configured at build-time. Change-Id: If7d9177283e8c692088ba8e30d6dfe52623c8cb9 Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> --- A src/arch/arm/include/armv4/arch/io.h A src/arch/arm/include/armv7/arch/io.h A src/arch/arm64/include/armv8/arch/io.h A src/arch/riscv/include/arch/io.h M src/device/Kconfig 5 files changed, 398 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/80372/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/80372?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: If7d9177283e8c692088ba8e30d6dfe52623c8cb9 Gerrit-Change-Number: 80372 Gerrit-PatchSet: 5 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: vc/google/chromeos: Move RAMOOPS region creation to BS_DEV_INIT_CHIPS
by Jamie Ryu (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Hannah Williams, Kapil Porwal, Wonkyu Kim. Jamie Ryu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82042?usp=email ) Change subject: vc/google/chromeos: Move RAMOOPS region creation to BS_DEV_INIT_CHIPS ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/82042?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: Ibae06362cd80eacb16f6cf0eed8c9aa1fbfb2535 Gerrit-Change-Number: 82042 Gerrit-PatchSet: 5 Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Wed, 24 Apr 2024 20:47:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: vc/google/chromeos: Move RAMOOPS region creation to BS_DEV_INIT_CHIPS
by Eran Mitrani (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Hannah Williams, Jamie Ryu, Kapil Porwal, Wonkyu Kim. Eran Mitrani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82042?usp=email ) Change subject: vc/google/chromeos: Move RAMOOPS region creation to BS_DEV_INIT_CHIPS ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82042?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: Ibae06362cd80eacb16f6cf0eed8c9aa1fbfb2535 Gerrit-Change-Number: 82042 Gerrit-PatchSet: 5 Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Wed, 24 Apr 2024 20:26:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: arch/arm64: Implement initial set of SMBIOS tables
by Julius Werner (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Arthur Heymans, Benjamin Doron, David Milosevic, Lean Sheng Tan, Martin L Roth, Maximilian Brune. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78285?usp=email ) Change subject: arch/arm64: Implement initial set of SMBIOS tables ...................................................................... Patch Set 15: Code-Review+2 (2 comments) Patchset: PS15: LGTM after the points Arthur mentioned. File src/arch/arm64/smbios.c: https://review.coreboot.org/c/coreboot/+/78285/comment/d5437765_d4578fe8 : PS15, Line 74: t->core_count = MAX_CPUS_ENABLED(t->core_count2); : t->thread_count = MAX_CPUS_ENABLED(t->thread_count2); > SMP is now a thing in ARM right? Maybe the SOC should provide this info? Maybe add a TODO comment? I think(?) that's what's happening here now. `smbios_cpu_get_core_counts()` will need to be implemented by each SoC or mainboard to provide those counts. -- To view, visit https://review.coreboot.org/c/coreboot/+/78285?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: Iedae0f26f168bd6d3af866e35d9d39ddb01abc15 Gerrit-Change-Number: 78285 Gerrit-PatchSet: 15 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Wed, 24 Apr 2024 20:10:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: arch/arm64: Support calling a trusted monitor
by Julius Werner (Code Review) 24 Apr '24

24 Apr '24

Attention is currently required from: Arthur Heymans, Benjamin Doron, David Milosevic. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78284?usp=email ) Change subject: arch/arm64: Support calling a trusted monitor ...................................................................... Patch Set 15: (2 comments) File src/arch/arm64/smc.c: https://review.coreboot.org/c/coreboot/+/78284/comment/d6343a3a_aa2c0355 : PS15, Line 9: inline > I don't think inline works without static: You want it to be exported at the compilation module leve […] I don't actually see the point of this function at all. It's not different from `smc()`. Anyone who calls this can just call `smc()` directly. Instead, I think this (`smc_call()`) should be removed and the above prototype for `smc()` (minus the `extern`) should be moved into `arch/smc.h` instead. https://review.coreboot.org/c/coreboot/+/78284/comment/7294616d_bebd55e0 : PS15, Line 15: inline _This_ is a function that, like Arthur says, should also be `static` in addition to `inline`. This doesn't need to be output into the binary by itself because it is not going to get called by assembly. It just exists in C to make it easier to write `smc()` calls with less arguments. This should probably also go in `arch/smc.h` instead so that other C code outside of this file could use it if they want to. -- To view, visit https://review.coreboot.org/c/coreboot/+/78284?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: I158db0b971aba722b3995d52162146aa406d1644 Gerrit-Change-Number: 78284 Gerrit-PatchSet: 15 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Wed, 24 Apr 2024 19:55:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024