[PATCH 2/7] Initial support for ARMv7 architecture

[PATCH 2/7] Initial support for ARMv7 architecture

[Leif Lindholm]

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



[-- Attachment #2: 0002-arm-support.patch --]
[-- Type: application/octet-stream, Size: 27294 bytes --]

=== modified file 'Makefile.util.def'
--- Makefile.util.def	2013-02-25 21:11:06 +0000
+++ Makefile.util.def	2013-03-24 12:56:20 +0000
@@ -149,6 +149,8 @@
   common = util/resolve.c;
   common = grub-core/kern/emu/argp_common.c;
 
+  arm = grub-core/kern/arm/dl.c;
+
   extra_dist = util/grub-mkimagexx.c;
 
   ldadd = libgrubmods.a;

=== modified file 'grub-core/Makefile.core.def'
--- grub-core/Makefile.core.def	2013-03-24 12:56:06 +0000
+++ grub-core/Makefile.core.def	2013-03-24 12:56:20 +0000
@@ -198,6 +198,10 @@
   sparc64_ieee1275 = kern/sparc64/dl.c;
   sparc64_ieee1275 = kern/sparc64/ieee1275/ieee1275.c;
 
+  arm = kern/arm/dl.c;
+  arm = kern/arm/cache.S;
+  arm = kern/arm/misc.S;
+
   emu = disk/host.c;
   emu = gnulib/progname.c;
   emu = gnulib/error.c;
@@ -1378,6 +1382,7 @@
   extra_dist = lib/powerpc/setjmp.S;
   extra_dist = lib/ia64/setjmp.S;
   extra_dist = lib/ia64/longjmp.S;
+  extra_dist = lib/arm/setjmp.S;
 };
 
 module = {

=== added directory 'grub-core/kern/arm'
=== added file 'grub-core/kern/arm/cache.S'
--- grub-core/kern/arm/cache.S	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/cache.S	2013-03-24 12:56:20 +0000
@@ -0,0 +1,251 @@
+/*
+ *  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/symbol.h>
+#include <grub/dl.h>
+
+	.file	"cache.S"
+	.text
+	.syntax	unified
+#if !defined (__thumb2__)
+	.arm
+#define ARM(x...)	x
+#define THUMB(x...)
+#else
+	.thumb
+#define THUMB(x...)	x
+#define ARM(x...)
+#endif
+
+	.align	2
+
+/*
+ * Simple cache maintenance functions
+ */
+
+@ r0 - *beg (inclusive)
+@ r1 - *end (exclusive)	
+clean_dcache_range:
+	@ Clean data cache range for range to point-of-unification
+	ldr	r2, dlinesz
+1:	cmp	r0, r1
+	bge	2f
+#ifdef DEBUG
+	push	{r0-r2, lr}
+	mov	r1, r2
+	mov	r2, r0
+	ldr	r0, =dcstr
+	bl	EXT_C(grub_printf)
+	pop	{r0-r2, lr}
+#endif
+	mcr	p15, 0, r0, c7, c11, 1	@ DCCMVAU
+	add	r0, r0, r2		@ Next line
+	b	1b
+2:	dsb
+	bx	lr
+
+@ r0 - *beg (inclusive)
+@ r1 - *end (exclusive)	
+invalidate_icache_range:
+	@ Invalidate instruction cache for range to point-of-unification
+	ldr	r2, ilinesz
+1:	cmp	r0, r1
+	bge	2f
+#ifdef DEBUG
+	push	{r0-r2, lr}
+	mov	r1, r2
+	mov	r2, r0
+	ldr	r0, =icstr
+	bl	EXT_C(grub_printf)
+	pop	{r0-r2, lr}
+#endif
+	mcr	p15, 0, r0, c7, c5, 1	@ ICIMVAU
+	add	r0, r0, r2		@ Next line
+	b	1b
+	@ Branch predictor invalidate all
+2:	mcr	p15, 0, r0, c7,	c5, 6	@ BPIALL
+	dsb
+	isb
+	bx	lr
+	
+@void __wrap___clear_cache(char *beg, char *end);
+FUNCTION(__wrap___clear_cache)
+	dmb
+	dsb
+	push	{r4-r6, lr}
+	ldr	r2, probed	@ If first call, probe cache sizes
+	cmp	r2, #0
+	bleq	probe_caches	@ This call corrupts r3
+	mov	r4, r0
+	mov	r5, r1
+	bl	clean_dcache_range
+	mov	r0, r4
+	mov	r1, r5
+	bl	invalidate_icache_range
+	pop	{r4-r6, pc}
+
+probe_caches:
+	push	{r4-r6, lr}
+	mrc 	p15, 0, r4, c0, c0, 1	@ Read Cache Type Register
+	mov	r5, #1
+	ubfx	r6, r4, #16, #4		@ Extract min D-cache num word log2
+	add	r6, r6, #2		@ words->bytes
+	lsl	r6, r5, r6		@ Convert to num bytes
+	ldr	r3, =dlinesz
+	str	r6, [r3]
+	and	r6, r4, #0xf		@ Extract min I-cache num word log2
+	add	r6, r6, #2		@ words->bytes
+	lsl	r6, r5, r6		@ Convert to num bytes
+	ldr	r3, =ilinesz
+	str	r6, [r3]
+	ldr	r3, =probed		@ Flag cache probing done
+	str	r5, [r3]
+	pop	{r4-r6, pc}
+
+#ifdef DEBUG
+dcstr:	.asciz	"cleaning %d bytes of D cache @ 0x%08x\n"
+icstr:	.asciz	"invalidating %d bytes of I cache @ 0x%08x\n"
+#endif
+	
+	.align	3
+probed:	.long	0
+dlinesz:
+	.long	0
+ilinesz:
+	.long	0
+
+@void grub_arch_sync_caches (void *address, grub_size_t len)
+FUNCTION(grub_arch_sync_caches)
+	add	r1, r0, r1
+	b	__wrap___clear_cache
+
+	@ r0  - CLIDR
+	@ r1  - LoC
+	@ r2  - current level
+	@ r3  - num sets
+	@ r4  - num ways
+	@ r5  - current set
+	@ r6  - current way
+	@ r7  - line size
+	@ r8  - scratch
+	@ r9  - scratch
+	@ r10 - scratch
+	@ r11 - scratch
+clean_invalidate_dcache:
+	push	{r4-r12, lr}
+	mrc 	p15, 1, r0, c0, c0, 1	@ Read CLIDR
+	ubfx	r1, r0, #24, #3		@ Extract LoC
+	
+	mov	r2, #0			@ First level, L1
+2:	and	r8, r0, #7		@ cache type at current level
+	cmp	r8, #2
+	blt	5f			@ instruction only, or none, skip level
+
+	@ set current cache level/type (for CSSIDR read)
+	lsl	r8, r2, #1
+	mcr	p15, 2, r8, c0, c0, 0	@ Write CSSELR (level, type: data/uni)
+
+	@ read current cache information
+	mrc	p15, 1, r8, c0, c0, 0	@ Read CSSIDR
+	ubfx	r3, r8, #13, #14	@ Number of sets -1
+	ubfx	r4, r8, #3, #9		@ Number of ways -1
+	and	r7, r8, #7		@ log2(line size in words) - 2
+	add	r7, r7, #2		@  adjust
+	mov	r8, #1
+	lsl	r7, r8, r7		@  -> line size in words
+	lsl	r7, r7, #2		@  -> bytes
+
+	@ set loop
+	mov	r5, #0			@ current set = 0
+3:	lsl	r8, r2, #1		@ insert level
+	clz	r9, r7			@ calculate set field offset
+	mov	r10, #31
+	sub	r9, r10, r9
+	lsl	r10, r5, r9
+	orr	r8, r8, r10		@ insert set field
+
+	@ way loop
+	@ calculate way field offset
+	mov	r6, #0			@ current way = 0
+	add	r10, r4, #1
+	clz	r9, r10			@ r9 = way field offset
+	add	r9, r9, #1
+4:	lsl	r10, r6, r9
+	orr	r11, r8, r10		@ insert way field	
+	
+	@ clean line by set/way
+	mcr	p15, 0, r11, c7, c14, 2	@ DCCISW
+	
+	@ next way
+	add	r6, r6, #1
+	cmp	r6, r4
+	ble	4b
+
+	@ next set
+	add	r5, r5, #1
+	cmp	r5, r3
+	ble	3b
+	
+	@ next level
+5:	lsr	r0, r0, #3		@ align next level CLIDR 'type' field
+	add	r2, r2, #1		@ increment cache level counter
+	cmp	r2, r1
+	blt	2b			@ outer loop
+
+	@ return
+6:	dsb
+	isb
+	pop	{r4-r12, pc}
+
+FUNCTION(grub_arm_disable_caches_mmu)
+	push	{r4, lr}
+
+	@ disable D-cache
+	mrc	p15, 0, r0, c1, c0, 0
+	bic	r0, r0, #(1 << 2)
+	mcr	p15, 0, r0, c1, c0, 0
+	dsb
+	isb
+
+	@ clean/invalidate D-cache
+	bl	clean_invalidate_dcache
+
+	@ disable I-cache
+	mrc	p15, 0, r0, c1, c0, 0
+	bic	r0, r0, #(1 << 12)
+	mcr	p15, 0, r0, c1, c0, 0
+	dsb
+	isb
+
+	@ invalidate I-cache (also invalidates branch predictors)
+	mcr	p15, 0, r0, c7, c5, 0
+	dsb
+	isb
+	
+	@ clear SCTLR M bit
+	mrc	p15, 0, r0, c1, c0, 0
+	bic	r0, r0, #(1 << 0)
+	mcr	p15, 0, r0, c1, c0, 0
+
+	mcr	p15, 0, r0, c8, c7, 0	@ invalidate TLB
+	mcr	p15, 0, r0, c7, c5, 6	@ invalidate branch predictor
+	dsb
+	isb
+
+	pop	{r4, pc}
+

=== added file 'grub-core/kern/arm/dl.c'
--- grub-core/kern/arm/dl.c	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/dl.c	2013-03-24 12:56:20 +0000
@@ -0,0 +1,490 @@
+/* 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>
+
+#ifdef GRUB_UTIL
+# include <grub/util/misc.h>
+#else
+# if !defined(__thumb2__)
+#  error "Relocations not implemented for A32 ("ARM") instruction set yet!"
+# endif
+
+grub_err_t reloc_jump24 (grub_uint32_t *addr, Elf32_Addr sym_addr);
+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);
+
+#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
+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))
+    {
+#ifndef GRUB_UTIL
+      return grub_error
+	(GRUB_ERR_BUG, N_("Relocation targeting wrong execution state"));
+#else
+      grub_util_error ("Relocation targeting wrong execution state");
+#endif
+    }
+
+  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))
+    {
+#ifdef GRUB_UTIL
+      grub_util_error ("Relocation out of range");
+#else
+      return grub_error
+	(GRUB_ERR_OUT_OF_RANGE, N_("THM_CALL Relocation out of range."));
+#endif
+    }
+
+#ifdef GRUB_UTIL
+  grub_util_info ("    relative destination = 0x%08x",
+		  (unsigned int)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
+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;
+}
+
+
+\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
+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;
+}
+
+
+\f
+/*************************************************
+ * 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)
+{
+  grub_dl_segment_t seg;
+  Elf_Rel *rel;
+  Elf_Sym *sym;
+  int i, entnum;
+
+  entnum = relhdr->sh_size / sizeof (Elf_Rel);
+
+  /* Find the target segment for this relocation section. */
+  seg = find_segment (mod->segment, relhdr->sh_info);
+  if (!seg)
+    return grub_error (GRUB_ERR_EOF, N_("relocation segment not found"));
+
+  rel = (Elf_Rel *) ((grub_addr_t) e + relhdr->sh_offset);
+
+  /* Step through all relocations */
+  for (i = 0, sym = mod->symtab; i < entnum; i++)
+    {
+      Elf_Addr *target, sym_addr;
+      int relsym, reltype;
+      grub_err_t retval;
+
+      if (seg->size < rel[i].r_offset)
+	return grub_error (GRUB_ERR_BAD_MODULE,
+			   "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);
+
+      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);
+	    if (retval != GRUB_ERR_NONE)
+	      return retval;
+	  }
+	  break;
+	case R_ARM_JUMP24:
+	  {
+	    retval = reloc_jump24 (target, sym_addr);
+	    if (retval != GRUB_ERR_NONE)
+	      return retval;
+	  }
+	  break;
+	case R_ARM_THM_CALL:
+	case R_ARM_THM_JUMP24:
+	  {
+	    /* Thumb instructions can be 16-bit aligned */
+	    retval = reloc_thm_call ((grub_uint16_t *) target, sym_addr);
+	    if (retval != GRUB_ERR_NONE)
+	      return retval;
+	  }
+	  break;
+	case R_ARM_THM_JUMP19:
+	  {
+	    /* Thumb instructions can be 16-bit aligned */
+	    retval = reloc_thm_jump19 ((grub_uint16_t *) target, sym_addr);
+	    if (retval != GRUB_ERR_NONE)
+	      return retval;
+	  }
+	  break;
+	default:
+	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			     N_("relocation 0x%x is not implemented yet"),
+			     reltype);
+	}
+    }
+
+  return GRUB_ERR_NONE;
+}
+
+
+/*
+ * Check if EHDR is a valid ELF header.
+ */
+grub_err_t
+grub_arch_dl_check_header (void *ehdr)
+{
+  Elf_Ehdr *e = ehdr;
+
+  /* Check the magic numbers.  */
+  if (e->e_ident[EI_CLASS] != ELFCLASS32
+      || e->e_ident[EI_DATA] != ELFDATA2LSB || e->e_machine != EM_ARM)
+    return grub_error (GRUB_ERR_BAD_OS,
+		       N_("invalid arch-dependent ELF magic"));
+
+  return GRUB_ERR_NONE;
+}
+
+/*
+ * Verify that provided ELF header contains reference to a symbol table
+ */
+static int
+has_symtab (Elf_Ehdr * e)
+{
+  int i;
+  Elf_Shdr *s;
+
+  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_SYMTAB)
+      return 1;
+
+  return 0;
+}
+
+/*
+ * grub_arch_dl_relocate_symbols():
+ *   Only externally visible function in this file.
+ *   Locates the relocations section of the ELF object, and calls
+ *   do_relocations() to deal with it.
+ */
+grub_err_t
+grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
+{
+  Elf_Ehdr *e = ehdr;
+  Elf_Shdr *s;
+  unsigned i;
+
+  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))
+
+  for (i = 0, s = FIRST_SHDR (e); i < e->e_shnum; i++, s = NEXT_SHDR (e, s))
+    {
+      grub_err_t ret;
+
+      switch (s->sh_type)
+	{
+	case SHT_REL:
+	  {
+	    /* Relocations, no addends */
+	    ret = do_relocations (s, e, mod);
+	    if (ret != GRUB_ERR_NONE)
+	      return ret;
+	  }
+	  break;
+	case SHT_NULL:
+	case SHT_PROGBITS:
+	case SHT_SYMTAB:
+	case SHT_STRTAB:
+	case SHT_NOBITS:
+	case SHT_ARM_ATTRIBUTES:
+	  break;
+	case SHT_RELA:
+	default:
+	  {
+	    grub_printf ("unhandled section_type: %d (0x%08x)\n",
+			 s->sh_type, s->sh_type);
+	    return GRUB_ERR_NOT_IMPLEMENTED_YET;
+	  };
+	}
+    }
+
+#undef FIRST_SHDR
+#undef NEXT_SHDR
+
+  return GRUB_ERR_NONE;
+}
+#endif /* ndef GRUB_UTIL */

