[v6 0/12] CI: Set up for an arm64 runner

[v6 0/12] CI: Set up for an arm64 runner

[Tom Rini]

Hey all,

This is picking up Simon's v5 of the above-named series and making a few
more changes so that the follow-up series I have leads to arm64 being
supported for almost all jobs. To quote Simon's cover letter:

All gitlab runners are currently amd64 machines. This series attempts to
create a docker image which can also support arm64 so that sandbox tests
can be run on it.

The TARGET_... environment variables for grub could perhaps be adjusted,
using the new variables, but I have not done that for now.

Adding to what Simon said, we now build grub for all architectures as
the reason to install it was to be able to use the binaries in QEMU.
That won't provide us with amd64 binaries on arm64 hosts so we can't use
that shortcut anymore.

Changes in v6:
- Re-introduce patches dropped in v4
- Add x86_64 toolchain
- Move to grub-2.12 and then build that (with -j$(nproc)) for all
  architectures

Changes in v5:
- Drop the comment in the gitlab file
- Add back a final missing sudo

Changes in v4:
- Leave the 'sudo' in

Changes in v3:
- Move ARGs to the top
- Revise documentation to explain a common error and building on arm64
- Use grub-efi and linux-image-generic for both archs
- Add patch to install toolchains on arm64 host
- Drop patches which remove grub and tracing for arm64

Changes in v2:
- Update docs also
- Add comments to the ARG variables
- Swap order so that amd64 is the exception

Link: https://lore.kernel.org/all/20241115134807.3068947-1-sjg@chromium.org/
-- 
Tom

[v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

The cache-flush function is incorrect which causes a crash in the
remoteproc tests with arm64.

Fix both problems by using map_sysmem() to convert an address to a
pointer and map_to_sysmem() to convert a pointer to an address.

Also update the image-loader's cache-flushing logic.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Changes in v6:
- Re-introduce

Changes in v2:
- Drop message about EFI_LOADER

 arch/sandbox/cpu/cache.c              |  8 +++++++-
 drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
 lib/efi_loader/efi_image_loader.c     |  3 ++-
 3 files changed, 20 insertions(+), 9 deletions(-)
---
 arch/sandbox/cpu/cache.c              |  8 +++++++-
 drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
 lib/efi_loader/efi_image_loader.c     |  3 ++-
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
index c8a5e64214b6..96b3da47e8ed 100644
--- a/arch/sandbox/cpu/cache.c
+++ b/arch/sandbox/cpu/cache.c
@@ -4,12 +4,18 @@
  */
 
 #include <cpu_func.h>
+#include <mapmem.h>
 #include <asm/state.h>
 
 void flush_cache(unsigned long addr, unsigned long size)
 {
+	void *ptr;
+
+	ptr = map_sysmem(addr, size);
+
 	/* Clang uses (char *) parameters, GCC (void *) */
-	__builtin___clear_cache((void *)addr, (void *)(addr + size));
+	__builtin___clear_cache(map_sysmem(addr, size), ptr + size);
+	unmap_sysmem(ptr);
 }
 
 void invalidate_icache_all(void)
diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
index ab1836b3f078..0b3941b7798d 100644
--- a/drivers/remoteproc/rproc-elf-loader.c
+++ b/drivers/remoteproc/rproc-elf-loader.c
@@ -6,6 +6,7 @@
 #include <dm.h>
 #include <elf.h>
 #include <log.h>
+#include <mapmem.h>
 #include <remoteproc.h>
 #include <asm/cache.h>
 #include <dm/device_compat.h>
@@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
 	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
 		void *dst = (void *)(uintptr_t)phdr->p_paddr;
 		void *src = (void *)addr + phdr->p_offset;
+		ulong dst_addr;
 
 		if (phdr->p_type != PT_LOAD)
 			continue;
@@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
 		if (phdr->p_filesz != phdr->p_memsz)
 			memset(dst + phdr->p_filesz, 0x00,
 			       phdr->p_memsz - phdr->p_filesz);
-		flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
-			    roundup((unsigned long)dst + phdr->p_filesz,
+		dst_addr = map_to_sysmem(dst);
+		flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
+			    roundup(dst_addr + phdr->p_filesz,
 				    ARCH_DMA_MINALIGN) -
-			    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
+			    rounddown(dst_addr, ARCH_DMA_MINALIGN));
 	}
 
 	return 0;
@@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
 	const struct dm_rproc_ops *ops;
 	Elf32_Shdr *shdr;
 	void *src, *dst;
+	ulong dst_addr;
 
 	shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
 	if (!shdr)
@@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
 		(ulong)dst, *rsc_size);
 
 	memcpy(dst, src, *rsc_size);
-	flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
-		    roundup((unsigned long)dst + *rsc_size,
-			    ARCH_DMA_MINALIGN) -
-		    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
+	dst_addr = map_to_sysmem(dst);
+	flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
+		    roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
+		    rounddown(dst_addr, ARCH_DMA_MINALIGN));
 
 	return 0;
 }
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 0ddf69a09183..bb58cf1badb7 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -13,6 +13,7 @@
 #include <efi_loader.h>
 #include <log.h>
 #include <malloc.h>
+#include <mapmem.h>
 #include <pe.h>
 #include <sort.h>
 #include <crypto/mscode.h>
@@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 	}
 
 	/* Flush cache */
