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

[Heinrich Schuchardt]

On 12.11.24 15:28, Heinrich Schuchardt wrote:

On 12.11.24 15:10, Simon Glass wrote:

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

We should throw out all these sandbox virtual addresses where they are
not needed.

Just add the missing conversion to drivers/remoteproc/rproc-elf-loader.c.

And change flush_cache() on all architectures to use void *.

Best regards

Heinrich

There are a number of places where the flush_cache invocation currently
assumes that sandbox virtual addresses are used.

We will have to wait until the complete separation of CLI from the rest
of U-Boot to clean this up.

Acked-by: Heinrich Schuchardt <xypron.g...@gmx.de>

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 <s...@chromium.org>
Fixes: 3286d223fd7 ("sandbox: implement invalidate_icache_all()")
---

(no changes since v2)

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

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

      /*