=== added file 'grub-core/kern/arm/misc.S'
--- grub-core/kern/arm/misc.S	1970-01-01 00:00:00 +0000
+++ grub-core/kern/arm/misc.S	2013-03-24 12:56:20 +0000
@@ -0,0 +1,44 @@
+/*
+ *  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/symbol.h>
+#include <grub/dl.h>
+
+	.file	"misc.S"
+	.text
+	.syntax	unified
+#if !defined (__thumb2__)
+	.arm
+#define ARM(x...)	x
+#define THUMB(x...)
+#else
+	.thumb
+#define THUMB(x...)	x
+#define ARM(x...)
+#endif
+
+	.align	2
+
+/*
+ * Null divide-by-zero handler
+ */
+FUNCTION(raise)
+	mov	r0, #0
+	bx	lr
+	
+	.end

=== added directory 'grub-core/lib/arm'
=== added file 'grub-core/lib/arm/setjmp.S'
--- grub-core/lib/arm/setjmp.S	1970-01-01 00:00:00 +0000
+++ grub-core/lib/arm/setjmp.S	2013-03-24 12:56:20 +0000
@@ -0,0 +1,55 @@
+/*
+ *  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/symbol.h>
+#include <grub/dl.h>
+
+	.file	"setjmp.S"
+	.syntax	unified
+#if !defined (__thumb2__)
+	.arm
+#define ARM(x...)	x
+#define THUMB(x...)
+#else
+	.thumb
+#define THUMB(x...)	x
+#define ARM(x...)
+#endif
+
+	.text
+
+/*
+ * int grub_setjmp (grub_jmp_buf env)
+ */
+FUNCTION(grub_setjmp)
+ THUMB(	mov	ip, sp			)
+ THUMB(	stm	r0, { r4-r11, ip, lr }	)
+ ARM(	stm	r0, { r4-r11, sp, lr }	)
+	mov	r0, #0
+	bx	lr
+
+/*
+ * int grub_longjmp (grub_jmp_buf env, int val)
+ */
+FUNCTION(grub_longjmp)
+ THUMB(	ldm	r0, { r4-r11, ip, lr }	)
+ THUMB(	mov	sp, ip			)
+ ARM(	ldm	r0, { r4-r11, sp, lr }	)
+	movs	r0, r1
+	moveq	r0, #1
+	bx	lr

