[PATCH 0/7] arm-uboot: support for different RAM bases

[PATCH 0/7] arm-uboot: support for different RAM bases

[Ian Campbell]

The current arm-uboot image type is hardcoded to run at address
0x08000000 which means that it cannot be used on ARM systems which do
not have RAM starting from address 0 (which is many of them). This is a
bit of a problem for distros.

The following series attempts to address this by only performing a
partial link of the arm-uboot kernel.img at build time and enhancing
grub-mkimage to finalise the link to a specific address while building
the core.img.

I have test this on a Midway system, which has RAM at address 0, linking
to a variety of addresses (0x{0,1,4}8000000) and on a cubieboard2 which
has RAM at 0x40000000-0x80000000 linked at 0x48000000. (there is an
unrelated issue on cubieboard2 relating to the scan for the uboot API
signature, which I'm still investigating, but I hacked around it here
and things worked fine otherwise).

I have done build only tests of arm64-efi, i386-pc and x86_64-efi. After
running grub-mkimage under faketime the resulting binaries are bit for
bit identical in every case (faketime due to the timestamp in the PE
header).

I haven't yet integrated this into grub-install, I need to think a bit
more about how to automagically determine the correct link address for a
given platform.

If no target address is given then the existing hardcoded address
(0x08000000) is used as the default.

Please CC on any replies since I'm not subscribed.

Ian.

[PATCH 1/7] mkimage: Refactor IMAGE_EFI checks into a function grub_image_needs_reloc.

[Ian Campbell]

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 util/grub-mkimagexx.c | 12 ++++++------
 util/mkimage.c        |  7 +++++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index 0a1ac9e..d059396 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -1371,7 +1371,7 @@ SUFFIX (locate_sections) (const char *kernel_path,
 	grub_util_info ("locating the section %s at 0x%"
 			GRUB_HOST_PRIxLONG_LONG,
 			name, (unsigned long long) current_address);
-	if (image_target->id != IMAGE_EFI)
+	if (!grub_image_needs_reloc(image_target))
 	  {
 	    current_address = grub_host_to_target_addr (s->sh_addr)
 	      - image_target->link_addr;
@@ -1413,7 +1413,7 @@ SUFFIX (locate_sections) (const char *kernel_path,
 	grub_util_info ("locating the section %s at 0x%"
 			GRUB_HOST_PRIxLONG_LONG,
 			name, (unsigned long long) current_address);
-	if (image_target->id != IMAGE_EFI)
+	if (!grub_image_needs_reloc(image_target))
 	  current_address = grub_host_to_target_addr (s->sh_addr)
 	    - image_target->link_addr;
 	section_addresses[i] = current_address;
@@ -1486,7 +1486,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
   for (i = 0; i < num_sections; i++)
     section_vaddresses[i] = section_addresses[i] + image_target->vaddr_offset;
 
-  if (image_target->id != IMAGE_EFI)
+  if (!grub_image_needs_reloc(image_target))
     {
       Elf_Addr current_address = *kernel_sz;
 
@@ -1507,7 +1507,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 	    grub_util_info ("locating the section %s at 0x%"
 			    GRUB_HOST_PRIxLONG_LONG,
 			    name, (unsigned long long) current_address);
-	    if (image_target->id != IMAGE_EFI)
+	    if (!grub_image_needs_reloc(image_target))
 	      current_address = grub_host_to_target_addr (s->sh_addr)
 		- image_target->link_addr;
 
@@ -1528,7 +1528,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
       || image_target->id == IMAGE_SPARC64_CDCORE)
     *kernel_sz = ALIGN_UP (*kernel_sz, image_target->mod_align);
 
-  if (image_target->id == IMAGE_EFI)
+  if (grub_image_needs_reloc(image_target))
     {
       symtab_section = NULL;
       for (i = 0, s = sections;
@@ -1588,7 +1588,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 
   out_img = xmalloc (*kernel_sz + total_module_size);
 
-  if (image_target->id == IMAGE_EFI)
+  if (grub_image_needs_reloc(image_target))
     {
       *start = SUFFIX (relocate_symbols) (e, sections, symtab_section,
 					  section_vaddresses, section_entsize,
diff --git a/util/mkimage.c b/util/mkimage.c
index 26d9816..645e296 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -922,6 +922,13 @@ grub_arm_reloc_jump24 (grub_uint32_t *target, Elf32_Addr sym_addr)
   return GRUB_ERR_NONE;
 }
 
+static int grub_image_needs_reloc(const struct grub_install_image_target_desc *target)
+{
+  if (target->id == IMAGE_EFI)
+    return 1;
+  return 0;
+}
+
 #pragma GCC diagnostic ignored "-Wcast-align"
 
 #define MKIMAGE_ELF32 1
-- 
1.8.4.rc3

[PATCH 2/7] mkimage: Replace hardcoded 0x400 in R_ARM_ABS32 relocation

[Ian Campbell]

In http://lists.gnu.org/archive/html/grub-devel/2013-04/msg00034.html Leif
suggested that this should actually be "SUFFIX (entry_point)" saying:
    _start gets bumped down a bit, so all relative relocations need to follow,
    and then I "un-adjust" for the absolutes. Not too proud of that bit of code.

I think this "bumping down" actually comes from the image type's vaddr_offset
and therefore it is more correct to adjust by this amount.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: leif.lindholm@linaro.org
---
 util/grub-mkimagexx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index d059396..f5e514f 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -868,7 +868,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
 		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
 				       (int) sym_addr, (int) sym_addr);
 		       /* Data will be naturally aligned */
-		       sym_addr += 0x400;
+		       sym_addr += image_target->vaddr_offset;
 		       *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr);
 		     }
 		     break;
-- 
1.8.4.rc3

[PATCH 3/7] mkimage: account for space for trampolines earlier

[Ian Campbell]

In particular we will want to do this before assigning space for .bss,
otherwise the trampoline will potentially overlap the start of .bss and might
be cleared before it is used.

I'm in two minds about whether this is fixing and actual (perhaps latent) bug
or not. AFAICT none of the existing targets which need trampolines require a
.bss assignment here (e.g. they generate PE images which means the .bss gets
taken care of at load time).

Apart from duplicating the surrounding if statement this is pure code motion.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 util/grub-mkimagexx.c | 77 ++++++++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index f5e514f..399ff6a 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -1481,6 +1481,46 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 						exec_size, kernel_sz, align,
 						image_target);
 
+  if (grub_image_needs_reloc(image_target))
+    {
+#ifdef MKIMAGE_ELF32
+      if (image_target->elf_target == EM_ARM)
+	{
+	  grub_size_t tramp;
+
+	  *kernel_sz = ALIGN_UP (*kernel_sz, 16);
+
+	  tramp = arm_get_trampoline_size (e, sections, section_entsize,
+					   num_sections, image_target);
+
+	  tramp_off = *kernel_sz;
+	  *kernel_sz += ALIGN_UP (tramp, 16);
+	}
+#endif
+
+#ifdef MKIMAGE_ELF64
+      if (image_target->elf_target == EM_IA_64)
+	{
+	  grub_size_t tramp;
+
+	  *kernel_sz = ALIGN_UP (*kernel_sz, 16);
+
+	  grub_ia64_dl_get_tramp_got_size (e, &tramp, &got);
+
+	  tramp_off = *kernel_sz;
+	  *kernel_sz += ALIGN_UP (tramp, 16);
+
+	  ia64jmp_off = *kernel_sz;
+	  ia64jmpnum = SUFFIX (count_funcs) (e, symtab_section,
+					     image_target);
+	  *kernel_sz += 16 * ia64jmpnum;
+
+	  ia64_got_off = *kernel_sz;
+	  *kernel_sz += ALIGN_UP (got, 16);
+	}
+#endif
+    }
+
   section_vaddresses = xmalloc (sizeof (*section_addresses) * num_sections);
 
   for (i = 0; i < num_sections; i++)
@@ -1540,43 +1580,6 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 	    break;
 	  }
 
-#ifdef MKIMAGE_ELF32
-      if (image_target->elf_target == EM_ARM)
-	{
-	  grub_size_t tramp;
-
-	  *kernel_sz = ALIGN_UP (*kernel_sz, 16);
-
-	  tramp = arm_get_trampoline_size (e, sections, section_entsize,
-					   num_sections, image_target);
-
-	  tramp_off = *kernel_sz;
-	  *kernel_sz += ALIGN_UP (tramp, 16);
-	}
-#endif
-
-#ifdef MKIMAGE_ELF64
-      if (image_target->elf_target == EM_IA_64)
-	{
-	  grub_size_t tramp;
-
-	  *kernel_sz = ALIGN_UP (*kernel_sz, 16);
-
-	  grub_ia64_dl_get_tramp_got_size (e, &tramp, &got);
-
-	  tramp_off = *kernel_sz;
-	  *kernel_sz += ALIGN_UP (tramp, 16);
-
-	  ia64jmp_off = *kernel_sz;
-	  ia64jmpnum = SUFFIX (count_funcs) (e, symtab_section,
-					     image_target);
-	  *kernel_sz += 16 * ia64jmpnum;
-
-	  ia64_got_off = *kernel_sz;
-	  *kernel_sz += ALIGN_UP (got, 16);
-	}
-#endif
-
       if (! symtab_section)
 	grub_util_error ("%s", _("no symbol table"));
     }