-	flush_cache((ulong)efi_reloc,
+	flush_cache(map_to_sysmem(efi_reloc),
 		    ALIGN(virt_size, EFI_CACHELINE_SIZE));
 
 	/*
-- 
2.43.0

[v6 02/12] test: Adjust print_ut test to use unsigned char

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

Since char is unsigned on arm64, this test currently fails. It seems
better to use unsigned anyway, since 0xff is written into the string at
the start. Update the terminator-assert to use a character instead of a
byte.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Changes in v6:
- Re-introduce

Changes in v2:
- Use '\0' instead of 0

 test/print_ut.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
---
 test/common/print.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/common/print.c b/test/common/print.c
index 464e425edee5..e3711b108091 100644
--- a/test/common/print.c
+++ b/test/common/print.c
@@ -241,7 +241,7 @@ COMMON_TEST(print_display_buffer, UTF_CONSOLE);
 
 static int print_hexdump_line(struct unit_test_state *uts)
 {
-	char *linebuf;
+	u8 *linebuf;
 	u8 *buf;
 	int i;
 
@@ -254,10 +254,10 @@ static int print_hexdump_line(struct unit_test_state *uts)
 	linebuf = map_sysmem(0x400, BUF_SIZE);
 	memset(linebuf, '\xff', BUF_SIZE);
 	ut_asserteq(-ENOSPC, hexdump_line(0, buf, 1, 0x10, 0, linebuf, 75));
-	ut_asserteq(-1, linebuf[0]);
+	ut_asserteq(0xff, linebuf[0]);
 	ut_asserteq(0x10, hexdump_line(0, buf, 1, 0x10, 0, linebuf, 76));
-	ut_asserteq(0, linebuf[75]);
-	ut_asserteq(-1, linebuf[76]);
+	ut_asserteq('\0', linebuf[75]);
+	ut_asserteq(0xff, linebuf[76]);
 
 	unmap_sysmem(buf);
 
-- 
2.43.0

[v6 03/12] docker: Add kernel.org x86_64 toolchain

[Tom Rini]

Add in the x86_64 toolchain, but do not enforce using it for sandbox.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- New patch
---
 tools/docker/Dockerfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 967ac89fbde8..0f6890b78218 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -27,6 +27,7 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-riscv64-linux.tar.xz | tar -C /opt -xJ
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-riscv32-linux.tar.xz | tar -C /opt -xJ
 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-sh2-linux.tar.xz | tar -C /opt -xJ
+RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-x86_64-linux.tar.xz | tar -C /opt -xJ
 
 # Manually install other toolchains
 RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz
@@ -280,7 +281,6 @@ RUN /bin/echo -e "[toolchain]\nroot = /usr" > ~/.buildman
 RUN /bin/echo -e "kernelorg = /opt/gcc-13.2.0-nolibc/*" >> ~/.buildman
 RUN /bin/echo -e "\n[toolchain-prefix]\nxtensa = /opt/2020.07/xtensa-dc233c-elf/bin/xtensa-dc233c-elf-" >> ~/.buildman;
 RUN /bin/echo -e "\n[toolchain-alias]\nsh = sh2" >> ~/.buildman
-RUN /bin/echo -e "\nsandbox = x86_64" >> ~/.buildman
 RUN /bin/echo -e "\nx86 = i386" >> ~/.buildman;
 
 # Add mkbootimg tool
-- 
2.43.0

[v6 04/12] docker: Use "make -j$(nproc)" when invoking make

[Tom Rini]

We had a few places that were not using "make -j$(nproc)" but instead
just plain "make" and so slowing down the overall build.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- New patch
---
 tools/docker/Dockerfile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 0f6890b78218..449745712de3 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -142,7 +142,7 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \
 	TARGET_STRIP=/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-strip \
 	TARGET_NM=/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-nm \
 	TARGET_RANLIB=/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-ranlib && \
-	make && \
+	make -j$(nproc) && \
 	./grub-mkimage -O arm64-efi -o /opt/grub/grubaa64.efi --prefix= -d \
 	grub-core cat chain configfile echo efinet ext2 fat halt help linux \
 	lsefisystab loadenv lvm minicmd normal part_msdos part_gpt reboot \
@@ -156,7 +156,7 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \
 	TARGET_STRIP=/opt/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-strip \
 	TARGET_NM=/opt/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-nm \
 	TARGET_RANLIB=/opt/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ranlib && \
-	make && \
+	make -j$(nproc) && \
 	./grub-mkimage -O arm-efi -o /opt/grub/grubarm.efi --prefix= -d \
 	grub-core cat chain configfile echo efinet ext2 fat halt help linux \
 	lsefisystab loadenv lvm minicmd normal part_msdos part_gpt reboot \
@@ -170,7 +170,7 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \
 	TARGET_STRIP=/opt/gcc-13.2.0-nolibc/riscv64-linux/bin/riscv64-linux-strip \
 	TARGET_NM=/opt/gcc-13.2.0-nolibc/riscv64-linux/bin/riscv64-linux-nm \
 	TARGET_RANLIB=/opt/gcc-13.2.0-nolibc/riscv64-linux/bin/riscv64-linux-ranlib && \
-	make && \
+	make -j$(nproc) && \
 	./grub-mkimage -O riscv64-efi -o /opt/grub/grubriscv64.efi --prefix= -d \
 	grub-core cat chain configfile echo efinet ext2 fat halt help linux \
 	lsefisystab loadenv lvm minicmd normal part_msdos part_gpt reboot \
@@ -196,7 +196,7 @@ RUN git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git /tmp/t
 	cd /tmp/tf-a/ && \
 	git checkout v2.10.0 && \
 	cd tools/fiptool && \
-	make && \
+	make -j$(nproc) && \
 	mkdir -p /usr/local/bin && \
 	cp fiptool /usr/local/bin && \
 	rm -rf /tmp/tf-a
-- 
2.43.0

[v6 05/12] docker: Update to grub-2.12

[Tom Rini]

The current release of grub is 2.12 and it will be good to pick this up
now so that we can update other parts of our stack.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- New patch
---
 tools/docker/Dockerfile | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 449745712de3..11beaedf9b30 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -55,6 +55,7 @@ RUN apt-get update && apt-get install -y \
 	flex \
 	gawk \
 	gdisk \
+	gettext \
 	git \
 	gnu-efi \
 	gnutls-dev \
@@ -128,11 +129,9 @@ RUN chmod +r /boot/vmlinu*
 # Build GRUB UEFI targets for ARM & RISC-V, 32-bit and 64-bit
 RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \
 	cd /tmp/grub && \
-	git checkout grub-2.06 && \
+	git checkout grub-2.12 && \
 	git config --global user.name "GitLab CI Runner" && \
 	git config --global user.email trini@konsulko.com && \
-	git cherry-pick 049efdd72eb7baa7b2bf8884391ee7fe650da5a0 && \
-	git cherry-pick 403d6540cd608b2706cfa0cb4713f7e4b490ff45 && \
 	./bootstrap && \
 	mkdir -p /opt/grub && \
 	./configure --target=aarch64 --with-platform=efi \
-- 
2.43.0

[v6 06/12] docker: Build grub for all architectures

[Tom Rini]

For consistency now, and future ease of testing with non-amd64 hosts,
build grub for all architectures rather than relying on host binaries
for i386/x86_64.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- New patch
---
 .azure-pipelines.yml    |  6 ++----
 .gitlab-ci.yml          |  6 ++----
 tools/docker/Dockerfile | 24 ++++++++++++++++++++++--
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 4ecf76eaa0b5..cc335e5f720c 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -245,8 +245,6 @@ stages:
           ln -s u_boot_boardenv_qemu_arm64_na.py /tmp/uboot-test-hooks/py/travis-ci/u_boot_boardenv_qemu_arm64_lwip_na.py
           ln -s travis-ci /tmp/uboot-test-hooks/bin/\`hostname\`
           ln -s travis-ci /tmp/uboot-test-hooks/py/\`hostname\`
-          grub-mkimage --prefix=\"\" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd
-          grub-mkimage --prefix=\"\" -o ~/grub_x64.efi -O x86_64-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd
           if [[ "\${TEST_PY_BD}" == "qemu-riscv32_spl" ]]; then
               wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.3.1/opensbi-1.3.1-rv-bin.tar.xz | tar -C /tmp -xJ;
               export OPENSBI=/tmp/opensbi-1.3.1-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin;
@@ -268,8 +266,8 @@ stages:
           fi
           pip install -r tools/buildman/requirements.txt
           tools/buildman/buildman -o \${UBOOT_TRAVIS_BUILD_DIR} -w -E -W -e --board \${TEST_PY_BD} \${OVERRIDE}
-          cp ~/grub_x86.efi \${UBOOT_TRAVIS_BUILD_DIR}/
-          cp ~/grub_x64.efi \${UBOOT_TRAVIS_BUILD_DIR}/
+          cp /opt/grub/grub_x86.efi \${UBOOT_TRAVIS_BUILD_DIR}/
+          cp /opt/grub/grub_x64.efi \${UBOOT_TRAVIS_BUILD_DIR}/
           cp /opt/grub/grubriscv64.efi \${UBOOT_TRAVIS_BUILD_DIR}/grub_riscv64.efi
           cp /opt/grub/grubaa64.efi \${UBOOT_TRAVIS_BUILD_DIR}/grub_arm64.efi
           cp /opt/grub/grubarm.efi \${UBOOT_TRAVIS_BUILD_DIR}/grub_arm.efi
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2164ad79a72b..23bbe2c24b8d 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -34,8 +34,6 @@ stages:
     - ln -s conf.qemu_arm64_na /tmp/uboot-test-hooks/bin/travis-ci/conf.qemu_arm64_lwip_na
     - ln -s travis-ci /tmp/uboot-test-hooks/bin/`hostname`
     - ln -s travis-ci /tmp/uboot-test-hooks/py/`hostname`
-    - grub-mkimage --prefix="" -o ~/grub_x86.efi -O i386-efi normal  echo lsefimmap lsefi lsefisystab efinet tftp minicmd
-    - grub-mkimage --prefix="" -o ~/grub_x64.efi -O x86_64-efi normal  echo lsefimmap lsefi lsefisystab efinet tftp minicmd
     - if [[ "${TEST_PY_BD}" == "qemu-riscv32_spl" ]]; then
         wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.3.1/opensbi-1.3.1-rv-bin.tar.xz | tar -C /tmp -xJ;
         export OPENSBI=/tmp/opensbi-1.3.1-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin;
@@ -62,8 +60,8 @@ stages:
       fi
     - tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E -W -e
         --board ${TEST_PY_BD} ${OVERRIDE}
-    - cp ~/grub_x86.efi $UBOOT_TRAVIS_BUILD_DIR/
-    - cp ~/grub_x64.efi $UBOOT_TRAVIS_BUILD_DIR/
+    - cp /opt/grub/grub_x86.efi $UBOOT_TRAVIS_BUILD_DIR/
+    - cp /opt/grub/grub_x64.efi $UBOOT_TRAVIS_BUILD_DIR/
     - cp /opt/grub/grubriscv64.efi $UBOOT_TRAVIS_BUILD_DIR/grub_riscv64.efi
     - cp /opt/grub/grubaa64.efi $UBOOT_TRAVIS_BUILD_DIR/grub_arm64.efi
     - cp /opt/grub/grubarm.efi $UBOOT_TRAVIS_BUILD_DIR/grub_arm.efi
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 11beaedf9b30..90c90bd05f0b 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -60,8 +60,6 @@ RUN apt-get update && apt-get install -y \
 	gnu-efi \
 	gnutls-dev \
 	graphviz \
-	grub-efi-amd64-bin \
-	grub-efi-ia32-bin \
 	help2man \
 	iasl \
 	imagemagick \
@@ -175,6 +173,28 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \
 	lsefisystab loadenv lvm minicmd normal part_msdos part_gpt reboot \
 	search search_fs_file search_fs_uuid search_label serial sleep test \
 	true && \
+	make clean && \
+	./configure --target=i386 --with-platform=efi \
+	CC=gcc \
+	TARGET_CC=/opt/gcc-13.2.0-nolibc/i386-linux/bin/i386-linux-gcc \
+	TARGET_OBJCOPY=/opt/gcc-13.2.0-nolibc/i386-linux/bin/i386-linux-objcopy \
+	TARGET_STRIP=/opt/gcc-13.2.0-nolibc/i386-linux/bin/i386-linux-strip \
+	TARGET_NM=/opt/gcc-13.2.0-nolibc/i386-linux/bin/i386-linux-nm \
+	TARGET_RANLIB=/opt/gcc-13.2.0-nolibc/i386-linux/bin/i386-linux-ranlib && \
+	make -j$(nproc) && \
+	./grub-mkimage -O i386-efi -o /opt/grub/grub_x86.efi --prefix= -d \
+	grub-core normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd && \
+	make clean && \
+	./configure --target=x86_64 --with-platform=efi \
+	CC=gcc \
+	TARGET_CC=/opt/gcc-13.2.0-nolibc/x86_64-linux/bin/x86_64-linux-gcc \
+	TARGET_OBJCOPY=/opt/gcc-13.2.0-nolibc/x86_64-linux/bin/x86_64-linux-objcopy \
+	TARGET_STRIP=/opt/gcc-13.2.0-nolibc/x86_64-linux/bin/x86_64-linux-strip \
+	TARGET_NM=/opt/gcc-13.2.0-nolibc/x86_64-linux/bin/x86_64-linux-nm \
+	TARGET_RANLIB=/opt/gcc-13.2.0-nolibc/x86_64-linux/bin/x86_64-linux-ranlib && \
+	make -j$(nproc) && \
+	./grub-mkimage -O x86_64-efi -o /opt/grub/grub_x64.efi --prefix= -d \
+	grub-core normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd && \
 	rm -rf /tmp/grub
 
 RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \
-- 
2.43.0

[v6 07/12] docker: Use cache mounts for apt

[Tom Rini]

Instead of deleting /var/lib/apt/lists after each relevant RUN line, use
a cache mount as is the current best practices.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- New patch
---
 tools/docker/Dockerfile | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 90c90bd05f0b..bc8d1ebbb1e8 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -10,7 +10,9 @@ LABEL org.opencontainers.image.description=" This image is for building U-Boot i
 ENV DEBIAN_FRONTEND=noninteractive
 
 # Add LLVM repository
-RUN apt-get update && apt-get install -y gnupg2 wget xz-utils && rm -rf /var/lib/apt/lists/*
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt,sharing=locked \
+    apt-get update && apt-get install -y gnupg2 wget xz-utils
 RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
 RUN echo deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main | tee /etc/apt/sources.list.d/llvm.list
 
@@ -33,7 +35,9 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_
 RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz
 
 # Update and install things from apt now
-RUN apt-get update && apt-get install -y \
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt,sharing=locked \
+    apt-get update && apt-get install -y \
 	automake \
 	autopoint \
 	bc \
@@ -118,8 +122,7 @@ RUN apt-get update && apt-get install -y \
 	vboot-utils \
 	xilinx-bootgen \
 	xxd \
-	zip \
-	&& rm -rf /var/lib/apt/lists/*
+	zip
 
 # Make kernels readable for libguestfs tools to work correctly
 RUN chmod +r /boot/vmlinu*
-- 
2.43.0

[v6 08/12] docker: Support building for multiple architectures

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

Add instructions on how to build the file for multiple architectures.
Add a message indicating what is happening.

Update the documentation as well.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- None
---
 doc/build/docker.rst    | 23 +++++++++++++++++++++--
 tools/docker/Dockerfile |  9 +++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/doc/build/docker.rst b/doc/build/docker.rst
index 45659b3b89dc..5896dd5ac4a6 100644
--- a/doc/build/docker.rst
+++ b/doc/build/docker.rst
@@ -1,11 +1,30 @@
 GitLab CI / U-Boot runner container
 ===================================
 
-In order to have a reproducible and portable build environment for CI we use a container for building in.  This means that developers can also reproduce the CI environment, to a large degree at least, locally.  This file is located in the tools/docker directory.  To build the image yourself
+In order to have a reproducible and portable build environment for CI we use a container for building in.  This means that developers can also reproduce the CI environment, to a large degree at least, locally.  This file is located in the tools/docker directory.
+
+The docker image supports both amd64 and arm64. Ensure that the
+'docker-buildx' Debian package is installed (or the equivalent on another
+distribution).
+
+You will need a multi-platform container, otherwise this error is shown::
+
+    ERROR: Multi-platform build is not supported for the docker driver.
+    Switch to a different driver, or turn on the containerd image store, and try again.
+
+You can add one with::
+
+    sudo docker buildx create --name multiarch --driver docker-container --use
+
+Building is supported on both amd64 (i.e. 64-bit x86) and arm64 machines. While
+both amd64 and arm64 happen in parallel, the non-native part will take
+considerably longer as it must use QEMU to emulate the foreign code.
+
+To build the image yourself::
 
 .. code-block:: bash
 
-    sudo docker build -t your-namespace:your-tag .
+    sudo docker buildx build --platform linux/arm64/v8,linux/amd64 -t your-namespace:your-tag .
 
 Or to use an existing container
 
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index bc8d1ebbb1e8..43cd37070e48 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -6,9 +6,18 @@ FROM ubuntu:jammy-20240808
 LABEL org.opencontainers.image.authors="Tom Rini <trini@konsulko.com>"
 LABEL org.opencontainers.image.description=" This image is for building U-Boot inside a container"
 
+# Used by docker to set the target platform: valid values are linux/arm64/v8
+# and linux/amd64
+ARG TARGETPLATFORM
+
+# Used by docker to set the build platform: the only valid value is linux/amd64
+ARG BUILDPLATFORM
+
 # Make sure apt is happy
 ENV DEBIAN_FRONTEND=noninteractive
 
+RUN echo "Building on $BUILDPLATFORM, for target $TARGETPLATFORM"
+
 # Add LLVM repository
 RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
     --mount=type=cache,target=/var/lib/apt,sharing=locked \
-- 
2.43.0

[v6 09/12] docker: Adjust installed packages slightly

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

We no longer need to install libc6-i386 so we can drop that. Switch to
installing linux-image-generic as that will be available on all hosts,
to provide the /boot/vmlinu* file that's requires for various tools.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- Adjust further now and reword the commit message.
---
 tools/docker/Dockerfile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 43cd37070e48..5d3bd4e24ab5 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -77,7 +77,6 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
 	iasl \
 	imagemagick \
 	iputils-ping \
-	libc6-i386 \
 	libconfuse-dev \
 	libgit2-dev \
 	libjson-glib-dev \
@@ -95,7 +94,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
 	libtool \
 	libudev-dev \
 	libusb-1.0-0-dev \
-	linux-image-kvm \
+	linux-image-generic \
 	lzma-alone \
 	lzop \
 	mount \
-- 
2.43.0

[v6 10/12] docker: Fix LegacyKeyValueFormat warning with PYTHONPATH

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

Fix a warning due to the syntax used for PYTHONPATH:

   LegacyKeyValueFormat: "ENV key=value" should be used instead of
      legacy "ENV key value" format (line 304)

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- None
---
 tools/docker/Dockerfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 5d3bd4e24ab5..9d10de5b8926 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -315,4 +315,4 @@ RUN /bin/echo -e "\nx86 = i386" >> ~/.buildman;
 
 # Add mkbootimg tool
 RUN git clone https://android.googlesource.com/platform/system/tools/mkbootimg /home/uboot/mkbootimg
-ENV PYTHONPATH "${PYTHONPATH}:/home/uboot/mkbootimg"
+ENV PYTHONPATH="${PYTHONPATH}:/home/uboot/mkbootimg"
-- 
2.43.0

[v6 11/12] docker: Install toolchains on arm64 host

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

Refactor the code to support downloading toolchains for arm64 as well as
x86_64

There doesn't seem to be an xtensa toolchain for arm64 at the same
location, so download that only on x86

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
Changes in v6:
- Add x86_64 to the toolchain list
---
 tools/docker/Dockerfile | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 9d10de5b8926..ce1ad7cb23a2 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -16,6 +16,15 @@ ARG BUILDPLATFORM
 # Make sure apt is happy
 ENV DEBIAN_FRONTEND=noninteractive
 
+# Set architectures to build for (leaving out ARM which is an exception)
+ENV ARCHS="aarch64 arc i386 m68k mips microblaze nios2 powerpc riscv64 riscv32 sh2 x86_64"
+
+# Mirror containing the toolchains
+ENV MIRROR=https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin
+
+# Toolchain version
+ENV TCVER=13.2.0
+
 RUN echo "Building on $BUILDPLATFORM, for target $TARGETPLATFORM"
 
 # Add LLVM repository
@@ -25,23 +34,25 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
 RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
 RUN echo deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main | tee /etc/apt/sources.list.d/llvm.list
 
-# Manually install the kernel.org "Crosstool" based toolchains for gcc-13.2.0
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-aarch64-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-arc-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-arm-linux-gnueabi.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-i386-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-m68k-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-mips-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-microblaze-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-nios2-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-powerpc-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-riscv64-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-riscv32-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-sh2-linux.tar.xz | tar -C /opt -xJ
-RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-x86_64-linux.tar.xz | tar -C /opt -xJ
+# Create a list of URLs to process, then pass them into a 'while read' loop
+RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then HOSTARCH=x86_64; else HOSTARCH=arm64; fi; ( \
+	# Manually install the kernel.org "Crosstool"-based toolchains
+	for arch in $ARCHS; do \
+		echo $MIRROR/$HOSTARCH/$TCVER/${HOSTARCH}-gcc-$TCVER-nolibc-${arch}-linux.tar.xz; \
+	done; \
+	\
+	# Deal with ARM, which has a 'gnueabi' suffix
+	echo $MIRROR/${HOSTARCH}/$TCVER/${HOSTARCH}-gcc-$TCVER-nolibc-arm-linux-gnueabi.tar.xz; \
+	\
+	) | while read url; do \
+		# Read the URL and unpack it into /opt
+		wget -O - $url | tar -C /opt -xJ; \
+	done
 
 # Manually install other toolchains
-RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz
+RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \
+		wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz; \
+	fi
 
 # Update and install things from apt now
 RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
-- 
2.43.0

[v6 12/12] CI: Add platform variable

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

Add a list of possible platforms that can be used by gitlab runners.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes in v6:
- None
---
 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 23bbe2c24b8d..f0113361a3fe 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -4,6 +4,7 @@ variables:
   DEFAULT_TAG: ""
   MIRROR_DOCKER: docker.io
   SJG_LAB: ""
+  PLATFORM: linux/amd64,linux/arm64
 
 default:
   tags:
-- 
2.43.0

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Heinrich Schuchardt]

On 27.11.24 18:17, Tom Rini wrote:
> From: Simon Glass <sjg@chromium.org>
>
> The cache-flush function is incorrect which causes a crash in the
> remoteproc tests with arm64.
>
> Fix both problems by using map_sysmem() to convert an address to a
> pointer and map_to_sysmem() to convert a pointer to an address.
>
> Also update the image-loader's cache-flushing logic.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
> Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Changes in v6:
> - Re-introduce
>
> Changes in v2:
> - Drop message about EFI_LOADER
>
>   arch/sandbox/cpu/cache.c              |  8 +++++++-
>   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
>   lib/efi_loader/efi_image_loader.c     |  3 ++-
>   3 files changed, 20 insertions(+), 9 deletions(-)
> ---
>   arch/sandbox/cpu/cache.c              |  8 +++++++-
>   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
>   lib/efi_loader/efi_image_loader.c     |  3 ++-
>   3 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
> index c8a5e64214b6..96b3da47e8ed 100644
> --- a/arch/sandbox/cpu/cache.c
> +++ b/arch/sandbox/cpu/cache.c
> @@ -4,12 +4,18 @@
>    */
>
>   #include <cpu_func.h>
> +#include <mapmem.h>
>   #include <asm/state.h>
>
>   void flush_cache(unsigned long addr, unsigned long size)
>   {
> +	void *ptr;
> +
> +	ptr = map_sysmem(addr, size);
> +
>   	/* Clang uses (char *) parameters, GCC (void *) */
> -	__builtin___clear_cache((void *)addr, (void *)(addr + size));
> +	__builtin___clear_cache(map_sysmem(addr, size), ptr + size);
> +	unmap_sysmem(ptr);
>   }
>
>   void invalidate_icache_all(void)
> diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> index ab1836b3f078..0b3941b7798d 100644
> --- a/drivers/remoteproc/rproc-elf-loader.c
> +++ b/drivers/remoteproc/rproc-elf-loader.c
> @@ -6,6 +6,7 @@
>   #include <dm.h>
>   #include <elf.h>
>   #include <log.h>
> +#include <mapmem.h>
>   #include <remoteproc.h>
>   #include <asm/cache.h>
>   #include <dm/device_compat.h>
> @@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
>   	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
>   		void *dst = (void *)(uintptr_t)phdr->p_paddr;
>   		void *src = (void *)addr + phdr->p_offset;
> +		ulong dst_addr;
>
>   		if (phdr->p_type != PT_LOAD)
>   			continue;
> @@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
>   		if (phdr->p_filesz != phdr->p_memsz)
>   			memset(dst + phdr->p_filesz, 0x00,
>   			       phdr->p_memsz - phdr->p_filesz);
> -		flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> -			    roundup((unsigned long)dst + phdr->p_filesz,
> +		dst_addr = map_to_sysmem(dst);
> +		flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> +			    roundup(dst_addr + phdr->p_filesz,
>   				    ARCH_DMA_MINALIGN) -
> -			    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> +			    rounddown(dst_addr, ARCH_DMA_MINALIGN));
>   	}
>
>   	return 0;
> @@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
>   	const struct dm_rproc_ops *ops;
>   	Elf32_Shdr *shdr;
>   	void *src, *dst;
> +	ulong dst_addr;
>
>   	shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
>   	if (!shdr)
> @@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
>   		(ulong)dst, *rsc_size);
>
>   	memcpy(dst, src, *rsc_size);
> -	flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> -		    roundup((unsigned long)dst + *rsc_size,
> -			    ARCH_DMA_MINALIGN) -
> -		    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> +	dst_addr = map_to_sysmem(dst);
> +	flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> +		    roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
> +		    rounddown(dst_addr, ARCH_DMA_MINALIGN));
>
>   	return 0;
>   }
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index 0ddf69a09183..bb58cf1badb7 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -13,6 +13,7 @@
>   #include <efi_loader.h>
>   #include <log.h>
>   #include <malloc.h>
> +#include <mapmem.h>
>   #include <pe.h>
>   #include <sort.h>
>   #include <crypto/mscode.h>
> @@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
>   	}
>
>   	/* Flush cache */
> -	flush_cache((ulong)efi_reloc,
> +	flush_cache(map_to_sysmem(efi_reloc),
>   		    ALIGN(virt_size, EFI_CACHELINE_SIZE));

It would be nice if we could be consistent:

include/cpu_func.h:66:
void flush_cache(unsigned long addr, unsigned long size);

arch/arc/lib/cache.c:773:
void flush_cache(unsigned long start, unsigned long size)

arch/sandbox/cpu/cache.c:9:
void flush_cache(unsigned long addr, unsigned long size)

Here sandbox calls __builtin___clear_cache() which needs a pointer.

The correct thing would be to change the signature of flush_cache to

flush_cache(void *addr, size_t size)

in all locations and do away with ulong here.

NAK to this patch.

Best regards

Heinrich

>
>   	/*

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6024 bytes --]

On Wed, Nov 27, 2024 at 07:40:05PM +0100, Heinrich Schuchardt wrote:
> On 27.11.24 18:17, Tom Rini wrote:
> > From: Simon Glass <sjg@chromium.org>
> > 
> > The cache-flush function is incorrect which causes a crash in the
> > remoteproc tests with arm64.
> > 
> > Fix both problems by using map_sysmem() to convert an address to a
> > pointer and map_to_sysmem() to convert a pointer to an address.
> > 
> > Also update the image-loader's cache-flushing logic.
> > 
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
> > Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > 
> > Changes in v6:
> > - Re-introduce
> > 
> > Changes in v2:
> > - Drop message about EFI_LOADER
> > 
> >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> >   3 files changed, 20 insertions(+), 9 deletions(-)
> > ---
> >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> >   3 files changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
> > index c8a5e64214b6..96b3da47e8ed 100644
> > --- a/arch/sandbox/cpu/cache.c
> > +++ b/arch/sandbox/cpu/cache.c
> > @@ -4,12 +4,18 @@
> >    */
> > 
> >   #include <cpu_func.h>
> > +#include <mapmem.h>
> >   #include <asm/state.h>
> > 
> >   void flush_cache(unsigned long addr, unsigned long size)
> >   {
> > +	void *ptr;
> > +
> > +	ptr = map_sysmem(addr, size);
> > +
> >   	/* Clang uses (char *) parameters, GCC (void *) */
> > -	__builtin___clear_cache((void *)addr, (void *)(addr + size));
> > +	__builtin___clear_cache(map_sysmem(addr, size), ptr + size);
> > +	unmap_sysmem(ptr);
> >   }
> > 
> >   void invalidate_icache_all(void)
> > diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> > index ab1836b3f078..0b3941b7798d 100644
> > --- a/drivers/remoteproc/rproc-elf-loader.c
> > +++ b/drivers/remoteproc/rproc-elf-loader.c
> > @@ -6,6 +6,7 @@
> >   #include <dm.h>
> >   #include <elf.h>
> >   #include <log.h>
> > +#include <mapmem.h>
> >   #include <remoteproc.h>
> >   #include <asm/cache.h>
> >   #include <dm/device_compat.h>
> > @@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> >   	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
> >   		void *dst = (void *)(uintptr_t)phdr->p_paddr;
> >   		void *src = (void *)addr + phdr->p_offset;
> > +		ulong dst_addr;
> > 
> >   		if (phdr->p_type != PT_LOAD)
> >   			continue;
> > @@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> >   		if (phdr->p_filesz != phdr->p_memsz)
> >   			memset(dst + phdr->p_filesz, 0x00,
> >   			       phdr->p_memsz - phdr->p_filesz);
> > -		flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > -			    roundup((unsigned long)dst + phdr->p_filesz,
> > +		dst_addr = map_to_sysmem(dst);
> > +		flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > +			    roundup(dst_addr + phdr->p_filesz,
> >   				    ARCH_DMA_MINALIGN) -
> > -			    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > +			    rounddown(dst_addr, ARCH_DMA_MINALIGN));
> >   	}
> > 
> >   	return 0;
> > @@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> >   	const struct dm_rproc_ops *ops;
> >   	Elf32_Shdr *shdr;
> >   	void *src, *dst;
> > +	ulong dst_addr;
> > 
> >   	shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
> >   	if (!shdr)
> > @@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> >   		(ulong)dst, *rsc_size);
> > 
> >   	memcpy(dst, src, *rsc_size);
> > -	flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > -		    roundup((unsigned long)dst + *rsc_size,
> > -			    ARCH_DMA_MINALIGN) -
> > -		    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > +	dst_addr = map_to_sysmem(dst);
> > +	flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > +		    roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
> > +		    rounddown(dst_addr, ARCH_DMA_MINALIGN));
> > 
> >   	return 0;
> >   }
> > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > index 0ddf69a09183..bb58cf1badb7 100644
> > --- a/lib/efi_loader/efi_image_loader.c
> > +++ b/lib/efi_loader/efi_image_loader.c
> > @@ -13,6 +13,7 @@
> >   #include <efi_loader.h>
> >   #include <log.h>
> >   #include <malloc.h>
> > +#include <mapmem.h>
> >   #include <pe.h>
> >   #include <sort.h>
> >   #include <crypto/mscode.h>
> > @@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> >   	}
> > 
> >   	/* Flush cache */
> > -	flush_cache((ulong)efi_reloc,
> > +	flush_cache(map_to_sysmem(efi_reloc),
> >   		    ALIGN(virt_size, EFI_CACHELINE_SIZE));
> 
> It would be nice if we could be consistent:
> 
> include/cpu_func.h:66:
> void flush_cache(unsigned long addr, unsigned long size);
> 
> arch/arc/lib/cache.c:773:
> void flush_cache(unsigned long start, unsigned long size)
> 
> arch/sandbox/cpu/cache.c:9:
> void flush_cache(unsigned long addr, unsigned long size)
> 
> Here sandbox calls __builtin___clear_cache() which needs a pointer.
> 
> The correct thing would be to change the signature of flush_cache to
> 
> flush_cache(void *addr, size_t size)
> 
> in all locations and do away with ulong here.
> 
> NAK to this patch.

I'm going to refer you back to yourself when Acked-by'ing this:
https://patchwork.ozlabs.org/project/uboot/patch/20241112141105.640189-2-sjg@chromium.org/#3412975

And I'm not even sure that re-working changing this to void* instead is
a good idea.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Heinrich Schuchardt]

