All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
@ 2013-03-24 17:01 Leif Lindholm
  2013-04-01  2:31 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-01 16:24 ` Francesco Lavra
  0 siblings, 2 replies; 16+ messages in thread
From: Leif Lindholm @ 2013-03-24 17:01 UTC (permalink / raw)
  To: grub-devel

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



[-- Attachment #2: 0007-uefi-support.patch --]
[-- Type: application/octet-stream, Size: 21885 bytes --]

=== modified file 'configure.ac'
--- configure.ac	2013-03-24 14:49:04 +0000
+++ configure.ac	2013-03-24 15:32:46 +0000
@@ -153,6 +153,7 @@
   mipsel-fuloong) platform=loongson ;;
   mipsel-loongson) ;;
   arm-uboot) ;;
+  arm-efi) ;;
   *-emu) ;;
   *) AC_MSG_ERROR([platform "$platform" is not supported for target CPU "$target_cpu"]) ;;
 esac
@@ -1159,6 +1160,7 @@
 AM_CONDITIONAL([COND_mipseb], [test x$target_cpu = xmips])
 AM_CONDITIONAL([COND_arm], [test x$target_cpu = xarm ])
 AM_CONDITIONAL([COND_arm_uboot], [test x$target_cpu = xarm -a x$platform = xuboot])
+AM_CONDITIONAL([COND_arm_efi], [test x$target_cpu = xarm -a x$platform = xefi])
 
 AM_CONDITIONAL([COND_HOST_HURD], [test x$host_kernel = xhurd])
 AM_CONDITIONAL([COND_HOST_LINUX], [test x$host_kernel = xlinux])

=== modified file 'gentpl.py'
--- gentpl.py	2013-03-24 14:49:04 +0000
+++ gentpl.py	2013-03-24 15:32:46 +0000
@@ -23,7 +23,7 @@
                    "i386_multiboot", "i386_ieee1275", "x86_64_efi",
                    "mips_loongson", "sparc64_ieee1275",
                    "powerpc_ieee1275", "mips_arc", "ia64_efi",
-                   "mips_qemu_mips", "arm_uboot" ]
+                   "mips_qemu_mips", "arm_uboot", "arm_efi" ]
 
 GROUPS = {}
 
@@ -36,10 +36,10 @@
 GROUPS["mips"]     = [ "mips_loongson", "mips_qemu_mips", "mips_arc" ]
 GROUPS["sparc64"]  = [ "sparc64_ieee1275" ]
 GROUPS["powerpc"]  = [ "powerpc_ieee1275" ]
-GROUPS["arm"]      = [ "arm_uboot" ]
+GROUPS["arm"]      = [ "arm_uboot", "arm_efi" ]
 
 # Groups based on firmware
-GROUPS["efi"]  = [ "i386_efi", "x86_64_efi", "ia64_efi" ]
+GROUPS["efi"]  = [ "i386_efi", "x86_64_efi", "ia64_efi", "arm_efi" ]
 GROUPS["ieee1275"]   = [ "i386_ieee1275", "sparc64_ieee1275", "powerpc_ieee1275" ]
 GROUPS["uboot"] = [ "arm_uboot" ]
 
@@ -64,7 +64,7 @@
 for i in GROUPS["terminfoinkernel"]: GROUPS["terminfomodule"].remove(i)
 
 # Flattened Device Trees (FDT)
-GROUPS["fdt"] = [ "arm_uboot" ]
+GROUPS["fdt"] = [ "arm_uboot", "arm_efi" ]
 
 # Miscelaneous groups schedulded to disappear in future
 GROUPS["i386_coreboot_multiboot_qemu"] = ["i386_coreboot", "i386_multiboot", "i386_qemu"]

=== modified file 'grub-core/Makefile.am'
--- grub-core/Makefile.am	2013-03-24 14:49:04 +0000
+++ grub-core/Makefile.am	2013-03-24 15:32:46 +0000
@@ -218,6 +218,12 @@
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
 endif
 
+if COND_arm_efi
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/arm/efi/loader.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
+endif
+
 if COND_emu
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/datetime.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/misc.h

=== modified file 'grub-core/Makefile.core.def'
--- grub-core/Makefile.core.def	2013-03-24 14:49:04 +0000
+++ grub-core/Makefile.core.def	2013-03-24 15:32:46 +0000
@@ -45,6 +45,9 @@
   ia64_efi_ldflags = '-Wl,-r,-d';
   ia64_efi_stripflags = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
+  arm_efi_ldflags          = '-Wl,-r,-d';
+  arm_efi_stripflags       = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
+
   i386_pc_ldflags          = '$(TARGET_IMG_LDFLAGS)';
   i386_pc_ldflags          = '$(TARGET_IMG_BASE_LDOPT),0x9000';
 
@@ -80,6 +83,7 @@
   sparc64_ieee1275_startup = kern/sparc64/ieee1275/crt0.S;
   powerpc_ieee1275_startup = kern/powerpc/ieee1275/startup.S;
   arm_uboot_startup = kern/arm/uboot/startup.S;
+  arm_efi_startup = kern/arm/efi/startup.S;
 
   common = kern/command.c;
   common = kern/corecmd.c;
@@ -152,6 +156,9 @@
   ia64_efi = kern/ia64/dl.c;
   ia64_efi = kern/ia64/dl_helper.c;
 
+  arm_efi = kern/arm/efi/init.c;
+  arm_efi = kern/arm/efi/misc.c;
+
   i386_pc = kern/i386/pc/init.c;
   i386_pc = kern/i386/pc/mmap.c;
   i386_pc = kern/i386/tsc.c;
@@ -714,6 +721,7 @@
   i386 = lib/i386/reboot_trampoline.S;
   ia64_efi = lib/efi/reboot.c;
   x86_64_efi = lib/efi/reboot.c;
+  arm_efi = lib/efi/reboot.c;
   powerpc_ieee1275 = lib/ieee1275/reboot.c;
   sparc64_ieee1275 = lib/ieee1275/reboot.c;
   mips_arc = lib/mips/arc/reboot.c;
@@ -1526,6 +1534,7 @@
 
   enable = x86;
   enable = ia64_efi;
+  enable = arm_efi;
   enable = mips;
 };
 

=== added directory 'grub-core/kern/arm/efi'
=== added file 'grub-core/kern/arm/efi/init.c'
--- grub-core/kern/arm/efi/init.c	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/efi/init.c	2013-03-24 15:32:46 +0000
@@ -0,0 +1,68 @@
+/* init.c - initialize an arm-based EFI system */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013 Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/env.h>
+#include <grub/kernel.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/time.h>
+#include <grub/efi/efi.h>
+
+/*
+ * A bit ugly, but functional - and should be completely portable.
+ */
+static grub_uint64_t
+grub_efi_get_time_ms(void)
+{
+  grub_efi_time_t now;
+  grub_uint64_t retval;
+  grub_efi_status_t status;
+
+  status = efi_call_2 (grub_efi_system_table->runtime_services->get_time,
+		       &now, NULL);
+  if (status != GRUB_EFI_SUCCESS)
+    {
+      grub_printf("No time!\n");
+      return 0;
+    }
+  retval = now.year * 365 * 24 * 60 * 60 * 1000;
+  retval += now.month * 30 * 24 * 60 * 60 * 1000;
+  retval += now.day * 24 * 60 * 60 * 1000;
+  retval += now.hour * 60 * 60 * 1000;
+  retval += now.minute * 60 * 1000;
+  retval += now.second * 1000;
+  retval += now.nanosecond / 1000;
+ 
+  grub_dprintf("timer", "timestamp: 0x%llx\n", retval);
+
+  return retval;
+}
+
+void
+grub_machine_init (void)
+{
+  grub_efi_init ();
+  grub_install_get_time_ms (grub_efi_get_time_ms);
+}
+
+void
+grub_machine_fini (void)
+{
+  grub_efi_fini ();
+}

=== added file 'grub-core/kern/arm/efi/misc.c'
--- grub-core/kern/arm/efi/misc.c	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/efi/misc.c	2013-03-24 15:43:19 +0000
@@ -0,0 +1,203 @@
+/* misc.c - various system functions for an arm-based EFI system */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013 Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/cpu/linux.h>
+#include <grub/cpu/system.h>
+#include <grub/efi/efi.h>
+#include <grub/machine/loader.h>
+
+static inline grub_size_t
+page_align (grub_size_t size)
+{
+  return (size + (1 << 12) - 1) & (~((1 << 12) - 1));
+}
+
+/* Find the optimal number of pages for the memory map. Is it better to
+   move this code to efi/mm.c?  */
+static grub_efi_uintn_t
+find_mmap_size (void)
+{
+  static grub_efi_uintn_t mmap_size = 0;
+
+  if (mmap_size != 0)
+    return mmap_size;
+  
+  mmap_size = (1 << 12);
+  while (1)
+    {
+      int ret;
+      grub_efi_memory_descriptor_t *mmap;
+      grub_efi_uintn_t desc_size;
+      
+      mmap = grub_malloc (mmap_size);
+      if (! mmap)
+	return 0;
+
+      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0, &desc_size, 0);
+      grub_free (mmap);
+      
+      if (ret < 0)
+	{
+	  grub_error (GRUB_ERR_IO, "cannot get memory map");
+	  return 0;
+	}
+      else if (ret > 0)
+	break;
+
+      mmap_size += (1 << 12);
+    }
+
+  /* Increase the size a bit for safety, because GRUB allocates more on
+     later, and EFI itself may allocate more.  */
+  mmap_size += (1 << 12);
+
+  return page_align (mmap_size);
+}
+
+#define NEXT_MEMORY_DESCRIPTOR(desc, size)      \
+  ((grub_efi_memory_descriptor_t *) ((char *) (desc) + (size)))
+#define PAGE_SHIFT 12
+
+void *
+grub_efi_allocate_loader_memory (grub_uint32_t min_offset, grub_uint32_t size)
+{
+  grub_efi_uintn_t desc_size;
+  grub_efi_memory_descriptor_t *mmap, *mmap_end;
+  grub_efi_uintn_t mmap_size, tmp_mmap_size;
+  grub_efi_memory_descriptor_t *desc;
+  void *mem = NULL;
+  grub_addr_t min_start = 0;
+
+  mmap_size = find_mmap_size();
+  if (!mmap_size)
+    return NULL;
+
+  mmap = grub_malloc(mmap_size);
+  if (!mmap)
+    return NULL;
+
+  tmp_mmap_size = mmap_size;
+  if (grub_efi_get_memory_map (&tmp_mmap_size, mmap, 0, &desc_size, 0) <= 0)
+    {
+      grub_error (GRUB_ERR_IO, "cannot get memory map");
+      goto fail;
+    }
+
+  mmap_end = NEXT_MEMORY_DESCRIPTOR (mmap, tmp_mmap_size);
+  /* Find lowest accessible RAM location */
+  {
+    int found = 0;
+    for (desc = mmap ; !found && (desc < mmap_end) ;
+	 desc = NEXT_MEMORY_DESCRIPTOR(desc, desc_size))
+      {
+	switch (desc->type)
+	  {
+	  case GRUB_EFI_CONVENTIONAL_MEMORY:
+	  case GRUB_EFI_LOADER_CODE:
+	  case GRUB_EFI_LOADER_DATA:
+	    min_start = desc->physical_start + min_offset;
+	    found = 1;
+	    break;
+	  default:
+	    break;
+	  }
+      }
+  }
+
+  /* First, find free pages for the real mode code
+     and the memory map buffer.  */
+  for (desc = mmap ; desc < mmap_end ;
+       desc = NEXT_MEMORY_DESCRIPTOR(desc, desc_size))
+    {
+      grub_uint64_t start, end;
+
+      grub_dprintf("mm", "%s: 0x%08x bytes @ 0x%08x\n",
+		   __FUNCTION__,
+		   (grub_uint32_t) (desc->num_pages << PAGE_SHIFT),
+		   (grub_uint32_t) (desc->physical_start));
+
+      if (desc->type != GRUB_EFI_CONVENTIONAL_MEMORY)
+	continue;
+
+      start = desc->physical_start;
+      end = start + (desc->num_pages << PAGE_SHIFT);
+      grub_dprintf("mm", "%s: start=0x%016llx, end=0x%016llx\n",
+		  __FUNCTION__, start, end);
+      start = start < min_start ? min_start : start;
+      if (start + size > end)
+	continue;
+      grub_dprintf("mm", "%s: let's allocate some (0x%x) pages @ 0x%08x...\n",
+		  __FUNCTION__, (size >> PAGE_SHIFT), (grub_addr_t) start);
+      mem = grub_efi_allocate_pages (start, (size >> PAGE_SHIFT) + 1);
+      grub_dprintf("mm", "%s: retval=0x%08x\n",
+		   __FUNCTION__, (grub_addr_t) mem);
+      if (! mem)
+	{
+	  grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
+	  goto fail;
+	}
+      break;
+    }
+
+  if (! mem)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
+      goto fail;
+    }
+
+  grub_free (mmap);
+  return mem;
+
+ fail:
+  grub_free (mmap);
+  return NULL;
+}
+
+grub_err_t
+grub_efi_prepare_platform (void)
+{
+  grub_efi_uintn_t mmap_size;
+  grub_efi_uintn_t map_key;
+  grub_efi_uintn_t desc_size;
+  grub_efi_uint32_t desc_version;
+  grub_efi_memory_descriptor_t *mmap_buf;
+  grub_err_t err;
+
+  /*
+   * Cloned from IA64
+   * Must be done after grub_machine_fini because map_key is used by
+   *exit_boot_services.
+   */
+  mmap_size = find_mmap_size ();
+  if (! mmap_size)
+    return GRUB_ERR_OUT_OF_MEMORY;
+  mmap_buf = grub_efi_allocate_pages (0, page_align (mmap_size) >> 12);
+  if (! mmap_buf)
+    return GRUB_ERR_OUT_OF_MEMORY;
+
+  err = grub_efi_finish_boot_services (&mmap_size, mmap_buf, &map_key,
+				       &desc_size, &desc_version);
+  if (err != GRUB_ERR_NONE)
+    return err;
+
+  grub_arm_disable_caches_mmu();
+  return GRUB_ERR_NONE;
+}

=== added file 'grub-core/kern/arm/efi/startup.S'
--- grub-core/kern/arm/efi/startup.S	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/efi/startup.S	2013-03-24 15:32:46 +0000
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2013 Free Software Foundation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ */
+
+#include <grub/symbol.h>
+
+	.file 	"startup.S"
+	.text
+	.arm
+FUNCTION(_start)
+	/*
+	 *  EFI_SYSTEM_TABLE and EFI_HANDLE are passed in r1/r0.
+	 */
+	ldr	ip, =EXT_C(grub_efi_image_handle)
+	str	r0, [ip]
+	ldr	ip, =EXT_C(grub_efi_system_table)
+	str	r1, [ip]
+	ldr	ip, =EXT_C(grub_main)
+	bx	ip
+	.thumb		@ For relocation debugging
+	blx	_start
+	.end

=== modified file 'grub-core/lib/efi/halt.c'
--- grub-core/lib/efi/halt.c	2011-01-03 01:28:14 +0000
+++ grub-core/lib/efi/halt.c	2013-03-24 15:32:46 +0000
@@ -28,7 +28,7 @@
 grub_halt (void)
 {
   grub_machine_fini ();
-#ifndef __ia64__
+#if !defined(__ia64__) && !defined(__arm__)
   grub_acpi_halt ();
 #endif
   efi_call_4 (grub_efi_system_table->runtime_services->reset_system,

=== added directory 'include/grub/arm/efi'
=== added file 'include/grub/arm/efi/loader.h'
--- include/grub/arm/efi/loader.h	1970-01-01 00:00:00 +0000
+++ include/grub/arm/efi/loader.h	2013-03-24 15:32:46 +0000
@@ -0,0 +1,26 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_LOADER_MACHINE_HEADER
+#define GRUB_LOADER_MACHINE_HEADER	1
+
+grub_err_t EXPORT_FUNC (grub_efi_prepare_platform) (void);
+void * EXPORT_FUNC (grub_efi_allocate_loader_memory) (grub_uint32_t min_offset,
+						      grub_uint32_t size);
+
+#endif /* ! GRUB_LOADER_MACHINE_HEADER */

=== added file 'include/grub/arm/efi/memory.h'
--- include/grub/arm/efi/memory.h	1970-01-01 00:00:00 +0000
+++ include/grub/arm/efi/memory.h	2013-03-24 15:32:46 +0000
@@ -0,0 +1,1 @@
+#include <grub/efi/memory.h>

=== modified file 'include/grub/efi/pe32.h'
--- include/grub/efi/pe32.h	2011-01-03 12:46:36 +0000
+++ include/grub/efi/pe32.h	2013-03-24 15:32:46 +0000
@@ -63,9 +63,10 @@
   grub_uint16_t characteristics;
 };
 
-#define GRUB_PE32_MACHINE_I386		0x14c
-#define GRUB_PE32_MACHINE_IA64		0x200
-#define GRUB_PE32_MACHINE_X86_64	0x8664
+#define GRUB_PE32_MACHINE_I386			0x14c
+#define GRUB_PE32_MACHINE_IA64			0x200
+#define GRUB_PE32_MACHINE_X86_64		0x8664
+#define GRUB_PE32_MACHINE_ARMTHUMB_MIXED	0x01c2
 
 #define GRUB_PE32_RELOCS_STRIPPED		0x0001
 #define GRUB_PE32_EXECUTABLE_IMAGE		0x0002
@@ -276,8 +277,10 @@
 #define GRUB_PE32_REL_BASED_HIGHLOW	3
 #define GRUB_PE32_REL_BASED_HIGHADJ	4
 #define GRUB_PE32_REL_BASED_MIPS_JMPADDR 5
+#define GRUB_PE32_REL_BASED_ARM_MOV32A  5
 #define GRUB_PE32_REL_BASED_SECTION	6
 #define GRUB_PE32_REL_BASED_REL		7
+#define GRUB_PE32_REL_BASED_ARM_MOV32T  7
 #define GRUB_PE32_REL_BASED_IA64_IMM64	9
 #define GRUB_PE32_REL_BASED_DIR64	10
 #define GRUB_PE32_REL_BASED_HIGH3ADJ	11

=== modified file 'util/grub-install.in'
--- util/grub-install.in	2013-03-24 14:49:04 +0000
+++ util/grub-install.in	2013-03-24 15:32:46 +0000
@@ -469,6 +469,8 @@
 	    # expansion.
 		ia64)
 		    efi_file=BOOTIA64.EFI ;;
+		arm)
+		    efi_file=BOOTARM.EFI ;;
 	    esac
 	else
 	    # It is convenient for each architecture to have a different
@@ -483,6 +485,8 @@
 	 # expansion.
 		ia64)
 		    efi_file=grubia64.efi ;;
+		arm)
+		    efi_file=grubarm.efi ;;
 		*)
 		    efi_file=grub.efi ;;
 	    esac

=== modified file 'util/grub-mkimage.c'
--- util/grub-mkimage.c	2013-03-24 14:49:04 +0000
+++ util/grub-mkimage.c	2013-03-24 15:32:46 +0000
@@ -472,6 +472,27 @@
       .mod_align = GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN,
       .link_align = 4
     },
+    {
+      .dirname = "arm-efi",
+      .names = { "arm-efi", NULL },
+      .voidp_sizeof = 4,
+      .bigendian = 0, 
+      .id = IMAGE_EFI, 
+      .flags = PLATFORM_FLAGS_NONE,
+      .total_module_size = TARGET_NO_FIELD,
+      .decompressor_compressed_size = TARGET_NO_FIELD,
+      .decompressor_uncompressed_size = TARGET_NO_FIELD,
+      .decompressor_uncompressed_addr = TARGET_NO_FIELD,
+      .section_align = GRUB_PE32_SECTION_ALIGNMENT,
+      .vaddr_offset = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE
+                                + GRUB_PE32_SIGNATURE_SIZE
+                                + sizeof (struct grub_pe32_coff_header)
+                                + sizeof (struct grub_pe32_optional_header)
+                                + 4 * sizeof (struct grub_pe32_section_table),
+                                GRUB_PE32_SECTION_ALIGNMENT),
+      .pe_target = GRUB_PE32_MACHINE_ARMTHUMB_MIXED,
+      .elf_target = EM_ARM,
+    },
   };
 
 #define grub_target_to_host32(x) (grub_target_to_host32_real (image_target, (x)))

=== modified file 'util/grub-mkimagexx.c'
--- util/grub-mkimagexx.c	2012-02-29 17:57:43 +0000
+++ util/grub-mkimagexx.c	2013-03-24 15:32:46 +0000
@@ -58,6 +58,11 @@
 #error "I'm confused"
 #endif
 
+static Elf_Addr SUFFIX (entry_point);
+
+grub_err_t reloc_thm_call (grub_uint16_t *addr, Elf32_Addr sym_addr);
+grub_err_t reloc_thm_jump19 (grub_uint16_t *addr, Elf32_Addr sym_addr);
+
 /* Relocate symbols; note that this function overwrites the symbol table.
    Return the address of a start symbol.  */
 static Elf_Addr
@@ -528,6 +533,48 @@
 		}
 	       break;
 #endif
+#if defined(MKIMAGE_ELF32)
+	     case EM_ARM:
+	       {
+		 sym_addr += addend;
+		 sym_addr -= SUFFIX (entry_point);
+		 switch (ELF_R_TYPE (info))
+		   {
+		   case R_ARM_ABS32:
+		     {
+		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
+				       (int) sym_addr, (int) sym_addr);
+		       /* Data will be naturally aligned */
+		       //      sym_addr -= offset;
+		       sym_addr += 0x400;
+		       *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr);
+		     }
+		     break;
+		   case R_ARM_THM_CALL:
+		   case R_ARM_THM_JUMP24:
+		     {
+		       grub_util_info ("  THM_JUMP24:\ttarget=0x%08x\toffset=(0x%08x)",	(unsigned int) target, sym_addr);
+		       sym_addr -= offset;
+		       /* Thumb instructions can be 16-bit aligned */
+		       reloc_thm_call ((grub_uint16_t *) target, sym_addr);
+		     }
+		     break;
+		   case R_ARM_THM_JUMP19:
+		     {
+		       grub_util_info ("  THM_JUMP19:\toffset=%d\t(0x%08x)",
+				       sym_addr, sym_addr);
+		       sym_addr -= offset;
+		       /* Thumb instructions can be 16-bit aligned */
+		       reloc_thm_jump19 ((grub_uint16_t *) target, sym_addr);
+		     }
+		     break;
+		   default:
+		     grub_util_error (_("relocation 0x%x is not implemented yet!"), ELF_R_TYPE (info));
+		     break;
+		   }
+		 break;
+	       }
+#endif /* MKIMAGE_ELF32 */
 	     default:
 	       grub_util_error ("unknown architecture type %d",
 				image_target->elf_target);
@@ -755,6 +802,46 @@
 		  break;
 		}
 		break;
+#if defined(MKIMAGE_ELF32)
+	      case EM_ARM:
+		switch (ELF_R_TYPE (info))
+		  {
+		    /* Relative relocations do not require fixup entries. */
+		  case R_ARM_JUMP24:
+		  case R_ARM_THM_CALL:
+		  case R_ARM_THM_JUMP19:
+		  case R_ARM_THM_JUMP24:
+		    {
+		      Elf_Addr addr;
+
+		      addr = section_address + offset;
+		      grub_util_info ("  %s:  not adding fixup: 0x%08x : 0x%08x", __FUNCTION__, (unsigned int) addr, (unsigned int) current_address);
+		    }
+		    break;
+		    /* Create fixup entry for PE/COFF loader */
+		  case R_ARM_ABS32:
+		    {      
+		      Elf_Addr addr;
+
+		      addr = section_address + offset;
+#if 0
+		      grub_util_info ("  %s:  add_fixup: 0x%08x : 0x%08x",
+				      __FUNCTION__, (unsigned int) addr,
+				      (unsigned int) current_address);
+#endif
+		      current_address
+			= SUFFIX (add_fixup_entry) (&lst,
+						    GRUB_PE32_REL_BASED_HIGHLOW,
+						    addr, 0, current_address,
+						    image_target);
+		    }
+		    break;
+		  default:
+		    grub_util_error (_("relocation 0x%x is not implemented yet2"), ELF_R_TYPE (info));
+		    break;
+		  }
+		break;
+#endif /* defined(MKIMAGE_ELF32) */
 	      default:
 		grub_util_error ("unknown machine type 0x%x", image_target->elf_target);
 	      }
@@ -1065,6 +1152,8 @@
       if (*start == 0)
 	grub_util_error ("start symbol is not defined");
       
+      SUFFIX (entry_point) = (Elf_Addr) *start;
+
       /* Resolve addresses in the virtual address space.  */
       SUFFIX (relocate_addresses) (e, sections, section_addresses, 
 				   section_entsize,


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-03-24 17:01 [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms Leif Lindholm
@ 2013-04-01  2:31 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-01 10:41   ` Francesco Lavra
  2013-04-03 17:50   ` Leif Lindholm
  2013-04-01 16:24 ` Francesco Lavra
  1 sibling, 2 replies; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-04-01  2:31 UTC (permalink / raw)
  To: The development of GNU GRUB

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

