coreboot-gerrit May 2024

coreboot-gerrit@coreboot.org

1 participants
3267 discussions

[XS] Change in coreboot[main]: soc/mediatek/mt8188: Decrease OP-TEE image size from 80 MB to 70 MB
by Gavin Liu (Code Review) 24 May '24

24 May '24

Attention is currently required from: Gavin Liu, Hung-Te Lin, Yidi Lin, Yu-Ping Wu. Hello Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82634?usp=email to look at the new patch set (#3). Change subject: soc/mediatek/mt8188: Decrease OP-TEE image size from 80 MB to 70 MB ...................................................................... soc/mediatek/mt8188: Decrease OP-TEE image size from 80 MB to 70 MB The secure buffer shrank from 42 MB to 32 MB, decreasing the total OP-TEE image size from 80 MB to 70 MB. BUG=b:246837563 TEST=emerge-geralt coreboot build coreboot and verify SVP works well Change-Id: I6729e65f83ef994fe59b5bd4ed098e6d3a847695 Signed-off-by: Gavin Liu <gavin.liu(a)mediatek.corp-partner.google.com> --- M src/soc/mediatek/mt8188/soc.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/82634/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/82634?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6729e65f83ef994fe59b5bd4ed098e6d3a847695 Gerrit-Change-Number: 82634 Gerrit-PatchSet: 3 Gerrit-Owner: Gavin Liu <gavin.liu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Gavin Liu <gavin.liu(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/brya/variants/trulo: Support OCP fault on A0/1 ports
by Subrata Banik (Code Review) 24 May '24

24 May '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Pranava Y N. Subrata Banik has posted comments on this change by Pranava Y N. ( https://review.coreboot.org/c/coreboot/+/82637?usp=email ) Change subject: mb/google/brya/variants/trulo: Support OCP fault on A0/1 ports ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/82637/comment/75541c5a_cb9cb4fd?us… : PS1, Line 14: No Signed-off-by line in commit message File src/mainboard/google/brya/variants/baseboard/trulo/devicetree.cb: https://review.coreboot.org/c/coreboot/+/82637/comment/df2a04ab_fb91b550?us… : PS1, Line 3: register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC1)" # USB3/2 Type A port A0 : register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC2)" # USB3/2 Type A port A1 : this should be added in the variant devicetree IMO as each variant could have different HW design -- To view, visit https://review.coreboot.org/c/coreboot/+/82637?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic17debc5eecebca8c000c43a660e1b52d2932f2a Gerrit-Change-Number: 82637 Gerrit-PatchSet: 1 Gerrit-Owner: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Fri, 24 May 2024 08:54:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: mb/google/brya/variants/trulo: Support OCP fault on A0/1 ports
by Pranava Y N (Code Review) 24 May '24

24 May '24

Pranava Y N has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82637?usp=email ) Change subject: mb/google/brya/variants/trulo: Support OCP fault on A0/1 ports ...................................................................... mb/google/brya/variants/trulo: Support OCP fault on A0/1 ports The devicetree entry and gpio.c updated to map the OC fault signal from A0/A1 ports in Trulo. BUG=b:335838378 TEST= NA Change-Id: Ic17debc5eecebca8c000c43a660e1b52d2932f2a --- M src/mainboard/google/brya/variants/baseboard/trulo/devicetree.cb M src/mainboard/google/brya/variants/trulo/gpio.c 2 files changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/82637/1 diff --git a/src/mainboard/google/brya/variants/baseboard/trulo/devicetree.cb b/src/mainboard/google/brya/variants/baseboard/trulo/devicetree.cb index a5e2217..c6ef3b3 100644 --- a/src/mainboard/google/brya/variants/baseboard/trulo/devicetree.cb +++ b/src/mainboard/google/brya/variants/baseboard/trulo/devicetree.cb @@ -1,4 +1,8 @@ chip soc/intel/alderlake + + register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC1)" # USB3/2 Type A port A0 + register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC2)" # USB3/2 Type A port A1 + device domain 0 on end end diff --git a/src/mainboard/google/brya/variants/trulo/gpio.c b/src/mainboard/google/brya/variants/trulo/gpio.c index beee6fc..1a6d1b1 100644 --- a/src/mainboard/google/brya/variants/trulo/gpio.c +++ b/src/mainboard/google/brya/variants/trulo/gpio.c @@ -8,7 +8,10 @@ /* Pad configuration in ramstage */ static const struct pad_config gpio_table[] = { - /* TODO */ + /* A14 : USB_OC1# ==> USB_A0_FAULT_ODL */ + PAD_CFG_NF_LOCK(GPP_A14, NONE, NF1, LOCK_CONFIG), + /* A15 : USB_OC2# ==> USB_A1_FAULT_ODL */ + PAD_CFG_NF_LOCK(GPP_A15, NONE, NF1, LOCK_CONFIG), }; /* Early pad configuration in bootblock */ -- To view, visit https://review.coreboot.org/c/coreboot/+/82637?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic17debc5eecebca8c000c43a660e1b52d2932f2a Gerrit-Change-Number: 82637 Gerrit-PatchSet: 1 Gerrit-Owner: Pranava Y N <pranavayn(a)google.com>