On 27.11.24 19:51, Tom Rini wrote:
> On Wed, Nov 27, 2024 at 07:40:05PM +0100, Heinrich Schuchardt wrote:
>> On 27.11.24 18:17, Tom Rini wrote:
>>> From: Simon Glass <sjg@chromium.org>
>>>
>>> The cache-flush function is incorrect which causes a crash in the
>>> remoteproc tests with arm64.
>>>
>>> Fix both problems by using map_sysmem() to convert an address to a
>>> pointer and map_to_sysmem() to convert a pointer to an address.
>>>
>>> Also update the image-loader's cache-flushing logic.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
>>> Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>
>>> Changes in v6:
>>> - Re-introduce
>>>
>>> Changes in v2:
>>> - Drop message about EFI_LOADER
>>>
>>>    arch/sandbox/cpu/cache.c              |  8 +++++++-
>>>    drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
>>>    lib/efi_loader/efi_image_loader.c     |  3 ++-
>>>    3 files changed, 20 insertions(+), 9 deletions(-)
>>> ---
>>>    arch/sandbox/cpu/cache.c              |  8 +++++++-
>>>    drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
>>>    lib/efi_loader/efi_image_loader.c     |  3 ++-
>>>    3 files changed, 20 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
>>> index c8a5e64214b6..96b3da47e8ed 100644
>>> --- a/arch/sandbox/cpu/cache.c
>>> +++ b/arch/sandbox/cpu/cache.c
>>> @@ -4,12 +4,18 @@
>>>     */
>>>
>>>    #include <cpu_func.h>
>>> +#include <mapmem.h>
>>>    #include <asm/state.h>
>>>
>>>    void flush_cache(unsigned long addr, unsigned long size)
>>>    {
>>> +	void *ptr;
>>> +
>>> +	ptr = map_sysmem(addr, size);
>>> +
>>>    	/* Clang uses (char *) parameters, GCC (void *) */
>>> -	__builtin___clear_cache((void *)addr, (void *)(addr + size));
>>> +	__builtin___clear_cache(map_sysmem(addr, size), ptr + size);
>>> +	unmap_sysmem(ptr);
>>>    }
>>>
>>>    void invalidate_icache_all(void)
>>> diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
>>> index ab1836b3f078..0b3941b7798d 100644
>>> --- a/drivers/remoteproc/rproc-elf-loader.c
>>> +++ b/drivers/remoteproc/rproc-elf-loader.c
>>> @@ -6,6 +6,7 @@
>>>    #include <dm.h>
>>>    #include <elf.h>
>>>    #include <log.h>
>>> +#include <mapmem.h>
>>>    #include <remoteproc.h>
>>>    #include <asm/cache.h>
>>>    #include <dm/device_compat.h>
>>> @@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
>>>    	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
>>>    		void *dst = (void *)(uintptr_t)phdr->p_paddr;
>>>    		void *src = (void *)addr + phdr->p_offset;
>>> +		ulong dst_addr;
>>>
>>>    		if (phdr->p_type != PT_LOAD)
>>>    			continue;
>>> @@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
>>>    		if (phdr->p_filesz != phdr->p_memsz)
>>>    			memset(dst + phdr->p_filesz, 0x00,
>>>    			       phdr->p_memsz - phdr->p_filesz);
>>> -		flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
>>> -			    roundup((unsigned long)dst + phdr->p_filesz,
>>> +		dst_addr = map_to_sysmem(dst);
>>> +		flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
>>> +			    roundup(dst_addr + phdr->p_filesz,
>>>    				    ARCH_DMA_MINALIGN) -
>>> -			    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
>>> +			    rounddown(dst_addr, ARCH_DMA_MINALIGN));
>>>    	}
>>>
>>>    	return 0;
>>> @@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
>>>    	const struct dm_rproc_ops *ops;
>>>    	Elf32_Shdr *shdr;
>>>    	void *src, *dst;
>>> +	ulong dst_addr;
>>>
>>>    	shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
>>>    	if (!shdr)
>>> @@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
>>>    		(ulong)dst, *rsc_size);
>>>
>>>    	memcpy(dst, src, *rsc_size);
>>> -	flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
>>> -		    roundup((unsigned long)dst + *rsc_size,
>>> -			    ARCH_DMA_MINALIGN) -
>>> -		    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
>>> +	dst_addr = map_to_sysmem(dst);
>>> +	flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
>>> +		    roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
>>> +		    rounddown(dst_addr, ARCH_DMA_MINALIGN));
>>>
>>>    	return 0;
>>>    }
>>> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
>>> index 0ddf69a09183..bb58cf1badb7 100644
>>> --- a/lib/efi_loader/efi_image_loader.c
>>> +++ b/lib/efi_loader/efi_image_loader.c
>>> @@ -13,6 +13,7 @@
>>>    #include <efi_loader.h>
>>>    #include <log.h>
>>>    #include <malloc.h>
>>> +#include <mapmem.h>
>>>    #include <pe.h>
>>>    #include <sort.h>
>>>    #include <crypto/mscode.h>
>>> @@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
>>>    	}
>>>
>>>    	/* Flush cache */
>>> -	flush_cache((ulong)efi_reloc,
>>> +	flush_cache(map_to_sysmem(efi_reloc),
>>>    		    ALIGN(virt_size, EFI_CACHELINE_SIZE));
>>
>> It would be nice if we could be consistent:
>>
>> include/cpu_func.h:66:
>> void flush_cache(unsigned long addr, unsigned long size);
>>
>> arch/arc/lib/cache.c:773:
>> void flush_cache(unsigned long start, unsigned long size)
>>
>> arch/sandbox/cpu/cache.c:9:
>> void flush_cache(unsigned long addr, unsigned long size)
>>
>> Here sandbox calls __builtin___clear_cache() which needs a pointer.
>>
>> The correct thing would be to change the signature of flush_cache to
>>
>> flush_cache(void *addr, size_t size)
>>
>> in all locations and do away with ulong here.
>>
>> NAK to this patch.
>
> I'm going to refer you back to yourself when Acked-by'ing this:
> https://patchwork.ozlabs.org/project/uboot/patch/20241112141105.640189-2-sjg@chromium.org/#3412975
>
> And I'm not even sure that re-working changing this to void* instead is
> a good idea.
>

Looking at
boot/bootm.c:647:       flush_cache(flush_start, ALIGN(load_end,
ARCH_DMA_MINALIGN) - flush_start);

where flush_start is derived the struct image_info field load. Would you
be able to tell if flush_start is a sandbox virtual address or a
pointer? I would not as struct image_info is not documented.

The sandbox implementation assumes that it is a pointer but elsewhere we
use ulong for sandbox virtual addresses (phys_addr_t).

For me this patch starts at the wrong end.

We should first properly describe the parameters of flush_cache(). Then
we can check if the code aligns with this.

Best regards

Heinrich

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Heinrich Schuchardt]