=== modified file 'grub-core/lib/setjmp.S'
--- grub-core/lib/setjmp.S	2012-01-18 13:04:52 +0000
+++ grub-core/lib/setjmp.S	2013-03-24 12:56:20 +0000
@@ -11,6 +11,8 @@
 #elif defined(__ia64__)
 #include "./ia64/setjmp.S"
 #include "./ia64/longjmp.S"
+#elif defined(__arm__)
+#include "./arm/setjmp.S"
 #else
 #error "Unknown target cpu type"
 #endif

=== added directory 'include/grub/arm'
=== added file 'include/grub/arm/system.h'
--- include/grub/arm/system.h	1970-01-01 00:00:00 +0000
+++ include/grub/arm/system.h	2013-03-24 12:56:20 +0000
@@ -0,0 +1,7 @@
+#ifndef GRUB_SYSTEM_CPU_HEADER
+#define GRUB_SYSTEM_CPU_HEADER
+
+void grub_arm_disable_caches_mmu (void);
+
+#endif /* ! GRUB_SYSTEM_CPU_HEADER */
+

=== added file 'include/grub/arm/time.h'
--- include/grub/arm/time.h	1970-01-01 00:00:00 +0000
+++ include/grub/arm/time.h	2013-03-24 12:56:20 +0000
@@ -0,0 +1,29 @@
+/*
+ *  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 KERNEL_CPU_TIME_HEADER
+#define KERNEL_CPU_TIME_HEADER	1
+
+static __inline void
+grub_cpu_idle (void)
+{
+  /* FIXME: this can't work until we handle interrupts.  */
+/*  __asm__ __volatile__ ("wfi"); */
+}
+
+#endif /* ! KERNEL_CPU_TIME_HEADER */

=== added file 'include/grub/arm/types.h'
--- include/grub/arm/types.h	1970-01-01 00:00:00 +0000
+++ include/grub/arm/types.h	2013-03-24 12:56:20 +0000
@@ -0,0 +1,34 @@
+/*
+ *  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_TYPES_CPU_HEADER
+#define GRUB_TYPES_CPU_HEADER	1
+
+/* The size of void *.  */
+#define GRUB_TARGET_SIZEOF_VOID_P	4
+
+/* The size of long.  */
+#define GRUB_TARGET_SIZEOF_LONG		4
+
+/* currently only support little-endian.  */
+#undef GRUB_TARGET_WORDS_BIGENDIAN
+
+/* Unaligned accesses only supported if MMU enabled */
+//#define GRUB_HAVE_UNALIGNED_ACCESS 1
+
+#endif /* ! GRUB_TYPES_CPU_HEADER */

=== modified file 'include/grub/elf.h'
--- include/grub/elf.h	2013-01-20 22:01:47 +0000
+++ include/grub/elf.h	2013-03-24 12:56:20 +0000
@@ -2098,6 +2098,7 @@
 #define R_ARM_LDR_SBREL_11_0	35
 #define R_ARM_ALU_SBREL_19_12	36
 #define R_ARM_ALU_SBREL_27_20	37
+#define R_ARM_THM_JUMP19	51
 #define R_ARM_TLS_GOTDESC	90
 #define R_ARM_TLS_CALL		91
 #define R_ARM_TLS_DESCSEQ	92

=== modified file 'include/grub/libgcc.h'
--- include/grub/libgcc.h	2013-03-03 14:57:30 +0000
+++ include/grub/libgcc.h	2013-03-24 12:56:20 +0000
@@ -112,3 +112,14 @@
 void EXPORT_FUNC (_savegpr_30) (void);
 void EXPORT_FUNC (_savegpr_31) (void);
 #endif
+
+#if defined (__arm__)
+void EXPORT_FUNC (__aeabi_idiv) (void);
+void EXPORT_FUNC (__aeabi_idivmod) (void);
+void EXPORT_FUNC (__aeabi_lasr) (void);
+void EXPORT_FUNC (__aeabi_llsl) (void);
+void EXPORT_FUNC (__aeabi_llsr) (void);
+void EXPORT_FUNC (__aeabi_uidiv) (void);
+void EXPORT_FUNC (__aeabi_uidivmod) (void);
+void EXPORT_FUNC (__wrap___clear_cache) (void *, void *);
+#endif

=== modified file 'include/grub/symbol.h'
--- include/grub/symbol.h	2012-05-28 15:49:18 +0000
+++ include/grub/symbol.h	2013-03-24 12:56:20 +0000
@@ -29,7 +29,11 @@
 
 #if HAVE_ASM_USCORE
 #ifdef ASM_FILE
-# define EXT_C(sym)	_ ## sym
+# ifndef (__arm__)
+#  define EXT_C(sym)	_ ## sym
+# else
+#  define EXT_C(sym)	% ## sym
+# endif
 #else
 # define EXT_C(sym)	"_" sym
 #endif

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Francesco Lavra]

On 03/24/2013 06:01 PM, Leif Lindholm wrote:
[...]
> === added file 'grub-core/kern/arm/cache.S'
> --- grub-core/kern/arm/cache.S	1970-01-01 00:00:00 +0000
> +++ grub-core/kern/arm/cache.S	2013-03-24 12:56:20 +0000
> @@ -0,0 +1,251 @@
> +/*
> + *  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/symbol.h>
> +#include <grub/dl.h>

This include is not needed.

> +
> +	.file	"cache.S"
> +	.text
> +	.syntax	unified
> +#if !defined (__thumb2__)
> +	.arm
> +#define ARM(x...)	x
> +#define THUMB(x...)
> +#else
> +	.thumb
> +#define THUMB(x...)	x
> +#define ARM(x...)
> +#endif
> +
> +	.align	2
> +
> +/*
> + * Simple cache maintenance functions
> + */
> +
> +@ r0 - *beg (inclusive)
> +@ r1 - *end (exclusive)	
> +clean_dcache_range:
> +	@ Clean data cache range for range to point-of-unification
> +	ldr	r2, dlinesz
> +1:	cmp	r0, r1
> +	bge	2f
> +#ifdef DEBUG
> +	push	{r0-r2, lr}
> +	mov	r1, r2
> +	mov	r2, r0
> +	ldr	r0, =dcstr
> +	bl	EXT_C(grub_printf)
> +	pop	{r0-r2, lr}
> +#endif
> +	mcr	p15, 0, r0, c7, c11, 1	@ DCCMVAU
> +	add	r0, r0, r2		@ Next line
> +	b	1b
> +2:	dsb
> +	bx	lr