1 0

[S] Change in coreboot[main]: libpayload: Inject head.S into libc, remove separate class
by Subrata Banik (Code Review) 24 May '24

24 May '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82478?usp=email ) ( 9 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: libpayload: Inject head.S into libc, remove separate class ...................................................................... libpayload: Inject head.S into libc, remove separate class Integrate head.S directly into libc and remove all instances of head.o. * Drop 'separate class' entry for head.S. * Drop special treament for head.o inside lpgcc. * Change the .text in `x86/head.S` to `.section .text._entry`. * Drop arch/mock/head.c, initially added as a dummy file. Change-Id: I156d781908fcc38d455bbf9f2c29e5ab95c7775a Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82478 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Julius Werner <jwerner(a)chromium.org> --- M payloads/libpayload/Makefile.mk M payloads/libpayload/arch/arm/Makefile.mk M payloads/libpayload/arch/arm64/Makefile.mk M payloads/libpayload/arch/mock/Makefile.mk D payloads/libpayload/arch/mock/head.c M payloads/libpayload/arch/x86/Makefile.mk M payloads/libpayload/arch/x86/head.S M payloads/libpayload/bin/lpgcc 8 files changed, 8 insertions(+), 34 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved diff --git a/payloads/libpayload/Makefile.mk b/payloads/libpayload/Makefile.mk index afaa06d..d654995 100644 --- a/payloads/libpayload/Makefile.mk +++ b/payloads/libpayload/Makefile.mk @@ -56,7 +56,6 @@ classes-$(CONFIG_LP_VBOOT_LIB) += vboot_fw classes-$(CONFIG_LP_VBOOT_LIB) += tlcl libraries := $(classes-y) -classes-y += head.o subdirs-y := arch/$(ARCHDIR-y) subdirs-y += crypto libc drivers libpci gdb @@ -97,7 +96,7 @@ cmp $@ $< 2>/dev/null || cp $< $@ library-targets = $(addsuffix .a,$(addprefix $(obj)/,$(libraries))) $(obj)/libpayload.a -lib: $$(library-targets) $(obj)/head.o +lib: $$(library-targets) extract_nth=$(word $(1), $(subst |, ,$(2))) @@ -116,17 +115,12 @@ printf " AR $(subst $(CURDIR)/,,$(@))\n" printf "create $@\n$(foreach objc,$(filter-out %.a,$^),addmod $(objc)\n)$(foreach lib,$(filter %.a,$^),addlib $(lib)\n)save\nend\n" | $(AR) -M -$(obj)/head.o: $(obj)/arch/$(ARCHDIR-y)/head.head.o.o - printf " CP $(subst $(CURDIR)/,,$(@))\n" - cp $^ $@ - install: real-target printf " INSTALL $(DESTDIR)/libpayload/lib\n" install -m 755 -d $(DESTDIR)/libpayload/lib install -m 644 $(library-targets) $(DESTDIR)/libpayload/lib/ install -m 644 arch/$(ARCHDIR-y)/libpayload.ldscript $(DESTDIR)/libpayload/lib/ install -m 755 -d $(DESTDIR)/libpayload/lib/$(ARCHDIR-y) - install -m 644 $(obj)/head.o $(DESTDIR)/libpayload/lib/$(ARCHDIR-y) printf " INSTALL $(DESTDIR)/libpayload/include\n" install -m 755 -d $(DESTDIR)/libpayload/include find include -type d -exec install -m755 -d $(DESTDIR)/libpayload/{} \; diff --git a/payloads/libpayload/arch/arm/Makefile.mk b/payloads/libpayload/arch/arm/Makefile.mk index f71138b..e6574e7 100644 --- a/payloads/libpayload/arch/arm/Makefile.mk +++ b/payloads/libpayload/arch/arm/Makefile.mk @@ -29,7 +29,7 @@ CFLAGS += -mthumb -march=armv7-a arm_asm_flags = -Wa,-mthumb -Wa,-mimplicit-it=always -Wa,-mno-warn-deprecated -head.o-y += head.S +libc-y += head.S libc-y += eabi_compat.c libc-y += main.c sysinfo.c libc-y += timer.c coreboot.c util.S @@ -44,5 +44,4 @@ libgdb-y += gdb.c # Add other classes here when you put assembly files into them! -head.o-S-ccopts += $(arm_asm_flags) libc-S-ccopts += $(arm_asm_flags) diff --git a/payloads/libpayload/arch/arm64/Makefile.mk b/payloads/libpayload/arch/arm64/Makefile.mk index b478c77..d6cc51e 100644 --- a/payloads/libpayload/arch/arm64/Makefile.mk +++ b/payloads/libpayload/arch/arm64/Makefile.mk @@ -29,7 +29,7 @@ CFLAGS += -march=armv8-a arm64_asm_flags = -head.o-y += head.S +libc-y += head.S libc-y += main.c sysinfo.c libc-y += timer.c coreboot.c util.S libc-y += virtual.c @@ -42,5 +42,4 @@ libgdb-y += gdb.c # Add other classes here when you put assembly files into them! - head.o-S-ccopts += $(arm64_asm_flags) - libc-S-ccopts += $(arm64_asm_flags) +libc-S-ccopts += $(arm64_asm_flags) diff --git a/payloads/libpayload/arch/mock/Makefile.mk b/payloads/libpayload/arch/mock/Makefile.mk index e3f10a6..0b418f0 100644 --- a/payloads/libpayload/arch/mock/Makefile.mk +++ b/payloads/libpayload/arch/mock/Makefile.mk @@ -1,7 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -head.o-y += head.c - libc-y += virtual.c CFLAGS += -Wno-address-of-packed-member diff --git a/payloads/libpayload/arch/mock/head.c b/payloads/libpayload/arch/mock/head.c deleted file mode 100644 index 55a691f..0000000 --- a/payloads/libpayload/arch/mock/head.c +++ /dev/null @@ -1,3 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -/* This file is empty on purpose. It should not be used. */ diff --git a/payloads/libpayload/arch/x86/Makefile.mk b/payloads/libpayload/arch/x86/Makefile.mk index 196415d..d2c33b4 100644 --- a/payloads/libpayload/arch/x86/Makefile.mk +++ b/payloads/libpayload/arch/x86/Makefile.mk @@ -30,7 +30,7 @@ CFLAGS += -mpreferred-stack-boundary=2 endif -head.o-y += head.S +libc-y += head.S libc-y += main.c sysinfo.c libc-y += timer.c coreboot.c util.S libc-y += exec.S virtual.c diff --git a/payloads/libpayload/arch/x86/head.S b/payloads/libpayload/arch/x86/head.S index 99842cd..26ba901 100644 --- a/payloads/libpayload/arch/x86/head.S +++ b/payloads/libpayload/arch/x86/head.S @@ -29,7 +29,7 @@ .code32 .global _entry - .text + .section .text._entry .align 4 /* diff --git a/payloads/libpayload/bin/lpgcc b/payloads/libpayload/bin/lpgcc index 724f5ec..a954d4f 100755 --- a/payloads/libpayload/bin/lpgcc +++ b/payloads/libpayload/bin/lpgcc @@ -201,14 +201,6 @@ $DEFAULT_CC $CMDLINE $_CFLAGS else _LIBGCC=`$DEFAULT_CC $_ARCHEXTRA -print-libgcc-file-name` - if [ -f $_ARCHLIBDIR/head.o ]; then - HEAD_O=$_ARCHLIBDIR/head.o - elif [ -f $_OBJ/head.o ]; then - HEAD_O=$_OBJ/head.o - else - echo "Could not find head.o" - exit 1 - fi if [ "$CONFIG_LP_ARM64_A53_ERRATUM_843419" = y ] && grep -q fix-cortex-a53-843419 $_XCOMPILE; then @@ -220,13 +212,8 @@ _LDFLAGS="$_LDFLAGS -Wl,--defsym=CONFIG_LP_STACK_SIZE=$CONFIG_LP_STACK_SIZE" if [ $DEBUGME -eq 1 ]; then - echo "$DEFAULT_CC $_LDFLAGS $HEAD_O $CMDLINE $_CFLAGS -lpayload $_LIBGCC" + echo "$DEFAULT_CC $_LDFLAGS $CMDLINE $_CFLAGS -lpayload $_LIBGCC" fi - # Note: $_ARCHLIBDIR/head.o must be the first object being linked, because it - # contains a Multiboot header. The Multiboot standard requires this - # header to be placed below 0x2000 in the resulting image. See: - # http://www.gnu.org/software/grub/manual/multiboot/html_node/OS-image-format… - - $DEFAULT_CC $_LDFLAGS $HEAD_O $CMDLINE $_CFLAGS -lpayload -xnone $_LIBGCC + $DEFAULT_CC $_LDFLAGS $CMDLINE $_CFLAGS -lpayload -xnone $_LIBGCC fi -- To view, visit https://review.coreboot.org/c/coreboot/+/82478?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I156d781908fcc38d455bbf9f2c29e5ab95c7775a Gerrit-Change-Number: 82478 Gerrit-PatchSet: 16 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: libpayload: x86: Move Multiboot header to include file
by Subrata Banik (Code Review) 24 May '24