> +static grub_uint64_t

> +grub_efi_get_time_ms(void)
> +{
> +  grub_efi_time_t now;
> +  grub_uint64_t retval;
> +  grub_efi_status_t status;
> +
> +  status = efi_call_2 (grub_efi_system_table->runtime_services->get_time,
> +		       &now, NULL);
> +  if (status != GRUB_EFI_SUCCESS)
> +    {
> +      grub_printf("No time!\n");
> +      return 0;

This is about the worse thing you can do. It will make any timeout go wrong.

> +    }
> +  retval = now.year * 365 * 24 * 60 * 60 * 1000;
> +  retval += now.month * 30 * 24 * 60 * 60 * 1000;
> +  retval += now.day * 24 * 60 * 60 * 1000;
> +  retval += now.hour * 60 * 60 * 1000;
> +  retval += now.minute * 60 * 1000;
> +  retval += now.second * 1000;
> +  retval += now.nanosecond / 1000;
> + 
> +  grub_dprintf("timer", "timestamp: 0x%llx\n", retval);
> +
> +  return retval;

This is almost a verbatim copy of what we had for i386 efi before but it
went haywire in many ways. Like jumps forward or backward around end of
month or when one sets datetime. Or around the summer/winter timezone
transition.
Does ARM have anything like TSC?

> +static inline grub_size_t
> +page_align (grub_size_t size)
> +{
> +  return (size + (1 << 12) - 1) & (~((1 << 12) - 1));
> +}

We already have ALIGN_UP




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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01  2:31 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-04-01 10:41   ` Francesco Lavra
  2013-04-01 11:23     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-03 17:50   ` Leif Lindholm
  1 sibling, 1 reply; 16+ messages in thread
From: Francesco Lavra @ 2013-04-01 10:41 UTC (permalink / raw)
  To: grub-devel

On 04/01/2013 04:31 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> +static grub_uint64_t
> 
>> +grub_efi_get_time_ms(void)
>> +{
>> +  grub_efi_time_t now;
>> +  grub_uint64_t retval;
>> +  grub_efi_status_t status;
>> +
>> +  status = efi_call_2 (grub_efi_system_table->runtime_services->get_time,
>> +		       &now, NULL);
>> +  if (status != GRUB_EFI_SUCCESS)
>> +    {
>> +      grub_printf("No time!\n");
>> +      return 0;
> 
> This is about the worse thing you can do. It will make any timeout go wrong.

How would you handle such a case? I guess a machine which can't provide
this runtime service would need some more work in its EFI firmware
before being ready for GRUB, so perhaps this is a moot point.

> 
>> +    }
>> +  retval = now.year * 365 * 24 * 60 * 60 * 1000;
>> +  retval += now.month * 30 * 24 * 60 * 60 * 1000;
>> +  retval += now.day * 24 * 60 * 60 * 1000;
>> +  retval += now.hour * 60 * 60 * 1000;
>> +  retval += now.minute * 60 * 1000;
>> +  retval += now.second * 1000;
>> +  retval += now.nanosecond / 1000;
>> + 
>> +  grub_dprintf("timer", "timestamp: 0x%llx\n", retval);
>> +
>> +  return retval;
> 
> This is almost a verbatim copy of what we had for i386 efi before but it
> went haywire in many ways. Like jumps forward or backward around end of
> month or when one sets datetime. Or around the summer/winter timezone
> transition.

I propose to re-use the existing function grub_datetime2unixtime()
(which handles correctly the number of days of each month, as well as
leap years), instead of doing the calculations here. And take into
account the time_zone member of grub_efi_time_t as well.
Here is how I would write it:

grub_uint64_t
grub_efi_get_time_ms (void)
{
  grub_efi_time_t efi_time;
  struct grub_datetime datetime;
  grub_int32_t unixtime;
  grub_uint64_t retval;

  if (efi_call_2 (grub_efi_system_table->runtime_services->get_time,
                  &efi_time, 0) != GRUB_EFI_SUCCESS)
    {
      grub_printf("No time!\n");
      return 0;
    }
  datetime.year = efi_time.year;
  datetime.month = efi_time.month;
  datetime.day = efi_time.day;
  datetime.hour = efi_time.hour;
  datetime.minute = efi_time.minute;
  datetime.second = efi_time.second;
  if (!grub_datetime2unixtime (&datetime, &unixtime))
	return 0;
  unixtime += 60 * efi_time.time_zone;
  retval = (grub_uint64_t)unixtime * 1000
           + efi_time.nanosecond / 1000000;
  grub_dprintf("timer", "timestamp: 0x%llx\n", retval);
  return retval;
}

Also, there is nothing ARM-specific in this function, so I would put it
in a generic EFI file like kern/efi/efi.c.

> Does ARM have anything like TSC?

ARMv7 does not have an architecturally-defined mechanism of retrieving
the timestamp, it's up to the system peripherals to provide time-keeping
capabilities.

--
Francesco


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01 10:41   ` Francesco Lavra
@ 2013-04-01 11:23     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-01 14:10       ` Francesco Lavra
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-04-01 11:23 UTC (permalink / raw)
  To: The development of GNU GRUB

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

>

> How would you handle such a case? I guess a machine which can't provide
> this runtime service would need some more work in its EFI firmware
> before being ready for GRUB, so perhaps this is a moot point.
> 

If no timing is available, you'd need at least return a count of calls
to the get_time function.

> I propose to re-use the existing function grub_datetime2unixtime()
> (which handles correctly the number of days of each month, as well as
> leap years), instead of doing the calculations here. And take into
> account the time_zone member of grub_efi_time_t as well.

get_time is wrong function for getting tsc. You should create a timer
event with 10000 units (=1 ms) and in its callback increase millisecond
counter.

> Also, there is nothing ARM-specific in this function, so I would put it
> in a generic EFI file like kern/efi/efi.c.
> 

it is ARM-specific by exclusion. All other EFI ports have TSC and don't
need to use EFI functions to retrieve it (other than for calibration on
ia64)


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01 11:23     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-04-01 14:10       ` Francesco Lavra
  2013-04-01 14:16         ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 16+ messages in thread
From: Francesco Lavra @ 2013-04-01 14:10 UTC (permalink / raw)
  To: grub-devel

On 04/01/2013 01:23 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> get_time is wrong function for getting tsc. You should create a timer
> event with 10000 units (=1 ms) and in its callback increase millisecond
> counter.

The problem is that such timer event would be machine-specific and
wouldn't work across different ARM SoCs. So the best place to handle
these machine-specific details would be in the platform firmware.
Unfortunately the EFI spec doesn't have a standard mechanism to retrieve
a timestamp counter, the most similar mechanism currently defined by the
spec is the get_time service.

--
Francesco


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01 14:10       ` Francesco Lavra
@ 2013-04-01 14:16         ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-01 14:49           ` Francesco Lavra
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-04-01 14:16 UTC (permalink / raw)
  To: The development of GNU GRUB

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

On 01.04.2013 16:10, Francesco Lavra wrote:

> On 04/01/2013 01:23 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> get_time is wrong function for getting tsc. You should create a timer
>> event with 10000 units (=1 ms) and in its callback increase millisecond
>> counter.
> 
> The problem is that such timer event would be machine-specific and
> wouldn't work across different ARM SoCs. So the best place to handle
> these machine-specific details would be in the platform firmware.
> Unfortunately the EFI spec doesn't have a standard mechanism to retrieve
> a timestamp counter, the most similar mechanism currently defined by the
> spec is the get_time service.

Looks at events in EFI spec. You can define a periodic event with 1ms
period and custom function as callback. Then you just need to count in
this function.

> 
> --
> Francesco
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01 14:16         ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-04-01 14:49           ` Francesco Lavra
  0 siblings, 0 replies; 16+ messages in thread
From: Francesco Lavra @ 2013-04-01 14:49 UTC (permalink / raw)
  To: grub-devel

On 04/01/2013 04:16 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 01.04.2013 16:10, Francesco Lavra wrote:
> 
>> On 04/01/2013 01:23 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>> get_time is wrong function for getting tsc. You should create a timer
>>> event with 10000 units (=1 ms) and in its callback increase millisecond
>>> counter.
>>
>> The problem is that such timer event would be machine-specific and
>> wouldn't work across different ARM SoCs. So the best place to handle
>> these machine-specific details would be in the platform firmware.
>> Unfortunately the EFI spec doesn't have a standard mechanism to retrieve
>> a timestamp counter, the most similar mechanism currently defined by the
>> spec is the get_time service.
> 
> Looks at events in EFI spec. You can define a periodic event with 1ms
> period and custom function as callback. Then you just need to count in
> this function.

You are right, I wasn't aware of this capability. I have no idea how
widespread is support of timer events in different ARM EFI
implementations. We'll find out...


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-03-24 17:01 [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms Leif Lindholm
  2013-04-01  2:31 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-04-01 16:24 ` Francesco Lavra
  2013-04-03 18:07   ` Leif Lindholm
  1 sibling, 1 reply; 16+ messages in thread
From: Francesco Lavra @ 2013-04-01 16:24 UTC (permalink / raw)
  To: grub-devel

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

On 03/24/2013 06:01 PM, Leif Lindholm wrote:
> === added file 'grub-core/kern/arm/efi/misc.c'
> --- grub-core/kern/arm/efi/misc.c	1970-01-01 00:00:00 +0000
> +++ grub-core/kern/arm/efi/misc.c	2013-03-24 15:43:19 +0000
[...]
> +void *
> +grub_efi_allocate_loader_memory (grub_uint32_t min_offset, grub_uint32_t size)
> +{
[...]
> +  for (desc = mmap ; desc < mmap_end ;
> +       desc = NEXT_MEMORY_DESCRIPTOR(desc, desc_size))
> +    {
> +      grub_uint64_t start, end;
> +
> +      grub_dprintf("mm", "%s: 0x%08x bytes @ 0x%08x\n",
> +		   __FUNCTION__,
> +		   (grub_uint32_t) (desc->num_pages << PAGE_SHIFT),
> +		   (grub_uint32_t) (desc->physical_start));
> +
> +      if (desc->type != GRUB_EFI_CONVENTIONAL_MEMORY)
> +	continue;
> +
> +      start = desc->physical_start;
> +      end = start + (desc->num_pages << PAGE_SHIFT);
> +      grub_dprintf("mm", "%s: start=0x%016llx, end=0x%016llx\n",
> +		  __FUNCTION__, start, end);
> +      start = start < min_start ? min_start : start;
> +      if (start + size > end)
> +	continue;
> +      grub_dprintf("mm", "%s: let's allocate some (0x%x) pages @ 0x%08x...\n",
> +		  __FUNCTION__, (size >> PAGE_SHIFT), (grub_addr_t) start);
> +      mem = grub_efi_allocate_pages (start, (size >> PAGE_SHIFT) + 1);
> +      grub_dprintf("mm", "%s: retval=0x%08x\n",
> +		   __FUNCTION__, (grub_addr_t) mem);
> +      if (! mem)
> +	{
> +	  grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
> +	  goto fail;
> +	}

This if block is redundant, since it is repeated just after the for()
loop. The break statement below would do.

> +      break;
> +    }
> +
> +  if (! mem)
> +    {
> +      grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
> +      goto fail;
> +    }
> +
> +  grub_free (mmap);
> +  return mem;
> +
> + fail:
> +  grub_free (mmap);
> +  return NULL;
> +}
[...]
> === added file 'grub-core/kern/arm/efi/startup.S'
> --- grub-core/kern/arm/efi/startup.S	1970-01-01 00:00:00 +0000
> +++ grub-core/kern/arm/efi/startup.S	2013-03-24 15:32:46 +0000
> @@ -0,0 +1,38 @@
> +/*
> + * (C) Copyright 2013 Free Software Foundation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + *
> + */

This license header doesn't follow exactly GRUB's template.

[...]
> --- util/grub-mkimage.c	2013-03-24 14:49:04 +0000
> +++ util/grub-mkimage.c	2013-03-24 15:32:46 +0000
> @@ -472,6 +472,27 @@
>        .mod_align = GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN,
>        .link_align = 4
>      },
> +    {
> +      .dirname = "arm-efi",
> +      .names = { "arm-efi", NULL },
> +      .voidp_sizeof = 4,
> +      .bigendian = 0, 
> +      .id = IMAGE_EFI,

Nitpick: the last two lines above have trailing whitespace.

> +      .flags = PLATFORM_FLAGS_NONE,
> +      .total_module_size = TARGET_NO_FIELD,
> +      .decompressor_compressed_size = TARGET_NO_FIELD,
> +      .decompressor_uncompressed_size = TARGET_NO_FIELD,
> +      .decompressor_uncompressed_addr = TARGET_NO_FIELD,
> +      .section_align = GRUB_PE32_SECTION_ALIGNMENT,
> +      .vaddr_offset = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE
> +                                + GRUB_PE32_SIGNATURE_SIZE
> +                                + sizeof (struct grub_pe32_coff_header)
> +                                + sizeof (struct grub_pe32_optional_header)
> +                                + 4 * sizeof (struct grub_pe32_section_table),
> +                                GRUB_PE32_SECTION_ALIGNMENT),

This ALIGN_UP() thing is a bit duplicated in this file. How about adding
a #define EFI32_HEADER_SIZE, analogous to EFI64_HEADER_SIZE, and using
the new define?
The attached patch does this simple refactoring and could be applied
right away, independently from the ARM port.

> +      .pe_target = GRUB_PE32_MACHINE_ARMTHUMB_MIXED,
> +      .elf_target = EM_ARM,
> +    },
>    };
>  
>  #define grub_target_to_host32(x) (grub_target_to_host32_real (image_target, (x)))
> 
> === modified file 'util/grub-mkimagexx.c'
> --- util/grub-mkimagexx.c	2012-02-29 17:57:43 +0000
> +++ util/grub-mkimagexx.c	2013-03-24 15:32:46 +0000
> @@ -58,6 +58,11 @@
>  #error "I'm confused"
>  #endif
>  
> +static Elf_Addr SUFFIX (entry_point);
> +
> +grub_err_t reloc_thm_call (grub_uint16_t *addr, Elf32_Addr sym_addr);
> +grub_err_t reloc_thm_jump19 (grub_uint16_t *addr, Elf32_Addr sym_addr);
> +
>  /* Relocate symbols; note that this function overwrites the symbol table.
>     Return the address of a start symbol.  */
>  static Elf_Addr
> @@ -528,6 +533,48 @@
>  		}
>  	       break;
>  #endif
> +#if defined(MKIMAGE_ELF32)
> +	     case EM_ARM:
> +	       {
> +		 sym_addr += addend;
> +		 sym_addr -= SUFFIX (entry_point);
> +		 switch (ELF_R_TYPE (info))
> +		   {
> +		   case R_ARM_ABS32:
> +		     {
> +		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
> +				       (int) sym_addr, (int) sym_addr);
> +		       /* Data will be naturally aligned */
> +		       //      sym_addr -= offset;

Maybe this commented line should be removed altogether?

> +		       sym_addr += 0x400;

Where does this 0x400 value come from?

> +		       *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr);
> +		     }
> +		     break;
> +		   case R_ARM_THM_CALL:
> +		   case R_ARM_THM_JUMP24:
> +		     {
> +		       grub_util_info ("  THM_JUMP24:\ttarget=0x%08x\toffset=(0x%08x)",	(unsigned int) target, sym_addr);
> +		       sym_addr -= offset;
> +		       /* Thumb instructions can be 16-bit aligned */
> +		       reloc_thm_call ((grub_uint16_t *) target, sym_addr);

reloc_thm_call() works with native endianness, so it cannot be used as
is here, as explained in the other mail a few hours ago.

> +		     }
> +		     break;
> +		   case R_ARM_THM_JUMP19:
> +		     {
> +		       grub_util_info ("  THM_JUMP19:\toffset=%d\t(0x%08x)",
> +				       sym_addr, sym_addr);
> +		       sym_addr -= offset;
> +		       /* Thumb instructions can be 16-bit aligned */
> +		       reloc_thm_jump19 ((grub_uint16_t *) target, sym_addr);

Ditto for reloc_thm_jump19(), it works with native endianness and cannot
be used as is.

--
Francesco

[-- Attachment #2: grub-mkimage-refactoring.patch --]
[-- Type: text/x-patch, Size: 1686 bytes --]

=== modified file 'util/grub-mkimage.c'
--- util/grub-mkimage.c	2013-03-07 07:17:24 +0000
+++ util/grub-mkimage.c	2013-03-31 13:50:50 +0000
@@ -91,6 +91,13 @@
   grub_uint16_t pe_target;
 };
 
+#define EFI32_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE		\
+				    + GRUB_PE32_SIGNATURE_SIZE		\
+				    + sizeof (struct grub_pe32_coff_header) \
+				    + sizeof (struct grub_pe32_optional_header) \
+				    + 4 * sizeof (struct grub_pe32_section_table), \
+				    GRUB_PE32_SECTION_ALIGNMENT)
+
 #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE		\
 				    + GRUB_PE32_SIGNATURE_SIZE		\
 				    + sizeof (struct grub_pe32_coff_header) \
