All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Relocator framework
@ 2009-08-03 12:06 Vladimir 'phcoder' Serbinenko
  2009-08-03 12:39 ` Vladimir 'phcoder' Serbinenko
  2009-08-04 20:52 ` Robert Millan
  0 siblings, 2 replies; 9+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2009-08-03 12:06 UTC (permalink / raw)
  To: The development of GRUB 2

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

Hello. As discussed on IRC it would be preferable if all loaders use
flexible technics for loading kernels using relocators as currently
multiboot and xnu does. Here is a relocator framework based on
multiboot and xnu relocator. As an advantage it switches from x86-64
to i386 if necessary. With it and some more patches I was able to load
mbtest on qemu64 with tianocore

-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

[-- Attachment #2: relocator.diff --]
[-- Type: text/plain, Size: 12659 bytes --]

diff --git a/conf/i386.rmk b/conf/i386.rmk
index 93f84ce..33d49b5 100644
--- a/conf/i386.rmk
+++ b/conf/i386.rmk
@@ -14,3 +14,9 @@ pkglib_MODULES += vga_text.mod
 vga_text_mod_SOURCES = term/i386/pc/vga_text.c term/i386/vga_common.c
 vga_text_mod_CFLAGS = $(COMMON_CFLAGS)
 vga_text_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
+pkglib_MODULES += relocator.mod
+relocator_mod_SOURCES = lib/i386/relocator.c lib/i386/relocator_asm.S lib/i386/relocator_backward.S
+relocator_mod_CFLAGS = $(COMMON_CFLAGS)
+relocator_mod_ASFLAGS = $(COMMON_ASFLAGS)
+relocator_mod_LDFLAGS = $(COMMON_LDFLAGS)
diff --git a/include/grub/i386/relocator.h b/include/grub/i386/relocator.h
new file mode 100644
index 0000000..cbc75ed
--- /dev/null
+++ b/include/grub/i386/relocator.h
@@ -0,0 +1,40 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  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_RELOCATOR_CPU_HEADER
+#define GRUB_RELOCATOR_CPU_HEADER	1
+
+struct grub_relocator32_state
+{
+  grub_uint32_t esp;
+  grub_uint32_t eax;
+  grub_uint32_t ebx;
+  grub_uint32_t ecx;
+  grub_uint32_t edx;
+  grub_uint32_t eip;
+};
+
+void *
+grub_relocator32_alloc (grub_size_t size);
+grub_err_t
+grub_relocator32_boot (void *relocator, grub_uint32_t dest,
+		       struct grub_relocator32_state state);
+void
+grub_relocator32_free (void *relocator);
+
+#endif /* ! GRUB_RELOCATOR_CPU_HEADER */
diff --git a/lib/i386/relocator.c b/lib/i386/relocator.c
new file mode 100644
index 0000000..6a9b3ce
--- /dev/null
+++ b/lib/i386/relocator.c
@@ -0,0 +1,154 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  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/mm.h>
+#include <grub/misc.h>
+
+#include <grub/types.h>
+#include <grub/types.h>
+#include <grub/err.h>
+
+#include <grub/i386/relocator.h>
+
+extern grub_uint8_t grub_relocator32_forward_start;
+extern grub_uint8_t grub_relocator32_forward_end;
+extern grub_uint8_t grub_relocator32_backward_start;
+extern grub_uint8_t grub_relocator32_backward_end;
+
+extern grub_uint32_t grub_relocator32_backward_dest;
+extern grub_uint32_t grub_relocator32_backward_size;
+#ifdef __x86_64__
+extern grub_uint64_t grub_relocator32_backward_src;
+#else
+extern grub_uint32_t grub_relocator32_backward_src;
+#endif
+
+extern grub_uint32_t grub_relocator32_forward_dest;
+extern grub_uint32_t grub_relocator32_forward_size;
+#ifdef __x86_64__
+extern grub_uint64_t grub_relocator32_forward_src;
+#else
+extern grub_uint32_t grub_relocator32_forward_src;
+#endif
+
+extern grub_uint32_t grub_relocator32_forward_eax;
+extern grub_uint32_t grub_relocator32_forward_ebx;
+extern grub_uint32_t grub_relocator32_forward_ecx;
+extern grub_uint32_t grub_relocator32_forward_edx;
+extern grub_uint32_t grub_relocator32_forward_eip;
+extern grub_uint32_t grub_relocator32_forward_esp;
+
+extern grub_uint32_t grub_relocator32_backward_eax;
+extern grub_uint32_t grub_relocator32_backward_ebx;
+extern grub_uint32_t grub_relocator32_backward_ecx;
+extern grub_uint32_t grub_relocator32_backward_edx;
+extern grub_uint32_t grub_relocator32_backward_eip;
+extern grub_uint32_t grub_relocator32_backward_esp;
+
+#define RELOCATOR_SIZEOF(x)	(&grub_relocator32_##x##_end - &grub_relocator32_##x##_start)
+#define RELOCATOR_ALIGN 16
+
+void *
+grub_relocator32_alloc (grub_size_t size)
+{
+  char *playground;
+
+  playground = grub_malloc ((RELOCATOR_SIZEOF (forward) + RELOCATOR_ALIGN)
+			    + size
+			    + (RELOCATOR_SIZEOF (backward) +
+			       RELOCATOR_ALIGN));
+  if (!playground)
+    return 0;
+
+  *(grub_size_t *) playground = size;
+
+  return playground + RELOCATOR_SIZEOF (forward);
+}
+
+void
+grub_relocator32_free (void *relocator)
+{
+  if (relocator)
+    grub_free ((char *) relocator - RELOCATOR_SIZEOF (forward));
+}
+
+
+grub_err_t
+grub_relocator32_boot (void *relocator, grub_uint32_t dest,
+		       struct grub_relocator32_state state)
+{
+  grub_size_t size;
+  char *playground;
+  void (*entry) ();
+
+  playground = (char *) relocator - RELOCATOR_SIZEOF (forward);
+  size = *(grub_size_t *) playground;
+
+  if (UINT_TO_PTR (dest) >= relocator)
+    {
+      int overhead;
+
+      overhead =
+	ALIGN_UP (dest - RELOCATOR_SIZEOF (backward) - RELOCATOR_ALIGN,
+		  RELOCATOR_ALIGN);
+      grub_relocator32_backward_dest = dest - overhead;
+      grub_relocator32_backward_src = PTR_TO_UINT64 (relocator - overhead);
+      grub_relocator32_backward_size = size + overhead;
+
+      grub_relocator32_backward_eax = state.eax;
+      grub_relocator32_backward_ebx = state.ebx;
+      grub_relocator32_backward_ecx = state.ecx;
+      grub_relocator32_backward_edx = state.edx;
+      grub_relocator32_backward_eip = state.eip;
+      grub_relocator32_backward_esp = state.esp;
+
+      grub_memmove (relocator - overhead,
+		    &grub_relocator32_backward_start,
+		    RELOCATOR_SIZEOF (backward));
+      entry = (void (*)()) (relocator - overhead);
+    }
+  else
+    {
+      int overhead;
+
+      overhead = ALIGN_UP (dest + size, RELOCATOR_ALIGN)
+	+ RELOCATOR_SIZEOF (forward) - (dest + size);
+
+      grub_relocator32_forward_dest = dest;
+      grub_relocator32_forward_src = PTR_TO_UINT64 (relocator);
+      grub_relocator32_forward_size = size + overhead;
+
+      grub_relocator32_forward_eax = state.eax;
+      grub_relocator32_forward_ebx = state.ebx;
+      grub_relocator32_forward_ecx = state.ecx;
+      grub_relocator32_forward_edx = state.edx;
+      grub_relocator32_forward_eip = state.eip;
+      grub_relocator32_forward_esp = state.esp;
+
+      grub_memmove (relocator + size + overhead - RELOCATOR_SIZEOF (forward),
+		    &grub_relocator32_forward_start,
+		    RELOCATOR_SIZEOF (forward));
+      entry =
+	(void (*)()) (relocator + size + overhead -
+		      RELOCATOR_SIZEOF (forward));
+    }
+  entry ();
+
+  /* Not reached.  */
+  return GRUB_ERR_NONE;
+}
diff --git a/lib/i386/relocator_asm.S b/lib/i386/relocator_asm.S
new file mode 100644
index 0000000..d022a68
--- /dev/null
+++ b/lib/i386/relocator_asm.S
@@ -0,0 +1,273 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  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>
+
+#ifdef BACKWARD
+#define RELOCATOR_VARIABLE(x) VARIABLE(grub_relocator32_backward_ ## x)
+#else
+#define RELOCATOR_VARIABLE(x) VARIABLE(grub_relocator32_forward_ ## x)
+#endif
+	.p2align	4	/* force 16-byte alignment */
+
+RELOCATOR_VARIABLE(start)
+#ifdef BACKWARD
+base:
+#endif
+	cli
+
+#ifndef __x86_64__
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE(dest)
+	.long 0
+	mov %eax, %edi
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE(src)
+	.long 0
+	mov %eax, %esi
+
+	/* mov imm32, %ecx */
+	.byte	0xb9
+RELOCATOR_VARIABLE(size)
+	.long 0
+	mov %edi, %eax
+#ifndef BACKWARD
+	add %ecx, %eax
+#endif
+	add $0x3, %ecx
+	shr $2, %ecx
+
+#ifdef BACKWARD
+	/* Backward movsl is implicitly off-by-four.  compensate that.  */
+	subl	$4,	%esi
+	subl	$4,	%edi
+
+	/* Backward copy.  */
+	std
+	addl	%ecx, %esi
+	addl	%ecx, %edi
+
+	rep
+	movsl
+
+#else
+	/* Forward copy.  */
+	cld
+	rep
+	movsl
+#endif
+	/* %eax contains now our new base.  */
+	mov %eax, %esi
+	add $(cont0 - base), %eax
+	jmp *%eax
+cont0:
+#else
+	xorq %rax, %rax
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE(dest)
+	.long 0
+	mov %eax, %edi
+
+	/* mov imm64, %rax */
+	.byte	0x48
+	.byte	0xb8
+RELOCATOR_VARIABLE(src)
+	.long 0, 0
+	mov %rax, %rsi
+
+	xorq %rcx, %rcx
+	/* mov imm32, %ecx */
+	.byte	0xb9
+RELOCATOR_VARIABLE(size)
+	.long 0
+
+	mov %rdi, %rax
+#ifndef BACKWARD
+	add %rcx, %rax
+#endif
+	add $0x3, %rcx
+	shr $2, %rcx
+
+#ifdef BACKWARD
+	/* Backward movsl is implicitly off-by-four.  compensate that.  */
+	subq	$4,	%rsi
+	subq	$4,	%rdi
+
+	/* Backward copy.  */
+	std
+	addq	%rcx, %rsi
+	addq	%rcx, %rdi
+
+	rep
+	movsl
+
+#else
+	/* Forward copy.  */
+	cld
+	rep
+	movsl
+#endif
+
+	/* %rax contains now our new 'base'.  */
+	mov %rax, %rsi
+#ifdef APPLE_CC
+	add $(cont0 - base), %eax
+#else
+	add $(cont0 - base), %rax
+#endif
+	jmp *%rax
+cont0:
+#ifdef APPLE_CC
+	lea (cont1 - base) (%esi, 1), %eax
+	mov %eax, (jump_vector - base) (%esi, 1)
+
+	lea (gdt - base) (%esi, 1), %eax
+	mov %eax, (gdt_addr - base) (%esi, 1)
+
+	/* Switch to compatibility mode. */
+
+	lgdt (gdtdesc - base) (%esi, 1)
+
+	/* Update %cs. Thanks to David Miller for pointing this mistake out. */
+	ljmp *(jump_vector - base) (%esi,1)
+#else
+	lea (cont1 - base) (%rsi, 1), %rax
+	mov %eax, (jump_vector - base) (%rsi, 1)
+
+	lea (gdt - base) (%rsi, 1), %rax
+	mov %rax, (gdt_addr - base) (%rsi, 1)
+
+	/* Switch to compatibility mode. */
+
+	lgdt (gdtdesc - base) (%rsi, 1)
+
+	/* Update %cs. Thanks to David Miller for pointing this mistake out. */
+	ljmp *(jump_vector - base) (%rsi, 1)
+#endif
+
+cont1:
+	.code32
+
+	/* Update other registers. */
+	mov $0x18, %eax
+	mov %eax, %ds
+	mov %eax, %es
+	mov %eax, %fs
+	mov %eax, %gs
+	mov %eax, %ss
+
+	/* Disable paging. */
+	mov %cr0, %eax
+	and $0x7fffffff, %eax
+	mov %eax, %cr0
+
+	/* Disable amd64. */
+	mov $0xc0000080, %ecx
+	rdmsr
+	and $0xfffffeff, %eax
+	wrmsr
+
+	/* Turn off PAE. */
+	movl %cr4, %eax
+	and $0xffffffcf, %eax
+	mov %eax, %cr4
+
+	jmp cont2
+cont2:
+#endif
+	.code32
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE (esp)
+	.long 0
+
+	movl %eax, %esp
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE (eax)
+	.long 0
+
+	/* mov imm32, %ebx */
+	.byte	0xbb
+RELOCATOR_VARIABLE (ebx)
+	.long 0
+
+	/* mov imm32, %ecx */
+	.byte	0xb9
+RELOCATOR_VARIABLE (ecx)
+	.long 0
+
+	/* mov imm32, %edx */
+	.byte	0xba
+RELOCATOR_VARIABLE (edx)
+	.long 0
+
+	/* Cleared direction flag is of no problem with any current
+	   payload and makes this implementation easier.  */
+	cld
+
+	.byte 0xea
+RELOCATOR_VARIABLE (eip)
+	.long 0
+#ifdef __x86_64__
+	.word 0x10
+#else
+	.word 0x08
+#endif
+
+#ifdef __x86_64__
+	/* GDT. Copied from loader/i386/linux.c. */
+	.p2align 4
+gdt:
+	/* NULL.  */
+	.byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+	/* Reserved.  */
+	.byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+	/* Code segment.  */
+	.byte 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x9A, 0xCF, 0x00
+
+	/* Data segment.  */
+	.byte 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x92, 0xCF, 0x00
+
+gdtdesc:
+	.word 31
+gdt_addr:
+	/* Filled by the code. */
+	.quad 0
+
+	.p2align 4
+jump_vector:
+	/* Jump location. Is filled by the code */
+	.long 0
+	.long 0x10
+#endif
+
+#ifndef BACKWARD
+base:
+#endif
+
+RELOCATOR_VARIABLE(end)
diff --git a/lib/i386/relocator_backward.S b/lib/i386/relocator_backward.S
new file mode 100644
index 0000000..0691347
--- /dev/null
+++ b/lib/i386/relocator_backward.S
@@ -0,0 +1,2 @@
+#define BACKWARD
+#include "relocator_asm.S"

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

* Re: [PATCH 1/2] Relocator framework
  2009-08-03 12:06 [PATCH 1/2] Relocator framework Vladimir 'phcoder' Serbinenko
@ 2009-08-03 12:39 ` Vladimir 'phcoder' Serbinenko
  2009-08-04 20:52 ` Robert Millan
  1 sibling, 0 replies; 9+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2009-08-03 12:39 UTC (permalink / raw)
  To: The development of GRUB 2

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

On Mon, Aug 3, 2009 at 2:06 PM, Vladimir 'phcoder'
Serbinenko<phcoder@gmail.com> wrote:
> Hello. As discussed on IRC it would be preferable if all loaders use
> flexible technics for loading kernels using relocators as currently
> multiboot and xnu does. Here is a relocator framework based on
> multiboot and xnu relocator. As an advantage it switches from x86-64
> to i386 if necessary. With it and some more patches I was able to load
> mbtest on qemu64 with tianocore
Small update: new function for increasing relocatable space
Can someone test it with situations requiring backward allocator? (dest > src)
A problem which is perhaps currently present is if abs(desc-src) <
RELOCATOR_SIZE then relocator may overwrite itself and fail. A
possible solution is to allocate a bit more space in relocatable
buffer to be able to change src by at most RELOCATOR_SIZE - this will
be enough to set dest=src in these situations and use a "null"
relocator (to be implemented)

>
> --
> Regards
> Vladimir 'phcoder' Serbinenko
>
> Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

[-- Attachment #2: relocator.diff --]
[-- Type: text/plain, Size: 13243 bytes --]

diff --git a/conf/i386.rmk b/conf/i386.rmk
index 93f84ce..33d49b5 100644
--- a/conf/i386.rmk
+++ b/conf/i386.rmk
@@ -14,3 +14,9 @@ pkglib_MODULES += vga_text.mod
 vga_text_mod_SOURCES = term/i386/pc/vga_text.c term/i386/vga_common.c
 vga_text_mod_CFLAGS = $(COMMON_CFLAGS)
 vga_text_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
+pkglib_MODULES += relocator.mod
+relocator_mod_SOURCES = lib/i386/relocator.c lib/i386/relocator_asm.S lib/i386/relocator_backward.S
+relocator_mod_CFLAGS = $(COMMON_CFLAGS)
+relocator_mod_ASFLAGS = $(COMMON_ASFLAGS)
+relocator_mod_LDFLAGS = $(COMMON_LDFLAGS)
diff --git a/include/grub/i386/relocator.h b/include/grub/i386/relocator.h
new file mode 100644
index 0000000..ef7fe23
--- /dev/null
+++ b/include/grub/i386/relocator.h
@@ -0,0 +1,41 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  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_RELOCATOR_CPU_HEADER
+#define GRUB_RELOCATOR_CPU_HEADER	1
+
+#include <grub/types.h>
+#include <grub/err.h>
+
+struct grub_relocator32_state
+{
+  grub_uint32_t esp;
+  grub_uint32_t eax;
+  grub_uint32_t ebx;
+  grub_uint32_t ecx;
+  grub_uint32_t edx;
+  grub_uint32_t eip;
+};
+
+void *grub_relocator32_alloc (grub_size_t size);
+grub_err_t grub_relocator32_boot (void *relocator, grub_uint32_t dest,
+				  struct grub_relocator32_state state);
+void *grub_relocator32_realloc (void *relocator, grub_size_t size);
+void grub_relocator32_free (void *relocator);
+
+#endif /* ! GRUB_RELOCATOR_CPU_HEADER */
diff --git a/lib/i386/relocator.c b/lib/i386/relocator.c
new file mode 100644
index 0000000..0205d8e
--- /dev/null
+++ b/lib/i386/relocator.c
@@ -0,0 +1,173 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  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/mm.h>
+#include <grub/misc.h>
+
+#include <grub/types.h>
+#include <grub/types.h>
+#include <grub/err.h>
+
+#include <grub/i386/relocator.h>
+
+extern grub_uint8_t grub_relocator32_forward_start;
+extern grub_uint8_t grub_relocator32_forward_end;
+extern grub_uint8_t grub_relocator32_backward_start;
+extern grub_uint8_t grub_relocator32_backward_end;
+
+extern grub_uint32_t grub_relocator32_backward_dest;
+extern grub_uint32_t grub_relocator32_backward_size;
+#ifdef __x86_64__
+extern grub_uint64_t grub_relocator32_backward_src;
+#else
+extern grub_uint32_t grub_relocator32_backward_src;
+#endif
+
+extern grub_uint32_t grub_relocator32_forward_dest;
+extern grub_uint32_t grub_relocator32_forward_size;
+#ifdef __x86_64__
+extern grub_uint64_t grub_relocator32_forward_src;
+#else
+extern grub_uint32_t grub_relocator32_forward_src;
+#endif
+
+extern grub_uint32_t grub_relocator32_forward_eax;
+extern grub_uint32_t grub_relocator32_forward_ebx;
+extern grub_uint32_t grub_relocator32_forward_ecx;
+extern grub_uint32_t grub_relocator32_forward_edx;
+extern grub_uint32_t grub_relocator32_forward_eip;
+extern grub_uint32_t grub_relocator32_forward_esp;
+
+extern grub_uint32_t grub_relocator32_backward_eax;
+extern grub_uint32_t grub_relocator32_backward_ebx;
+extern grub_uint32_t grub_relocator32_backward_ecx;
+extern grub_uint32_t grub_relocator32_backward_edx;
+extern grub_uint32_t grub_relocator32_backward_eip;
+extern grub_uint32_t grub_relocator32_backward_esp;
+
+#define RELOCATOR_SIZEOF(x)	(&grub_relocator32_##x##_end - &grub_relocator32_##x##_start)
+#define RELOCATOR_ALIGN 16
+
+void *
+grub_relocator32_alloc (grub_size_t size)
+{
+  char *playground;
+
+  playground = grub_malloc ((RELOCATOR_SIZEOF (forward) + RELOCATOR_ALIGN)
+			    + size
+			    + (RELOCATOR_SIZEOF (backward) +
+			       RELOCATOR_ALIGN));
+  if (!playground)
+    return 0;
+
+  *(grub_size_t *) playground = size;
+
+  return playground + RELOCATOR_SIZEOF (forward);
+}
+
+void *
+grub_relocator32_realloc (void *relocator, grub_size_t size)
+{
+  char *playground;
+
+  playground = (char *) relocator - RELOCATOR_SIZEOF (forward);
+
+  playground = grub_realloc (playground,
+			     (RELOCATOR_SIZEOF (forward) + RELOCATOR_ALIGN)
+			     + size
+			     + (RELOCATOR_SIZEOF (backward) + RELOCATOR_ALIGN));
+  if (!playground)
+    return 0;
+
+  *(grub_size_t *) playground = size;
+
+  return playground + RELOCATOR_SIZEOF (forward);
+}
+
+void
+grub_relocator32_free (void *relocator)
+{
+  if (relocator)
+    grub_free ((char *) relocator - RELOCATOR_SIZEOF (forward));
+}
+
+
+grub_err_t
+grub_relocator32_boot (void *relocator, grub_uint32_t dest,
+		       struct grub_relocator32_state state)
+{
+  grub_size_t size;
+  char *playground;
+  void (*entry) ();
+
+  playground = (char *) relocator - RELOCATOR_SIZEOF (forward);
+  size = *(grub_size_t *) playground;
+
+  if (UINT_TO_PTR (dest) >= relocator)
+    {
+      int overhead;
+
+      overhead =
+	ALIGN_UP (dest - RELOCATOR_SIZEOF (backward) - RELOCATOR_ALIGN,
+		  RELOCATOR_ALIGN);
+      grub_relocator32_backward_dest = dest - overhead;
+      grub_relocator32_backward_src = PTR_TO_UINT64 (relocator - overhead);
+      grub_relocator32_backward_size = size + overhead;
+
+      grub_relocator32_backward_eax = state.eax;
+      grub_relocator32_backward_ebx = state.ebx;
+      grub_relocator32_backward_ecx = state.ecx;
+      grub_relocator32_backward_edx = state.edx;
+      grub_relocator32_backward_eip = state.eip;
+      grub_relocator32_backward_esp = state.esp;
+
+      grub_memmove (relocator - overhead,
+		    &grub_relocator32_backward_start,
+		    RELOCATOR_SIZEOF (backward));
+      entry = (void (*)()) (relocator - overhead);
+    }
+  else
+    {
+      int overhead;
+
+      overhead = ALIGN_UP (dest + size, RELOCATOR_ALIGN)
+	+ RELOCATOR_SIZEOF (forward) - (dest + size);
+
+      grub_relocator32_forward_dest = dest;
+      grub_relocator32_forward_src = PTR_TO_UINT64 (relocator);
+      grub_relocator32_forward_size = size + overhead;
+
+      grub_relocator32_forward_eax = state.eax;
+      grub_relocator32_forward_ebx = state.ebx;
+      grub_relocator32_forward_ecx = state.ecx;
+      grub_relocator32_forward_edx = state.edx;
+      grub_relocator32_forward_eip = state.eip;
+      grub_relocator32_forward_esp = state.esp;
+
+      grub_memmove (relocator + size + overhead - RELOCATOR_SIZEOF (forward),
+		    &grub_relocator32_forward_start,
+		    RELOCATOR_SIZEOF (forward));
+      entry =
+	(void (*)()) (relocator + size + overhead -
+		      RELOCATOR_SIZEOF (forward));
+    }
+  entry ();
+
+  /* Not reached.  */
+  return GRUB_ERR_NONE;
+}
diff --git a/lib/i386/relocator_asm.S b/lib/i386/relocator_asm.S
new file mode 100644
index 0000000..d022a68
--- /dev/null
+++ b/lib/i386/relocator_asm.S
@@ -0,0 +1,273 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  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>
+
+#ifdef BACKWARD
+#define RELOCATOR_VARIABLE(x) VARIABLE(grub_relocator32_backward_ ## x)
+#else
+#define RELOCATOR_VARIABLE(x) VARIABLE(grub_relocator32_forward_ ## x)
+#endif
+	.p2align	4	/* force 16-byte alignment */
+
+RELOCATOR_VARIABLE(start)
+#ifdef BACKWARD
+base:
+#endif
+	cli
+
+#ifndef __x86_64__
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE(dest)
+	.long 0
+	mov %eax, %edi
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE(src)
+	.long 0
+	mov %eax, %esi
+
+	/* mov imm32, %ecx */
+	.byte	0xb9
+RELOCATOR_VARIABLE(size)
+	.long 0
+	mov %edi, %eax
+#ifndef BACKWARD
+	add %ecx, %eax
+#endif
+	add $0x3, %ecx
+	shr $2, %ecx
+
+#ifdef BACKWARD
+	/* Backward movsl is implicitly off-by-four.  compensate that.  */
+	subl	$4,	%esi
+	subl	$4,	%edi
+
+	/* Backward copy.  */
+	std
+	addl	%ecx, %esi
+	addl	%ecx, %edi
+
+	rep
+	movsl
+
+#else
+	/* Forward copy.  */
+	cld
+	rep
+	movsl
+#endif
+	/* %eax contains now our new base.  */
+	mov %eax, %esi
+	add $(cont0 - base), %eax
+	jmp *%eax
+cont0:
+#else
+	xorq %rax, %rax
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE(dest)
+	.long 0
+	mov %eax, %edi
+
+	/* mov imm64, %rax */
+	.byte	0x48
+	.byte	0xb8
+RELOCATOR_VARIABLE(src)
+	.long 0, 0
+	mov %rax, %rsi
+
+	xorq %rcx, %rcx
+	/* mov imm32, %ecx */
+	.byte	0xb9
+RELOCATOR_VARIABLE(size)
+	.long 0
+
+	mov %rdi, %rax
+#ifndef BACKWARD
+	add %rcx, %rax
+#endif
+	add $0x3, %rcx
+	shr $2, %rcx
+
+#ifdef BACKWARD
+	/* Backward movsl is implicitly off-by-four.  compensate that.  */
+	subq	$4,	%rsi
+	subq	$4,	%rdi
+
+	/* Backward copy.  */
+	std
+	addq	%rcx, %rsi
+	addq	%rcx, %rdi
+
+	rep
+	movsl
+
+#else
+	/* Forward copy.  */
+	cld
+	rep
+	movsl
+#endif
+
+	/* %rax contains now our new 'base'.  */
+	mov %rax, %rsi
+#ifdef APPLE_CC
+	add $(cont0 - base), %eax
+#else
+	add $(cont0 - base), %rax
+#endif
+	jmp *%rax
+cont0:
+#ifdef APPLE_CC
+	lea (cont1 - base) (%esi, 1), %eax
+	mov %eax, (jump_vector - base) (%esi, 1)
+
+	lea (gdt - base) (%esi, 1), %eax
+	mov %eax, (gdt_addr - base) (%esi, 1)
+
+	/* Switch to compatibility mode. */
+
+	lgdt (gdtdesc - base) (%esi, 1)
+
+	/* Update %cs. Thanks to David Miller for pointing this mistake out. */
+	ljmp *(jump_vector - base) (%esi,1)
+#else
+	lea (cont1 - base) (%rsi, 1), %rax
+	mov %eax, (jump_vector - base) (%rsi, 1)
+
+	lea (gdt - base) (%rsi, 1), %rax
+	mov %rax, (gdt_addr - base) (%rsi, 1)
+
+	/* Switch to compatibility mode. */
+
+	lgdt (gdtdesc - base) (%rsi, 1)
+
+	/* Update %cs. Thanks to David Miller for pointing this mistake out. */
+	ljmp *(jump_vector - base) (%rsi, 1)
+#endif
+
+cont1:
+	.code32
+
+	/* Update other registers. */
+	mov $0x18, %eax
+	mov %eax, %ds
+	mov %eax, %es
+	mov %eax, %fs
+	mov %eax, %gs
+	mov %eax, %ss
+
+	/* Disable paging. */
+	mov %cr0, %eax
+	and $0x7fffffff, %eax
+	mov %eax, %cr0
+
+	/* Disable amd64. */
+	mov $0xc0000080, %ecx
+	rdmsr
+	and $0xfffffeff, %eax
+	wrmsr
+
+	/* Turn off PAE. */
+	movl %cr4, %eax
+	and $0xffffffcf, %eax
+	mov %eax, %cr4
+
+	jmp cont2
+cont2:
+#endif
+	.code32
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE (esp)
+	.long 0
+
+	movl %eax, %esp
+
+	/* mov imm32, %eax */
+	.byte	0xb8
+RELOCATOR_VARIABLE (eax)
+	.long 0
+
+	/* mov imm32, %ebx */
+	.byte	0xbb
+RELOCATOR_VARIABLE (ebx)
+	.long 0
+
+	/* mov imm32, %ecx */
+	.byte	0xb9
+RELOCATOR_VARIABLE (ecx)
+	.long 0
+
+	/* mov imm32, %edx */
+	.byte	0xba
+RELOCATOR_VARIABLE (edx)
+	.long 0
+
+	/* Cleared direction flag is of no problem with any current
+	   payload and makes this implementation easier.  */
+	cld
+
+	.byte 0xea
+RELOCATOR_VARIABLE (eip)
+	.long 0
+#ifdef __x86_64__
+	.word 0x10
+#else
+	.word 0x08
+#endif
+
+#ifdef __x86_64__
+	/* GDT. Copied from loader/i386/linux.c. */
+	.p2align 4
+gdt:
+	/* NULL.  */
+	.byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+	/* Reserved.  */
+	.byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+	/* Code segment.  */
+	.byte 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x9A, 0xCF, 0x00
+
+	/* Data segment.  */
+	.byte 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x92, 0xCF, 0x00
+
+gdtdesc:
+	.word 31
+gdt_addr:
+	/* Filled by the code. */
+	.quad 0
+
+	.p2align 4
+jump_vector:
+	/* Jump location. Is filled by the code */
+	.long 0
+	.long 0x10
+#endif
+
+#ifndef BACKWARD
+base:
+#endif
+
+RELOCATOR_VARIABLE(end)
diff --git a/lib/i386/relocator_backward.S b/lib/i386/relocator_backward.S
new file mode 100644
index 0000000..0691347
--- /dev/null
+++ b/lib/i386/relocator_backward.S
@@ -0,0 +1,2 @@
+#define BACKWARD
+#include "relocator_asm.S"

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

* Re: [PATCH 1/2] Relocator framework
  2009-08-03 12:06 [PATCH 1/2] Relocator framework Vladimir 'phcoder' Serbinenko
  2009-08-03 12:39 ` Vladimir 'phcoder' Serbinenko
@ 2009-08-04 20:52 ` Robert Millan
  2009-08-05 10:18   ` Vladimir 'phcoder' Serbinenko
  1 sibling, 1 reply; 9+ messages in thread
From: Robert Millan @ 2009-08-04 20:52 UTC (permalink / raw)
  To: The development of GRUB 2

On Mon, Aug 03, 2009 at 02:06:03PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Hello. As discussed on IRC it would be preferable if all loaders use
> flexible technics for loading kernels using relocators as currently
> multiboot and xnu does.

Well, as for what I said on IRC, I'm not sure if it'd be preferable.  I
think it *could* be, if it makes things simpler & more maintainable (which
greatly depends on how the patches for multiboot.c/linux.c look like).

Anyway, I'll comment some things on this patch:

> +#ifdef __x86_64__
> +extern grub_uint64_t grub_relocator32_backward_src;
> +#else
> +extern grub_uint32_t grub_relocator32_backward_src;
> +#endif

You could make this a pointer, or grub_uintptr_t
(the latter we don't yet have, it seems like a good excuse to
add it if a pointer is not suitable :-)).

> +#ifndef __x86_64__
> +	/* mov imm32, %eax */
> +	.byte	0xb8
> +RELOCATOR_VARIABLE(dest)
> +	.long 0
> +	mov %eax, %edi

Please use size qualifiers (e.g. movl).

Also, remember to indent the first parameter like we do elsewhere
(e.g. ".long\t0", "movl\t%eax, %edi", etc).

> +	/* Backward movsl is implicitly off-by-four.  compensate that.  */
> +	subl	$4,	%esi
> +	subl	$4,	%edi
> +
> +	/* Backward copy.  */
> +	std
> +	addl	%ecx, %esi
> +	addl	%ecx, %edi
> +
> +	rep
> +	movsl

Are you sure moving to movsl is a good idea?  Maybe the payload size is not
4-aligned.

> +#ifdef APPLE_CC
> +	add $(cont0 - base), %eax
> +#else
> +	add $(cont0 - base), %rax
> +#endif

What's the issue at hand?  Apple assembler [1] can't add an inmediate to
%rax ?

Truncating it seems like it could be problematic if %eax overflows.

[1] btw, APPLE_CC macro for assembler checks is really confusing

> +	/* Update %cs. Thanks to David Miller for pointing this mistake out. */
> +	ljmp *(jump_vector - base) (%esi,1)

Comments ought to be for useful information.  For praise/acknoledgements (and I
think David deserves it) we have the THANKS file.

> +	/* Disable paging. */
> +	mov %cr0, %eax
> +	and $0x7fffffff, %eax
> +	mov %eax, %cr0
> +
> +	/* Disable amd64. */
> +	mov $0xc0000080, %ecx
> +	rdmsr
> +	and $0xfffffeff, %eax
> +	wrmsr
> +
> +	/* Turn off PAE. */
> +	movl %cr4, %eax
> +	and $0xffffffcf, %eax
> +	mov %eax, %cr4

Please use macros for such things (see GRUB_MEMORY_MACHINE_CR0_PE_ON).

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."



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

* Re: [PATCH 1/2] Relocator framework
  2009-08-04 20:52 ` Robert Millan
@ 2009-08-05 10:18   ` Vladimir 'phcoder' Serbinenko
  2009-08-07 11:06     ` Robert Millan
  0 siblings, 1 reply; 9+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2009-08-05 10:18 UTC (permalink / raw)
  To: The development of GRUB 2

>> +#ifdef __x86_64__
>> +extern grub_uint64_t grub_relocator32_backward_src;
>> +#else
>> +extern grub_uint32_t grub_relocator32_backward_src;
>> +#endif
>
> You could make this a pointer, or grub_uintptr_t
> (the latter we don't yet have, it seems like a good excuse to
> add it if a pointer is not suitable :-)).
grub_addr_t would actually do the job.
>
>> +#ifndef __x86_64__
>> +     /* mov imm32, %eax */
>> +     .byte   0xb8
>> +RELOCATOR_VARIABLE(dest)
>> +     .long 0
>> +     mov %eax, %edi
>
> Please use size qualifiers (e.g. movl).
>
> Also, remember to indent the first parameter like we do elsewhere
> (e.g. ".long\t0", "movl\t%eax, %edi", etc).
>
Ok
>> +     /* Backward movsl is implicitly off-by-four.  compensate that.  */
>> +     subl    $4,     %esi
>> +     subl    $4,     %edi
>> +
>> +     /* Backward copy.  */
>> +     std
>> +     addl    %ecx, %esi
>> +     addl    %ecx, %edi
>> +
>> +     rep
>> +     movsl
>
> Are you sure moving to movsl is a good idea?  Maybe the payload size is not
> 4-aligned.
>
On AFAIK x86 movsl works on unaligned addresses too. I'll recheck
>> +#ifdef APPLE_CC
>> +     add $(cont0 - base), %eax
>> +#else
>> +     add $(cont0 - base), %rax
>> +#endif
>
> What's the issue at hand?  Apple assembler [1] can't add an inmediate to
> %rax ?
>
Apple linker can't handle 64-bit differences
> Truncating it seems like it could be problematic if %eax overflows.
>
Actually it's not a problem since this code is put together with
kernel which executes in 32-bit mode so it can't be over 4GiB. If we
base 64-bit relocator it will be a problem if OS wants to go over
4GiB.
> [1] btw, APPLE_CC macro for assembler checks is really confusing
>
I'll try with __apple__
> Please use macros for such things (see GRUB_MEMORY_MACHINE_CR0_PE_ON).
Will look into it
>
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git



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

* Re: [PATCH 1/2] Relocator framework
  2009-08-05 10:18   ` Vladimir 'phcoder' Serbinenko
@ 2009-08-07 11:06     ` Robert Millan
  2009-08-07 11:16       ` Marco Gerards
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Millan @ 2009-08-07 11:06 UTC (permalink / raw)
  To: The development of GRUB 2

On Wed, Aug 05, 2009 at 12:18:17PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> >> +#ifdef __x86_64__
> >> +extern grub_uint64_t grub_relocator32_backward_src;
> >> +#else
> >> +extern grub_uint32_t grub_relocator32_backward_src;
> >> +#endif
> >
> > You could make this a pointer, or grub_uintptr_t
> > (the latter we don't yet have, it seems like a good excuse to
> > add it if a pointer is not suitable :-)).
> grub_addr_t would actually do the job.

Ah yes, of course.

> > Are you sure moving to movsl is a good idea?  Maybe the payload size is not
> > 4-aligned.
> >
> On AFAIK x86 movsl works on unaligned addresses too. I'll recheck

But maybe there's some corner case in which we would overwrite something if
we copy 3 bytes more than necessary?  I didn't check if this would be possible.

> >> +#ifdef APPLE_CC
> >> +     add $(cont0 - base), %eax
> >> +#else
> >> +     add $(cont0 - base), %rax
> >> +#endif
> >
> > What's the issue at hand?  Apple assembler [1] can't add an inmediate to
> > %rax ?
> >
> Apple linker can't handle 64-bit differences

This seems like a bug.  Can GNU binutils be used on MacOS to resolve this?

If it's workable, I'd rather make binutils a build requirement than adding
more of this (the other APPLE_CC ifdefs will probably need some review too,
but there's no hurry about that).

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."



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

* Re: [PATCH 1/2] Relocator framework
  2009-08-07 11:06     ` Robert Millan
@ 2009-08-07 11:16       ` Marco Gerards
  2009-08-07 12:01         ` Vladimir 'phcoder' Serbinenko
  0 siblings, 1 reply; 9+ messages in thread
From: Marco Gerards @ 2009-08-07 11:16 UTC (permalink / raw)
  To: The development of GRUB 2

Robert Millan <rmh@aybabtu.com> writes:

[...]

>> Apple linker can't handle 64-bit differences
>
> This seems like a bug.  Can GNU binutils be used on MacOS to resolve this?
>
> If it's workable, I'd rather make binutils a build requirement than adding
> more of this (the other APPLE_CC ifdefs will probably need some review too,
> but there's no hurry about that).

Agreed.  GRUB is a GNU project and we can depend on GNU programs.
Especially since we use some GNU extensions.

--
Marco




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

* Re: [PATCH 1/2] Relocator framework
  2009-08-07 11:16       ` Marco Gerards
@ 2009-08-07 12:01         ` Vladimir 'phcoder' Serbinenko
  2009-08-08  3:52           ` Pavel Roskin
  0 siblings, 1 reply; 9+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2009-08-07 12:01 UTC (permalink / raw)
  To: The development of GRUB 2

On Fri, Aug 7, 2009 at 1:16 PM, Marco Gerards<mgerards@xs4all.nl> wrote:
> Robert Millan <rmh@aybabtu.com> writes:
>
> [...]
>
>>> Apple linker can't handle 64-bit differences
>>
>> This seems like a bug.  Can GNU binutils be used on MacOS to resolve this?
>>
>> If it's workable, I'd rather make binutils a build requirement than adding
>> more of this (the other APPLE_CC ifdefs will probably need some review too,
>> but there's no hurry about that).
>
> Agreed.  GRUB is a GNU project and we can depend on GNU programs.
> Especially since we use some GNU extensions.
>
Apple's compiler is based GCC but binutils aren't and they pose the
most of problems. Actualy the most problematic bit was that I didn't
know that unless you prefix variable with L_ apple's assembler treats
it as global. Adding L_ in this code solves problem. Yves proposed to
add a LOCAL(x) macro to symbol.h this way the assembler code is also
more readable. I also did further adjustments in this patch to
decrease the number of ifdefs but had no time to test it and so
couldn't submit it
> --
> Marco
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git



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

* Re: [PATCH 1/2] Relocator framework
  2009-08-07 12:01         ` Vladimir 'phcoder' Serbinenko
@ 2009-08-08  3:52           ` Pavel Roskin
  2009-08-08 13:11             ` Vladimir 'phcoder' Serbinenko
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Roskin @ 2009-08-08  3:52 UTC (permalink / raw)
  To: The development of GRUB 2

On Fri, 2009-08-07 at 14:01 +0200, Vladimir 'phcoder' Serbinenko wrote:

> Apple's compiler is based GCC but binutils aren't and they pose the
> most of problems. Actualy the most problematic bit was that I didn't
> know that unless you prefix variable with L_ apple's assembler treats
> it as global. Adding L_ in this code solves problem. Yves proposed to
> add a LOCAL(x) macro to symbol.h this way the assembler code is also
> more readable. I also did further adjustments in this patch to
> decrease the number of ifdefs but had no time to test it and so
> couldn't submit it

I would prefer if you don't commit any patches that introduce any more
preprocessor conditionals for the Apple compiler or assembler.

It would make it hard to debug problems reported by others, as different
compilers would produce different binaries.

If we are using some feature that is just nice but not needed (such as
binary & in lnxboot.S to set the image size to 1024), it would be better
to avoid using that feature for all compilers rather than for the
compilers that don't support it.

-- 
Regards,
Pavel Roskin



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

* Re: [PATCH 1/2] Relocator framework
  2009-08-08  3:52           ` Pavel Roskin
@ 2009-08-08 13:11             ` Vladimir 'phcoder' Serbinenko
  0 siblings, 0 replies; 9+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2009-08-08 13:11 UTC (permalink / raw)
  To: The development of GRUB 2

On Sat, Aug 8, 2009 at 5:52 AM, Pavel Roskin<proski@gnu.org> wrote:
> On Fri, 2009-08-07 at 14:01 +0200, Vladimir 'phcoder' Serbinenko wrote:
>
>> Apple's compiler is based GCC but binutils aren't and they pose the
>> most of problems. Actualy the most problematic bit was that I didn't
>> know that unless you prefix variable with L_ apple's assembler treats
>> it as global. Adding L_ in this code solves problem. Yves proposed to
>> add a LOCAL(x) macro to symbol.h this way the assembler code is also
>> more readable. I also did further adjustments in this patch to
>> decrease the number of ifdefs but had no time to test it and so
>> couldn't submit it
>
> I would prefer if you don't commit any patches that introduce any more
> preprocessor conditionals for the Apple compiler or assembler.
LOCAL(x) can always be expanded to L_## x but it will make code more
readable and modifiable in future. And it will allow a bunch of other
ifdef's go out.
>
> It would make it hard to debug problems reported by others, as different
> compilers would produce different binaries.
>
Even just changing compiler options you produce different binary
> If we are using some feature that is just nice but not needed (such as
> binary & in lnxboot.S to set the image size to 1024), it would be better
> to avoid using that feature for all compilers rather than for the
> compilers that don't support it.
>
lnxboot.S with Apple's toolchain is disabled temporarily awaiting
someone to find a definitive solution.
Why do our 2 new maintainers propose to shave off useful features?
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git



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

end of thread, other threads:[~2009-08-08 13:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-03 12:06 [PATCH 1/2] Relocator framework Vladimir 'phcoder' Serbinenko
2009-08-03 12:39 ` Vladimir 'phcoder' Serbinenko
2009-08-04 20:52 ` Robert Millan
2009-08-05 10:18   ` Vladimir 'phcoder' Serbinenko
2009-08-07 11:06     ` Robert Millan
2009-08-07 11:16       ` Marco Gerards
2009-08-07 12:01         ` Vladimir 'phcoder' Serbinenko
2009-08-08  3:52           ` Pavel Roskin
2009-08-08 13:11             ` Vladimir 'phcoder' Serbinenko

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.