24 May '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82533?usp=email ) Change subject: libpayload: x86: Move Multiboot header to include file ...................................................................... libpayload: x86: Move Multiboot header to include file This moves the multiboot header into its own include file, simplifying head.S and making it easier to include/exclude the multiboot header based on config options. BUG=b:242829490 TEST=Able to build and boot google/rex. Change-Id: I59a22dfe36044b4dd64a5b028a134be7a7d02a48 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82533 Reviewed-by: Julius Werner <jwerner(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M payloads/libpayload/arch/x86/head.S A payloads/libpayload/arch/x86/multiboot_header.inc 2 files changed, 58 insertions(+), 27 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved diff --git a/payloads/libpayload/arch/x86/head.S b/payloads/libpayload/arch/x86/head.S index 2bac700..99842cd 100644 --- a/payloads/libpayload/arch/x86/head.S +++ b/payloads/libpayload/arch/x86/head.S @@ -38,37 +38,14 @@ * change anything. */ _entry: - jmp _init - .align 4 - -#define MB_MAGIC 0x1BADB002 -#define MB_FLAGS 0x00010003 - -mb_header: - .long MB_MAGIC - .long MB_FLAGS - .long -(MB_MAGIC + MB_FLAGS) - .long mb_header - .long _start - .long _edata - .long _end - .long _init - -/* - * This function saves off the previous stack and switches us to our - * own execution environment. - */ -_init: + /* Add multiboot header and jump around it when building with multiboot support. */ +#if CONFIG(LP_MULTIBOOT) + #include "multiboot_header.inc" +#endif /* No interrupts, please. */ cli -#if CONFIG(LP_MULTIBOOT) - /* Store EAX and EBX */ - movl %eax, loader_eax - movl %ebx, loader_ebx -#endif - /* save pointer to coreboot tables */ movl 4(%esp), %eax movl %eax, cb_header_ptr diff --git a/payloads/libpayload/arch/x86/multiboot_header.inc b/payloads/libpayload/arch/x86/multiboot_header.inc new file mode 100644 index 0000000..e321dc4 --- /dev/null +++ b/payloads/libpayload/arch/x86/multiboot_header.inc @@ -0,0 +1,54 @@ +/* + * + * Copyright (C) 2008 Advanced Micro Devices, Inc. + * Copyright (C) 2017 Patrick Rudolph <siro(a)das-labor.org> + * Copyright (C) 2024 Google Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#define MB_MAGIC 0x1BADB002 +#define MB_FLAGS 0x00010003 + + jmp _init + +/* + * Note: The Multiboot standard requires Multiboot header to be placed + * below 0x2000 in the resulting image. See: + * http://www.gnu.org/software/grub/manual/multiboot/html_node/OS-image-format… + */ +mb_header: + .long MB_MAGIC + .long MB_FLAGS + .long -(MB_MAGIC + MB_FLAGS) + .long mb_header + .long _start + .long _edata + .long _end + .long _init + +_init: + /* Store EAX and EBX */ + movl %eax, loader_eax + movl %ebx, loader_ebx -- To view, visit https://review.coreboot.org/c/coreboot/+/82533?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I59a22dfe36044b4dd64a5b028a134be7a7d02a48 Gerrit-Change-Number: 82533 Gerrit-PatchSet: 11 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[L] Change in coreboot[main]: libpayload: Add x86_64 (64-bit) support
by Subrata Banik (Code Review) 24 May '24