@@ -182,12 +189,7 @@
       .decompressor_uncompressed_size = TARGET_NO_FIELD,
       .decompressor_uncompressed_addr = TARGET_NO_FIELD,
       .section_align = GRUB_PE32_SECTION_ALIGNMENT,
-      .vaddr_offset = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE
-				+ GRUB_PE32_SIGNATURE_SIZE
-				+ sizeof (struct grub_pe32_coff_header)
-				+ sizeof (struct grub_pe32_optional_header)
-				+ 4 * sizeof (struct grub_pe32_section_table),
-				GRUB_PE32_SECTION_ALIGNMENT),
+      .vaddr_offset = EFI32_HEADER_SIZE,
       .pe_target = GRUB_PE32_MACHINE_I386,
       .elf_target = EM_386,
     },
@@ -1101,12 +1103,7 @@
 	int reloc_addr;
 
 	if (image_target->voidp_sizeof == 4)
-	  header_size = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE
-				  + GRUB_PE32_SIGNATURE_SIZE
-				  + sizeof (struct grub_pe32_coff_header)
-				  + sizeof (struct grub_pe32_optional_header)
-				  + 4 * sizeof (struct grub_pe32_section_table),
-				  GRUB_PE32_SECTION_ALIGNMENT);
+	  header_size = EFI32_HEADER_SIZE;
 	else
 	  header_size = EFI64_HEADER_SIZE;
 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01  2:31 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-01 10:41   ` Francesco Lavra
@ 2013-04-03 17:50   ` Leif Lindholm
  1 sibling, 0 replies; 16+ messages in thread