-- 
1.8.4.rc3

[PATCH 4/7] mkimage: make R_ARM_ABS32 debug output more consistent

[Ian Campbell]

The decimal target is not very useful and all the other arm reloc debugging
prints a hex target and offset, so do the same.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 util/grub-mkimagexx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index 399ff6a..759e838 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -865,8 +865,10 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
 		   {
 		   case R_ARM_ABS32:
 		     {
-		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
-				       (int) sym_addr, (int) sym_addr);
+		       grub_util_info ("  ABS32:\ttarget=0x%08lx\toffset=(0x%08x)",
+				       (unsigned long) ((char *) target
+							- (char *) e),
+				       sym_addr);
 		       /* Data will be naturally aligned */
 		       sym_addr += image_target->vaddr_offset;
 		       *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr);
-- 
1.8.4.rc3

[PATCH 5/7] mkimage: allow linking at address 0

[Ian Campbell]

Running from address 0 is plausible at least in principal. Use the maximum
possible address as a sentinal instead.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 util/grub-mkimagexx.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index 759e838..b4216ff 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -64,6 +64,8 @@
 
 static Elf_Addr SUFFIX (entry_point);
 
+#define INVALID_START_ADDR (~((Elf_Addr)0))
+
 static void
 SUFFIX (generate_elf) (const struct grub_install_image_target_desc *image_target,
 		       int note, char **core_img, size_t *core_size,
@@ -380,7 +382,7 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
 {
   Elf_Word symtab_size, sym_size, num_syms;
   Elf_Off symtab_offset;
-  Elf_Addr start_address = 0;
+  Elf_Addr start_address = INVALID_START_ADDR;
   Elf_Sym *sym;
   Elf_Word i;
   Elf_Shdr *strtab_section;
@@ -439,7 +441,7 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
 		      (unsigned long long) sym->st_value,
 		      (unsigned long long) section_addresses[cur_index]);
 
-      if (! start_address)
+      if (start_address == INVALID_START_ADDR)
 	if (strcmp (name, "_start") == 0 || strcmp (name, "start") == 0)
 	  start_address = sym->st_value;
     }
@@ -1602,7 +1604,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 					  ia64jmp_off 
 					  + image_target->vaddr_offset,
 					  image_target);
-      if (*start == 0)
+      if (*start == INVALID_START_ADDR)
 	grub_util_error ("start symbol is not defined");
 
       SUFFIX (entry_point) = (Elf_Addr) *start;
@@ -1664,3 +1666,4 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 #undef Elf_Section
 #undef ELF_ST_TYPE
 #undef XEN_NOTE_SIZE
+#undef INVALID_START_ADDR
-- 
1.8.4.rc3

[PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Ian Campbell]

To do so support the specification of a "target_address" (which must be 0 for
image types which do not have this flag) and a new image flag to signal the
need for absolute relocation. If this flag is present then image->link_addr is
the default target.

Account for the target address when relocating.

Fabricate correct addresses for absolute symbols relating to the bss
("__bss_start" and "_end").