24 May '24

1 0

[L] Change in coreboot[main]: libpayload: Add x86_64 (64-bit) support
by Subrata Banik (Code Review) 24 May '24

24 May '24

Attention is currently required from: Arthur Heymans, Julius Werner, Kapil Porwal. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/81968?usp=email ) Change subject: libpayload: Add x86_64 (64-bit) support ...................................................................... Patch Set 74: (9 comments) File payloads/libpayload/arch/x86/head_64.S: https://review.coreboot.org/c/coreboot/+/81968/comment/22ba63cc_14e901f9?us… : PS74, Line 38: * Payload Entry Point Behavior with below code. > This table is okay but I think there should also be a more explicit warning that this code needs to […] Acknowledged https://review.coreboot.org/c/coreboot/+/81968/comment/91db6ef5_3d576609?us… : PS74, Line 41: LP_ARCH_X86_64 > This column seems pointless. Acknowledged https://review.coreboot.org/c/coreboot/+/81968/comment/6abd2a75_c3ac8662?us… : PS74, Line 121: > What about `fninit`? initially I thought we don't need this code because modern 64-bit processors automatically initialize the FPU and configure the necessary control bits upon entering 64-bit mode. However, this code now may executed while transitioning from either a 32-bit coreboot and 64-bit coreboot hence, make sense to keep those logic here as well File payloads/libpayload/arch/x86/pt.S: https://review.coreboot.org/c/coreboot/+/81968/comment/773a4bd2_796c691d?us… : PS74, Line 51: .global pde_table > I think these names are problematic now because you're using `pde_table` for the PDE table in the 2M […] Acknowledged https://review.coreboot.org/c/coreboot/+/81968/comment/ab401341_931ad161?us… : PS74, Line 65: .type init_page_table, @function > Should be a similar warning here, I'd suggest: […] Acknowledged https://review.coreboot.org/c/coreboot/+/81968/comment/43f48856_c1e77602?us… : PS74, Line 75: mov $(_PRES + _RW + _US + _A + _D), %eax > I still don't understand why you're not setting `_PS` here tbh. Am I misunderstanding how this works? Table 4-17 "Format of an IA-32e Page-Directory Entry that Maps a 2-MByte Page" says "7 (PS) | Page size; must be 1 (otherwise, this entry references a page table; see Table 4-18)". you are right, _PS is needed. I thought I added that but looking at upstream, it might have lost in patch trail > > Did you test this code (commenting out the `jnz` above to force it to run) and confirm that it works this way? yes, i have tested this flow as well where 1GB paging is not supported https://review.coreboot.org/c/coreboot/+/81968/comment/5c8c1773_f9f2d200?us… : PS74, Line 82: mov %ebx, 4(%rsi, %rcx, 8) > Actually, the same argument as below also applies here: we're only writing 2MB tables for the low 4G […] Acknowledged https://review.coreboot.org/c/coreboot/+/81968/comment/4b483ecc_6966366a?us… : PS74, Line 104: cmp %edx, %eax > Sorry, I'm very confused about what you're doing here and what you're using EDX for (instead of EAX like with the other versions of this loop). What's the point of adding EDX to EAX and then comparing with EAX? You're not initializing EAX anywhere so it would still point to the end of the PDE table from the previous loop (and then you're adding more to it)? > > In general, the point of this overflow check is just to see if we need to increment EBX because EAX overflowed over the 4GB limit. In this case, since we know that EAX just walks the PDE table and libpayload (including the entire PDE table in BSS) is linked below 4GB, we can actually skip that entire check because we know that it will never cross that boundary (and EBX will never need to be anything other than 0). So in fact I think we can simplify this entire thing to: > ``` > mov $4, %edi > mov $(_PRES + _RW + _US + _A + pde_table), %eax > mov $0, %ebx > mov $0, %ecx > mov $pdpe_table, %esi > > fill_pdpe: > mov %eax, (%rsi, %rcx, 8) > mov $0, 4(%rsi, rcx, 8) > add $4096, %eax > inc %ecx > cmp %edi, %ecx > jb fill_pdpe > ``` > (I don't know if the linker supports a relocation for a construct like `mov $(_PRES + _RW + _US + _A + pde_table), %eax`. If not, then just do `mov $pde_table, %eax` and `add $(_PRES + _RW + _US + _A), %eax`.) you are right and if u follow the previous patch set, I had added that initially then may be while adding 64-bit API, I got confused that I had to handle the overflow but true this is <4GB table creation. https://review.coreboot.org/c/coreboot/+/81968/comment/55d91030_02bf4b90?us… : PS74, Line 116: jmp leave > nit: Easier to just do `ret` here? Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/81968?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I69fda47bedf1a14807b1515c4aed6e3a1d5b8585 Gerrit-Change-Number: 81968 Gerrit-PatchSet: 74 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 24 May 2024 08:08:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: libpayload/arm64: Support FEAT_CCIDX
by Yidi Lin (Code Review) 24 May '24