If the start address (initial value of r0 in this function) is not
aligned to a cache line boundary, the last cache line in the range to be
cleaned might not be cleaned, depending on the value of r1 (the end
address). To handle correctly such cases, the initial value of r0 should
be aligned to the cache line boundary.

> +
> +@ r0 - *beg (inclusive)
> +@ r1 - *end (exclusive)	
> +invalidate_icache_range:
> +	@ Invalidate instruction cache for range to point-of-unification
> +	ldr	r2, ilinesz
> +1:	cmp	r0, r1
> +	bge	2f
> +#ifdef DEBUG
> +	push	{r0-r2, lr}
> +	mov	r1, r2
> +	mov	r2, r0
> +	ldr	r0, =icstr
> +	bl	EXT_C(grub_printf)
> +	pop	{r0-r2, lr}
> +#endif
> +	mcr	p15, 0, r0, c7, c5, 1	@ ICIMVAU
> +	add	r0, r0, r2		@ Next line
> +	b	1b

The same applies here: the initial value of r0 should be aligned to the
cache line boundary.

> +	@ Branch predictor invalidate all
> +2:	mcr	p15, 0, r0, c7,	c5, 6	@ BPIALL
> +	dsb
> +	isb
> +	bx	lr
> +	
> +@void __wrap___clear_cache(char *beg, char *end);
> +FUNCTION(__wrap___clear_cache)
> +	dmb
> +	dsb
> +	push	{r4-r6, lr}
> +	ldr	r2, probed	@ If first call, probe cache sizes
> +	cmp	r2, #0

If the -mimplicit-it=thumb assembler option is removed from patch 1/7
(as I think it should), then here goes the instruction:

	it	eq

> +	bleq	probe_caches	@ This call corrupts r3
> +	mov	r4, r0
> +	mov	r5, r1
> +	bl	clean_dcache_range
> +	mov	r0, r4
> +	mov	r1, r5
> +	bl	invalidate_icache_range
> +	pop	{r4-r6, pc}
> +
> +probe_caches:
> +	push	{r4-r6, lr}
> +	mrc 	p15, 0, r4, c0, c0, 1	@ Read Cache Type Register
> +	mov	r5, #1
> +	ubfx	r6, r4, #16, #4		@ Extract min D-cache num word log2
> +	add	r6, r6, #2		@ words->bytes
> +	lsl	r6, r5, r6		@ Convert to num bytes
> +	ldr	r3, =dlinesz
> +	str	r6, [r3]
> +	and	r6, r4, #0xf		@ Extract min I-cache num word log2
> +	add	r6, r6, #2		@ words->bytes
> +	lsl	r6, r5, r6		@ Convert to num bytes
> +	ldr	r3, =ilinesz
> +	str	r6, [r3]
> +	ldr	r3, =probed		@ Flag cache probing done
> +	str	r5, [r3]
> +	pop	{r4-r6, pc}
> +
> +#ifdef DEBUG
> +dcstr:	.asciz	"cleaning %d bytes of D cache @ 0x%08x\n"
> +icstr:	.asciz	"invalidating %d bytes of I cache @ 0x%08x\n"
> +#endif
> +	
> +	.align	3
> +probed:	.long	0
> +dlinesz:
> +	.long	0
> +ilinesz:
> +	.long	0
> +
> +@void grub_arch_sync_caches (void *address, grub_size_t len)
> +FUNCTION(grub_arch_sync_caches)
> +	add	r1, r0, r1
> +	b	__wrap___clear_cache
> +
> +	@ r0  - CLIDR
> +	@ r1  - LoC
> +	@ r2  - current level
> +	@ r3  - num sets
> +	@ r4  - num ways
> +	@ r5  - current set
> +	@ r6  - current way
> +	@ r7  - line size
> +	@ r8  - scratch
> +	@ r9  - scratch
> +	@ r10 - scratch
> +	@ r11 - scratch
> +clean_invalidate_dcache:
> +	push	{r4-r12, lr}
> +	mrc 	p15, 1, r0, c0, c0, 1	@ Read CLIDR
> +	ubfx	r1, r0, #24, #3		@ Extract LoC
> +	
> +	mov	r2, #0			@ First level, L1
> +2:	and	r8, r0, #7		@ cache type at current level
> +	cmp	r8, #2
> +	blt	5f			@ instruction only, or none, skip level
> +
> +	@ set current cache level/type (for CSSIDR read)
> +	lsl	r8, r2, #1
> +	mcr	p15, 2, r8, c0, c0, 0	@ Write CSSELR (level, type: data/uni)
> +
> +	@ read current cache information
> +	mrc	p15, 1, r8, c0, c0, 0	@ Read CSSIDR
> +	ubfx	r3, r8, #13, #14	@ Number of sets -1
> +	ubfx	r4, r8, #3, #9		@ Number of ways -1
> +	and	r7, r8, #7		@ log2(line size in words) - 2
> +	add	r7, r7, #2		@  adjust
> +	mov	r8, #1
> +	lsl	r7, r8, r7		@  -> line size in words
> +	lsl	r7, r7, #2		@  -> bytes
> +
> +	@ set loop
> +	mov	r5, #0			@ current set = 0
> +3:	lsl	r8, r2, #1		@ insert level
> +	clz	r9, r7			@ calculate set field offset
> +	mov	r10, #31
> +	sub	r9, r10, r9
> +	lsl	r10, r5, r9
> +	orr	r8, r8, r10		@ insert set field
> +
> +	@ way loop
> +	@ calculate way field offset
> +	mov	r6, #0			@ current way = 0
> +	add	r10, r4, #1
> +	clz	r9, r10			@ r9 = way field offset
> +	add	r9, r9, #1
> +4:	lsl	r10, r6, r9
> +	orr	r11, r8, r10		@ insert way field	
> +	
> +	@ clean line by set/way

Nitpick: the comment should be "clean and invalidate line by set/way".

[...]
> === added file 'grub-core/kern/arm/dl.c'
> --- grub-core/kern/arm/dl.c	1970-01-01 00:00:00 +0000
> +++ grub-core/kern/arm/dl.c	2013-03-24 12:56:20 +0000
[...]
> +/*
> + * R_ARM_THM_JUMP19
> + *
> + * Relocate conditional Thumb (T32) B<c>.W
> + */
> +grub_err_t
> +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;

Shouldn't it be 0xfbc0d000? Bit 11 of the second half-word corresponds
to field J2, and as such is part of the offset.

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

It looks to me like some shifts are wrong here. It should be:

  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 */
> +#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;

The mask should be 0xfffff.

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

As above, it looks like some shifts are wrong. It should be:

  insword |= ((offset >> 19) & 1) << 26
    | ((offset >> 18) & 1) << 11
    | ((offset >> 17) & 1) << 13
    | ((offset >> 11) & 0x3f) << 16
    | (offset & 0x7ff);

> +  *addr = insword >> 16;
> +  *(addr + 1) = 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
> 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;
> }

If sym_addr has its least significant bit set, it means it addresses a
Thumb instruction, and in this case an inter-working veneer must be used
to effect the transition from ARM to Thumb state. Implementing veneers
is probably overkill here, but I think in this case this function should
error out as the relocation is not supported, instead of silently
performing a wrong relocation.