On 27.11.24 19:40, Heinrich Schuchardt wrote:
> On 27.11.24 18:17, Tom Rini wrote:
>> From: Simon Glass <sjg@chromium.org>
>>
>> The cache-flush function is incorrect which causes a crash in the
>> remoteproc tests with arm64.
>>
>> Fix both problems by using map_sysmem() to convert an address to a
>> pointer and map_to_sysmem() to convert a pointer to an address.
>>
>> Also update the image-loader's cache-flushing logic.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
>> Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>
>> Changes in v6:
>> - Re-introduce
>>
>> Changes in v2:
>> - Drop message about EFI_LOADER
>>
>>   arch/sandbox/cpu/cache.c              |  8 +++++++-
>>   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
>>   lib/efi_loader/efi_image_loader.c     |  3 ++-
>>   3 files changed, 20 insertions(+), 9 deletions(-)
>> ---
>>   arch/sandbox/cpu/cache.c              |  8 +++++++-
>>   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
>>   lib/efi_loader/efi_image_loader.c     |  3 ++-
>>   3 files changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
>> index c8a5e64214b6..96b3da47e8ed 100644
>> --- a/arch/sandbox/cpu/cache.c
>> +++ b/arch/sandbox/cpu/cache.c
>> @@ -4,12 +4,18 @@
>>    */
>>
>>   #include <cpu_func.h>
>> +#include <mapmem.h>
>>   #include <asm/state.h>
>>
>>   void flush_cache(unsigned long addr, unsigned long size)
>>   {
>> +    void *ptr;
>> +
>> +    ptr = map_sysmem(addr, size);
>> +
>>       /* Clang uses (char *) parameters, GCC (void *) */
>> -    __builtin___clear_cache((void *)addr, (void *)(addr + size));
>> +    __builtin___clear_cache(map_sysmem(addr, size), ptr + size);
>> +    unmap_sysmem(ptr);
>>   }