24 May '24

Yidi Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82636?usp=email ) Change subject: libpayload/arm64: Support FEAT_CCIDX ...................................................................... libpayload/arm64: Support FEAT_CCIDX ARM SoC supports FEAT_CCIDX after ARMv8.3. The register field description of CCSIDR_EL1 is different when FEAT_CCIDX is implemented. If numsets and associativity from CCSIDR_EL1 are not correct, the system would hang during mmu_disable(). Rather than assuming that FEAT_CCIDX is not implemented, this patch adds a check to dcache_apply_all to use the right register format. Reference: - https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/12770 BUG=b:317015456 TEST=mmu_disable works on the FEAT_CCIDX supported SoC. Change-Id: I892009890f6ae889e87c877ffffd76a33d1dc789 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M payloads/libpayload/arch/arm64/cpu.S 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/82636/1 diff --git a/payloads/libpayload/arch/arm64/cpu.S b/payloads/libpayload/arch/arm64/cpu.S index 70a1044..1b54df5 100644 --- a/payloads/libpayload/arch/arm64/cpu.S +++ b/payloads/libpayload/arch/arm64/cpu.S @@ -41,6 +41,9 @@ mov w10, #0 // w10 = 2 * cache level mov w8, #1 // w8 = constant 0b1 + mrs x12, id_aa64mmfr2_el1 // read ID_AA64MMFR2_EL1 + ubfx x12, x12, #20, #4 // [23:20] - CCIDX support + 1: //next_level add w2, w10, w10, lsr #1 // calculate 3 * cache level lsr w1, w0, w2 // extract 3-bit cache type for this level @@ -52,8 +55,14 @@ mrs x1, ccsidr_el1 // w1 = read ccsidr and w2, w1, #7 // w2 = log2(linelen_bytes) - 4 add w2, w2, #4 // w2 = log2(linelen_bytes) - ubfx w4, w1, #3, #10 // w4 = associativity - 1 (also - // max way number) + + cbz x12, 11f // check FEAT_CCIDX for associativity + // branch to 11 if FEAT_CCIDX is not implemented + ubfx x4, x1, #3, #21 // x4 = associativity CCSIDR_EL1[23:3] + b 12f +11: + ubfx x4, x1, #3, #10 // x4 = associativity CCSIDR_EL1[12:3] +12: clz w5, w4 // w5 = 32 - log2(ways) // (bit position of way in DC) lsl w9, w4, w5 // w9 = max way number @@ -61,7 +70,13 @@ lsl w16, w8, w5 // w16 = amount to decrement (way // number per iteration) 2: //next_way - ubfx w7, w1, #13, #15 // w7 = max set #, right aligned + cbz x12, 21f // check FEAT_CCIDX for numsets + // branch to 21 if FEAT_CCIDX is not implemented + ubfx x7, x1, #32, #24 // x7(w7) = numsets CCSIDR_EL1[55:32] + b 22f +21: + ubfx w7, w1, #13, #15 // w7 = numsets CCSIDR_EL1[27:13] +22: lsl w7, w7, w2 // w7 = max set #, DC aligned lsl w17, w8, w2 // w17 = amount to decrement (set // number per iteration) -- To view, visit https://review.coreboot.org/c/coreboot/+/82636?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I892009890f6ae889e87c877ffffd76a33d1dc789 Gerrit-Change-Number: 82636 Gerrit-PatchSet: 1 Gerrit-Owner: Yidi Lin <yidilin(a)google.com>