[...]
> +/*
> + * 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)
> +{
> +  grub_dl_segment_t seg;
> +  Elf_Rel *rel;
> +  Elf_Sym *sym;
> +  int i, entnum;
> +
> +  entnum = relhdr->sh_size / sizeof (Elf_Rel);
> +
> +  /* Find the target segment for this relocation section. */
> +  seg = find_segment (mod->segment, relhdr->sh_info);
> +  if (!seg)
> +    return grub_error (GRUB_ERR_EOF, N_("relocation segment not found"));
> +
> +  rel = (Elf_Rel *) ((grub_addr_t) e + relhdr->sh_offset);
> +
> +  /* Step through all relocations */
> +  for (i = 0, sym = mod->symtab; i < entnum; i++)
> +    {
> +      Elf_Addr *target, sym_addr;
> +      int relsym, reltype;
> +      grub_err_t retval;
> +
> +      if (seg->size < rel[i].r_offset)
> +	return grub_error (GRUB_ERR_BAD_MODULE,
> +			   "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 is declared as Elf_Addr *, so the cast to Elf_Word * seems
inappropriate, even though they are practically the same type.
Since target can point at either a 32 bit word or a 16 bit half-word,
I'm wondering whether void * would be a more appropriate type here.

[...]
> === added file 'grub-core/kern/arm/misc.S'
> --- grub-core/kern/arm/misc.S	1970-01-01 00:00:00 +0000
> +++ grub-core/kern/arm/misc.S	2013-03-24 12:56:20 +0000
> @@ -0,0 +1,44 @@
> +/*
> + *  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/symbol.h>
> +#include <grub/dl.h>

Unneeded include.

> +
> +	.file	"misc.S"
> +	.text
> +	.syntax	unified
> +#if !defined (__thumb2__)
> +	.arm
> +#define ARM(x...)	x
> +#define THUMB(x...)
> +#else
> +	.thumb
> +#define THUMB(x...)	x
> +#define ARM(x...)
> +#endif
> +
> +	.align	2
> +
> +/*
> + * Null divide-by-zero handler
> + */
> +FUNCTION(raise)
> +	mov	r0, #0
> +	bx	lr
> +	
> +	.end
> 
[...]
> === added file 'grub-core/lib/arm/setjmp.S'
> --- grub-core/lib/arm/setjmp.S	1970-01-01 00:00:00 +0000
> +++ grub-core/lib/arm/setjmp.S	2013-03-24 12:56:20 +0000
> @@ -0,0 +1,55 @@
> +/*
> + *  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/symbol.h>
> +#include <grub/dl.h>

Unneeded include.

> +
> +	.file	"setjmp.S"
> +	.syntax	unified
> +#if !defined (__thumb2__)
> +	.arm
> +#define ARM(x...)	x
> +#define THUMB(x...)
> +#else
> +	.thumb
> +#define THUMB(x...)	x
> +#define ARM(x...)
> +#endif
> +
> +	.text
> +
> +/*
> + * int grub_setjmp (grub_jmp_buf env)
> + */
> +FUNCTION(grub_setjmp)
> + THUMB(	mov	ip, sp			)
> + THUMB(	stm	r0, { r4-r11, ip, lr }	)
> + ARM(	stm	r0, { r4-r11, sp, lr }	)
> +	mov	r0, #0
> +	bx	lr
> +
> +/*
> + * int grub_longjmp (grub_jmp_buf env, int val)
> + */
> +FUNCTION(grub_longjmp)
> + THUMB(	ldm	r0, { r4-r11, ip, lr }	)
> + THUMB(	mov	sp, ip			)
> + ARM(	ldm	r0, { r4-r11, sp, lr }	)
> +	movs	r0, r1

As above, I would drop the -mimplicit-it=thumb assembler option and here
I would insert the instruction:

	it	eq

> +	moveq	r0, #1
> +	bx	lr
> 

--
Francesco

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

> === added directory 'grub-core/kern/arm'

> === added file 'grub-core/kern/arm/cache.S'
> --- grub-core/kern/arm/cache.S	1970-01-01 00:00:00 +0000
> +++ grub-core/kern/arm/cache.S	2013-03-24 12:56:20 +0000
> @@ -0,0 +1,251 @@
> +/*
> + *  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/symbol.h>
> +#include <grub/dl.h>
> +
> +	.file	"cache.S"
> +	.text
> +	.syntax	unified
> +#if !defined (__thumb2__)
> +	.arm
> +#define ARM(x...)	x
> +#define THUMB(x...)
> +#else
> +	.thumb
> +#define THUMB(x...)	x
> +#define ARM(x...)
> +#endif

What are THUMB/ARM macros for? I don't see any use of them

> +
> +	.align	2
> +
> +/*
> + * Simple cache maintenance functions
> + */
> +
> +@ r0 - *beg (inclusive)
> +@ r1 - *end (exclusive)	
> +clean_dcache_range:
> +	@ Clean data cache range for range to point-of-unification
> +	ldr	r2, dlinesz
> +1:	cmp	r0, r1
> +	bge	2f
> +#ifdef DEBUG
> +	push	{r0-r2, lr}
> +	mov	r1, r2
> +	mov	r2, r0
> +	ldr	r0, =dcstr
> +	bl	EXT_C(grub_printf)
> +	pop	{r0-r2, lr}
> +#endif

Could you use grub_dprintf framework for debug messages which are still
useful and remove the rest?

> +#ifdef DEBUG
> +	push	{r0-r2, lr}
> +	mov	r1, r2
> +	mov	r2, r0
> +	ldr	r0, =icstr
> +	bl	EXT_C(grub_printf)
> +	pop	{r0-r2, lr}
> +#endif

Ditto
> +@void __wrap___clear_cache(char *beg, char *end);

> +FUNCTION(__wrap___clear_cache)

Now that we removed all nested functions would it be possible to remove
this function in order to make linker fail if any nested functions appear?

Can't really review cache flushing due to my very limited ARM experience.

> +#ifdef GRUB_UTIL
> +# include <grub/util/misc.h>
> +#else

Ditto. Use grub_dprintf

> +# if !defined(__thumb2__)
> +#  error "Relocations not implemented for A32 ("ARM") instruction set yet!"
> +# endif
> +
> +grub_err_t reloc_jump24 (grub_uint32_t *addr, Elf32_Addr sym_addr);
> +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);
> +
> +#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 */

This would need to be platform-independent. I don't see how this would
be ARM-specific.

> +  tmp = *target;
> +  tmp += sym_addr;
> +  *target = tmp;


Why not *target += sym_addr and put it inline?

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

Why not use grub_get_unaligned32

> +  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))
> +    {
> +#ifndef GRUB_UTIL
> +      return grub_error
> +	(GRUB_ERR_BUG, N_("Relocation targeting wrong execution state"));

This would be GRUB_ERR_BAD_MODULE

> +
> +  offset_low = -16777216;
> +  offset_high = is_blx ? 16777212 : 16777214;

Why not write these values in hex?

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

What is this target? Why is it dependent whether we're in util or not

> +  if ((offset < offset_low) || (offset > offset_high))
> +    {
> +#ifdef GRUB_UTIL
> +      grub_util_error ("Relocation out of range");
> +#else
> +      return grub_error
> +	(GRUB_ERR_OUT_OF_RANGE, N_("THM_CALL Relocation out of range."));

ERR_BAD_MODULE ditto

> +#ifdef GRUB_UTIL
> +#pragma GCC diagnostic ignored "-Wcast-align"
> +  grub_util_info ("    *target = 0x%08x", *((unsigned int *)target));
> +#endif

grub_get_unaligned.

> +  *addr = insword >> 16;
> +  *(addr + 1) = insword & 0xffff;

Use grub_set_unaligned+

> +/*************************************************
> + * 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;
> +}
> +

Could you just put this directly in the code?

> +/*
> + * grub_arch_dl_relocate_symbols():
> + *   Only externally visible function in this file.
> + *   Locates the relocations section of the ELF object, and calls
> + *   do_relocations() to deal with it.
> + */
> +grub_err_t
> +grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
> +{
> +  Elf_Ehdr *e = ehdr;
> +  Elf_Shdr *s;
> +  unsigned i;
> +
> +  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))
> +
> +  for (i = 0, s = FIRST_SHDR (e); i < e->e_shnum; i++, s = NEXT_SHDR (e, s))
> +    {
> +      grub_err_t ret;
> +
> +      switch (s->sh_type)
> +	{
> +	case SHT_REL:
> +	  {
> +	    /* Relocations, no addends */
> +	    ret = do_relocations (s, e, mod);
> +	    if (ret != GRUB_ERR_NONE)
> +	      return ret;
> +	  }
> +	  break;
> +	case SHT_NULL:
> +	case SHT_PROGBITS:
> +	case SHT_SYMTAB:
> +	case SHT_STRTAB:
> +	case SHT_NOBITS:
> +	case SHT_ARM_ATTRIBUTES:
> +	  break;
> +	case SHT_RELA:
> +	default:
> +	  {
> +	    grub_printf ("unhandled section_type: %d (0x%08x)\n",
> +			 s->sh_type, s->sh_type);
> +	    return GRUB_ERR_NOT_IMPLEMENTED_YET;
> +	  };
> +	}
> +    }


Why is this file much bigger and very different from similar file for
other architectures? Perhaps you should make dl.c malloc in single chunk
like already done for ia64 and ppc. It may simplify the code but
additionally will solve few problems with relocation overflows.

> +
> +#if defined (__arm__)
> +void EXPORT_FUNC (__aeabi_idiv) (void);
> +void EXPORT_FUNC (__aeabi_idivmod) (void);

Are these for all or only 64-bit divisions?

> === modified file 'include/grub/symbol.h'
> --- include/grub/symbol.h	2012-05-28 15:49:18 +0000
> +++ include/grub/symbol.h	2013-03-24 12:56:20 +0000
> @@ -29,7 +29,11 @@
>  
>  #if HAVE_ASM_USCORE
>  #ifdef ASM_FILE
> -# define EXT_C(sym)	_ ## sym
> +# ifndef (__arm__)
> +#  define EXT_C(sym)	_ ## sym
> +# else
> +#  define EXT_C(sym)	% ## sym

Why % ? Shouldn't configure tests adjusted as well?


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

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Francesco Lavra]

