[PATCH 0/9] CI: Set up for an arm64 runner

[PATCH 0/9] CI: Set up for an arm64 runner

[Simon Glass]

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.


Simon Glass (9):
  sandbox: efi_loader: Correct use of addresses as pointers
  test: Adjust print_ut test to use unsigned char
  docker: Support building for multiple architectures
  docker: Adjust packages for arm64
  docker: Drop grub for arm64
  docker: Drop tracing for arm64
  docker: Drop coreboot for arm64
  docker: Fix LegacyKeyValueFormat warning with PYTHONPATH
  CI: Add platform variable

 .gitlab-ci.yml                        |  1 +
 arch/sandbox/cpu/cache.c              |  8 +++++-
 drivers/remoteproc/rproc-elf-loader.c | 18 +++++++-----
 lib/efi_loader/efi_image_loader.c     |  3 +-
 test/print_ut.c                       |  6 ++--
 tools/docker/Dockerfile               | 40 +++++++++++++++++++--------
 6 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH 1/9] sandbox: efi_loader: Correct use of addresses as pointers

[Simon Glass]

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.

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));
 
 	/*
-- 
2.34.1

Re: [PATCH 1/9] sandbox: efi_loader: Correct use of addresses as pointers

[Heinrich Schuchardt]

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));
> 
> 	/*

Re: [PATCH 1/9] sandbox: efi_loader: Correct use of addresses as pointers

[Simon Glass]

Hi Heinrich,

On Mon, 4 Nov 2024 at 13:37, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> 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.

For now I'll just remove that comment from this commit. We use ulong
in U-Boot for addresses. Everything works fine and we only get in
trouble when we use a ulong as a pointer. Let's discuss this in more
detail at some point.

>
> 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(-)
> >

Regards,
Simon

[PATCH 2/9] test: Adjust print_ut test to use unsigned char

[Simon Glass]

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]);
-	ut_asserteq(-1, linebuf[76]);
+	ut_asserteq(0xff, linebuf[76]);
 
 	unmap_sysmem(buf);
 
-- 
2.34.1

Re: [PATCH 2/9] test: Adjust print_ut test to use unsigned char

[Heinrich Schuchardt]

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?

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

>-	ut_asserteq(-1, linebuf[76]);
>+	ut_asserteq(0xff, linebuf[76]);
> 
> 	unmap_sysmem(buf);
> 

Re: [PATCH 2/9] test: Adjust print_ut test to use unsigned char

[Simon Glass]

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

[PATCH 3/9] docker: Support building for multiple architectures

[Simon Glass]

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 -
-- 
2.34.1

Re: [PATCH 3/9] docker: Support building for multiple architectures

[Tom Rini]

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

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.

-- 
Tom

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

Re: [PATCH 3/9] docker: Support building for multiple architectures

[Heinrich Schuchardt]

Am 4. November 2024 14:39:47 MEZ schrieb Simon Glass <sjg@chromium.org>:
>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

Please, add comments describing what these variables are used for.

Best regards

Heinrich

>+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 -

[PATCH 4/9] docker: Adjust packages for arm64

[Simon Glass]

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-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 if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
+		EXTRA_PACKAGES="grub-efi-arm64-bin linux-image-generic"; \
+	else \
+		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
-- 
2.34.1

Re: [PATCH 4/9] docker: Adjust packages for arm64

[Tom Rini]

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

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

> 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-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 if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
> +		EXTRA_PACKAGES="grub-efi-arm64-bin linux-image-generic"; \
> +	else \
> +		EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \
> +	fi; \

Lets try using grub-efi (dummy package that should grab the correct
architecture one) and see if linux-image-generic provides the vmlinux
file we annoyingly need for both architectures. I _think_ we can stop
explicitly listing libc6-i386, but may need to list grub-efi-ia32-bin
too as grub-efi will only grab the amd64 one.

-- 
Tom

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

Re: [PATCH 4/9] docker: Adjust packages for arm64

[Heinrich Schuchardt]

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-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 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

Re: [PATCH 4/9] docker: Adjust packages for arm64

[Tom Rini]

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

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-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 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.

-- 
Tom

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

Re: [PATCH 4/9] docker: Adjust packages for arm64

[Simon Glass]

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-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 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

[PATCH 5/9] docker: Drop grub for arm64

[Simon Glass]

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 && \
 	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 && \
-- 
2.34.1

Re: [PATCH 5/9] docker: Drop grub for arm64

[Tom Rini]

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

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

> 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(-)

What is the build failure? Depending on what other prospects for arm64
build hardware come through I'm hoping to look harder at more build
options, not just sandbox (but of course, QEMU shouldn't matter what the
host is).

-- 
Tom

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

Re: [PATCH 5/9] docker: Drop grub for arm64

[Simon Glass]

Hi Tom,

On Mon, 4 Nov 2024 at 06:56, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Nov 04, 2024 at 06:39:49AM -0700, Simon Glass wrote:
>
> > 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(-)
>
> What is the build failure? Depending on what other prospects for arm64
> build hardware come through I'm hoping to look harder at more build
> options, not just sandbox (but of course, QEMU shouldn't matter what the
> host is).

field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast
-Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var
-Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes
-Wmissing-declarations -Wformat=2
686.0 checking for aarch64-gcc...
/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc
686.0 checking for aarch64-objcopy...
/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-objcopy
686.0 checking for aarch64-strip...
/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-strip
686.0 checking for aarch64-nm...
/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-nm
686.0 checking for aarch64-ranlib...
/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-ranlib
686.0 ./configure: line 32517:
/opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc: No such
file or directory
686.1 checking which extra warnings work...
689.6 checking if compiling with clang... yes
689.8 checking for options to compile assembly...
689.8 checking whether -freg-struct-return works... no
690.0 checking for options to get soft-float... no
690.8 configure: error: could not force soft-float

I suspect it is just that the toolchains need fiddling. I'd much
rather get something running and iterate on it after that.

Regards,
Simon

Re: [PATCH 5/9] docker: Drop grub for arm64

[Tom Rini]

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

On Tue, Nov 05, 2024 at 08:15:05AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 4 Nov 2024 at 06:56, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Nov 04, 2024 at 06:39:49AM -0700, Simon Glass wrote:
> >
> > > 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(-)
> >
> > What is the build failure? Depending on what other prospects for arm64
> > build hardware come through I'm hoping to look harder at more build
> > options, not just sandbox (but of course, QEMU shouldn't matter what the
> > host is).
> 
> field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast
> -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var
> -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes
> -Wmissing-declarations -Wformat=2
> 686.0 checking for aarch64-gcc...
> /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc
> 686.0 checking for aarch64-objcopy...
> /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-objcopy
> 686.0 checking for aarch64-strip...
> /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-strip
> 686.0 checking for aarch64-nm...
> /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-nm
> 686.0 checking for aarch64-ranlib...
> /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-ranlib
> 686.0 ./configure: line 32517:
> /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc: No such
> file or directory
> 686.1 checking which extra warnings work...
> 689.6 checking if compiling with clang... yes
> 689.8 checking for options to compile assembly...
> 689.8 checking whether -freg-struct-return works... no
> 690.0 checking for options to get soft-float... no
> 690.8 configure: error: could not force soft-float
> 
> I suspect it is just that the toolchains need fiddling. I'd much
> rather get something running and iterate on it after that.

That's the toolchain that should be used to build sandbox, so you should
figure out what's going on. And I think you missed fiddling with
~/.buildman so that it sets the sandbox toolchain to the right one and
not x86_64.

-- 
Tom

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

Re: [PATCH 5/9] docker: Drop grub for arm64

[Simon Glass]

Hi Tom,

On Tue, 5 Nov 2024 at 08:44, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Nov 05, 2024 at 08:15:05AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 4 Nov 2024 at 06:56, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Nov 04, 2024 at 06:39:49AM -0700, Simon Glass wrote:
> > >
> > > > 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(-)
> > >
> > > What is the build failure? Depending on what other prospects for arm64
> > > build hardware come through I'm hoping to look harder at more build
> > > options, not just sandbox (but of course, QEMU shouldn't matter what the
> > > host is).
> >
> > field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast
> > -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var
> > -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes
> > -Wmissing-declarations -Wformat=2
> > 686.0 checking for aarch64-gcc...
> > /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc
> > 686.0 checking for aarch64-objcopy...
> > /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-objcopy
> > 686.0 checking for aarch64-strip...
> > /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-strip
> > 686.0 checking for aarch64-nm...
> > /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-nm
> > 686.0 checking for aarch64-ranlib...
> > /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-ranlib
> > 686.0 ./configure: line 32517:
> > /opt/gcc-13.2.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc: No such
> > file or directory
> > 686.1 checking which extra warnings work...
> > 689.6 checking if compiling with clang... yes
> > 689.8 checking for options to compile assembly...
> > 689.8 checking whether -freg-struct-return works... no
> > 690.0 checking for options to get soft-float... no
> > 690.8 configure: error: could not force soft-float
> >
> > I suspect it is just that the toolchains need fiddling. I'd much
> > rather get something running and iterate on it after that.
>
> That's the toolchain that should be used to build sandbox, so you should
> figure out what's going on. And I think you missed fiddling with
> ~/.buildman so that it sets the sandbox toolchain to the right one and
> not x86_64.

sandbox should be built with the host toolchain, right? The one it is
complaining about is the amd64-host aarch64 toolchain, which we don't
want or need yet.

The grub part (and the cross toolchains) needs more work, but we don't
need it for sandbox, or at least the sandbox tests seem to pass
without it. Are you wanting to run qemu on arm64 to boot grub?

I believe I've taken a reasonable stab at a good starting point for
this work. Do you agree?

Regards,
Simon

Re: [PATCH 5/9] docker: Drop grub for arm64

[Heinrich Schuchardt]

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 && \

Re: [PATCH 5/9] docker: Drop grub for arm64

[Tom Rini]

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

On Mon, Nov 04, 2024 at 10:04:39PM +0100, Heinrich Schuchardt wrote:
> 
> 
> 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.

Well, lets do that upgrade separately.

-- 
Tom

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

[PATCH 6/9] docker: Drop tracing for arm64

[Simon Glass]

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.

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 && \
-- 
2.34.1

Re: [PATCH 6/9] docker: Drop tracing for arm64

[Tom Rini]

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

On Mon, Nov 04, 2024 at 06:39:50AM -0700, 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.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

What's the build error? We may well want to support this, yes.

-- 
Tom

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

Re: [PATCH 6/9] docker: Drop tracing for arm64

[Heinrich Schuchardt]

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 && \

Re: [PATCH 6/9] docker: Drop tracing for arm64

[Simon Glass]

Hi,

On Mon, 4 Nov 2024 at 14:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> 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.

49.20 /usr/bin/ld: /usr/local/lib64/libtracefs.so: undefined reference
to `trace_seq_vprintf'

I agree we probably want to support this, but I think we can sort it out later.

Regards,
Simon

[PATCH 7/9] docker: Drop coreboot for arm64

[Simon Glass]

This isn't needed for sandbox builds and coreboot isn't much used on
arm64, nor does U-Boot have a build that supports running from coreboot
on any ARM target.

So drop it.

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 73fdd1a4302..ae3d4685869 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -258,7 +258,8 @@ RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then  \
     fi
 
 # Build coreboot
-RUN wget -O - https://coreboot.org/releases/coreboot-24.08.tar.xz | tar -C /tmp -xJ && \
+RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \
+    wget -O - https://coreboot.org/releases/coreboot-24.08.tar.xz | tar -C /tmp -xJ && \
     cd /tmp/coreboot-24.08 && \
     make crossgcc-i386 CPUS=$(nproc) && \
     make -C payloads/coreinfo olddefconfig && \
@@ -269,7 +270,8 @@ RUN wget -O - https://coreboot.org/releases/coreboot-24.08.tar.xz | tar -C /tmp
     make olddefconfig && \
     make -j $(nproc) && \
     sudo mkdir /opt/coreboot && \
-    sudo cp build/coreboot.rom build/cbfstool /opt/coreboot/
+    sudo cp build/coreboot.rom build/cbfstool /opt/coreboot/; \
+    fi
 
 # Create our user/group
 RUN echo uboot ALL=NOPASSWD: ALL > /etc/sudoers.d/uboot
-- 
2.34.1

Re: [PATCH 7/9] docker: Drop coreboot for arm64

[Tom Rini]

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

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

> This isn't needed for sandbox builds and coreboot isn't much used on
> arm64, nor does U-Boot have a build that supports running from coreboot
> on any ARM target.
> 
> So drop it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

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

[PATCH 8/9] docker: Fix LegacyKeyValueFormat warning with PYTHONPATH

[Simon Glass]

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>
---

 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"
-- 
2.34.1

Re: [PATCH 8/9] docker: Fix LegacyKeyValueFormat warning with PYTHONPATH

[Tom Rini]

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

On Mon, Nov 04, 2024 at 06:39:52AM -0700, 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: Tom Rini <trini@konsulko.com>

-- 
Tom

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

Re: [PATCH 8/9] docker: Fix LegacyKeyValueFormat warning with PYTHONPATH

[Heinrich Schuchardt]

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"

[PATCH 9/9] CI: Add platform variable

[Simon Glass]

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

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

 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 0aeda53bc2d..e152060b046 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -3,6 +3,7 @@
 variables:
   DEFAULT_TAG: ""
   MIRROR_DOCKER: docker.io
+  PLATFORM: linux/amd64,linux/arm64
 
 default:
   tags:
-- 
2.34.1

Re: [PATCH 9/9] CI: Add platform variable

[Tom Rini]

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

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

> 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>

-- 
Tom

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

Re: [PATCH 0/9] CI: Set up for an arm64 runner

[Tom Rini]

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

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

> 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.

Thanks for starting on this.

-- 
Tom

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

Re: [PATCH 0/9] CI: Set up for an arm64 runner

[Ilias Apalodimas]

Thanks for this

On Mon, 4 Nov 2024 at 13:55, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Nov 04, 2024 at 06:39:44AM -0700, Simon Glass wrote:
>
> > 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.
>
> Thanks for starting on this.

Tom I'll probably have an arm64 runner available next week. Let me
know how/when start testing this

Cheers
/Ilias
>
> --
> Tom