1 0

[S] Change in coreboot[main]: arch/arm64: Support FEAT_CCIDX
by Yidi Lin (Code Review) 24 May '24

24 May '24

Yidi Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82635?usp=email ) Change subject: arch/arm64: Support FEAT_CCIDX ...................................................................... arch/arm64: Support FEAT_CCIDX ARM SoC supports FEAT_CCIDX after ARMv8.3. The register field description of CCSIDR_EL1 is different when FEAT_CCIDX is implemented. If numsets and associativity from CCSIDR_EL1 are not correct, the system would hang during mmu_disable(). Rather than assuming that FEAT_CCIDX is not implemented, this patch adds a check to dcache_apply_all to use the right register format. Reference: - https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/12770 BUG=b:317015456 TEST=mmu_disable works on the FEAT_CCIDX supported SoC. Change-Id: Ieadd0d9dfb8911039b3d36c9419af4ae04ed814c Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M src/arch/arm64/armv8/cpu.S 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/82635/1 diff --git a/src/arch/arm64/armv8/cpu.S b/src/arch/arm64/armv8/cpu.S index a40ee64..ff13cf6 100644 --- a/src/arch/arm64/armv8/cpu.S +++ b/src/arch/arm64/armv8/cpu.S @@ -16,6 +16,9 @@ mov w10, #0 // w10 = 2 * cache level mov w8, #1 // w8 = constant 0b1 + mrs x12, id_aa64mmfr2_el1 // read ID_AA64MMFR2_EL1 + ubfx x12, x12, #20, #4 // [23:20] - CCIDX support + 1: //next_level add w2, w10, w10, lsr #1 // calculate 3 * cache level lsr w1, w0, w2 // extract 3-bit cache type for this level @@ -27,8 +30,14 @@ mrs x1, ccsidr_el1 // w1 = read ccsidr and w2, w1, #7 // w2 = log2(linelen_bytes) - 4 add w2, w2, #4 // w2 = log2(linelen_bytes) - ubfx w4, w1, #3, #10 // w4 = associativity - 1 (also - // max way number) + + cbz x12, 11f // check FEAT_CCIDX for associativity + // branch to 11 if FEAT_CCIDX is not implemented + ubfx x4, x1, #3, #21 // x4 = associativity CCSIDR_EL1[23:3] + b 12f +11: + ubfx x4, x1, #3, #10 // x4 = associativity CCSIDR_EL1[12:3] +12: clz w5, w4 // w5 = 32 - log2(ways) // (bit position of way in DC) lsl w9, w4, w5 // w9 = max way number @@ -36,7 +45,13 @@ lsl w16, w8, w5 // w16 = amount to decrement (way // number per iteration) 2: //next_way - ubfx w7, w1, #13, #15 // w7 = max set #, right aligned + cbz x12, 21f // check FEAT_CCIDX for numsets + // branch to 21 if FEAT_CCIDX is not implemented + ubfx x7, x1, #32, #24 // x7(w7) = numsets CCSIDR_EL1[55:32] + b 22f +21: + ubfx w7, w1, #13, #15 // w7 = numsets CCSIDR_EL1[27:13] +22: lsl w7, w7, w2 // w7 = max set #, DC aligned lsl w17, w8, w2 // w17 = amount to decrement (set // number per iteration) -- To view, visit https://review.coreboot.org/c/coreboot/+/82635?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ieadd0d9dfb8911039b3d36c9419af4ae04ed814c Gerrit-Change-Number: 82635 Gerrit-PatchSet: 1 Gerrit-Owner: Yidi Lin <yidilin(a)google.com>

1 0

[M] Change in coreboot[main]: mb/google/ovis/var/deku: Set PsysPL2 value to 178W
by Sumeet R Pawnikar (Code Review) 24 May '24

24 May '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun, Tony Huang. Sumeet R Pawnikar has posted comments on this change by Tony Huang. ( https://review.coreboot.org/c/coreboot/+/82200?usp=email ) Change subject: mb/google/ovis/var/deku: Set PsysPL2 value to 178W ...................................................................... Patch Set 33: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82200?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9ce3a8f843a87e81d404778aaf250b876b6801eb Gerrit-Change-Number: 82200 Gerrit-PatchSet: 33 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Robert Chen <robert.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 24 May 2024 07:45:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2024