On 04/01/2013 03:28 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> +grub_err_t
>> +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));
> 
> Why not use grub_get_unaligned32

It's not a simple unaligned access, it's actually about accessing two
naturally aligned 16-bit halfwords, and concatenating them by putting
the first value in the most significant bytes. Each halfword is encoded
in little endian, but the set of two halfwords is encoded "most
significant halfword first".
The problem here is in the GRUB_UTIL case, where accessing each of the
two halfwords could cause endianness issues. In this case two calls to
grub_target_to_host16() should be used to retrieve the instruction word.

>> +#ifdef GRUB_UTIL
>> +#pragma GCC diagnostic ignored "-Wcast-align"
>> +  grub_util_info ("    *target = 0x%08x", *((unsigned int *)target));
>> +#endif
> 
> grub_get_unaligned.

Ditto, it's not a simple unaligned access.

> 
>> +  *addr = insword >> 16;
>> +  *(addr + 1) = insword & 0xffff;
> 
> Use grub_set_unaligned+

The same applies here, in the util case a couple of
grub_host_to_target16() calls must be used to store back the instruction
word.

--
Francesco

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Leif Lindholm]

On Mon, Apr 01, 2013 at 03:28:17AM +0200, Vladimir '??-coder/phcoder' Serbinenko wrote:
> > === added directory 'grub-core/kern/arm'
> 
> > === added file 'grub-core/kern/arm/cache.S'
> > --- grub-core/kern/arm/cache.S	1970-01-01 00:00:00 +0000
> > +++ grub-core/kern/arm/cache.S	2013-03-24 12:56:20 +0000
> > @@ -0,0 +1,251 @@
> > +/*
> > + *  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/symbol.h>
> > +#include <grub/dl.h>
> > +
> > +	.file	"cache.S"
> > +	.text
> > +	.syntax	unified
> > +#if !defined (__thumb2__)
> > +	.arm
> > +#define ARM(x...)	x
> > +#define THUMB(x...)
> > +#else
> > +	.thumb
> > +#define THUMB(x...)	x
> > +#define ARM(x...)
> > +#endif
> 
> What are THUMB/ARM macros for? I don't see any use of them

Used for conditionals where one set of instructions is required when
assembling for ARM instruction set and another for Thumb.
(See lib/arm/setjmp.S for an example.)
Being lazy, I tend to stick it in as boilerplate.
Can drop for now in files not using it.

> > +
> > +	.align	2
> > +
> > +/*
> > + * Simple cache maintenance functions
> > + */
> > +
> > +@ r0 - *beg (inclusive)
> > +@ r1 - *end (exclusive)	
> > +clean_dcache_range:
> > +	@ Clean data cache range for range to point-of-unification
> > +	ldr	r2, dlinesz
> > +1:	cmp	r0, r1
> > +	bge	2f
> > +#ifdef DEBUG
> > +	push	{r0-r2, lr}
> > +	mov	r1, r2
> > +	mov	r2, r0
> > +	ldr	r0, =dcstr
> > +	bl	EXT_C(grub_printf)
> > +	pop	{r0-r2, lr}
> > +#endif
> 
> Could you use grub_dprintf framework for debug messages which are still
> useful and remove the rest?

Sure - these are propably all a bit excessive for now, so will drop.

> > +#ifdef DEBUG
> > +	push	{r0-r2, lr}
> > +	mov	r1, r2
> > +	mov	r2, r0
> > +	ldr	r0, =icstr
> > +	bl	EXT_C(grub_printf)
> > +	pop	{r0-r2, lr}
> > +#endif
> 
> Ditto
> > +@void __wrap___clear_cache(char *beg, char *end);
> 
> > +FUNCTION(__wrap___clear_cache)
> 
> Now that we removed all nested functions would it be possible to remove
> this function in order to make linker fail if any nested functions appear?

Tested it, and it seems to work.
It won't actually fail at link time though, but when running grub-mkimage,
so will require an install test, not just a build test.

> Can't really review cache flushing due to my very limited ARM experience.
> 
> > +#ifdef GRUB_UTIL
> > +# include <grub/util/misc.h>
> > +#else
> 
> Ditto. Use grub_dprintf
> 
> > +# if !defined(__thumb2__)
> > +#  error "Relocations not implemented for A32 ("ARM") instruction set yet!"
> > +# endif
> > +
> > +grub_err_t reloc_jump24 (grub_uint32_t *addr, Elf32_Addr sym_addr);
> > +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);
> > +
> > +#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 */
> 
> This would need to be platform-independent. I don't see how this would
> be ARM-specific.

Fair enough - I may just strip it out.

> > +  tmp = *target;
> > +  tmp += sym_addr;
> > +  *target = tmp;
> 
> 
> Why not *target += sym_addr and put it inline?

In a development version I actually extended all of these operations to use
grub_host_to_target/grub_target_to_host, but decided against including that
in my patchset since it ends up being fairly invasive around grub-mkimage
and the util headers. Ended up keeping the logic in place.

It's a simple enough function, so I can put it inline, but I opted to put
it separately since the other relocation handlers are.

> > +grub_err_t
> > +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));
> 
> Why not use grub_get_unaligned32

Like Francesco said, this is endianess as well as alignment, so I should
probable edit the comment to reflect.
 