This functionality is not yet used by any image type and is connected only to
grub-mkimage ("-T option) and not yet to grub-install.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 include/grub/util/install.h |  2 +-
 util/grub-install-common.c  |  4 +--
 util/grub-mkimage.c         |  7 +++++
 util/grub-mkimagexx.c       | 74 +++++++++++++++++++++++++++++++--------------
 util/mkimage.c              | 24 ++++++++++++---
 5 files changed, 81 insertions(+), 30 deletions(-)

diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index bc987aa..b7ecf27 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -167,7 +167,7 @@ struct grub_install_image_target_desc;
 
 void
 grub_install_generate_image (const char *dir, const char *prefix,
-			     FILE *out,
+			     FILE *out, grub_uint64_t target_address,
 			     const char *outname, char *mods[],
 			     char *memdisk_path, char **pubkey_paths,
 			     size_t npubkeys,
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index ae26875..82a44f2 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -495,8 +495,8 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
   if (!tgt)
     grub_util_error (_("unknown target format %s\n"), mkimage_target);
 
-  grub_install_generate_image (dir, prefix, fp, outname,
-			       modules.entries, memdisk_path,
+  grub_install_generate_image (dir, prefix, fp, 0,
+			       outname, modules.entries, memdisk_path,
 			       pubkeys, npubkeys, config_path, tgt,
 			       note, compression);
   while (dc--)
diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index a2bd4c1..267beaa 100644
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -65,6 +65,7 @@ static struct argp_option options[] = {
    /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
    N_("use images and modules under DIR [default=%s/<platform>]"), 0},
   {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
+  {"target-address", 'T', N_("ADDR"), 0, N_("set kernel target address [default=%d]"), 0},
   {"memdisk",  'm', N_("FILE"), 0,
    /* TRANSLATORS: "memdisk" here isn't an identifier, it can be translated.
     "embed" is a verb (command description).  "*/
@@ -126,6 +127,7 @@ struct arguments
   int note;
   const struct grub_install_image_target_desc *image_target;
   grub_compression_t comp;
+  grub_uint64_t target_address;
 };
 
 static error_t
@@ -217,6 +219,10 @@ argp_parser (int key, char *arg, struct argp_state *state)
       arguments->prefix = xstrdup (arg);
       break;
 
+    case 'T':
+      arguments->target_address = strtoull (arg, NULL, 0);
+      break;
+
     case 'v':
       verbosity++;
       break;
@@ -289,6 +295,7 @@ main (int argc, char *argv[])
 
   grub_install_generate_image (arguments.dir,
 			       arguments.prefix ? : DEFAULT_DIRECTORY, fp,
+			       arguments.target_address,
 			       arguments.output, arguments.modules,
 			       arguments.memdisk, arguments.pubkeys,
 			       arguments.npubkeys, arguments.config,
diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index b4216ff..186d259 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -378,6 +378,7 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
 			   Elf_Shdr *symtab_section, Elf_Addr *section_addresses,
 			   Elf_Half section_entsize, Elf_Half num_sections,
 			   void *jumpers, Elf_Addr jumpers_addr,
+			   Elf_Addr bss_addr, size_t bss_size,
 			   const struct grub_install_image_target_desc *image_target)
 {
   Elf_Word symtab_size, sym_size, num_syms;
@@ -416,10 +417,14 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
         }
       else if (cur_index == STN_UNDEF)
 	{
-	  if (sym->st_name)
+	  if (strcmp (name, "__bss_start") == 0 && bss_addr)
+	      sym->st_value = bss_addr;
+	  else if (strcmp (name, "_end") == 0 && bss_addr)
+	      sym->st_value = bss_addr + bss_size;
+	  else if (sym->st_name)
 	    grub_util_error ("undefined symbol %s", name);
-	  else
-	    continue;
+
+	  continue;
 	}
       else if (cur_index >= num_sections)
 	grub_util_error ("section %d does not exist", cur_index);
@@ -584,7 +589,7 @@ static void
 SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
 			     Elf_Addr *section_addresses,
 			     Elf_Half section_entsize, Elf_Half num_sections,
-			     const char *strtab,
+			     const char *strtab, grub_uint64_t target_address,
 			     char *pe_target, Elf_Addr tramp_off,
 			     Elf_Addr got_off,
 			     const struct grub_install_image_target_desc *image_target)
@@ -867,6 +872,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
 		   {
 		   case R_ARM_ABS32:
 		     {
+		       sym_addr += target_address;
 		       grub_util_info ("  ABS32:\ttarget=0x%08lx\toffset=(0x%08x)",
 				       (unsigned long) ((char *) target
 							- (char *) e),
@@ -928,7 +934,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
 			   grub_uint32_t tr_addr;
 			   grub_int32_t new_offset;
 			   tr_addr = (char *) tr - (char *) pe_target
-			     - target_section_addr;
+			     - (target_address - target_section_addr);
 			   new_offset = sym_addr - tr_addr - 12;
 
 			   /* There is no immediate version of bx, only register one...  */
@@ -1337,6 +1343,7 @@ static Elf_Addr *
 SUFFIX (locate_sections) (const char *kernel_path,
 			  Elf_Shdr *sections, Elf_Half section_entsize,
 			  Elf_Half num_sections, const char *strtab,
+			  grub_uint64_t target_address,
 			  size_t *exec_size, size_t *kernel_sz,
 			  size_t *all_align,
 			  const struct grub_install_image_target_desc *image_target)
@@ -1351,7 +1358,7 @@ SUFFIX (locate_sections) (const char *kernel_path,
   section_addresses = xmalloc (sizeof (*section_addresses) * num_sections);
   memset (section_addresses, 0, sizeof (*section_addresses) * num_sections);
 
-  current_address = 0;
+  current_address = target_address;
 
   for (i = 0, s = sections;
        i < num_sections;
@@ -1391,6 +1398,15 @@ SUFFIX (locate_sections) (const char *kernel_path,
 		grub_util_error ("%s", msg);
 	      }
 	  }
+	else if (!grub_image_needs_abs_reloc(image_target))
+	  {
+	    if (grub_host_to_target_addr (s->sh_addr))
+	      grub_util_error (_("`%s' is miscompiled: its start address is 0x%llx"
+				 " but this platform uses late relocation"),
+			       kernel_path,
+			       (unsigned long long) grub_host_to_target_addr (s->sh_addr));
+	  }
+
 	section_addresses[i] = current_address;
 	current_address += grub_host_to_target_addr (s->sh_size);
       }
@@ -1398,7 +1414,7 @@ SUFFIX (locate_sections) (const char *kernel_path,
   current_address = ALIGN_UP (current_address + image_target->vaddr_offset,
 			      image_target->section_align)
     - image_target->vaddr_offset;
-  *exec_size = current_address;
+  *exec_size = current_address - target_address;
 
   /* .data */
   for (i = 0, s = sections;
@@ -1426,14 +1442,15 @@ SUFFIX (locate_sections) (const char *kernel_path,
 
   current_address = ALIGN_UP (current_address + image_target->vaddr_offset,
 			      image_target->section_align) - image_target->vaddr_offset;
-  *kernel_sz = current_address;
+  *kernel_sz = current_address - target_address;
   return section_addresses;
 }
 
 static char *
 SUFFIX (load_image) (const char *kernel_path, size_t *exec_size, 
 		     size_t *kernel_sz, size_t *bss_size,
-		     size_t total_module_size, grub_uint64_t *start,
+		     size_t total_module_size, grub_uint64_t target_address,
+		     grub_uint64_t *start,
 		     void **reloc_section, size_t *reloc_size,
 		     size_t *align,
 		     const struct grub_install_image_target_desc *image_target)
@@ -1444,6 +1461,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
   Elf_Shdr *sections;
   Elf_Addr *section_addresses;
   Elf_Addr *section_vaddresses;
+  Elf_Addr bss_addr = 0;
   int i;
   Elf_Shdr *s;
   Elf_Half num_sections;
@@ -1455,6 +1473,9 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
   Elf_Shdr *symtab_section = 0;
   grub_size_t got = 0;
 
+  if (target_address && !grub_image_needs_abs_reloc(image_target))
+    grub_util_error("cannot perform absolute relocation for this image type");
+
   *start = 0;
 
   kernel_size = grub_util_get_image_size (kernel_path);
@@ -1481,7 +1502,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 
   section_addresses = SUFFIX (locate_sections) (kernel_path,
 						sections, section_entsize,
-						num_sections, strtab,
+						num_sections, strtab, target_address,
 						exec_size, kernel_sz, align,
 						image_target);
 
@@ -1530,9 +1551,9 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
   for (i = 0; i < num_sections; i++)
     section_vaddresses[i] = section_addresses[i] + image_target->vaddr_offset;
 
-  if (!grub_image_needs_reloc(image_target))
+  if (!grub_image_needs_reloc(image_target) || grub_image_needs_abs_reloc(image_target))
     {
-      Elf_Addr current_address = *kernel_sz;
+      Elf_Addr current_address = target_address + *kernel_sz;
 
       for (i = 0, s = sections;
 	   i < num_sections;
@@ -1558,11 +1579,16 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 	    section_vaddresses[i] = current_address
 	      + image_target->vaddr_offset;
 	    current_address += grub_host_to_target_addr (s->sh_size);
+
+	    if (!bss_addr)
+	      bss_addr = section_vaddresses[i];
 	  }
       current_address = ALIGN_UP (current_address + image_target->vaddr_offset,
 				  image_target->section_align)
 	- image_target->vaddr_offset;
-      *bss_size = current_address - *kernel_sz;
+      *bss_size = current_address - *kernel_sz - target_address;
+      grub_util_info ("locating bss at 0x%x-0x%x (0x%x bytes)",
+		      (int)bss_addr, (int)(bss_addr + *bss_size), (int)*bss_size);
     }
   else
     *bss_size = 0;
@@ -1603,6 +1629,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 					  (char *) out_img + ia64jmp_off, 
 					  ia64jmp_off 
 					  + image_target->vaddr_offset,
+					  bss_addr, *bss_size,
 					  image_target);
       if (*start == INVALID_START_ADDR)
 	grub_util_error ("start symbol is not defined");
@@ -1612,17 +1639,18 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
       /* Resolve addresses in the virtual address space.  */
       SUFFIX (relocate_addresses) (e, sections, section_addresses, 
 				   section_entsize,
-				   num_sections, strtab,
+				   num_sections, strtab, target_address,
 				   out_img, tramp_off, ia64_got_off,
 				   image_target);
 
-      *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
-						 section_vaddresses, sections,
-						 section_entsize, num_sections,
-						 strtab, ia64jmp_off
-						 + image_target->vaddr_offset,
-						 2 * ia64jmpnum + (got / 8),
-						 image_target);
+      if (!grub_image_needs_abs_reloc(image_target))
+	  *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
+						     section_vaddresses, sections,
+						     section_entsize, num_sections,
+						     strtab, ia64jmp_off
+						     + image_target->vaddr_offset,
+						     2 * ia64jmpnum + (got / 8),
+						     image_target);
     }
 
   for (i = 0, s = sections;
@@ -1632,10 +1660,10 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
 	|| SUFFIX (is_text_section) (s, image_target))
       {
 	if (grub_target_to_host32 (s->sh_type) == SHT_NOBITS)
-	  memset (out_img + section_addresses[i], 0,
+	  memset (out_img + section_addresses[i] - target_address, 0,
 		  grub_host_to_target_addr (s->sh_size));
 	else
-	  memcpy (out_img + section_addresses[i],
+	  memcpy (out_img + section_addresses[i] - target_address,
 		  kernel_img + grub_host_to_target_addr (s->sh_offset),
 		  grub_host_to_target_addr (s->sh_size));
       }
diff --git a/util/mkimage.c b/util/mkimage.c
index 645e296..7b29acf 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -74,6 +74,7 @@ struct grub_install_image_target_desc
       PLATFORM_FLAGS_NONE = 0,
       PLATFORM_FLAGS_DECOMPRESSORS = 2,
       PLATFORM_FLAGS_MODULES_BEFORE_KERNEL = 4,
+      PLATFORM_FLAGS_ABS_RELOC = 8,
     } flags;
   unsigned total_module_size;
   unsigned decompressor_compressed_size;
@@ -922,11 +923,16 @@ grub_arm_reloc_jump24 (grub_uint32_t *target, Elf32_Addr sym_addr)
   return GRUB_ERR_NONE;
 }
 
+static int grub_image_needs_abs_reloc(const struct grub_install_image_target_desc *target)
+{
+  return target->flags & PLATFORM_FLAGS_ABS_RELOC;
+}
+
 static int grub_image_needs_reloc(const struct grub_install_image_target_desc *target)
 {
   if (target->id == IMAGE_EFI)
     return 1;
-  return 0;
+  return grub_image_needs_abs_reloc(target);
 }
 
 #pragma GCC diagnostic ignored "-Wcast-align"
@@ -987,7 +993,8 @@ grub_install_get_image_targets_string (void)
 
 void
 grub_install_generate_image (const char *dir, const char *prefix,
-			     FILE *out, const char *outname, char *mods[],
+			     FILE *out, grub_uint64_t target_address,
+			     const char *outname, char *mods[],
 			     char *memdisk_path, char **pubkey_paths,
 			     size_t npubkeys, char *config_path,
 			     const struct grub_install_image_target_desc *image_target,
@@ -1024,6 +1031,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
   else
     total_module_size = sizeof (struct grub_module_info32);
 
+  if (!target_address && grub_image_needs_abs_reloc(image_target))
+    {
+      grub_util_info ("Using default target address 0x%llx",
+		      (unsigned long long)image_target->link_addr);
+      target_address = image_target->link_addr;
+    }
+
   {
     size_t i;
     for (i = 0; i < npubkeys; i++)
@@ -1069,11 +1083,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
 
   if (image_target->voidp_sizeof == 4)
     kernel_img = load_image32 (kernel_path, &exec_size, &kernel_size, &bss_size,
-			       total_module_size, &start_address, &rel_section,
+			       total_module_size, target_address,
+			       &start_address, &rel_section,
 			       &reloc_size, &align, image_target);
   else
     kernel_img = load_image64 (kernel_path, &exec_size, &kernel_size, &bss_size,
-			       total_module_size, &start_address, &rel_section,
+			       total_module_size, target_address,
+			       &start_address, &rel_section,
 			       &reloc_size, &align, image_target);
   if (image_target->id == IMAGE_XEN && align < 4096)
     align = 4096;
-- 
1.8.4.rc3

Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Leif Lindholm]

On Sun, Dec 29, 2013 at 06:47:35PM +0000, Ian Campbell wrote:
> To do so support the specification of a "target_address" (which must be 0 for
> image types which do not have this flag) and a new image flag to signal the
> need for absolute relocation. If this flag is present then image->link_addr is
> the default target.
> 
> Account for the target address when relocating.
> 
> Fabricate correct addresses for absolute symbols relating to the bss
> ("__bss_start" and "_end").
> 
> This functionality is not yet used by any image type and is connected only to
> grub-mkimage ("-T option) and not yet to grub-install.
> 
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> ---
>  include/grub/util/install.h |  2 +-
>  util/grub-install-common.c  |  4 +--
>  util/grub-mkimage.c         |  7 +++++
>  util/grub-mkimagexx.c       | 74 +++++++++++++++++++++++++++++++--------------
>  util/mkimage.c              | 24 ++++++++++++---
>  5 files changed, 81 insertions(+), 30 deletions(-)
> 
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index bc987aa..b7ecf27 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -167,7 +167,7 @@ struct grub_install_image_target_desc;
>  
>  void
>  grub_install_generate_image (const char *dir, const char *prefix,
> -			     FILE *out,
> +			     FILE *out, grub_uint64_t target_address,
>  			     const char *outname, char *mods[],
>  			     char *memdisk_path, char **pubkey_paths,
>  			     size_t npubkeys,
> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> index ae26875..82a44f2 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -495,8 +495,8 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
>    if (!tgt)
>      grub_util_error (_("unknown target format %s\n"), mkimage_target);
>  
> -  grub_install_generate_image (dir, prefix, fp, outname,
> -			       modules.entries, memdisk_path,
> +  grub_install_generate_image (dir, prefix, fp, 0,
> +			       outname, modules.entries, memdisk_path,
>  			       pubkeys, npubkeys, config_path, tgt,
>  			       note, compression);
>    while (dc--)
> diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> index a2bd4c1..267beaa 100644
> --- a/util/grub-mkimage.c
> +++ b/util/grub-mkimage.c
> @@ -65,6 +65,7 @@ static struct argp_option options[] = {
>     /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
>     N_("use images and modules under DIR [default=%s/<platform>]"), 0},
>    {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
> +  {"target-address", 'T', N_("ADDR"), 0, N_("set kernel target address [default=%d]"), 0},

For this to actually print a default value, you need to also update
the help_filter function. Also, you probably want a %p here.
---
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -65,7 +65,7 @@ static struct argp_option options[] = {
    /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
    N_("use images and modules under DIR [default=%s/<platform>]"), 0},
   {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
-  {"target-address", 'T', N_("ADDR"), 0, N_("set kernel target address [default=%d]"), 0},
+  {"target-address", 'T', N_("ADDR"), 0, N_("set kernel target address [default=%p]"), 0},
   {"memdisk",  'm', N_("FILE"), 0,
    /* TRANSLATORS: "memdisk" here isn't an identifier, it can be translated.
     "embed" is a verb (command description).  "*/
@@ -104,6 +104,8 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))
        free (formats);
        return ret;
       }
+    case 'T':
+       return xasprintf (text, GRUB_KERNEL_ARM_UBOOT_DEFAULT_LINK_ADDR);
     default:
       return (char *) text;
     }

---
(Of course, you probably want to abstract away the ARM_UBOOT bit...)

>    {"memdisk",  'm', N_("FILE"), 0,
>     /* TRANSLATORS: "memdisk" here isn't an identifier, it can be translated.
>      "embed" is a verb (command description).  "*/
> @@ -126,6 +127,7 @@ struct arguments
>    int note;
>    const struct grub_install_image_target_desc *image_target;
>    grub_compression_t comp;
> +  grub_uint64_t target_address;
>  };
>  
>  static error_t
> @@ -217,6 +219,10 @@ argp_parser (int key, char *arg, struct argp_state *state)
>        arguments->prefix = xstrdup (arg);
>        break;
>  
> +    case 'T':
> +      arguments->target_address = strtoull (arg, NULL, 0);
> +      break;
> +
>      case 'v':
>        verbosity++;
>        break;
> @@ -289,6 +295,7 @@ main (int argc, char *argv[])
>  
>    grub_install_generate_image (arguments.dir,
>  			       arguments.prefix ? : DEFAULT_DIRECTORY, fp,
> +			       arguments.target_address,
>  			       arguments.output, arguments.modules,
>  			       arguments.memdisk, arguments.pubkeys,
>  			       arguments.npubkeys, arguments.config,
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index b4216ff..186d259 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -378,6 +378,7 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
>  			   Elf_Shdr *symtab_section, Elf_Addr *section_addresses,
>  			   Elf_Half section_entsize, Elf_Half num_sections,
>  			   void *jumpers, Elf_Addr jumpers_addr,
> +			   Elf_Addr bss_addr, size_t bss_size,
>  			   const struct grub_install_image_target_desc *image_target)
>  {
>    Elf_Word symtab_size, sym_size, num_syms;
> @@ -416,10 +417,14 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
>          }
>        else if (cur_index == STN_UNDEF)
>  	{
> -	  if (sym->st_name)
> +	  if (strcmp (name, "__bss_start") == 0 && bss_addr)
> +	      sym->st_value = bss_addr;
> +	  else if (strcmp (name, "_end") == 0 && bss_addr)
> +	      sym->st_value = bss_addr + bss_size;
> +	  else if (sym->st_name)
>  	    grub_util_error ("undefined symbol %s", name);
> -	  else
> -	    continue;
> +
> +	  continue;
>  	}
>        else if (cur_index >= num_sections)
>  	grub_util_error ("section %d does not exist", cur_index);
> @@ -584,7 +589,7 @@ static void
>  SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
>  			     Elf_Addr *section_addresses,
>  			     Elf_Half section_entsize, Elf_Half num_sections,
> -			     const char *strtab,
> +			     const char *strtab, grub_uint64_t target_address,
>  			     char *pe_target, Elf_Addr tramp_off,
>  			     Elf_Addr got_off,
>  			     const struct grub_install_image_target_desc *image_target)
> @@ -867,6 +872,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
>  		   {
>  		   case R_ARM_ABS32:
>  		     {
> +		       sym_addr += target_address;
>  		       grub_util_info ("  ABS32:\ttarget=0x%08lx\toffset=(0x%08x)",
>  				       (unsigned long) ((char *) target
>  							- (char *) e),
> @@ -928,7 +934,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
>  			   grub_uint32_t tr_addr;
>  			   grub_int32_t new_offset;
>  			   tr_addr = (char *) tr - (char *) pe_target
> -			     - target_section_addr;
> +			     - (target_address - target_section_addr);
>  			   new_offset = sym_addr - tr_addr - 12;
>  
>  			   /* There is no immediate version of bx, only register one...  */
> @@ -1337,6 +1343,7 @@ static Elf_Addr *
>  SUFFIX (locate_sections) (const char *kernel_path,
>  			  Elf_Shdr *sections, Elf_Half section_entsize,
>  			  Elf_Half num_sections, const char *strtab,
> +			  grub_uint64_t target_address,
>  			  size_t *exec_size, size_t *kernel_sz,
>  			  size_t *all_align,
>  			  const struct grub_install_image_target_desc *image_target)
> @@ -1351,7 +1358,7 @@ SUFFIX (locate_sections) (const char *kernel_path,
>    section_addresses = xmalloc (sizeof (*section_addresses) * num_sections);
>    memset (section_addresses, 0, sizeof (*section_addresses) * num_sections);
>  
> -  current_address = 0;
> +  current_address = target_address;
>  
>    for (i = 0, s = sections;
>         i < num_sections;
> @@ -1391,6 +1398,15 @@ SUFFIX (locate_sections) (const char *kernel_path,
>  		grub_util_error ("%s", msg);
>  	      }
>  	  }
> +	else if (!grub_image_needs_abs_reloc(image_target))
> +	  {
> +	    if (grub_host_to_target_addr (s->sh_addr))
> +	      grub_util_error (_("`%s' is miscompiled: its start address is 0x%llx"
> +				 " but this platform uses late relocation"),
> +			       kernel_path,
> +			       (unsigned long long) grub_host_to_target_addr (s->sh_addr));
> +	  }
> +
>  	section_addresses[i] = current_address;
>  	current_address += grub_host_to_target_addr (s->sh_size);
>        }
> @@ -1398,7 +1414,7 @@ SUFFIX (locate_sections) (const char *kernel_path,
>    current_address = ALIGN_UP (current_address + image_target->vaddr_offset,
>  			      image_target->section_align)
>      - image_target->vaddr_offset;
> -  *exec_size = current_address;
> +  *exec_size = current_address - target_address;
>  
>    /* .data */
>    for (i = 0, s = sections;
> @@ -1426,14 +1442,15 @@ SUFFIX (locate_sections) (const char *kernel_path,
>  
>    current_address = ALIGN_UP (current_address + image_target->vaddr_offset,
>  			      image_target->section_align) - image_target->vaddr_offset;
> -  *kernel_sz = current_address;
> +  *kernel_sz = current_address - target_address;
>    return section_addresses;
>  }
>  
>  static char *
>  SUFFIX (load_image) (const char *kernel_path, size_t *exec_size, 
>  		     size_t *kernel_sz, size_t *bss_size,
> -		     size_t total_module_size, grub_uint64_t *start,
> +		     size_t total_module_size, grub_uint64_t target_address,
> +		     grub_uint64_t *start,
>  		     void **reloc_section, size_t *reloc_size,
>  		     size_t *align,
>  		     const struct grub_install_image_target_desc *image_target)
> @@ -1444,6 +1461,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>    Elf_Shdr *sections;
>    Elf_Addr *section_addresses;
>    Elf_Addr *section_vaddresses;
> +  Elf_Addr bss_addr = 0;
>    int i;
>    Elf_Shdr *s;
>    Elf_Half num_sections;
> @@ -1455,6 +1473,9 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>    Elf_Shdr *symtab_section = 0;
>    grub_size_t got = 0;
>  
> +  if (target_address && !grub_image_needs_abs_reloc(image_target))
> +    grub_util_error("cannot perform absolute relocation for this image type");
> +
>    *start = 0;
>  
>    kernel_size = grub_util_get_image_size (kernel_path);
> @@ -1481,7 +1502,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>  
>    section_addresses = SUFFIX (locate_sections) (kernel_path,
>  						sections, section_entsize,
> -						num_sections, strtab,
> +						num_sections, strtab, target_address,
>  						exec_size, kernel_sz, align,
>  						image_target);
>  
> @@ -1530,9 +1551,9 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>    for (i = 0; i < num_sections; i++)
>      section_vaddresses[i] = section_addresses[i] + image_target->vaddr_offset;
>  
> -  if (!grub_image_needs_reloc(image_target))
> +  if (!grub_image_needs_reloc(image_target) || grub_image_needs_abs_reloc(image_target))
>      {
> -      Elf_Addr current_address = *kernel_sz;
> +      Elf_Addr current_address = target_address + *kernel_sz;
>  
>        for (i = 0, s = sections;
>  	   i < num_sections;
> @@ -1558,11 +1579,16 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>  	    section_vaddresses[i] = current_address
>  	      + image_target->vaddr_offset;
>  	    current_address += grub_host_to_target_addr (s->sh_size);
> +
> +	    if (!bss_addr)
> +	      bss_addr = section_vaddresses[i];
>  	  }
>        current_address = ALIGN_UP (current_address + image_target->vaddr_offset,
>  				  image_target->section_align)
>  	- image_target->vaddr_offset;
> -      *bss_size = current_address - *kernel_sz;
> +      *bss_size = current_address - *kernel_sz - target_address;
> +      grub_util_info ("locating bss at 0x%x-0x%x (0x%x bytes)",
> +		      (int)bss_addr, (int)(bss_addr + *bss_size), (int)*bss_size);
>      }
>    else
>      *bss_size = 0;
> @@ -1603,6 +1629,7 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>  					  (char *) out_img + ia64jmp_off, 
>  					  ia64jmp_off 
>  					  + image_target->vaddr_offset,
> +					  bss_addr, *bss_size,
>  					  image_target);
>        if (*start == INVALID_START_ADDR)
>  	grub_util_error ("start symbol is not defined");
> @@ -1612,17 +1639,18 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>        /* Resolve addresses in the virtual address space.  */
>        SUFFIX (relocate_addresses) (e, sections, section_addresses, 
>  				   section_entsize,
> -				   num_sections, strtab,
> +				   num_sections, strtab, target_address,
>  				   out_img, tramp_off, ia64_got_off,
>  				   image_target);
>  
> -      *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
> -						 section_vaddresses, sections,
> -						 section_entsize, num_sections,
> -						 strtab, ia64jmp_off
> -						 + image_target->vaddr_offset,
> -						 2 * ia64jmpnum + (got / 8),
> -						 image_target);
> +      if (!grub_image_needs_abs_reloc(image_target))
> +	  *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
> +						     section_vaddresses, sections,
> +						     section_entsize, num_sections,
> +						     strtab, ia64jmp_off
> +						     + image_target->vaddr_offset,
> +						     2 * ia64jmpnum + (got / 8),
> +						     image_target);

The above is only indentation changes?

>      }
>  
>    for (i = 0, s = sections;
> @@ -1632,10 +1660,10 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>  	|| SUFFIX (is_text_section) (s, image_target))
>        {
>  	if (grub_target_to_host32 (s->sh_type) == SHT_NOBITS)
> -	  memset (out_img + section_addresses[i], 0,
> +	  memset (out_img + section_addresses[i] - target_address, 0,
>  		  grub_host_to_target_addr (s->sh_size));
>  	else
> -	  memcpy (out_img + section_addresses[i],
> +	  memcpy (out_img + section_addresses[i] - target_address,
>  		  kernel_img + grub_host_to_target_addr (s->sh_offset),
>  		  grub_host_to_target_addr (s->sh_size));
>        }
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 645e296..7b29acf 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -74,6 +74,7 @@ struct grub_install_image_target_desc
>        PLATFORM_FLAGS_NONE = 0,
>        PLATFORM_FLAGS_DECOMPRESSORS = 2,
>        PLATFORM_FLAGS_MODULES_BEFORE_KERNEL = 4,
> +      PLATFORM_FLAGS_ABS_RELOC = 8,
>      } flags;
>    unsigned total_module_size;
>    unsigned decompressor_compressed_size;
> @@ -922,11 +923,16 @@ grub_arm_reloc_jump24 (grub_uint32_t *target, Elf32_Addr sym_addr)
>    return GRUB_ERR_NONE;
>  }
>  
> +static int grub_image_needs_abs_reloc(const struct grub_install_image_target_desc *target)
> +{
> +  return target->flags & PLATFORM_FLAGS_ABS_RELOC;
> +}
> +
>  static int grub_image_needs_reloc(const struct grub_install_image_target_desc *target)
>  {
>    if (target->id == IMAGE_EFI)
>      return 1;
> -  return 0;
> +  return grub_image_needs_abs_reloc(target);
>  }
>  
>  #pragma GCC diagnostic ignored "-Wcast-align"
> @@ -987,7 +993,8 @@ grub_install_get_image_targets_string (void)
>  
>  void
>  grub_install_generate_image (const char *dir, const char *prefix,
> -			     FILE *out, const char *outname, char *mods[],
> +			     FILE *out, grub_uint64_t target_address,
> +			     const char *outname, char *mods[],
>  			     char *memdisk_path, char **pubkey_paths,
>  			     size_t npubkeys, char *config_path,
>  			     const struct grub_install_image_target_desc *image_target,
> @@ -1024,6 +1031,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
>    else
>      total_module_size = sizeof (struct grub_module_info32);
>  
> +  if (!target_address && grub_image_needs_abs_reloc(image_target))
> +    {
> +      grub_util_info ("Using default target address 0x%llx",
> +		      (unsigned long long)image_target->link_addr);
> +      target_address = image_target->link_addr;
> +    }
> +
>    {
>      size_t i;
>      for (i = 0; i < npubkeys; i++)
> @@ -1069,11 +1083,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
>  
>    if (image_target->voidp_sizeof == 4)
>      kernel_img = load_image32 (kernel_path, &exec_size, &kernel_size, &bss_size,
> -			       total_module_size, &start_address, &rel_section,
> +			       total_module_size, target_address,
> +			       &start_address, &rel_section,
>  			       &reloc_size, &align, image_target);
>    else
>      kernel_img = load_image64 (kernel_path, &exec_size, &kernel_size, &bss_size,
> -			       total_module_size, &start_address, &rel_section,
> +			       total_module_size, target_address,
> +			       &start_address, &rel_section,
>  			       &reloc_size, &align, image_target);
>    if (image_target->id == IMAGE_XEN && align < 4096)
>      align = 4096;
> -- 
> 1.8.4.rc3
> 

Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 29.12.2013 23:53, Leif Lindholm wrote:
> For this to actually print a default value, you need to also update
> the help_filter function. Also, you probably want a %p here.

This is not a pointer in the address space of running process, so %p is
not the right qualifier for it. 0x%x if needed only on 32-bit arch or
0x%llx if on 64-bit as well would be a better alternative.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Ian Campbell]

On Sun, 2013-12-29 at 22:53 +0000, Leif Lindholm wrote:
> > diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> > index a2bd4c1..267beaa 100644
> > --- a/util/grub-mkimage.c
> > +++ b/util/grub-mkimage.c
> > @@ -65,6 +65,7 @@ static struct argp_option options[] = {
> >     /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
> >     N_("use images and modules under DIR [default=%s/<platform>]"), 0},
> >    {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
> > +  {"target-address", 'T', N_("ADDR"), 0, N_("set kernel target address [default=%d]"), 0},
> 
> For this to actually print a default value, you need to also update
> the help_filter function. Also, you probably want a %p here.

I hadn't noticed that, but given the lack of any default in the above it
now seems obvious. Thanks.

[...]
> (Of course, you probably want to abstract away the ARM_UBOOT bit...)

That might be tricky in practice -- what do you do if the user hasn't
given any target yet?

I'm more than a little inclined to suggest that this ought to say
(literally) "[default=PLATFORM-SPECIFIC]" or something non-committal. A
lot simpler with no odd corner cases.

[...] 
> > -      *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
> > -						 section_vaddresses, sections,
> > -						 section_entsize, num_sections,
> > -						 strtab, ia64jmp_off
> > -						 + image_target->vaddr_offset,
> > -						 2 * ia64jmpnum + (got / 8),
> > -						 image_target);
> > +      if (!grub_image_needs_abs_reloc(image_target))
> > +	  *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
> > +						     section_vaddresses, sections,
> > +						     section_entsize, num_sections,
> > +						     strtab, ia64jmp_off
> > +						     + image_target->vaddr_offset,
> > +						     2 * ia64jmpnum + (got / 8),
> > +						     image_target);
> 
> The above is only indentation changes?

Other than the addition of the if conditional, the rest is just the
indentation changes arising from that addition.

Ian.

Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Andrey Borzenkov]

В Вс, 29/12/2013 в 18:47 +0000, Ian Campbell пишет:
> @@ -1530,9 +1551,9 @@ SUFFIX (load_image) (const char *kernel_path, size_t *exec_size,
>    for (i = 0; i < num_sections; i++)
>      section_vaddresses[i] = section_addresses[i] + image_target->vaddr_offset;
>  
> -  if (!grub_image_needs_reloc(image_target))
> +  if (!grub_image_needs_reloc(image_target) || grub_image_needs_abs_reloc(image_target))
>      {

grub_image_needs_reloc implies grub_image_need_abs_reloc. Looks like one
of hunks should be dropped?

> @@ -922,11 +923,16 @@ grub_arm_reloc_jump24 (grub_uint32_t *target, Elf32_Addr sym_addr)
>  static int grub_image_needs_reloc(const struct grub_install_image_target_desc *target)
>  {
>    if (target->id == IMAGE_EFI)
>      return 1;
> -  return 0;
> +  return grub_image_needs_abs_reloc(target);
>  }
>  

Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Francesco Lavra]

On 12/29/2013 07:47 PM, Ian Campbell wrote:
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index b4216ff..186d259 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -378,6 +378,7 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
>  			   Elf_Shdr *symtab_section, Elf_Addr *section_addresses,
>  			   Elf_Half section_entsize, Elf_Half num_sections,
>  			   void *jumpers, Elf_Addr jumpers_addr,
> +			   Elf_Addr bss_addr, size_t bss_size,
>  			   const struct grub_install_image_target_desc *image_target)
>  {
>    Elf_Word symtab_size, sym_size, num_syms;
> @@ -416,10 +417,14 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
>          }
>        else if (cur_index == STN_UNDEF)
>  	{
> -	  if (sym->st_name)
> +	  if (strcmp (name, "__bss_start") == 0 && bss_addr)
> +	      sym->st_value = bss_addr;
> +	  else if (strcmp (name, "_end") == 0 && bss_addr)
> +	      sym->st_value = bss_addr + bss_size;
> +	  else if (sym->st_name)
>  	    grub_util_error ("undefined symbol %s", name);
> -	  else
> -	    continue;
> +
> +	  continue;
>  	}
>        else if (cur_index >= num_sections)
>  	grub_util_error ("section %d does not exist", cur_index);
> @@ -584,7 +589,7 @@ static void
>  SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
>  			     Elf_Addr *section_addresses,
>  			     Elf_Half section_entsize, Elf_Half num_sections,
> -			     const char *strtab,
> +			     const char *strtab, grub_uint64_t target_address,
>  			     char *pe_target, Elf_Addr tramp_off,
>  			     Elf_Addr got_off,
>  			     const struct grub_install_image_target_desc *image_target)
> @@ -867,6 +872,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
>  		   {
>  		   case R_ARM_ABS32:
>  		     {
> +		       sym_addr += target_address;
>  		       grub_util_info ("  ABS32:\ttarget=0x%08lx\toffset=(0x%08x)",
>  				       (unsigned long) ((char *) target
>  							- (char *) e),
> @@ -928,7 +934,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
>  			   grub_uint32_t tr_addr;
>  			   grub_int32_t new_offset;
>  			   tr_addr = (char *) tr - (char *) pe_target
> -			     - target_section_addr;
> +			     - (target_address - target_section_addr);

target_section_addr was being subtracted from tr before, now it's being
added; I guess this is not intentional.

--
Francesco

Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Ian Campbell]

On Tue, 2013-12-31 at 15:19 +0100, Francesco Lavra wrote:
> On 12/29/2013 07:47 PM, Ian Campbell wrote:
> > diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> > index b4216ff..186d259 100644
> > --- a/util/grub-mkimagexx.c
> > +++ b/util/grub-mkimagexx.c
> > @@ -378,6 +378,7 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
> >  			   Elf_Shdr *symtab_section, Elf_Addr *section_addresses,
> >  			   Elf_Half section_entsize, Elf_Half num_sections,
> >  			   void *jumpers, Elf_Addr jumpers_addr,
> > +			   Elf_Addr bss_addr, size_t bss_size,
> >  			   const struct grub_install_image_target_desc *image_target)
> >  {
> >    Elf_Word symtab_size, sym_size, num_syms;
> > @@ -416,10 +417,14 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
> >          }
> >        else if (cur_index == STN_UNDEF)
> >  	{
> > -	  if (sym->st_name)
> > +	  if (strcmp (name, "__bss_start") == 0 && bss_addr)
> > +	      sym->st_value = bss_addr;
> > +	  else if (strcmp (name, "_end") == 0 && bss_addr)
> > +	      sym->st_value = bss_addr + bss_size;
> > +	  else if (sym->st_name)
> >  	    grub_util_error ("undefined symbol %s", name);
> > -	  else
> > -	    continue;
> > +
> > +	  continue;
> >  	}
> >        else if (cur_index >= num_sections)
> >  	grub_util_error ("section %d does not exist", cur_index);
> > @@ -584,7 +589,7 @@ static void
> >  SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
> >  			     Elf_Addr *section_addresses,
> >  			     Elf_Half section_entsize, Elf_Half num_sections,
> > -			     const char *strtab,
> > +			     const char *strtab, grub_uint64_t target_address,
> >  			     char *pe_target, Elf_Addr tramp_off,
> >  			     Elf_Addr got_off,
> >  			     const struct grub_install_image_target_desc *image_target)
> > @@ -867,6 +872,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
> >  		   {
> >  		   case R_ARM_ABS32:
> >  		     {
> > +		       sym_addr += target_address;
> >  		       grub_util_info ("  ABS32:\ttarget=0x%08lx\toffset=(0x%08x)",
> >  				       (unsigned long) ((char *) target
> >  							- (char *) e),
> > @@ -928,7 +934,7 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr *sections,
> >  			   grub_uint32_t tr_addr;
> >  			   grub_int32_t new_offset;
> >  			   tr_addr = (char *) tr - (char *) pe_target
> > -			     - target_section_addr;
> > +			     - (target_address - target_section_addr);
> 
> target_section_addr was being subtracted from tr before, now it's being
> added; I guess this is not intentional.

Hrm, yes, it does look suspicious. Good spot.

I added some debug prints to all of this and it came out correct, i.e.
with this change tr_addr (which is actually an offset) is 0x9290 and
without it tr_addr is 0xb8009290, which is clearly nonsense. The
trampolines are placed at 0x9290-0x92a0 (n.b. .bss, which is supposed to
be right after the trampolines, is from offset 0x92a0, which matches).

I suppose something elsewhere accounts for this offset the other way
around and everything comes out in the wash. I'll investigate properly
in the new year.

The R_ARM_THM_{CALL,JUMP*} case is also unchanged, but I don't actually
see any interworking in that direction so that case never triggers, it
should certainly be changed to be the same in both cases, whatever that
turns out to be.

Ian.

> 
> --
> Francesco
> 

[PATCH 7/7] arm-uboot: support relocation at installation time

[Ian Campbell]

Partially link kernel.img and take advantage of the new
PLATFORM_FLAGS_ABS_RELOC support to link the final kernel image for a
particular address.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
 grub-core/Makefile.core.def | 2 +-
 include/grub/offsets.h      | 2 +-
 util/mkimage.c              | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index e5e558c..f14a2c4 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -95,7 +95,7 @@ kernel = {
   i386_qemu_cppflags     = '-DGRUB_BOOT_MACHINE_LINK_ADDR=$(GRUB_BOOT_MACHINE_LINK_ADDR)';
   emu_cflags = '$(CFLAGS_GNULIB)';
   emu_cppflags = '$(CPPFLAGS_GNULIB)';
-  arm_uboot_ldflags       = '-Wl,-Ttext=0x08000000';
+  arm_uboot_ldflags       = '-Wl,-r,-d';
   arm_uboot_stripflags    = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
   i386_pc_startup = kern/i386/pc/startup.S;
diff --git a/include/grub/offsets.h b/include/grub/offsets.h
index 85e7401..bb14190 100644
--- a/include/grub/offsets.h
+++ b/include/grub/offsets.h
@@ -120,7 +120,7 @@
 
 #define GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN 	0x8
 #define GRUB_KERNEL_ARM_UBOOT_TOTAL_MODULE_SIZE	0x4
-#define GRUB_KERNEL_ARM_UBOOT_LINK_ADDR		0x08000000
+#define GRUB_KERNEL_ARM_UBOOT_DEFAULT_LINK_ADDR 0x08000000
 
 /* Minimal gap between _end and the start of the modules.  It's a hack
    for PowerMac to prevent "CLAIM failed" error.  The real fix is to
diff --git a/util/mkimage.c b/util/mkimage.c
index 7b29acf..112092e 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -549,14 +549,14 @@ static const struct grub_install_image_target_desc image_targets[] =
       .voidp_sizeof = 4,
       .bigendian = 0,
       .id = IMAGE_UBOOT, 
-      .flags = PLATFORM_FLAGS_NONE,
+      .flags = PLATFORM_FLAGS_ABS_RELOC,
       .total_module_size = GRUB_KERNEL_ARM_UBOOT_TOTAL_MODULE_SIZE,
       .decompressor_compressed_size = TARGET_NO_FIELD,
       .decompressor_uncompressed_size = TARGET_NO_FIELD,
       .decompressor_uncompressed_addr = TARGET_NO_FIELD,
       .section_align = GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN,
       .vaddr_offset = 0,
-      .link_addr = GRUB_KERNEL_ARM_UBOOT_LINK_ADDR,
+      .link_addr = GRUB_KERNEL_ARM_UBOOT_DEFAULT_LINK_ADDR,
       .elf_target = EM_ARM,
       .mod_gap = GRUB_KERNEL_ARM_UBOOT_MOD_GAP,
       .mod_align = GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN,
@@ -1802,8 +1802,8 @@ grub_install_generate_image (const char *dir, const char *prefix,
       hdr->ih_magic = grub_cpu_to_be32_compile_time (GRUB_UBOOT_IH_MAGIC);
       hdr->ih_time = grub_cpu_to_be32 (time (0));
       hdr->ih_size = grub_cpu_to_be32 (core_size);
-      hdr->ih_load = grub_cpu_to_be32 (image_target->link_addr);
-      hdr->ih_ep = grub_cpu_to_be32 (image_target->link_addr);
+      hdr->ih_load = grub_cpu_to_be32 (target_address);
+      hdr->ih_ep = grub_cpu_to_be32 (start_address);
       hdr->ih_type = GRUB_UBOOT_IH_TYPE_KERNEL;
       hdr->ih_os = GRUB_UBOOT_IH_OS_LINUX;
       hdr->ih_arch = GRUB_UBOOT_IH_ARCH_ARM;
-- 
1.8.4.rc3

Re: [PATCH 0/7] arm-uboot: support for different RAM bases

[Ian Campbell]

On Sun, 2013-12-29 at 18:47 +0000, Ian Campbell wrote:
> (there is an
> unrelated issue on cubieboard2 relating to the scan for the uboot API
> signature, which I'm still investigating, but I hacked around it here
> and things worked fine otherwise). 

FYI: http://lists.denx.de/pipermail/u-boot/2013-December/169918.html

Ian.

Re: [PATCH 0/7] arm-uboot: support for different RAM bases

[Leif Lindholm]

Hi Ian,

On Sun, Dec 29, 2013 at 06:47:19PM +0000, Ian Campbell wrote:
> The current arm-uboot image type is hardcoded to run at address
> 0x08000000 which means that it cannot be used on ARM systems which do
> not have RAM starting from address 0 (which is many of them). This is a
> bit of a problem for distros.
> 
> The following series attempts to address this by only performing a
> partial link of the arm-uboot kernel.img at build time and enhancing
> grub-mkimage to finalise the link to a specific address while building
> the core.img.

Good stuff.

> I have test this on a Midway system, which has RAM at address 0, linking
> to a variety of addresses (0x{0,1,4}8000000) and on a cubieboard2 which
> has RAM at 0x40000000-0x80000000 linked at 0x48000000. (there is an
> unrelated issue on cubieboard2 relating to the scan for the uboot API
> signature, which I'm still investigating, but I hacked around it here
> and things worked fine otherwise).

We can add BeagleBone Black to the list (0x88000000 and 0x90000000).
(Although I'm having issues actually booting Linux on it, which I
had when I just manually changed the link address, GRUB runs fine.)

/
    Leif

Re: [PATCH 0/7] arm-uboot: support for different RAM bases

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 29.12.2013 19:47, Ian Campbell wrote:
> The current arm-uboot image type is hardcoded to run at address
> 0x08000000 which means that it cannot be used on ARM systems which do
> not have RAM starting from address 0 (which is many of them). This is a
> bit of a problem for distros.
> 
> The following series attempts to address this by only performing a
> partial link of the arm-uboot kernel.img at build time and enhancing
> grub-mkimage to finalise the link to a specific address while building
> the core.img.
> 
> I have test this on a Midway system, which has RAM at address 0, linking
> to a variety of addresses (0x{0,1,4}8000000) and on a cubieboard2 which
> has RAM at 0x40000000-0x80000000 linked at 0x48000000. (there is an
> unrelated issue on cubieboard2 relating to the scan for the uboot API
> signature, which I'm still investigating, but I hacked around it here
> and things worked fine otherwise).
> 
Is there a way to make uboot to load GRUB at some appropriate address.
We can do relocations in the startup code if needed.
> I have done build only tests of arm64-efi, i386-pc and x86_64-efi. After
> running grub-mkimage under faketime the resulting binaries are bit for
> bit identical in every case (faketime due to the timestamp in the PE
> header).
> 
> I haven't yet integrated this into grub-install, I need to think a bit
> more about how to automagically determine the correct link address for a
> given platform.
> 
> If no target address is given then the existing hardcoded address
> (0x08000000) is used as the default.
> 
> Please CC on any replies since I'm not subscribed.
> 
> Ian.
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

Re: [PATCH 0/7] arm-uboot: support for different RAM bases

[Ian Campbell]

On Mon, 2013-12-30 at 02:55 +0100, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> Is there a way to make uboot to load GRUB at some appropriate address.

I don't think there is with the uImage format which grub uses today. It
contains a load address in the header which AFAIK is an absolute address
with no scope for "u-boot chooses" or "offset from start of RAM" or
anything like that.

In theory we could drop the uImage header and just us a plain Linux
zImage via u-boot's "bootz" command, which would then put things in the
hands of the u-boot boot script and environment, but that is not all
that widely available yet, whereas "bootm" is pretty much everywhere.

> We can do relocations in the startup code if needed.

The above notwithstanding I did wonder about this but it seems like
potentially complex code to write in the early asm portion of things.

I also vaguely played with -fpic/fpie but not with much success and I
don't know if that solution would be workable in practice.

Ian.