From: Leif Lindholm @ 2013-04-03 17:50 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: phcoder

On Mon, Apr 01, 2013 at 04:31:13AM +0200, Vladimir '??-coder/phcoder' Serbinenko wrote:
> > +static grub_uint64_t
> 
> > +grub_efi_get_time_ms(void)
> > +{
> > +  grub_efi_time_t now;
> > +  grub_uint64_t retval;
> > +  grub_efi_status_t status;
> > +
> > +  status = efi_call_2 (grub_efi_system_table->runtime_services->get_time,
> > +		       &now, NULL);
> > +  if (status != GRUB_EFI_SUCCESS)
> > +    {
> > +      grub_printf("No time!\n");
> > +      return 0;
> 
> This is about the worse thing you can do. It will make any timeout go wrong.
 
What would be a better way?

> > +    }
> > +  retval = now.year * 365 * 24 * 60 * 60 * 1000;
> > +  retval += now.month * 30 * 24 * 60 * 60 * 1000;
> > +  retval += now.day * 24 * 60 * 60 * 1000;
> > +  retval += now.hour * 60 * 60 * 1000;
> > +  retval += now.minute * 60 * 1000;
> > +  retval += now.second * 1000;
> > +  retval += now.nanosecond / 1000;
> > + 
> > +  grub_dprintf("timer", "timestamp: 0x%llx\n", retval);
> > +
> > +  return retval;
> 
> This is almost a verbatim copy of what we had for i386 efi before but it
> went haywire in many ways. Like jumps forward or backward around end of
> month or when one sets datetime. Or around the summer/winter timezone
> transition.
> Does ARM have anything like TSC?
 
Not standardised and architecturally required.
The closest thing we have in base ARMv7-A is the performance monitor
cycle counter - but the performance monitor extension is "strongly
recommended" rather than required.

> > +static inline grub_size_t
> > +page_align (grub_size_t size)
> > +{
> > +  return (size + (1 << 12) - 1) & (~((1 << 12) - 1));
> > +}
> 
> We already have ALIGN_UP

In my defense, that is a verbatim copy from loader/ia64/efi/linux.c :)
There is a comment in that file that some code that I am now copying
in my port might be better to move to efi/mm.c...