> > +  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))
> > +    {
> > +#ifndef GRUB_UTIL
> > +      return grub_error
> > +	(GRUB_ERR_BUG, N_("Relocation targeting wrong execution state"));
> 
> This would be GRUB_ERR_BAD_MODULE
> 
> > +
> > +  offset_low = -16777216;
> > +  offset_high = is_blx ? 16777212 : 16777214;
> 
> Why not write these values in hex?
 
They are written in decimal in the ARM Architecture Reference Manual.
I could change them if you wish.

> > +
> > +  /* 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
> 
> What is this target? Why is it dependent whether we're in util or not
 
In the util case, this ends up being called from the EFI relocation code,
so cannot be fully resolved. This may end up being true also for any future
relocatable ELF image for U-Boot.

> > +  if ((offset < offset_low) || (offset > offset_high))
> > +    {
> > +#ifdef GRUB_UTIL
> > +      grub_util_error ("Relocation out of range");
> > +#else
> > +      return grub_error
> > +	(GRUB_ERR_OUT_OF_RANGE, N_("THM_CALL Relocation out of range."));
> 
> ERR_BAD_MODULE ditto
> 
> > +#ifdef GRUB_UTIL
> > +#pragma GCC diagnostic ignored "-Wcast-align"
> > +  grub_util_info ("    *target = 0x%08x", *((unsigned int *)target));
> > +#endif
> 
> grub_get_unaligned.
> 
> > +  *addr = insword >> 16;
> > +  *(addr + 1) = insword & 0xffff;
> 
> Use grub_set_unaligned+
> 
> > +/*************************************************
> > + * 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;
> > +}
> > +
> 
> Could you just put this directly in the code?

I can, if preferred.

> > +/*
> > + * grub_arch_dl_relocate_symbols():
> > + *   Only externally visible function in this file.
> > + *   Locates the relocations section of the ELF object, and calls
> > + *   do_relocations() to deal with it.
> > + */
> > +grub_err_t
> > +grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
> > +{
> > +  Elf_Ehdr *e = ehdr;
> > +  Elf_Shdr *s;
> > +  unsigned i;
> > +
> > +  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))
> > +
> > +  for (i = 0, s = FIRST_SHDR (e); i < e->e_shnum; i++, s = NEXT_SHDR (e, s))
> > +    {
> > +      grub_err_t ret;
> > +
> > +      switch (s->sh_type)
> > +	{
> > +	case SHT_REL:
> > +	  {
> > +	    /* Relocations, no addends */
> > +	    ret = do_relocations (s, e, mod);
> > +	    if (ret != GRUB_ERR_NONE)
> > +	      return ret;
> > +	  }
> > +	  break;
> > +	case SHT_NULL:
> > +	case SHT_PROGBITS:
> > +	case SHT_SYMTAB:
> > +	case SHT_STRTAB:
> > +	case SHT_NOBITS:
> > +	case SHT_ARM_ATTRIBUTES:
> > +	  break;
> > +	case SHT_RELA:
> > +	default:
> > +	  {
> > +	    grub_printf ("unhandled section_type: %d (0x%08x)\n",
> > +			 s->sh_type, s->sh_type);
> > +	    return GRUB_ERR_NOT_IMPLEMENTED_YET;
> > +	  };
> > +	}
> > +    }
> 
> 
> Why is this file much bigger and very different from similar file for
> other architectures? Perhaps you should make dl.c malloc in single chunk
> like already done for ia64 and ppc. It may simplify the code but
> additionally will solve few problems with relocation overflows.
 
Three reasons:
- It contains (potentially redundant) additional debug functionality.
- Thumb2 relocations are a bit fiddly.
- I implement also code used for the EFI case, which for ia64 is inlined
  (causing duplication) in util/grub-mkimagexx.c.

> > +
> > +#if defined (__arm__)
> > +void EXPORT_FUNC (__aeabi_idiv) (void);
> > +void EXPORT_FUNC (__aeabi_idivmod) (void);
> 
> Are these for all or only 64-bit divisions?

All.
 
> > === modified file 'include/grub/symbol.h'
> > --- include/grub/symbol.h	2012-05-28 15:49:18 +0000
> > +++ include/grub/symbol.h	2013-03-24 12:56:20 +0000
> > @@ -29,7 +29,11 @@
> >  
> >  #if HAVE_ASM_USCORE
> >  #ifdef ASM_FILE
> > -# define EXT_C(sym)	_ ## sym
> > +# ifndef (__arm__)
> > +#  define EXT_C(sym)	_ ## sym
> > +# else
> > +#  define EXT_C(sym)	% ## sym
> 
> Why % ? Shouldn't configure tests adjusted as well?
 
Empirical tests showed it worked :)
Yes, probably - in acinclude.m4?

/
    Leif

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Leif Lindholm]

On Sat, Mar 30, 2013 at 04:15:54PM +0100, Francesco Lavra wrote:
> On 03/24/2013 06:01 PM, Leif Lindholm wrote:
> [...]
> > === added file 'grub-core/kern/arm/cache.S'
> > --- grub-core/kern/arm/cache.S	1970-01-01 00:00:00 +0000
> > +++ grub-core/kern/arm/cache.S	2013-03-24 12:56:20 +0000
> > @@ -0,0 +1,251 @@
> > +/*
> > + *  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/symbol.h>
> > +#include <grub/dl.h>
> 
> This include is not needed.
> 
> > +
> > +	.file	"cache.S"
> > +	.text
> > +	.syntax	unified
> > +#if !defined (__thumb2__)
> > +	.arm
> > +#define ARM(x...)	x
> > +#define THUMB(x...)
> > +#else
> > +	.thumb
> > +#define THUMB(x...)	x
> > +#define ARM(x...)
> > +#endif
> > +
> > +	.align	2
> > +
> > +/*
> > + * Simple cache maintenance functions
> > + */
> > +
> > +@ r0 - *beg (inclusive)
> > +@ r1 - *end (exclusive)	
> > +clean_dcache_range:
> > +	@ Clean data cache range for range to point-of-unification
> > +	ldr	r2, dlinesz
> > +1:	cmp	r0, r1
> > +	bge	2f
> > +#ifdef DEBUG
> > +	push	{r0-r2, lr}
> > +	mov	r1, r2
> > +	mov	r2, r0
> > +	ldr	r0, =dcstr
> > +	bl	EXT_C(grub_printf)
> > +	pop	{r0-r2, lr}
> > +#endif
> > +	mcr	p15, 0, r0, c7, c11, 1	@ DCCMVAU
> > +	add	r0, r0, r2		@ Next line
> > +	b	1b
> > +2:	dsb
> > +	bx	lr
> 
> If the start address (initial value of r0 in this function) is not
> aligned to a cache line boundary, the last cache line in the range to be
> cleaned might not be cleaned, depending on the value of r1 (the end
> address). To handle correctly such cases, the initial value of r0 should
> be aligned to the cache line boundary.
 
ACK. Fixed, will be in next version.

> > +
> > +@ r0 - *beg (inclusive)
> > +@ r1 - *end (exclusive)	
> > +invalidate_icache_range:
> > +	@ Invalidate instruction cache for range to point-of-unification
> > +	ldr	r2, ilinesz
> > +1:	cmp	r0, r1
> > +	bge	2f
> > +#ifdef DEBUG
> > +	push	{r0-r2, lr}
> > +	mov	r1, r2
> > +	mov	r2, r0
> > +	ldr	r0, =icstr
> > +	bl	EXT_C(grub_printf)
> > +	pop	{r0-r2, lr}
> > +#endif
> > +	mcr	p15, 0, r0, c7, c5, 1	@ ICIMVAU
> > +	add	r0, r0, r2		@ Next line
> > +	b	1b
> 
> The same applies here: the initial value of r0 should be aligned to the
> cache line boundary.
 
And again.