I missed this part when looking at the EFI change.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Simon Glass]

Hi Heinrich,

On Wed, 27 Nov 2024 at 11:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 27.11.24 18:17, Tom Rini wrote:
> > From: Simon Glass <sjg@chromium.org>
> >
> > The cache-flush function is incorrect which causes a crash in the
> > remoteproc tests with arm64.
> >
> > Fix both problems by using map_sysmem() to convert an address to a
> > pointer and map_to_sysmem() to convert a pointer to an address.
> >
> > Also update the image-loader's cache-flushing logic.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
> > Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > Changes in v6:
> > - Re-introduce
> >
> > Changes in v2:
> > - Drop message about EFI_LOADER
> >
> >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> >   3 files changed, 20 insertions(+), 9 deletions(-)
> > ---
> >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> >   3 files changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
> > index c8a5e64214b6..96b3da47e8ed 100644
> > --- a/arch/sandbox/cpu/cache.c
> > +++ b/arch/sandbox/cpu/cache.c
> > @@ -4,12 +4,18 @@
> >    */
> >
> >   #include <cpu_func.h>
> > +#include <mapmem.h>
> >   #include <asm/state.h>
> >
> >   void flush_cache(unsigned long addr, unsigned long size)
> >   {
> > +     void *ptr;
> > +
> > +     ptr = map_sysmem(addr, size);
> > +
> >       /* Clang uses (char *) parameters, GCC (void *) */
> > -     __builtin___clear_cache((void *)addr, (void *)(addr + size));
> > +     __builtin___clear_cache(map_sysmem(addr, size), ptr + size);
> > +     unmap_sysmem(ptr);
> >   }
> >
> >   void invalidate_icache_all(void)
> > diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> > index ab1836b3f078..0b3941b7798d 100644
> > --- a/drivers/remoteproc/rproc-elf-loader.c
> > +++ b/drivers/remoteproc/rproc-elf-loader.c
> > @@ -6,6 +6,7 @@
> >   #include <dm.h>
> >   #include <elf.h>
> >   #include <log.h>
> > +#include <mapmem.h>
> >   #include <remoteproc.h>
> >   #include <asm/cache.h>
> >   #include <dm/device_compat.h>
> > @@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> >       for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
> >               void *dst = (void *)(uintptr_t)phdr->p_paddr;
> >               void *src = (void *)addr + phdr->p_offset;
> > +             ulong dst_addr;
> >
> >               if (phdr->p_type != PT_LOAD)
> >                       continue;
> > @@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> >               if (phdr->p_filesz != phdr->p_memsz)
> >                       memset(dst + phdr->p_filesz, 0x00,
> >                              phdr->p_memsz - phdr->p_filesz);
> > -             flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > -                         roundup((unsigned long)dst + phdr->p_filesz,
> > +             dst_addr = map_to_sysmem(dst);
> > +             flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > +                         roundup(dst_addr + phdr->p_filesz,
> >                                   ARCH_DMA_MINALIGN) -
> > -                         rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > +                         rounddown(dst_addr, ARCH_DMA_MINALIGN));
> >       }
> >
> >       return 0;
> > @@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> >       const struct dm_rproc_ops *ops;
> >       Elf32_Shdr *shdr;
> >       void *src, *dst;
> > +     ulong dst_addr;
> >
> >       shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
> >       if (!shdr)
> > @@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> >               (ulong)dst, *rsc_size);
> >
> >       memcpy(dst, src, *rsc_size);
> > -     flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > -                 roundup((unsigned long)dst + *rsc_size,
> > -                         ARCH_DMA_MINALIGN) -
> > -                 rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > +     dst_addr = map_to_sysmem(dst);
> > +     flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > +                 roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
> > +                 rounddown(dst_addr, ARCH_DMA_MINALIGN));
> >
> >       return 0;
> >   }
> > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > index 0ddf69a09183..bb58cf1badb7 100644
> > --- a/lib/efi_loader/efi_image_loader.c
> > +++ b/lib/efi_loader/efi_image_loader.c
> > @@ -13,6 +13,7 @@
> >   #include <efi_loader.h>
> >   #include <log.h>
> >   #include <malloc.h>
> > +#include <mapmem.h>
> >   #include <pe.h>
> >   #include <sort.h>
> >   #include <crypto/mscode.h>
> > @@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> >       }
> >
> >       /* Flush cache */
> > -     flush_cache((ulong)efi_reloc,
> > +     flush_cache(map_to_sysmem(efi_reloc),
> >                   ALIGN(virt_size, EFI_CACHELINE_SIZE));
>
> It would be nice if we could be consistent:
>
> include/cpu_func.h:66:
> void flush_cache(unsigned long addr, unsigned long size);
>
> arch/arc/lib/cache.c:773:
> void flush_cache(unsigned long start, unsigned long size)
>
> arch/sandbox/cpu/cache.c:9:
> void flush_cache(unsigned long addr, unsigned long size)

Patches are welcome :-)