/
    Leif


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-01 16:24 ` Francesco Lavra
@ 2013-04-03 18:07   ` Leif Lindholm
  2013-04-03 20:01     ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 16+ messages in thread
From: Leif Lindholm @ 2013-04-03 18:07 UTC (permalink / raw)
  To: The development of GNU GRUB

On Mon, Apr 01, 2013 at 06:24:43PM +0200, Francesco Lavra wrote:
> On 03/24/2013 06:01 PM, Leif Lindholm wrote:
> > === added file 'grub-core/kern/arm/efi/misc.c'
> > --- grub-core/kern/arm/efi/misc.c	1970-01-01 00:00:00 +0000
> > +++ grub-core/kern/arm/efi/misc.c	2013-03-24 15:43:19 +0000
> [...]
> > +void *
> > +grub_efi_allocate_loader_memory (grub_uint32_t min_offset, grub_uint32_t size)
> > +{
> [...]
> > +	{
> > +	  grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
> > +	  goto fail;
> > +	}
> 
> This if block is redundant, since it is repeated just after the for()
> loop. The break statement below would do.
 
OK.

> [...]
> > === added file 'grub-core/kern/arm/efi/startup.S'
> > --- grub-core/kern/arm/efi/startup.S	1970-01-01 00:00:00 +0000
> > +++ grub-core/kern/arm/efi/startup.S	2013-03-24 15:32:46 +0000
> > @@ -0,0 +1,38 @@
> > +/*
> > + * (C) Copyright 2013 Free Software Foundation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> > + * MA 02111-1307 USA
> > + *
> > + */
> 
> This license header doesn't follow exactly GRUB's template.

You're right - I thought I had sanity checked all these...
I think this also came from copying ia64 code.
 
> [...]
> > --- util/grub-mkimage.c	2013-03-24 14:49:04 +0000
> > +++ util/grub-mkimage.c	2013-03-24 15:32:46 +0000
> > @@ -472,6 +472,27 @@
> >        .mod_align = GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN,
> >        .link_align = 4
> >      },
> > +    {
> > +      .dirname = "arm-efi",
> > +      .names = { "arm-efi", NULL },
> > +      .voidp_sizeof = 4,
> > +      .bigendian = 0, 
> > +      .id = IMAGE_EFI,
> 
> Nitpick: the last two lines above have trailing whitespace.
 
Argh!
Fixed.

> > +      .flags = PLATFORM_FLAGS_NONE,
> > +      .total_module_size = TARGET_NO_FIELD,
> > +      .decompressor_compressed_size = TARGET_NO_FIELD,
> > +      .decompressor_uncompressed_size = TARGET_NO_FIELD,
> > +      .decompressor_uncompressed_addr = TARGET_NO_FIELD,
> > +      .section_align = GRUB_PE32_SECTION_ALIGNMENT,
> > +      .vaddr_offset = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE
> > +                                + GRUB_PE32_SIGNATURE_SIZE
> > +                                + sizeof (struct grub_pe32_coff_header)
> > +                                + sizeof (struct grub_pe32_optional_header)
> > +                                + 4 * sizeof (struct grub_pe32_section_table),
> > +                                GRUB_PE32_SECTION_ALIGNMENT),
> 
> This ALIGN_UP() thing is a bit duplicated in this file. How about adding
> a #define EFI32_HEADER_SIZE, analogous to EFI64_HEADER_SIZE, and using
> the new define?
> The attached patch does this simple refactoring and could be applied
> right away, independently from the ARM port.
 
Quite happy to use that instead.