> > +	@ Branch predictor invalidate all
> > +2:	mcr	p15, 0, r0, c7,	c5, 6	@ BPIALL
> > +	dsb
> > +	isb
> > +	bx	lr
> > +	
> > +@void __wrap___clear_cache(char *beg, char *end);
> > +FUNCTION(__wrap___clear_cache)
> > +	dmb
> > +	dsb
> > +	push	{r4-r6, lr}
> > +	ldr	r2, probed	@ If first call, probe cache sizes
> > +	cmp	r2, #0
> 
> If the -mimplicit-it=thumb assembler option is removed from patch 1/7
> (as I think it should), then here goes the instruction:
> 
> 	it	eq
> 
> > +	bleq	probe_caches	@ This call corrupts r3
> > +	mov	r4, r0
> > +	mov	r5, r1
> > +	bl	clean_dcache_range
> > +	mov	r0, r4
> > +	mov	r1, r5
> > +	bl	invalidate_icache_range
> > +	pop	{r4-r6, pc}
> > +
> > +probe_caches:
> > +	push	{r4-r6, lr}
> > +	mrc 	p15, 0, r4, c0, c0, 1	@ Read Cache Type Register
> > +	mov	r5, #1
> > +	ubfx	r6, r4, #16, #4		@ Extract min D-cache num word log2
> > +	add	r6, r6, #2		@ words->bytes
> > +	lsl	r6, r5, r6		@ Convert to num bytes
> > +	ldr	r3, =dlinesz
> > +	str	r6, [r3]
> > +	and	r6, r4, #0xf		@ Extract min I-cache num word log2
> > +	add	r6, r6, #2		@ words->bytes
> > +	lsl	r6, r5, r6		@ Convert to num bytes
> > +	ldr	r3, =ilinesz
> > +	str	r6, [r3]
> > +	ldr	r3, =probed		@ Flag cache probing done
> > +	str	r5, [r3]
> > +	pop	{r4-r6, pc}
> > +
> > +#ifdef DEBUG
> > +dcstr:	.asciz	"cleaning %d bytes of D cache @ 0x%08x\n"
> > +icstr:	.asciz	"invalidating %d bytes of I cache @ 0x%08x\n"
> > +#endif
> > +	
> > +	.align	3
> > +probed:	.long	0
> > +dlinesz:
> > +	.long	0
> > +ilinesz:
> > +	.long	0
> > +
> > +@void grub_arch_sync_caches (void *address, grub_size_t len)
> > +FUNCTION(grub_arch_sync_caches)
> > +	add	r1, r0, r1
> > +	b	__wrap___clear_cache
> > +
> > +	@ r0  - CLIDR
> > +	@ r1  - LoC
> > +	@ r2  - current level
> > +	@ r3  - num sets
> > +	@ r4  - num ways
> > +	@ r5  - current set
> > +	@ r6  - current way
> > +	@ r7  - line size
> > +	@ r8  - scratch
> > +	@ r9  - scratch
> > +	@ r10 - scratch
> > +	@ r11 - scratch
> > +clean_invalidate_dcache:
> > +	push	{r4-r12, lr}
> > +	mrc 	p15, 1, r0, c0, c0, 1	@ Read CLIDR
> > +	ubfx	r1, r0, #24, #3		@ Extract LoC
> > +	
> > +	mov	r2, #0			@ First level, L1
> > +2:	and	r8, r0, #7		@ cache type at current level
> > +	cmp	r8, #2
> > +	blt	5f			@ instruction only, or none, skip level
> > +
> > +	@ set current cache level/type (for CSSIDR read)
> > +	lsl	r8, r2, #1
> > +	mcr	p15, 2, r8, c0, c0, 0	@ Write CSSELR (level, type: data/uni)
> > +
> > +	@ read current cache information
> > +	mrc	p15, 1, r8, c0, c0, 0	@ Read CSSIDR
> > +	ubfx	r3, r8, #13, #14	@ Number of sets -1
> > +	ubfx	r4, r8, #3, #9		@ Number of ways -1
> > +	and	r7, r8, #7		@ log2(line size in words) - 2
> > +	add	r7, r7, #2		@  adjust
> > +	mov	r8, #1
> > +	lsl	r7, r8, r7		@  -> line size in words
> > +	lsl	r7, r7, #2		@  -> bytes
> > +
> > +	@ set loop
> > +	mov	r5, #0			@ current set = 0
> > +3:	lsl	r8, r2, #1		@ insert level
> > +	clz	r9, r7			@ calculate set field offset
> > +	mov	r10, #31
> > +	sub	r9, r10, r9
> > +	lsl	r10, r5, r9
> > +	orr	r8, r8, r10		@ insert set field
> > +
> > +	@ way loop
> > +	@ calculate way field offset
> > +	mov	r6, #0			@ current way = 0
> > +	add	r10, r4, #1
> > +	clz	r9, r10			@ r9 = way field offset
> > +	add	r9, r9, #1
> > +4:	lsl	r10, r6, r9
> > +	orr	r11, r8, r10		@ insert way field	
> > +	
> > +	@ clean line by set/way
> 
> Nitpick: the comment should be "clean and invalidate line by set/way".
 
Definitely, the current comment is incorrect.

> [...]
> > === added file 'grub-core/kern/arm/dl.c'
> > --- grub-core/kern/arm/dl.c	1970-01-01 00:00:00 +0000
> > +++ grub-core/kern/arm/dl.c	2013-03-24 12:56:20 +0000
> [...]
> > +/*
> > + * R_ARM_THM_JUMP19
> > + *
> > + * Relocate conditional Thumb (T32) B<c>.W
> > + */
> > +grub_err_t
> > +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;
> 
> Shouldn't it be 0xfbc0d000? Bit 11 of the second half-word corresponds
> to field J2, and as such is part of the offset.

Err, yes - certainly.
 
> > +
> > +  /* 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);
> 
> It looks to me like some shifts are wrong here. It should be:
> 
>   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);
 
ACK.

> > +
> > +  /* 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;
> 
> The mask should be 0xfffff.
 
ACK.

> > +
> > +  /* 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);
> 
> As above, it looks like some shifts are wrong. It should be:
> 
>   insword |= ((offset >> 19) & 1) << 26
>     | ((offset >> 18) & 1) << 11
>     | ((offset >> 17) & 1) << 13
>     | ((offset >> 11) & 0x3f) << 16
>     | (offset & 0x7ff);

ACK. The scary thing is I have a GRUB happily running under UEFI,
successfully loading a Linux kernel from MMC, using this code...
 
Fixing for next version.

> > +  *addr = insword >> 16;
> > +  *(addr + 1) = 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
> > 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;
> > }
> 
> If sym_addr has its least significant bit set, it means it addresses a
> Thumb instruction, and in this case an inter-working veneer must be used
> to effect the transition from ARM to Thumb state. Implementing veneers
> is probably overkill here, but I think in this case this function should
> error out as the relocation is not supported, instead of silently
> performing a wrong relocation.
 
Yes, fair enough - I did it for R_THM_JUMP24, so just being lazy here.

> [...]
> > +/*
> > + * 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)
> > +{
> > +  grub_dl_segment_t seg;
> > +  Elf_Rel *rel;
> > +  Elf_Sym *sym;
> > +  int i, entnum;
> > +
> > +  entnum = relhdr->sh_size / sizeof (Elf_Rel);
> > +
> > +  /* Find the target segment for this relocation section. */
> > +  seg = find_segment (mod->segment, relhdr->sh_info);
> > +  if (!seg)
> > +    return grub_error (GRUB_ERR_EOF, N_("relocation segment not found"));
> > +
> > +  rel = (Elf_Rel *) ((grub_addr_t) e + relhdr->sh_offset);
> > +
> > +  /* Step through all relocations */
> > +  for (i = 0, sym = mod->symtab; i < entnum; i++)
> > +    {
> > +      Elf_Addr *target, sym_addr;
> > +      int relsym, reltype;
> > +      grub_err_t retval;
> > +
> > +      if (seg->size < rel[i].r_offset)
> > +	return grub_error (GRUB_ERR_BAD_MODULE,
> > +			   "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 is declared as Elf_Addr *, so the cast to Elf_Word * seems
> inappropriate, even though they are practically the same type.
> Since target can point at either a 32 bit word or a 16 bit half-word,
> I'm wondering whether void * would be a more appropriate type here.
 
void * makes sense to me.

/
    Leif

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 24.03.2013 18:01, Leif Lindholm wrote:

>  
> +  arm = grub-core/kern/arm/dl.c;
> +

Missed this one. It's wrong. All tools are exactly the same on all
platforms. The only thing that varies is tool availability but if the
tool is available on too platforms it has to do exactly the same thing.


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

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Leif Lindholm]

On Tue, Apr 09, 2013 at 01:27:52AM +0200, Vladimir '??-coder/phcoder' Serbinenko wrote:
> On 24.03.2013 18:01, Leif Lindholm wrote:
> >  
> > +  arm = grub-core/kern/arm/dl.c;
> > +
> 
> Missed this one. It's wrong. All tools are exactly the same on all
> platforms. The only thing that varies is tool availability but if the
> tool is available on too platforms it has to do exactly the same thing.

Yes, sorry, will fix in next rev.

/
    Leif

Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 09.04.2013 12:34, Leif Lindholm wrote:

> On Tue, Apr 09, 2013 at 01:27:52AM +0200, Vladimir '??-coder/phcoder' Serbinenko wrote:
>> On 24.03.2013 18:01, Leif Lindholm wrote:
>>>  
>>> +  arm = grub-core/kern/arm/dl.c;
>>> +
>>
>> Missed this one. It's wrong. All tools are exactly the same on all
>> platforms. The only thing that varies is tool availability but if the
>> tool is available on too platforms it has to do exactly the same thing.
> 
> Yes, sorry, will fix in next rev.

Could you make a participation request on savannah, so we can use common
branch for it? I've fixed locally some of the problems.

> 
> /
>     Leif
> 
> _______________________________________________
> 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 --]