They should all be ulong

Regards,
Simon

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6546 bytes --]

On Wed, Nov 27, 2024 at 02:09:42PM -0700, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 27 Nov 2024 at 11:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 27.11.24 18:17, Tom Rini wrote:
> > > From: Simon Glass <sjg@chromium.org>
> > >
> > > The cache-flush function is incorrect which causes a crash in the
> > > remoteproc tests with arm64.
> > >
> > > Fix both problems by using map_sysmem() to convert an address to a
> > > pointer and map_to_sysmem() to convert a pointer to an address.
> > >
> > > Also update the image-loader's cache-flushing logic.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
> > > Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >
> > > Changes in v6:
> > > - Re-introduce
> > >
> > > Changes in v2:
> > > - Drop message about EFI_LOADER
> > >
> > >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> > >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> > >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> > >   3 files changed, 20 insertions(+), 9 deletions(-)
> > > ---
> > >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> > >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> > >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> > >   3 files changed, 20 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
> > > index c8a5e64214b6..96b3da47e8ed 100644
> > > --- a/arch/sandbox/cpu/cache.c
> > > +++ b/arch/sandbox/cpu/cache.c
> > > @@ -4,12 +4,18 @@
> > >    */
> > >
> > >   #include <cpu_func.h>
> > > +#include <mapmem.h>
> > >   #include <asm/state.h>
> > >
> > >   void flush_cache(unsigned long addr, unsigned long size)
> > >   {
> > > +     void *ptr;
> > > +
> > > +     ptr = map_sysmem(addr, size);
> > > +
> > >       /* Clang uses (char *) parameters, GCC (void *) */
> > > -     __builtin___clear_cache((void *)addr, (void *)(addr + size));
> > > +     __builtin___clear_cache(map_sysmem(addr, size), ptr + size);
> > > +     unmap_sysmem(ptr);
> > >   }
> > >
> > >   void invalidate_icache_all(void)
> > > diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> > > index ab1836b3f078..0b3941b7798d 100644
> > > --- a/drivers/remoteproc/rproc-elf-loader.c
> > > +++ b/drivers/remoteproc/rproc-elf-loader.c
> > > @@ -6,6 +6,7 @@
> > >   #include <dm.h>
> > >   #include <elf.h>
> > >   #include <log.h>
> > > +#include <mapmem.h>
> > >   #include <remoteproc.h>
> > >   #include <asm/cache.h>
> > >   #include <dm/device_compat.h>
> > > @@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> > >       for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
> > >               void *dst = (void *)(uintptr_t)phdr->p_paddr;
> > >               void *src = (void *)addr + phdr->p_offset;
> > > +             ulong dst_addr;
> > >
> > >               if (phdr->p_type != PT_LOAD)
> > >                       continue;
> > > @@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> > >               if (phdr->p_filesz != phdr->p_memsz)
> > >                       memset(dst + phdr->p_filesz, 0x00,
> > >                              phdr->p_memsz - phdr->p_filesz);
> > > -             flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > > -                         roundup((unsigned long)dst + phdr->p_filesz,
> > > +             dst_addr = map_to_sysmem(dst);
> > > +             flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > > +                         roundup(dst_addr + phdr->p_filesz,
> > >                                   ARCH_DMA_MINALIGN) -
> > > -                         rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > > +                         rounddown(dst_addr, ARCH_DMA_MINALIGN));
> > >       }
> > >
> > >       return 0;
> > > @@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> > >       const struct dm_rproc_ops *ops;
> > >       Elf32_Shdr *shdr;
> > >       void *src, *dst;
> > > +     ulong dst_addr;
> > >
> > >       shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
> > >       if (!shdr)
> > > @@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> > >               (ulong)dst, *rsc_size);
> > >
> > >       memcpy(dst, src, *rsc_size);
> > > -     flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > > -                 roundup((unsigned long)dst + *rsc_size,
> > > -                         ARCH_DMA_MINALIGN) -
> > > -                 rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > > +     dst_addr = map_to_sysmem(dst);
> > > +     flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > > +                 roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
> > > +                 rounddown(dst_addr, ARCH_DMA_MINALIGN));
> > >
> > >       return 0;
> > >   }
> > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > > index 0ddf69a09183..bb58cf1badb7 100644
> > > --- a/lib/efi_loader/efi_image_loader.c
> > > +++ b/lib/efi_loader/efi_image_loader.c
> > > @@ -13,6 +13,7 @@
> > >   #include <efi_loader.h>
> > >   #include <log.h>
> > >   #include <malloc.h>
> > > +#include <mapmem.h>
> > >   #include <pe.h>
> > >   #include <sort.h>
> > >   #include <crypto/mscode.h>
> > > @@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> > >       }
> > >
> > >       /* Flush cache */
> > > -     flush_cache((ulong)efi_reloc,
> > > +     flush_cache(map_to_sysmem(efi_reloc),
> > >                   ALIGN(virt_size, EFI_CACHELINE_SIZE));
> >
> > It would be nice if we could be consistent:
> >
> > include/cpu_func.h:66:
> > void flush_cache(unsigned long addr, unsigned long size);
> >
> > arch/arc/lib/cache.c:773:
> > void flush_cache(unsigned long start, unsigned long size)
> >
> > arch/sandbox/cpu/cache.c:9:
> > void flush_cache(unsigned long addr, unsigned long size)
> 
> Patches are welcome :-)
> 
> They should all be ulong