> > === modified file 'util/grub-mkimagexx.c'
> > --- util/grub-mkimagexx.c	2012-02-29 17:57:43 +0000
> > +++ util/grub-mkimagexx.c	2013-03-24 15:32:46 +0000
[...]
> >     Return the address of a start symbol.  */
> >  static Elf_Addr
> > @@ -528,6 +533,48 @@
> >  		}
> >  	       break;
> >  #endif
> > +#if defined(MKIMAGE_ELF32)
> > +	     case EM_ARM:
> > +	       {
> > +		 sym_addr += addend;
> > +		 sym_addr -= SUFFIX (entry_point);
> > +		 switch (ELF_R_TYPE (info))
> > +		   {
> > +		   case R_ARM_ABS32:
> > +		     {
> > +		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
> > +				       (int) sym_addr, (int) sym_addr);
> > +		       /* Data will be naturally aligned */
> > +		       //      sym_addr -= offset;
> 
> Maybe this commented line should be removed altogether?
 
Yes.

> > +		       sym_addr += 0x400;
> 
> Where does this 0x400 value come from?
 
*cough* make that "SUFFIX (entry_point)".
_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.

> > +		       *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr);
> > +		     }
> > +		     break;
> > +		   case R_ARM_THM_CALL:
> > +		   case R_ARM_THM_JUMP24:
> > +		     {
> > +		       grub_util_info ("  THM_JUMP24:\ttarget=0x%08x\toffset=(0x%08x)",	(unsigned int) target, sym_addr);
> > +		       sym_addr -= offset;
> > +		       /* Thumb instructions can be 16-bit aligned */
> > +		       reloc_thm_call ((grub_uint16_t *) target, sym_addr);
> 
> reloc_thm_call() works with native endianness, so it cannot be used as
> is here, as explained in the other mail a few hours ago.
 
I don't see grub-mkimage currently being fully cross-platform anyway,
so I would (as mentioned in previous email) prefer to postpone any
such adjustments until the basic support is in. I have the patches
for it, if not entirely up to date.

> > +		     }
> > +		     break;
> > +		   case R_ARM_THM_JUMP19:
> > +		     {
> > +		       grub_util_info ("  THM_JUMP19:\toffset=%d\t(0x%08x)",
> > +				       sym_addr, sym_addr);
> > +		       sym_addr -= offset;
> > +		       /* Thumb instructions can be 16-bit aligned */
> > +		       reloc_thm_jump19 ((grub_uint16_t *) target, sym_addr);
> 
> Ditto for reloc_thm_jump19(), it works with native endianness and cannot
> be used as is.
 
Ditto.

/
    Leif


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-03 18:07   ` Leif Lindholm
@ 2013-04-03 20:01     ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-04-04 12:54       ` Leif Lindholm
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-04-03 20:01 UTC (permalink / raw)
  To: The development of GNU GRUB

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

On 03.04.2013 20:07, Leif Lindholm wrote:

> I don't see grub-mkimage currently being fully cross-platform anyway,
> so I would (as mentioned in previous email) prefer to postpone any
> such adjustments until the basic support is in. I have the patches
> for it, if not entirely up to date.

grub-mkimage is fully cross-platform and is intended to be usable this
way. If you see a problem, please tell.


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-03 20:01     ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-04-04 12:54       ` Leif Lindholm
  2013-04-09 17:30         ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 16+ messages in thread
From: Leif Lindholm @ 2013-04-04 12:54 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: phcoder

On Wed, Apr 03, 2013 at 10:01:59PM +0200, Vladimir '??-coder/phcoder' Serbinenko wrote:
> > I don't see grub-mkimage currently being fully cross-platform anyway,
> > so I would (as mentioned in previous email) prefer to postpone any
> > such adjustments until the basic support is in. I have the patches
> > for it, if not entirely up to date.
> 
> grub-mkimage is fully cross-platform and is intended to be usable this
> way. If you see a problem, please tell.

Well, the straightforward relocations for EM_386 and EM_X86_64 look OK,
but much of the fiddly IA64 patching that goes on in grub-mkimagexx.c
(add_value_to_slot_* and make_trampoline) does not appear
endianess-safe to me.

Since I didn't want to have to duplicate my relocation handling
between kernel and grub-mkimage, I use kern/arm/dl.c for both.

In order to do that in an endianess-safe way, I need to be able to
export target platform information over there, as well as the
host_to_target/target_to_host macros. This would involve moving these
macros, as well as the struct image_target_desc definition, out of
grub-mkimage.c.

However, this would also make it possible to do the same for IA64,
and get rid of some code duplication between grub-mkimagexx.c and
kern/ia64/dl.c.

/
    Leif


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-04 12:54       ` Leif Lindholm
@ 2013-04-09 17:30         ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-05-09 18:08           ` Leif Lindholm
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-04-09 17:30 UTC (permalink / raw)
  To: Leif Lindholm, The development of GRUB 2

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

On 04.04.2013 14:54, Leif Lindholm wrote:

> On Wed, Apr 03, 2013 at 10:01:59PM +0200, Vladimir '??-coder/phcoder' Serbinenko wrote:
>>> I don't see grub-mkimage currently being fully cross-platform anyway,
>>> so I would (as mentioned in previous email) prefer to postpone any
>>> such adjustments until the basic support is in. I have the patches
>>> for it, if not entirely up to date.
>>
>> grub-mkimage is fully cross-platform and is intended to be usable this
>> way. If you see a problem, please tell.
> 
> Well, the straightforward relocations for EM_386 and EM_X86_64 look OK,
> but much of the fiddly IA64 patching that goes on in grub-mkimagexx.c
> (add_value_to_slot_* and make_trampoline) does not appear
> endianess-safe to me.
> 

Fixed

> Since I didn't want to have to duplicate my relocation handling
> between kernel and grub-mkimage, I use kern/arm/dl.c for both.
> 

Could you move the common functions to dl_helper.c as in ISA64?

> In order to do that in an endianess-safe way, I need to be able to
> export target platform information over there, as well as the
> host_to_target/target_to_host macros. This would involve moving these
> macros, as well as the struct image_target_desc definition, out of
> grub-mkimage.c.

Is big-endian ARM of any interest at all? I was under impression that it
was limited to few devices and not supported by either u-boot or EFI.
You still have grub_le_to_cpu / grub_cpu_to_le

> 
> However, this would also make it possible to do the same for IA64,
> and get rid of some code duplication between grub-mkimagexx.c and
> kern/ia64/dl.c.
> 

I already did. For IA64 I simply used le_to_cpu/cpu_to_le as ia64-efi is
always little-endian


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-04-09 17:30         ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-05-09 18:08           ` Leif Lindholm
  2013-05-11  8:42             ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-05-11 18:00             ` Francesco Lavra
  0 siblings, 2 replies; 16+ messages in thread
From: Leif Lindholm @ 2013-05-09 18:08 UTC (permalink / raw)
  To: Vladimir 'φ-coder/phcoder' Serbinenko
  Cc: The development of GRUB 2

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

On Tue, Apr 09, 2013 at 07:30:46PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > Since I didn't want to have to duplicate my relocation handling
> > between kernel and grub-mkimage, I use kern/arm/dl.c for both.
> 
> Could you move the common functions to dl_helper.c as in ISA64?
 
Done in the attached patch.

> > In order to do that in an endianess-safe way, I need to be able to
> > export target platform information over there, as well as the
> > host_to_target/target_to_host macros. This would involve moving these
> > macros, as well as the struct image_target_desc definition, out of
> > grub-mkimage.c.
> 
> Is big-endian ARM of any interest at all? I was under impression that it
> was limited to few devices and not supported by either u-boot or EFI.
> You still have grub_le_to_cpu / grub_cpu_to_le
 
It shouldn't be of much interest at the boot stage, but even if it is
there are two things that make life easier:
1) With the endianess support introduced in ARMv6 (supported by RasPi),
   instructions are always little-endian. This would leave only the
   abs32 relocations as needing to be configured for a specific target.
2) The UEFI spec (2.3.1) states that on an ARM processor, little-endian
   will always be the configured state of the data interface(s).

> > However, this would also make it possible to do the same for IA64,
> > and get rid of some code duplication between grub-mkimagexx.c and
> > kern/ia64/dl.c.
> 
> I already did. For IA64 I simply used le_to_cpu/cpu_to_le as ia64-efi is
> always little-endian

I was referring more to functions like add_value_to_slot_* that are
duplicated between these files. But of course, given the IA64 fixed
endianess, the other part of my comment was irrelevant :)

Attached is a patch that:
- incoorporates Francesco's bug fixes
- splits out relocation work into dl_helper.c
- cleans up code slightly
- makes relocations use le_to_cpu/cpu_to_le
- changes all grub_util printout calls to grub_dprintf 

/
    Leif

[-- Attachment #2: arm-dl-rework.patch --]
[-- Type: text/x-diff, Size: 19427 bytes --]

=== modified file 'Makefile.util.def'
--- Makefile.util.def	2013-04-12 14:54:54 +0000
+++ Makefile.util.def	2013-05-09 16:38:09 +0000
@@ -150,7 +150,7 @@
   common = util/resolve.c;
   common = grub-core/kern/emu/argp_common.c;
 
-  common = grub-core/kern/arm/dl.c;
+  common = grub-core/kern/arm/dl_helper.c;
 
   extra_dist = util/grub-mkimagexx.c;
 

=== modified file 'grub-core/Makefile.core.def'
--- grub-core/Makefile.core.def	2013-04-12 14:55:38 +0000
+++ grub-core/Makefile.core.def	2013-05-09 14:43:56 +0000
@@ -209,6 +209,7 @@
   sparc64_ieee1275 = kern/sparc64/ieee1275/ieee1275.c;
 
   arm = kern/arm/dl.c;
+  arm = kern/arm/dl_helper.c;
   arm = kern/arm/cache.S;
   arm = kern/arm/misc.S;
 

=== modified file 'grub-core/kern/arm/dl.c'
--- grub-core/kern/arm/dl.c	2013-04-12 14:53:58 +0000
+++ grub-core/kern/arm/dl.c	2013-05-09 16:44:58 +0000
@@ -25,272 +25,9 @@
 #include <grub/i18n.h>
 #include <grub/arm/reloc.h>
 
-#ifdef GRUB_UTIL
-# include <grub/util/misc.h>
-#else
-
-#ifdef DL_DEBUG
-static const char *symstrtab;
-
-/*
- * This is a bit of a hack, setting the symstrtab pointer to the last STRTAB
- * section in the module (which is where symbol names are in the objects I've
- * inspected manually). 
- */
-static void
-set_symstrtab (Elf_Ehdr * e)
-{
-  int i;
-  Elf_Shdr *s;
-
-  symstrtab = NULL;
-
-  for (i = 0, s = (Elf_Shdr *) ((grub_uint32_t) e + e->e_shoff);
-       i < e->e_shnum;
-       i++, s = (Elf_Shdr *) ((grub_uint32_t) s + e->e_shentsize))
-    if (s->sh_type == SHT_STRTAB)
-      symstrtab = (void *) ((grub_addr_t) e + s->sh_offset);
-}
-
-static const char *
-get_symbolname (Elf_Sym * sym)
-{
-  const char *symbolname = symstrtab + sym->st_name;
-
-  return (*symbolname ? symbolname : NULL);
-}
-#endif /* DL_DEBUG */
-
-/*
- * R_ARM_ABS32
- *
- * Simple relocation of 32-bit value (in literal pool)
- */
-static grub_err_t
-reloc_abs32 (Elf_Word *target, Elf_Addr sym_addr)
-{
-  Elf_Addr tmp;
-
-  tmp = *target;
-  tmp += sym_addr;
-  *target = tmp;
-#if 0 //def GRUB_UTIL
-  grub_util_info ("  %s:  reloc_abs32 0x%08x => 0x%08x", __FUNCTION__,
-		  (unsigned int) sym_addr, (unsigned int) tmp);
-#endif
-
-  return GRUB_ERR_NONE;
-}
-#endif /* ndef GRUB_UTIL */
-
-
-/********************************************************************
- * Thumb (T32) relocations:                                         *
- *                                                                  *
- * 32-bit Thumb instructions can be 16-bit aligned, and are fetched *
- * little-endian, requiring some additional fiddling.               *
- ********************************************************************/
-
-/*
- * R_ARM_THM_CALL/THM_JUMP24
- *
- * Relocate Thumb (T32) instruction set relative branches:
- *   B.W, BL and BLX
- */
-grub_err_t
-grub_arm_reloc_thm_call (grub_uint16_t *target, Elf32_Addr sym_addr)
-{
-  grub_int32_t offset, offset_low, offset_high;
-  grub_uint32_t sign, j1, j2, is_blx;
-  grub_uint32_t insword, insmask;
-
-  /* Extract instruction word in alignment-safe manner */
-  insword = (*target << 16) | (*(target + 1));
-  insmask = 0xf800d000;
-
-  /* B.W/BL or BLX? Affects range and expected target state */
-  if (((insword >> 12) & 0xd) == 0xc)
-    is_blx = 1;
-  else
-    is_blx = 0;
-
-  /* If BLX, target symbol must be ARM (target address LSB == 0) */
-  if (is_blx && (sym_addr & 1))
-    return grub_error (GRUB_ERR_BUG,
-		       N_("Relocation targeting wrong execution state"));
-
-  offset_low = -16777216;
-  offset_high = is_blx ? 16777212 : 16777214;
-
-  /* Extract bitfields from instruction words */
-  sign = (insword >> 26) & 1;
-  j1 = (insword >> 13) & 1;
-  j2 = (insword >> 11) & 1;
-  offset = (sign << 24) | ((~(j1 ^ sign) & 1) << 23) |
-    ((~(j2 ^ sign) & 1) << 22) |
-    ((insword & 0x03ff0000) >> 4) | ((insword & 0x000007ff) << 1);
-
-  /* Sign adjust and calculate offset */
-  if (offset & (1 << 24))
-    offset -= (1 << 25);
-#ifdef GRUB_UTIL
-  grub_util_info ("    sym_addr = 0x%08x", sym_addr);
-#endif
-#ifdef GRUB_UTIL
-  offset += sym_addr;
-#else
-  offset += sym_addr - (grub_uint32_t) target;
-#endif
-#ifdef DEBUG
-  grub_printf(" %s: target=0x%08x, sym_addr=0x%08x, offset=%d\n",
-	      is_blx ? "BLX" : "BL", (unsigned int) target, sym_addr, offset);
-#endif
-
-  if ((offset < offset_low) || (offset > offset_high))
-    return grub_error (GRUB_ERR_OUT_OF_RANGE,
-		       N_("THM_CALL Relocation out of range."));
-
-#ifdef GRUB_UTIL
-  grub_util_info ("    relative destination = 0x%08lx",
-		  (unsigned long)target + offset);
-#endif
-
-  /* Reassemble instruction word */
-  sign = (offset >> 24) & 1;
-  j1 = sign ^ (~(offset >> 23) & 1);
-  j2 = sign ^ (~(offset >> 22) & 1);
-  insword = (insword & insmask) |
-    (sign << 26) |
-    (((offset >> 12) & 0x03ff) << 16) |
-    (j1 << 13) | (j2 << 11) | ((offset >> 1) & 0x07ff);
-
-  /* Write instruction word back in alignment-safe manner */
-  *target = (insword >> 16) & 0xffff;
-  *(target + 1) = insword & 0xffff;
-
-#ifdef GRUB_UTIL
-#pragma GCC diagnostic ignored "-Wcast-align"
-  grub_util_info ("    *target = 0x%08x", *((unsigned int *)target));
-#endif
-
-  return GRUB_ERR_NONE;
-}
-
-/*
- * R_ARM_THM_JUMP19
- *
- * Relocate conditional Thumb (T32) B<c>.W
- */
-grub_err_t
-grub_arm_reloc_thm_jump19 (grub_uint16_t *addr, Elf32_Addr sym_addr)
-{
-  grub_int32_t offset;
-  grub_uint32_t insword, insmask;
-
-  /* Extract instruction word in alignment-safe manner */
-  insword = (*addr) << 16 | *(addr + 1);
-  insmask = 0xfbc0d800;
-
-  /* Extract and sign extend offset */
-  offset = ((insword >> 26) & 1) << 18
-    | ((insword >> 11) & 1) << 17
-    | ((insword >> 13) & 1) << 16
-    | ((insword >> 16) & 0x3f) << 11
-    | (insword & 0x7ff);
-  offset <<= 1;
-  if (offset & (1 << 19))
-    offset -= (1 << 20);
-
-  /* Adjust and re-truncate offset */
-#ifdef GRUB_UTIL
-  offset += sym_addr;
-#else
-  offset += sym_addr - (grub_uint32_t) addr;
-#endif
-  if ((offset > 1048574) || (offset < -1048576))
-    {
-      return grub_error
-        (GRUB_ERR_OUT_OF_RANGE, N_("THM_JUMP19 Relocation out of range."));
-    }
-
-  offset >>= 1;
-  offset &= 0x7ffff;
-
-  /* Reassemble instruction word and write back */
-  insword &= insmask;
-  insword |= ((offset >> 18) & 1) << 26
-    | ((offset >> 17) & 1) << 11
-    | ((offset >> 16) & 1) << 13
-    | ((offset >> 11) & 0x3f) << 16
-    | (offset & 0x7ff);
-  *addr = insword >> 16;
-  *(addr + 1) = insword & 0xffff;
-  return GRUB_ERR_NONE;
-}
-
-
-
-/***********************************************************
- * ARM (A32) relocations:                                  *
- *                                                         *
- * ARM instructions are 32-bit in size and 32-bit aligned. *
- ***********************************************************/
-
-/*
- * R_ARM_JUMP24
- *
- * Relocate ARM (A32) B
- */
-grub_err_t
-grub_arm_reloc_jump24 (grub_uint32_t *addr, Elf32_Addr sym_addr)
-{
-  grub_uint32_t insword;
-  grub_int32_t offset;
-
-  insword = *addr;
-
-  offset = (insword & 0x00ffffff) << 2;
-  if (offset & 0x02000000)
-    offset -= 0x04000000;
-#ifdef GRUB_UTIL
-  offset += sym_addr;
-#else
-  offset += sym_addr - (grub_uint32_t) addr;
-#endif
-
-  insword &= 0xff000000;
-  insword |= (offset >> 2) & 0x00ffffff;
-
-  *addr = insword;
-
-  return GRUB_ERR_NONE;
-}
-
-
-
 /*************************************************
  * Runtime dynamic linker with helper functions. *
  *************************************************/
-#ifndef GRUB_UTIL
-/*
- * find_segment(): finds a module segment matching sh_info
- */
-static grub_dl_segment_t
-find_segment (grub_dl_segment_t seg, Elf32_Word sh_info)
-{
-  for (; seg; seg = seg->next)
-    if (seg->section == sh_info)
-      return seg;
-
-  return NULL;
-}
-
-
-/*
- * do_relocations():
- *   Iterate over all relocations in section, calling appropriate functions
- *   for patching.
- */
 static grub_err_t
 do_relocations (Elf_Shdr * relhdr, Elf_Ehdr * e, grub_dl_t mod)
 {
@@ -302,7 +39,9 @@
   entnum = relhdr->sh_size / sizeof (Elf_Rel);
 
   /* Find the target segment for this relocation section. */
-  seg = find_segment (mod->segment, relhdr->sh_info);
+  for (seg = mod->segment ; seg ; seg = seg->next)
+    if (seg->section == relhdr->sh_info)
+      break;
   if (!seg)
     return grub_error (GRUB_ERR_EOF, N_("relocation segment not found"));
 
@@ -320,22 +59,16 @@
 			   "reloc offset is out of the segment");
       relsym = ELF_R_SYM (rel[i].r_info);
       reltype = ELF_R_TYPE (rel[i].r_info);
-      target = (Elf_Word *) ((grub_addr_t) seg->addr + rel[i].r_offset);
+      target = (void *) ((grub_addr_t) seg->addr + rel[i].r_offset);
 
       sym_addr = sym[relsym].st_value;
 
-#ifdef DL_DEBUG
-
-      grub_printf ("%s: 0x%08x -> %s @ 0x%08x\n", __FUNCTION__,
-		   (grub_addr_t) sym_addr, get_symbolname (sym), sym->st_value);
-#endif
-
       switch (reltype)
 	{
 	case R_ARM_ABS32:
 	  {
 	    /* Data will be naturally aligned */
-	    retval = reloc_abs32 (target, sym_addr);
+	    retval = grub_arm_reloc_abs32 (target, sym_addr);
 	    if (retval != GRUB_ERR_NONE)
 	      return retval;
 	  }
@@ -427,10 +160,6 @@
   if (!has_symtab (e))
     return grub_error (GRUB_ERR_BAD_MODULE, N_("no symbol table"));
 
-#ifdef DL_DEBUG
-  set_symstrtab (e);
-#endif
-
 #define FIRST_SHDR(x) ((Elf_Shdr *) ((grub_addr_t)(x) + (x)->e_shoff))
 #define NEXT_SHDR(x, y) ((Elf_Shdr *) ((grub_addr_t)(y) + (x)->e_shentsize))
 
@@ -458,7 +187,7 @@
 	case SHT_RELA:
 	default:
 	  {
-	    grub_printf ("unhandled section_type: %d (0x%08x)\n",
+	    grub_dprintf ("dl", "unhandled section_type: %d (0x%08x)\n",
 			 s->sh_type, s->sh_type);
 	    return GRUB_ERR_NOT_IMPLEMENTED_YET;
 	  };
@@ -470,4 +199,3 @@
 
   return GRUB_ERR_NONE;
 }
-#endif /* ndef GRUB_UTIL */

=== added file 'grub-core/kern/arm/dl_helper.c'
--- grub-core/kern/arm/dl_helper.c	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/dl_helper.c	2013-05-09 17:12:32 +0000
@@ -0,0 +1,222 @@
+/* dl.c - arch-dependent part of loadable module support */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/dl.h>
+#include <grub/elf.h>
+#include <grub/misc.h>
+#include <grub/err.h>
+#include <grub/mm.h>
+#include <grub/i18n.h>
+#include <grub/arm/reloc.h>
+
+/*
+ * R_ARM_ABS32
+ *
+ * Simple relocation of 32-bit value (in literal pool)
+ */
+grub_err_t
+grub_arm_reloc_abs32 (Elf32_Word *target, Elf32_Addr sym_addr)
+{
+  Elf32_Addr tmp;
+
+  tmp = grub_le_to_cpu32 (*target);
+  tmp += sym_addr;
+  *target = grub_cpu_to_le32 (tmp);
+  grub_dprintf ("dl", "  %s:  reloc_abs32 0x%08x => 0x%08x", __FUNCTION__,
+		(unsigned int) sym_addr, (unsigned int) tmp);
+
+  return GRUB_ERR_NONE;
+}
+
+/********************************************************************
+ * Thumb (T32) relocations:                                         *
+ *                                                                  *
+ * 32-bit Thumb instructions can be 16-bit aligned, and are fetched *
+ * little-endian, requiring some additional fiddling.               *
+ ********************************************************************/
+
+/*
+ * R_ARM_THM_CALL/THM_JUMP24
+ *
+ * Relocate Thumb (T32) instruction set relative branches:
+ *   B.W, BL and BLX
+ */
+grub_err_t
+grub_arm_reloc_thm_call (grub_uint16_t *target, Elf32_Addr sym_addr)
+{
+  grub_int32_t offset, offset_low, offset_high;
+  grub_uint32_t sign, j1, j2, is_blx;
+  grub_uint32_t insword, insmask;
+
+  /* Extract instruction word in alignment-safe manner */
+  insword = (grub_le_to_cpu16 (*target) << 16)
+    | (grub_le_to_cpu16(*(target + 1)));
+  insmask = 0xf800d000;
+
+  /* B.W/BL or BLX? Affects range and expected target state */
+  if (((insword >> 12) & 0xd) == 0xc)
+    is_blx = 1;
+  else
+    is_blx = 0;
+
+  /* If BLX, target symbol must be ARM (target address LSB == 0) */
+  if (is_blx && (sym_addr & 1))
+    return grub_error (GRUB_ERR_BAD_MODULE,
+		       N_("Relocation targeting wrong execution state"));
+
+  offset_low = -16777216;
+  offset_high = is_blx ? 16777212 : 16777214;
+
+  /* Extract bitfields from instruction words */
+  sign = (insword >> 26) & 1;
+  j1 = (insword >> 13) & 1;
+  j2 = (insword >> 11) & 1;
+  offset = (sign << 24) | ((~(j1 ^ sign) & 1) << 23) |
+    ((~(j2 ^ sign) & 1) << 22) |
+    ((insword & 0x03ff0000) >> 4) | ((insword & 0x000007ff) << 1);
+
+  /* Sign adjust and calculate offset */
+  if (offset & (1 << 24))
+    offset -= (1 << 25);
+
+  grub_dprintf ("dl", "    sym_addr = 0x%08x", sym_addr);
+
+  offset += sym_addr;
+#ifndef GRUB_UTIL
+  offset -= (grub_uint32_t) target;
+#endif
+
+  grub_dprintf("dl", " %s: target=0x%08x, sym_addr=0x%08x, offset=%d\n",
+	      is_blx ? "BLX" : "BL", (unsigned int) target, sym_addr, offset);
+
+  if ((offset < offset_low) || (offset > offset_high))
+    return grub_error (GRUB_ERR_BAD_MODULE,
+		       N_("THM_CALL Relocation out of range."));
+
+  grub_dprintf ("dl", "    relative destination = 0x%08lx",
+		(unsigned long)target + offset);
+
+  /* Reassemble instruction word */
+  sign = (offset >> 24) & 1;
+  j1 = sign ^ (~(offset >> 23) & 1);
+  j2 = sign ^ (~(offset >> 22) & 1);
+  insword = (insword & insmask) |
+    (sign << 26) |
+    (((offset >> 12) & 0x03ff) << 16) |
+    (j1 << 13) | (j2 << 11) | ((offset >> 1) & 0x07ff);
+
+  /* Write instruction word back in alignment-safe manner */
+  *target = grub_cpu_to_le16 ((insword >> 16) & 0xffff);
+  *(target + 1) = grub_cpu_to_le16 (insword & 0xffff);
+
+  grub_dprintf ("dl", "    *insword = 0x%08x", insword);
+
+  return GRUB_ERR_NONE;
+}
+
+/*
+ * R_ARM_THM_JUMP19
+ *
+ * Relocate conditional Thumb (T32) B<c>.W
+ */
+grub_err_t
+grub_arm_reloc_thm_jump19 (grub_uint16_t *target, Elf32_Addr sym_addr)
+{
+  grub_int32_t offset;
+  grub_uint32_t insword, insmask;
+
+  /* Extract instruction word in alignment-safe manner */
+  insword = grub_le_to_cpu16 ((*target)) << 16
+    | grub_le_to_cpu16 (*(target + 1));
+  insmask = 0xfbc0d000;
+
+  /* Extract and sign extend offset */
+  offset = ((insword >> 26) & 1) << 19
+    | ((insword >> 11) & 1) << 18
+    | ((insword >> 13) & 1) << 17
+    | ((insword >> 16) & 0x3f) << 11
+    | (insword & 0x7ff);
+  offset <<= 1;
+  if (offset & (1 << 20))
+    offset -= (1 << 21);
+
+  /* Adjust and re-truncate offset */
+  offset += sym_addr;
+#ifndef GRUB_UTIL
+  offset -= (grub_uint32_t) target;
+#endif
+  if ((offset > 1048574) || (offset < -1048576))
+    return grub_error (GRUB_ERR_BAD_MODULE,
+		       N_("THM_JUMP19 Relocation out of range."));
+
+  offset >>= 1;
+  offset &= 0xfffff;
+
+  /* Reassemble instruction word and write back */
+  insword &= insmask;
+  insword |= ((offset >> 19) & 1) << 26
+    | ((offset >> 18) & 1) << 11
+    | ((offset >> 17) & 1) << 13
+    | ((offset >> 11) & 0x3f) << 16
+    | (offset & 0x7ff);
+  *target = grub_cpu_to_le16 (insword >> 16);
+  *(target + 1) = grub_cpu_to_le16 (insword & 0xffff);
+  return GRUB_ERR_NONE;
+}
+
+
+\f
+/***********************************************************
+ * ARM (A32) relocations:                                  *
+ *                                                         *
+ * ARM instructions are 32-bit in size and 32-bit aligned. *
+ ***********************************************************/
+
+/*
+ * R_ARM_JUMP24
+ *
+ * Relocate ARM (A32) B
+ */
+grub_err_t
+grub_arm_reloc_jump24 (grub_uint32_t *target, Elf32_Addr sym_addr)
+{
+  grub_uint32_t insword;
+  grub_int32_t offset;
+
+  if (sym_addr & 1)
+    return grub_error (GRUB_ERR_BAD_MODULE,
+		       N_("Relocation targeting wrong execution state"));
+
+  insword = grub_le_to_cpu32 (*target);
+
+  offset = (insword & 0x00ffffff) << 2;
+  if (offset & 0x02000000)
+    offset -= 0x04000000;
+  offset += sym_addr;
+#ifdef GRUB_UTIL
+  offset -= (grub_uint32_t) target;
+#endif
+
+  insword &= 0xff000000;
+  insword |= (offset >> 2) & 0x00ffffff;
+
+  *target = grub_cpu_to_le32 (insword);
+
+  return GRUB_ERR_NONE;
+}

=== modified file 'include/grub/arm/reloc.h'
--- include/grub/arm/reloc.h	2013-04-12 14:53:58 +0000
+++ include/grub/arm/reloc.h	2013-05-09 14:48:08 +0000
@@ -19,6 +19,7 @@
 #ifndef GRUB_ARM_RELOC_H
 #define GRUB_ARM_RELOC_H 1
 
+grub_err_t grub_arm_reloc_abs32 (grub_uint32_t *addr, Elf32_Addr sym_addr);
 grub_err_t grub_arm_reloc_jump24 (grub_uint32_t *addr, Elf32_Addr sym_addr);
 grub_err_t grub_arm_reloc_thm_call (grub_uint16_t *addr, Elf32_Addr sym_addr);
 grub_err_t grub_arm_reloc_thm_jump19 (grub_uint16_t *addr, Elf32_Addr sym_addr);

=== modified file 'util/grub-install.in'
--- util/grub-install.in	2013-04-12 14:46:51 +0000
+++ util/grub-install.in	2013-05-09 15:17:30 +0000
@@ -803,7 +803,7 @@
     fi
 
     # Try to make this image bootable using the EFI Boot Manager, if available.
-    efibootmgr="`which efibootmgr`"
+    efibootmgr="`which efibootmgr`" || true
     if test "$removable" = no && test -n "$efi_distributor" && \
 	test -n "$efibootmgr"; then
         # On Linux, we need the efivars kernel modules.

=== modified file 'util/grub-mkimagexx.c'
--- util/grub-mkimagexx.c	2013-04-12 14:53:58 +0000
+++ util/grub-mkimagexx.c	2013-05-09 16:34:59 +0000
@@ -542,7 +542,6 @@
 		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
 				       (int) sym_addr, (int) sym_addr);
 		       /* Data will be naturally aligned */
-		       //      sym_addr -= offset;
 		       sym_addr += 0x400;
 		       *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr);
 		     }
@@ -566,9 +565,9 @@
 		       grub_util_info ("  THM_JUMP19:\toffset=%d\t(0x%08x)",
 				       sym_addr, sym_addr);
 		       sym_addr -= offset;
+
 		       /* Thumb instructions can be 16-bit aligned */
-		       err = grub_arm_reloc_thm_jump19 ((grub_uint16_t *) target,
-							sym_addr);
+		       err = grub_arm_reloc_thm_jump19 ((grub_uint16_t *) target, sym_addr);
 		       if (err)
 			 grub_util_error ("%s", grub_errmsg);
 		     }
@@ -825,15 +824,10 @@
 		    break;
 		    /* Create fixup entry for PE/COFF loader */
 		  case R_ARM_ABS32:
-		    {      
+		    {
 		      Elf_Addr addr;
 
 		      addr = section_address + offset;
-#if 0
-		      grub_util_info ("  %s:  add_fixup: 0x%08x : 0x%08x",
-				      __FUNCTION__, (unsigned int) addr,
-				      (unsigned int) current_address);
-#endif
 		      current_address
 			= SUFFIX (add_fixup_entry) (&lst,
 						    GRUB_PE32_REL_BASED_HIGHLOW,
@@ -842,7 +836,7 @@
 		    }
 		    break;
 		  default:
-		    grub_util_error (_("relocation 0x%x is not implemented yet2"), ELF_R_TYPE (info));
+		    grub_util_error (_("fixup for relocation 0x%x not implemented"), ELF_R_TYPE (info));
 		    break;
 		  }
 		break;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-05-09 18:08           ` Leif Lindholm
@ 2013-05-11  8:42             ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-05-11 18:00             ` Francesco Lavra
  1 sibling, 0 replies; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-05-11  8:42 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: The development of GRUB 2

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

Committed to arm branch after fixing few bugs.

On 09.05.2013 20:08, Leif Lindholm wrote:

> On Tue, Apr 09, 2013 at 07:30:46PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>> Since I didn't want to have to duplicate my relocation handling
>>> between kernel and grub-mkimage, I use kern/arm/dl.c for both.
>>
>> Could you move the common functions to dl_helper.c as in ISA64?
>  
> Done in the attached patch.
> 
>>> In order to do that in an endianess-safe way, I need to be able to
>>> export target platform information over there, as well as the
>>> host_to_target/target_to_host macros. This would involve moving these
>>> macros, as well as the struct image_target_desc definition, out of
>>> grub-mkimage.c.
>>
>> Is big-endian ARM of any interest at all? I was under impression that it
>> was limited to few devices and not supported by either u-boot or EFI.
>> You still have grub_le_to_cpu / grub_cpu_to_le
>  
> It shouldn't be of much interest at the boot stage, but even if it is
> there are two things that make life easier:
> 1) With the endianess support introduced in ARMv6 (supported by RasPi),
>    instructions are always little-endian. This would leave only the
>    abs32 relocations as needing to be configured for a specific target.
> 2) The UEFI spec (2.3.1) states that on an ARM processor, little-endian
>    will always be the configured state of the data interface(s).
> 
>>> However, this would also make it possible to do the same for IA64,
>>> and get rid of some code duplication between grub-mkimagexx.c and
>>> kern/ia64/dl.c.
>>
>> I already did. For IA64 I simply used le_to_cpu/cpu_to_le as ia64-efi is
>> always little-endian
> 
> I was referring more to functions like add_value_to_slot_* that are
> duplicated between these files. But of course, given the IA64 fixed
> endianess, the other part of my comment was irrelevant :)
> 
> Attached is a patch that:
> - incoorporates Francesco's bug fixes
> - splits out relocation work into dl_helper.c
> - cleans up code slightly
> - makes relocations use le_to_cpu/cpu_to_le
> - changes all grub_util printout calls to grub_dprintf 
> 
> /
>     Leif




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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms
  2013-05-09 18:08           ` Leif Lindholm
  2013-05-11  8:42             ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-05-11 18:00             ` Francesco Lavra
  1 sibling, 0 replies; 16+ messages in thread
From: Francesco Lavra @ 2013-05-11 18:00 UTC (permalink / raw)
  To: The development of GNU GRUB
  Cc: Vladimir 'φ-coder/phcoder' Serbinenko, Leif Lindholm

On 05/09/2013 08:08 PM, Leif Lindholm wrote:
> Attached is a patch that:
> - incoorporates Francesco's bug fixes
> - splits out relocation work into dl_helper.c
> - cleans up code slightly
> - makes relocations use le_to_cpu/cpu_to_le
> - changes all grub_util printout calls to grub_dprintf 
[...]
> === modified file 'util/grub-mkimagexx.c'
> --- util/grub-mkimagexx.c	2013-04-12 14:53:58 +0000
> +++ util/grub-mkimagexx.c	2013-05-09 16:34:59 +0000
> @@ -542,7 +542,6 @@
>  		       grub_util_info ("  ABS32:\toffset=%d\t(0x%08x)",
>  				       (int) sym_addr, (int) sym_addr);
>  		       /* Data will be naturally aligned */
> -		       //      sym_addr -= offset;
>  		       sym_addr += 0x400;

This 0x400 constant still bothers me a bit :)

--
Francesco


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-05-11 17:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-24 17:01 [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms Leif Lindholm
2013-04-01  2:31 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-04-01 10:41   ` Francesco Lavra
2013-04-01 11:23     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-04-01 14:10       ` Francesco Lavra
2013-04-01 14:16         ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-04-01 14:49           ` Francesco Lavra
2013-04-03 17:50   ` Leif Lindholm
2013-04-01 16:24 ` Francesco Lavra
2013-04-03 18:07   ` Leif Lindholm
2013-04-03 20:01     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-04-04 12:54       ` Leif Lindholm
2013-04-09 17:30         ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-05-09 18:08           ` Leif Lindholm
2013-05-11  8:42             ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-05-11 18:00             ` Francesco Lavra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.