Am 4. November 2024 14:39:45 MEZ schrieb 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.

As discussed some time ago, it would be good to update the way EFI_LOADER uses addresses and pointers, particularly around memory allocation. Ideally we would use addresses internally, with pointers only exposed via the external API, even where pointers are in fact u64 values.

No, we should get rid of sandbox virtual addresses where possible, e.g. in LMB.

They are are only relevant for the sandbox's command line interface and device-tree.

This will minimize the number of conversions, which only satisfy the needs of the sandbox and have no relevance on real systems including QEMU.

Do not wag the dog with the tail.

Best regards

Heinrich

Signed-off-by: Simon Glass sjg@chromium.org Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")

---

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 c8a5e64214b..96b3da47e8e 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 ab1836b3f07..0b3941b7798 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 0ddf69a0918..bb58cf1badb 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));

  /*

Hi Heinrich,

On Mon, 4 Nov 2024 at 13:51, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

Am 4. November 2024 14:39:46 MEZ schrieb 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.

Signed-off-by: Simon Glass sjg@chromium.org

test/print_ut.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/print_ut.c b/test/print_ut.c index f5e607b21a3..7e5c015f3a7 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -242,7 +242,7 @@ PRINT_TEST(print_display_buffer, UTF_CONSOLE);

static int print_hexdump_line(struct unit_test_state *uts) {

•  char *linebuf;
  

•  u8 *linebuf;
  u8 *buf;
  int i;
  
  

@@ -255,10 +255,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]);
  

Should we add a ut_asserteq_str() here to check all characters?

I believe it is...that's why it checks the \0 at the end of the string. Or are you asking about the bytes after the string?

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

...

•  ut_asserteq(-1, linebuf[76]);
  

•  ut_asserteq(0xff, linebuf[76]);
  
  unmap_sysmem(buf);
  
  

Regards, Simon

On Mon, Nov 04, 2024 at 06:39:47AM -0700, Simon Glass wrote:

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

Signed-off-by: Simon Glass sjg@chromium.org

tools/docker/Dockerfile | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 967ac89fbde..c9d794082c8 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -2,6 +2,9 @@ # This Dockerfile is used to build an image containing basic stuff to be used # to build U-Boot and run our test suites.

+# Build with (for example): +# docker buildx build --platform linux/arm64/v8,linux/amd64 --tag sjg20/u-boot-gitlab-ci-runner-multiarch:jammy-20240808-03Nov2024 .

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" @@ -9,6 +12,10 @@ LABEL org.opencontainers.image.description=" This image is for building U-Boot i # Make sure apt is happy ENV DEBIAN_FRONTEND=noninteractive

+ARG TARGETPLATFORM +ARG BUILDPLATFORM +RUN echo "Building $BUILDPLATFORM, for target $TARGETPLATFORM"

# Add LLVM repository RUN apt-get update && apt-get install -y gnupg2 wget xz-utils && rm -rf /var/lib/apt/lists/* RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -

Please update doc/build/docker.rst as well with this change, thanks.

Am 4. November 2024 14:39:48 MEZ schrieb Simon Glass sjg@chromium.org:

The package names are slightly different for arm64 and we don't seem to have a linux-image-kvm package. Provide a different set for arm64

Signed-off-by: Simon Glass sjg@chromium.org

tools/docker/Dockerfile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index c9d794082c8..c4c3dc5a901 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -39,7 +39,12 @@ 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-20... | tar -C /opt -xz

# Update and install things from apt now -RUN apt-get update && apt-get install -y \ +RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \

Please, use 'arm64' and not 'linux/arm64' as variable value.

• EXTRA_PACKAGES="grub-efi-arm64-bin linux-image-generic"; \
  

Replace, arm64 by the variable, which may take the value 'riscv64'.

• else \

This 'else' does not work for riscv64.

The exception is amd64 and not arm64. Only amd64 has two GRUB versions.

Best regards

Heinrich

• EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \
  

• fi; \
• apt-get update && apt-get install -y \ automake \ autopoint \ bc \

@@ -65,13 +70,10 @@ RUN apt-get update && apt-get install -y \ gnu-efi \ gnutls-dev \ graphviz \

• grub-efi-amd64-bin \
• grub-efi-ia32-bin \ help2man \ iasl \ imagemagick \ iputils-ping \
• libc6-i386 \ libconfuse-dev \ libgit2-dev \ libjson-glib-dev \

@@ -89,7 +91,6 @@ RUN apt-get update && apt-get install -y \ libtool \ libudev-dev \ libusb-1.0-0-dev \

• linux-image-kvm \ lzma-alone \ lzop \ mount \

@@ -126,6 +127,7 @@ RUN apt-get update && apt-get install -y \ xilinx-bootgen \ xxd \ zip \

• ${EXTRA_PACKAGES} \ && rm -rf /var/lib/apt/lists/*

# Make kernels readable for libguestfs tools to work correctly

Hi Tom, Heinrich,

On Mon, 4 Nov 2024 at 14:34, Tom Rini trini@konsulko.com wrote:

On Mon, Nov 04, 2024 at 09:58:50PM +0100, Heinrich Schuchardt wrote:

...

Am 4. November 2024 14:39:48 MEZ schrieb Simon Glass sjg@chromium.org:

...

The package names are slightly different for arm64 and we don't seem to have a linux-image-kvm package. Provide a different set for arm64

Signed-off-by: Simon Glass sjg@chromium.org

tools/docker/Dockerfile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index c9d794082c8..c4c3dc5a901 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -39,7 +39,12 @@ 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-20... | tar -C /opt -xz

# Update and install things from apt now -RUN apt-get update && apt-get install -y \ +RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \

Please, use 'arm64' and not 'linux/arm64' as variable value.

Getting back to your previous comment about documenting these variables, I took it that these are NOT free-form but rather what Docker (et al) require for proper multi-architecture containering.

Yes, that's my understanding (limited though it is so far)

Regards, Simon

Am 4. November 2024 14:39:49 MEZ schrieb Simon Glass sjg@chromium.org:

This doesn't build on arm64 at present, so drop it for now. We only expect to run sandbox tests, so perhaps it isn't needed.

Signed-off-by: Simon Glass sjg@chromium.org

tools/docker/Dockerfile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index c4c3dc5a901..60106a82801 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -134,7 +134,9 @@ RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \ 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 && \ +# This fails on arm64 +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \

• git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ cd /tmp/grub && \ git checkout grub-2.06 && \

Upstream GRUB builds for arm64 and riscv64.

Please, retry with grub-2.12.

Best regards

Heinrich

git config --global user.name "GitLab CI Runner" && \ @@ -184,7 +186,8 @@ 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 && \

• rm -rf /tmp/grub

• rm -rf /tmp/grub; \
• fi

RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \ cd /tmp/qemu && \

On 11/4/24 14:39, Simon Glass wrote:

This doesn't build on arm64 at present, so drop it for now. This can be revisited if we wish to support arm64 for the sandbox trace-test.

I had no problems building the trace libraries on Ubuntu 24.04 arm64. So the problem you experienced should be easily fixable.

Best regards

Heinrich

Signed-off-by: Simon Glass sjg@chromium.org

tools/docker/Dockerfile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 60106a82801..73fdd1a4302 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -240,7 +240,8 @@ RUN git clone https://github.com/stefanberger/swtpm /tmp/swtpm && \ rm -rf /tmp/swtpm

# Build trace-cmd -RUN mkdir /tmp/trace && \ +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \

• mkdir /tmp/trace && \ git clone https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git /tmp/trace/libtraceevent && \ cd /tmp/trace/libtraceevent && \ make -j$(nproc) && \

@@ -253,7 +254,8 @@ RUN mkdir /tmp/trace && \ cd /tmp/trace/trace-cmd && \ make -j$(nproc) && \ sudo make install && \

• rm -rf /tmp/trace

• rm -rf /tmp/trace; \

• fi

  # Build coreboot RUN wget -O - https://coreboot.org/releases/coreboot-24.08.tar.xz | tar -C /tmp -xJ && \

On 11/4/24 14:39, Simon Glass wrote:

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

---

tools/docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index ae3d4685869..c42e0890897 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -301,4 +301,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"