Please note that they are all consistent in prototypes and
functionality. arc just says "start" rather than "addr" and everyone
uses them in the same way. The issue, as the Fixes tag shows, is that
sandbox's isn't complete.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [v6 03/12] docker: Add kernel.org x86_64 toolchain

[Simon Glass]

On Wed, 27 Nov 2024 at 10:22, Tom Rini <trini@konsulko.com> wrote:
>
> Add in the x86_64 toolchain, but do not enforce using it for sandbox.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Changes in v6:
> - New patch
> ---
>  tools/docker/Dockerfile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [v6 04/12] docker: Use "make -j$(nproc)" when invoking make

[Simon Glass]

On Wed, 27 Nov 2024 at 10:22, Tom Rini <trini@konsulko.com> wrote:
>
> We had a few places that were not using "make -j$(nproc)" but instead
> just plain "make" and so slowing down the overall build.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Changes in v6:
> - New patch
> ---
>  tools/docker/Dockerfile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [v6 05/12] docker: Update to grub-2.12

[Simon Glass]

On Wed, 27 Nov 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
>
> The current release of grub is 2.12 and it will be good to pick this up
> now so that we can update other parts of our stack.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Changes in v6:
> - New patch
> ---
>  tools/docker/Dockerfile | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [v6 06/12] docker: Build grub for all architectures

[Simon Glass]

On Wed, 27 Nov 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
>
> For consistency now, and future ease of testing with non-amd64 hosts,
> build grub for all architectures rather than relying on host binaries
> for i386/x86_64.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Changes in v6:
> - New patch
> ---
>  .azure-pipelines.yml    |  6 ++----
>  .gitlab-ci.yml          |  6 ++----
>  tools/docker/Dockerfile | 24 ++++++++++++++++++++++--
>  3 files changed, 26 insertions(+), 10 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [v6 07/12] docker: Use cache mounts for apt

