Re: [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC

[Heinrich Schuchardt]

On 10/10/21 23:48, marek.va...@gmail.com wrote:

From: Marek Vasut <marek.vasut+rene...@gmail.com>

In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
relocation code breaks down because it assumes gd->relocaddr
points to relocated U-Boot code, which is not the case. Add
special case for handling GD_FLG_SKIP_RELOC, which uses the
__image_copy_start instead of gd->relocaddr for efi loader
code relocation source address.

GD_FLG_SKIP_RELOC is only to be used by the x86 EFI app which is
incompatible with CONFIG_EFI_LOADER=y.
lib/efi/efi_app.c:131:  board_init_f(GD_FLG_SKIP_RELOC);

Why do we need this patch?

Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Tom Rini <tr...@konsulko.com>
---
  common/board_r.c             |  7 ++++++-
  lib/efi_loader/efi_runtime.c | 19 ++++++++++++++++---
  2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 31a59c585a..3e95123f95 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -178,7 +178,12 @@ static int initr_reloc_global_data(void)
         */
        efi_save_gd();

-       efi_runtime_relocate(gd->relocaddr, NULL);
+#ifdef CONFIG_ARM

Why should this be ARM specific?

Best regards

Heinrich

+       if (gd->flags & GD_FLG_SKIP_RELOC)
+               efi_runtime_relocate((ulong)__image_copy_start, NULL);
+       else
+#endif
+               efi_runtime_relocate(gd->relocaddr, NULL);
  #endif

        return 0;
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 93a695fc27..876ca09106 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -17,6 +17,8 @@
  #include <asm/global_data.h>
  #include <u-boot/crc.h>

+#include <asm/sections.h>
+
  /* For manual relocation support */
  DECLARE_GLOBAL_DATA_PTR;

@@ -629,13 +631,23 @@ out:
        return ret;
  }

+static unsigned long efi_get_reloc_start(void)
+{
+#ifdef CONFIG_ARM
+       if (gd->flags & GD_FLG_SKIP_RELOC)
+               return (unsigned long)__image_copy_start;
+       else
+#endif
+               return gd->relocaddr;
+}
+
  static __efi_runtime void efi_relocate_runtime_table(ulong offset)
  {
        ulong patchoff;
        void **pos;

        /* Relocate the runtime services pointers */
-       patchoff = offset - gd->relocaddr;
+       patchoff = offset - efi_get_reloc_start();
        for (pos = (void **)&efi_runtime_services.get_time;
             pos <= (void **)&efi_runtime_services.query_variable_info; ++pos) {
                if (*pos)
@@ -681,7 +693,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc 
*map)
                ulong *p;
                ulong newaddr;

-               p = (void*)((ulong)rel->offset - base) + gd->relocaddr;
+               p = (void*)((ulong)rel->offset - base) + efi_get_reloc_start();

                /*
                 * The runtime services table is updated in
@@ -852,7 +864,8 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
                map = (void*)virtmap + (descriptor_size * i);
                if (map->type == EFI_RUNTIME_SERVICES_CODE) {
                        ulong new_offset = map->virtual_start -
-                                          map->physical_start + gd->relocaddr;
+                                          map->physical_start +
+                                          efi_get_reloc_start();

                        efi_relocate_runtime_table(new_offset);
                        efi_runtime_relocate(new_offset, map);