[Simon Glass]

On Wed, 27 Nov 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
>
> Instead of deleting /var/lib/apt/lists after each relevant RUN line, use
> a cache mount as is the current best practices.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Changes in v6:
> - New patch
> ---
>  tools/docker/Dockerfile | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [v6 01/12] sandbox: efi_loader: Correct use of addresses as pointers

[Simon Glass]

Hi Tom,

On Wed, 27 Nov 2024 at 14:14, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Nov 27, 2024 at 02:09:42PM -0700, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 27 Nov 2024 at 11:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > > On 27.11.24 18:17, Tom Rini wrote:
> > > > From: Simon Glass <sjg@chromium.org>
> > > >
> > > > The cache-flush function is incorrect which causes a crash in the
> > > > remoteproc tests with arm64.
> > > >
> > > > Fix both problems by using map_sysmem() to convert an address to a
> > > > pointer and map_to_sysmem() to convert a pointer to an address.
> > > >
> > > > Also update the image-loader's cache-flushing logic.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
> > > > Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > >
> > > > Changes in v6:
> > > > - Re-introduce
> > > >
> > > > Changes in v2:
> > > > - Drop message about EFI_LOADER
> > > >
> > > >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> > > >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> > > >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> > > >   3 files changed, 20 insertions(+), 9 deletions(-)
> > > > ---
> > > >   arch/sandbox/cpu/cache.c              |  8 +++++++-
> > > >   drivers/remoteproc/rproc-elf-loader.c | 18 +++++++++++-------
> > > >   lib/efi_loader/efi_image_loader.c     |  3 ++-
> > > >   3 files changed, 20 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/arch/sandbox/cpu/cache.c b/arch/sandbox/cpu/cache.c
> > > > index c8a5e64214b6..96b3da47e8ed 100644
> > > > --- a/arch/sandbox/cpu/cache.c
> > > > +++ b/arch/sandbox/cpu/cache.c
> > > > @@ -4,12 +4,18 @@
> > > >    */
> > > >
> > > >   #include <cpu_func.h>
> > > > +#include <mapmem.h>
> > > >   #include <asm/state.h>
> > > >
> > > >   void flush_cache(unsigned long addr, unsigned long size)
> > > >   {
> > > > +     void *ptr;
> > > > +
> > > > +     ptr = map_sysmem(addr, size);
> > > > +
> > > >       /* Clang uses (char *) parameters, GCC (void *) */
> > > > -     __builtin___clear_cache((void *)addr, (void *)(addr + size));
> > > > +     __builtin___clear_cache(map_sysmem(addr, size), ptr + size);
> > > > +     unmap_sysmem(ptr);
> > > >   }
> > > >
> > > >   void invalidate_icache_all(void)
> > > > diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> > > > index ab1836b3f078..0b3941b7798d 100644
> > > > --- a/drivers/remoteproc/rproc-elf-loader.c
> > > > +++ b/drivers/remoteproc/rproc-elf-loader.c
> > > > @@ -6,6 +6,7 @@
> > > >   #include <dm.h>
> > > >   #include <elf.h>
> > > >   #include <log.h>
> > > > +#include <mapmem.h>
> > > >   #include <remoteproc.h>
> > > >   #include <asm/cache.h>
> > > >   #include <dm/device_compat.h>
> > > > @@ -180,6 +181,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> > > >       for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
> > > >               void *dst = (void *)(uintptr_t)phdr->p_paddr;
> > > >               void *src = (void *)addr + phdr->p_offset;
> > > > +             ulong dst_addr;
> > > >
> > > >               if (phdr->p_type != PT_LOAD)
> > > >                       continue;
> > > > @@ -195,10 +197,11 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
> > > >               if (phdr->p_filesz != phdr->p_memsz)
> > > >                       memset(dst + phdr->p_filesz, 0x00,
> > > >                              phdr->p_memsz - phdr->p_filesz);
> > > > -             flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > > > -                         roundup((unsigned long)dst + phdr->p_filesz,
> > > > +             dst_addr = map_to_sysmem(dst);
> > > > +             flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > > > +                         roundup(dst_addr + phdr->p_filesz,
> > > >                                   ARCH_DMA_MINALIGN) -
> > > > -                         rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > > > +                         rounddown(dst_addr, ARCH_DMA_MINALIGN));
> > > >       }
> > > >
> > > >       return 0;
> > > > @@ -377,6 +380,7 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> > > >       const struct dm_rproc_ops *ops;
> > > >       Elf32_Shdr *shdr;
> > > >       void *src, *dst;
> > > > +     ulong dst_addr;
> > > >
> > > >       shdr = rproc_elf32_find_rsc_table(dev, fw_addr, fw_size);
> > > >       if (!shdr)
> > > > @@ -398,10 +402,10 @@ int rproc_elf32_load_rsc_table(struct udevice *dev, ulong fw_addr,
> > > >               (ulong)dst, *rsc_size);
> > > >
> > > >       memcpy(dst, src, *rsc_size);
> > > > -     flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> > > > -                 roundup((unsigned long)dst + *rsc_size,
> > > > -                         ARCH_DMA_MINALIGN) -
> > > > -                 rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> > > > +     dst_addr = map_to_sysmem(dst);
> > > > +     flush_cache(rounddown(dst_addr, ARCH_DMA_MINALIGN),
> > > > +                 roundup(dst_addr + *rsc_size, ARCH_DMA_MINALIGN) -
> > > > +                 rounddown(dst_addr, ARCH_DMA_MINALIGN));
> > > >
> > > >       return 0;
> > > >   }
> > > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > > > index 0ddf69a09183..bb58cf1badb7 100644
> > > > --- a/lib/efi_loader/efi_image_loader.c
> > > > +++ b/lib/efi_loader/efi_image_loader.c
> > > > @@ -13,6 +13,7 @@
> > > >   #include <efi_loader.h>
> > > >   #include <log.h>
> > > >   #include <malloc.h>
> > > > +#include <mapmem.h>
> > > >   #include <pe.h>
> > > >   #include <sort.h>
> > > >   #include <crypto/mscode.h>
> > > > @@ -977,7 +978,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> > > >       }
> > > >
> > > >       /* Flush cache */
> > > > -     flush_cache((ulong)efi_reloc,
> > > > +     flush_cache(map_to_sysmem(efi_reloc),
> > > >                   ALIGN(virt_size, EFI_CACHELINE_SIZE));
> > >
> > > It would be nice if we could be consistent:
> > >
> > > include/cpu_func.h:66:
> > > void flush_cache(unsigned long addr, unsigned long size);
> > >
> > > arch/arc/lib/cache.c:773:
> > > void flush_cache(unsigned long start, unsigned long size)
> > >
> > > arch/sandbox/cpu/cache.c:9:
> > > void flush_cache(unsigned long addr, unsigned long size)
> >
> > Patches are welcome :-)
> >
> > They should all be ulong
>
> Please note that they are all consistent in prototypes and
> functionality. arc just says "start" rather than "addr" and everyone
> uses them in the same way. The issue, as the Fixes tag shows, is that
> sandbox's isn't complete.

I wasn't suggesting you change this patch, just suggesting what
Heinrich might do if he wants to send a separate patch.

Regards,
Simon

Re: [v6 0/12] CI: Set up for an arm64 runner

[Tom Rini]

On Wed, 27 Nov 2024 11:17:18 -0600, Tom Rini wrote:

> Hey all,
> 
> This is picking up Simon's v5 of the above-named series and making a few
> more changes so that the follow-up series I have leads to arm64 being
> supported for almost all jobs. To quote Simon's cover letter:
> 
> All gitlab runners are currently amd64 machines. This series attempts to
> create a docker image which can also support arm64 so that sandbox tests
> can be run on it.
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom

Re: [v6 0/12] CI: Set up for an arm64 runner

[Simon Glass]

On Tue, 3 Dec 2024 at 12:57, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, 27 Nov 2024 11:17:18 -0600, Tom Rini wrote:
>
> > Hey all,
> >
> > This is picking up Simon's v5 of the above-named series and making a few
> > more changes so that the follow-up series I have leads to arm64 being
> > supported for almost all jobs. To quote Simon's cover letter:
> >
> > All gitlab runners are currently amd64 machines. This series attempts to
> > create a docker image which can also support arm64 so that sandbox tests
> > can be run on it.
> >
> > [...]
>
> Applied to u-boot/next, thanks!

Applied to sjg/master, thanks!


- Simon