[PATCH] split realmode and loader routines out of startup.S

[PATCH] split realmode and loader routines out of startup.S

[Robert Millan]

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


This patch splits realmode and loader routines out of startup.S.  The idea
is that the LinuxBIOS port can be adapted to share more code with the rest
of GRUB instead of duplicating it.

This is quite critical stuff, so even if the change seems trivial I'd suggest
being careful, since I don't trust myself too much.  Of course, I've tested
that it can still boot Linux and Multiboot (on qemu only).  Perhaps testing
on real hardware would be appropiate (but I don't have this handy atm).

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: split.diff --]
[-- Type: text/x-diff, Size: 17777 bytes --]

2007-10-16  Robert Millan  <rmh@aybabtu.com>

	* kern/i386/loader.S: New file.

	* kern/i386/pc/startup.S (grub_linux_prot_size): Moved to ...
	* kern/i386/loader.S (grub_linux_prot_size): ... here.
	* kern/i386/pc/startup.S (grub_linux_tmp_addr): Moved to ...
	* kern/i386/loader.S (grub_linux_tmp_addr): ... here.
	* kern/i386/pc/startup.S (grub_linux_real_addr): Moved to ...
	* kern/i386/loader.S (grub_linux_real_addr): ... here.
	* kern/i386/pc/startup.S (grub_linux_boot_zimage): Moved to ...
	* kern/i386/loader.S (grub_linux_boot_zimage): ... here.
	* kern/i386/pc/startup.S (grub_linux_boot_bzimage): Moved to ...
	* kern/i386/loader.S (grub_linux_boot_bzimage): ... here.
	* kern/i386/pc/startup.S (grub_multiboot_real_boot): Moved to ...
	* kern/i386/loader.S (grub_multiboot_real_boot): ... here.
	* kern/i386/pc/startup.S (grub_multiboot2_real_boot): Moved to ...
	* kern/i386/loader.S (grub_multiboot2_real_boot): ... here.

	* kern/i386/realmode.S: New file.

	* kern/i386/pc/startup.S (protstack): Moved to ...
	* kern/i386/realmode.S (protstack): ... here.
	* kern/i386/pc/startup.S (gdt): Moved to ...
	* kern/i386/realmode.S (gdt): ... here.
	* kern/i386/pc/startup.S (prot_to_real): Moved to ...
	* kern/i386/realmode.S (prot_to_real): ... here.

	* kern/i386/pc/startup.S: Include `kern/i386/loader.S' and
	`kern/i386/realmode.S'.

diff -Nurp grub2/kern/i386/loader.S grub2.split/kern/i386/loader.S
--- grub2/kern/i386/loader.S	1970-01-01 01:00:00.000000000 +0100
+++ grub2.split/kern/i386/loader.S	2007-10-16 22:44:21.000000000 +0200
@@ -0,0 +1,157 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 1999,2000,2001,2002,2003,2005,2006,2007 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/>.
+ */
+
+
+/*
+ * Note: These functions defined in this file may be called from C.
+ *       Be careful of that you must not modify some registers. Quote
+ *       from gcc-2.95.2/gcc/config/i386/i386.h:
+	
+   1 for registers not available across function calls.
+   These must include the FIXED_REGISTERS and also any
+   registers that can be used without being saved.
+   The latter must include the registers where values are returned
+   and the register where structure-value addresses are passed.
+   Aside from that, you can include as many other registers as you like.
+
+  ax,dx,cx,bx,si,di,bp,sp,st,st1,st2,st3,st4,st5,st6,st7,arg
+{  1, 1, 1, 0, 0, 0, 0, 1, 1,  1,  1,  1,  1,  1,  1,  1,  1 }
+ */
+
+/*
+ * Note: GRUB is compiled with the options -mrtd and -mregparm=3.
+ *       So the first three arguments are passed in %eax, %edx, and %ecx,
+ *       respectively, and if a function has a fixed number of arguments
+ *       and the number if greater than three, the function must return
+ *       with "ret $N" where N is ((the number of arguments) - 3) * 4.
+ */
+
+/*
+ *  This is the area for all of the special variables.
+ */
+
+	.p2align	2	/* force 4-byte alignment */
+	
+/*
+ * void grub_linux_boot_zimage (void)
+ */
+VARIABLE(grub_linux_prot_size)
+	.long	0
+VARIABLE(grub_linux_tmp_addr)
+	.long	0
+VARIABLE(grub_linux_real_addr)
+	.long	0
+	
+FUNCTION(grub_linux_boot_zimage)
+	/* copy the kernel */
+	movl	EXT_C(grub_linux_prot_size), %ecx
+	addl	$3, %ecx
+	shrl	$2, %ecx
+	movl	$GRUB_LINUX_BZIMAGE_ADDR, %esi
+	movl	$GRUB_LINUX_ZIMAGE_ADDR, %edi
+	cld
+	rep
+	movsl
+
+FUNCTION(grub_linux_boot_bzimage)
+	call	EXT_C(grub_dl_unload_all)
+	
+	movl	EXT_C(grub_linux_real_addr), %ebx
+
+	/* copy the real mode code */
+	movl	EXT_C(grub_linux_tmp_addr), %esi
+	movl	%ebx, %edi
+	movl	$GRUB_LINUX_SETUP_MOVE_SIZE, %ecx
+	cld
+	rep
+	movsb
+
+	/* change %ebx to the segment address */
+	shrl	$4, %ebx
+	movl	%ebx, %eax
+	addl	$0x20, %eax
+	movw	%ax, linux_setup_seg
+
+	/* XXX new stack pointer in safe area for calling functions */
+	movl	$0x4000, %esp
+	call	EXT_C(grub_stop_floppy)
+
+	/* final setup for linux boot */
+	call	prot_to_real
+	.code16
+
+	cli
+	movw	%bx, %ss
+	movw	$GRUB_LINUX_SETUP_STACK, %sp
+
+	movw	%bx, %ds
+	movw	%bx, %es
+	movw	%bx, %fs
+	movw	%bx, %gs
+
+	/* ljmp */
+	.byte	0xea
+	.word	0
+linux_setup_seg:
+	.word	0
+	.code32
+
+		
+/*
+ * This starts the multiboot kernel.
+ */
+
+FUNCTION(grub_multiboot_real_boot)
+	/* Push the entry address on the stack.  */
+	pushl	%eax
+	/* Move the address of the multiboot information structure to ebx.  */
+	movl	%edx,%ebx
+	
+	/* Unload all modules and stop the floppy driver.  */
+	call	EXT_C(grub_dl_unload_all)
+	call	EXT_C(grub_stop_floppy)
+
+	/* Interrupts should be disabled.  */
+	cli
+	
+	/* Move the magic value into eax and jump to the kernel.  */
+	movl	$MULTIBOOT_MAGIC2,%eax
+	popl	%ecx
+	jmp 	*%ecx
+
+/*
+ * This starts the multiboot 2 kernel.
+ */
+
+FUNCTION(grub_multiboot2_real_boot)
+        /* Push the entry address on the stack.  */
+        pushl   %eax
+        /* Move the address of the multiboot information structure to ebx.  */
+        movl    %edx,%ebx
+
+        /* Unload all modules and stop the floppy driver.  */
+        call    EXT_C(grub_dl_unload_all)
+        call    EXT_C(grub_stop_floppy)
+
+        /* Interrupts should be disabled.  */
+        cli
+
+        /* Move the magic value into eax and jump to the kernel.  */
+        movl    $MULTIBOOT2_BOOTLOADER_MAGIC,%eax 
+        popl    %ecx
+        jmp     *%ecx
diff -Nurp grub2/kern/i386/pc/startup.S grub2.split/kern/i386/pc/startup.S
--- grub2/kern/i386/pc/startup.S	2007-07-25 21:29:24.000000000 +0200
+++ grub2.split/kern/i386/pc/startup.S	2007-10-16 22:46:25.000000000 +0200
@@ -242,6 +242,7 @@ codestart:
 	 */
 	call EXT_C(grub_main)
 
+#include "kern/i386/realmode.S"
 
 /*
  *  This is the area for all of the special variables.
@@ -249,9 +250,6 @@ codestart:
 
 	.p2align	2	/* force 4-byte alignment */
 
-protstack:
-	.long	GRUB_MEMORY_MACHINE_PROT_STACK
-
 VARIABLE(grub_boot_drive)
 	.long	0
 
@@ -271,65 +269,7 @@ VARIABLE(grub_apm_bios_info)
 	.word	0	/* cseg_16_len */
 	.word	0	/* dseg_16_len */
 	
-/*
- * This is the Global Descriptor Table
- *
- *  An entry, a "Segment Descriptor", looks like this:
- *
- * 31          24         19   16                 7           0
- * ------------------------------------------------------------
- * |             | |B| |A|       | |   |1|0|E|W|A|            |
- * | BASE 31..24 |G|/|L|V| LIMIT |P|DPL|  TYPE   | BASE 23:16 |  4
- * |             | |D| |L| 19..16| |   |1|1|C|R|A|            |
- * ------------------------------------------------------------
- * |                             |                            |
- * |        BASE 15..0           |       LIMIT 15..0          |  0
- * |                             |                            |
- * ------------------------------------------------------------
- *
- *  Note the ordering of the data items is reversed from the above
- *  description.
- */
-
 	.p2align	2	/* force 4-byte alignment */
-gdt:
-	.word	0, 0
-	.byte	0, 0, 0, 0
-
-	/* -- code segment --
-	 * base = 0x00000000, limit = 0xFFFFF (4 KiB Granularity), present
-	 * type = 32bit code execute/read, DPL = 0
-	 */
-	.word	0xFFFF, 0
-	.byte	0, 0x9A, 0xCF, 0
-
-	/* -- data segment --
-	 * base = 0x00000000, limit 0xFFFFF (4 KiB Granularity), present
-	 * type = 32 bit data read/write, DPL = 0 
-	 */
-	.word	0xFFFF, 0
-	.byte	0, 0x92, 0xCF, 0
-
-	/* -- 16 bit real mode CS --
-	 * base = 0x00000000, limit 0x0FFFF (1 B Granularity), present
-	 * type = 16 bit code execute/read only/conforming, DPL = 0
-	 */
-	.word	0xFFFF, 0
-	.byte	0, 0x9E, 0, 0
-
-	/* -- 16 bit real mode DS --
-	 * base = 0x00000000, limit 0x0FFFF (1 B Granularity), present
-	 * type = 16 bit data read/write, DPL = 0
-	 */
-	.word	0xFFFF, 0
-	.byte	0, 0x92, 0, 0
-
-
-/* this is the GDT descriptor */
-gdtdesc:
-	.word	0x27			/* limit */
-	.long	gdt			/* addr */
-
 	
 /*
  *  These next two routines, "real_to_prot" and "prot_to_real" are structured
@@ -382,69 +322,6 @@ protcseg:
 	/* return on the old (or initialized) stack! */
 	ret
 
-
-prot_to_real:
-	/* just in case, set GDT */
-	lgdt	gdtdesc
-
-	/* save the protected mode stack */
-	movl	%esp, %eax
-	movl	%eax, protstack
-
-	/* get the return address */
-	movl	(%esp), %eax
-	movl	%eax, GRUB_MEMORY_MACHINE_REAL_STACK
-
-	/* set up new stack */
-	movl	$GRUB_MEMORY_MACHINE_REAL_STACK, %eax
-	movl	%eax, %esp
-	movl	%eax, %ebp
-
-	/* set up segment limits */
-	movw	$GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG, %ax
-	movw	%ax, %ds
-	movw	%ax, %es
-	movw	%ax, %fs
-	movw	%ax, %gs
-	movw	%ax, %ss
-
-	/* this might be an extra step */
-	/* jump to a 16 bit segment */
-	ljmp	$GRUB_MEMORY_MACHINE_PSEUDO_REAL_CSEG, $tmpcseg
-
-tmpcseg:
-	.code16
-
-	/* clear the PE bit of CR0 */
-	movl	%cr0, %eax
-	andl 	$(~GRUB_MEMORY_MACHINE_CR0_PE_ON), %eax
-	movl	%eax, %cr0
-
-	/* flush prefetch queue, reload %cs */
-	DATA32	ljmp	$0, $realcseg
-
-realcseg:
-	/* we are in real mode now
-	 * set up the real mode segment registers : DS, SS, ES
-	 */
-	/* zero %eax */
-	xorl	%eax, %eax
-
-	movw	%ax, %ds
-	movw	%ax, %es
-	movw	%ax, %fs
-	movw	%ax, %gs
-	movw	%ax, %ss
-
-	/* restore interrupts */
-	sti
-
-	/* return on new stack! */
-	DATA32	ret
-
-	.code32
-
-
 /*
  * grub_gate_a20(int on)
  *
@@ -730,115 +607,7 @@ FUNCTION(grub_chainloader_real_boot)
 	ljmp	$0, $GRUB_MEMORY_MACHINE_BOOT_LOADER_ADDR
 	.code32
 
-
-/*
- * void grub_linux_boot_zimage (void)
- */
-VARIABLE(grub_linux_prot_size)
-	.long	0
-VARIABLE(grub_linux_tmp_addr)
-	.long	0
-VARIABLE(grub_linux_real_addr)
-	.long	0
-	
-FUNCTION(grub_linux_boot_zimage)
-	/* copy the kernel */
-	movl	EXT_C(grub_linux_prot_size), %ecx
-	addl	$3, %ecx
-	shrl	$2, %ecx
-	movl	$GRUB_LINUX_BZIMAGE_ADDR, %esi
-	movl	$GRUB_LINUX_ZIMAGE_ADDR, %edi
-	cld
-	rep
-	movsl
-
-FUNCTION(grub_linux_boot_bzimage)
-	call	EXT_C(grub_dl_unload_all)
-	
-	movl	EXT_C(grub_linux_real_addr), %ebx
-
-	/* copy the real mode code */
-	movl	EXT_C(grub_linux_tmp_addr), %esi
-	movl	%ebx, %edi
-	movl	$GRUB_LINUX_SETUP_MOVE_SIZE, %ecx
-	cld
-	rep
-	movsb
-
-	/* change %ebx to the segment address */
-	shrl	$4, %ebx
-	movl	%ebx, %eax
-	addl	$0x20, %eax
-	movw	%ax, linux_setup_seg
-
-	/* XXX new stack pointer in safe area for calling functions */
-	movl	$0x4000, %esp
-	call	EXT_C(grub_stop_floppy)
-
-	/* final setup for linux boot */
-	call	prot_to_real
-	.code16
-
-	cli
-	movw	%bx, %ss
-	movw	$GRUB_LINUX_SETUP_STACK, %sp
-
-	movw	%bx, %ds
-	movw	%bx, %es
-	movw	%bx, %fs
-	movw	%bx, %gs
-
-	/* ljmp */
-	.byte	0xea
-	.word	0
-linux_setup_seg:
-	.word	0
-	.code32
-
-		
-/*
- * This starts the multiboot kernel.
- */
-
-FUNCTION(grub_multiboot_real_boot)
-	/* Push the entry address on the stack.  */
-	pushl	%eax
-	/* Move the address of the multiboot information structure to ebx.  */
-	movl	%edx,%ebx
-	
-	/* Unload all modules and stop the floppy driver.  */
-	call	EXT_C(grub_dl_unload_all)
-	call	EXT_C(grub_stop_floppy)
-
-	/* Interrupts should be disabled.  */
-	cli
-	
-	/* Move the magic value into eax and jump to the kernel.  */
-	movl	$MULTIBOOT_MAGIC2,%eax
-	popl	%ecx
-	jmp 	*%ecx
-
-/*
- * This starts the multiboot 2 kernel.
- */
-
-FUNCTION(grub_multiboot2_real_boot)
-        /* Push the entry address on the stack.  */
-        pushl   %eax
-        /* Move the address of the multiboot information structure to ebx.  */
-        movl    %edx,%ebx
-
-        /* Unload all modules and stop the floppy driver.  */
-        call    EXT_C(grub_dl_unload_all)
-        call    EXT_C(grub_stop_floppy)
-
-        /* Interrupts should be disabled.  */
-        cli
-
-        /* Move the magic value into eax and jump to the kernel.  */
-        movl    $MULTIBOOT2_BOOTLOADER_MAGIC,%eax 
-        popl    %ecx
-        jmp     *%ecx
+#include "kern/i386/loader.S"
 	
 /*
  *   int grub_biosdisk_rw_int13_extensions (int ah, int drive, void *dap)
diff -Nurp grub2/kern/i386/realmode.S grub2.split/kern/i386/realmode.S
--- grub2/kern/i386/realmode.S	1970-01-01 01:00:00.000000000 +0100
+++ grub2.split/kern/i386/realmode.S	2007-10-16 23:01:27.000000000 +0200
@@ -0,0 +1,178 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 1999,2000,2001,2002,2003,2005,2006,2007 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/>.
+ */
+
+
+/*
+ * Note: These functions defined in this file may be called from C.
+ *       Be careful of that you must not modify some registers. Quote
+ *       from gcc-2.95.2/gcc/config/i386/i386.h:
+	
+   1 for registers not available across function calls.
+   These must include the FIXED_REGISTERS and also any
+   registers that can be used without being saved.
+   The latter must include the registers where values are returned
+   and the register where structure-value addresses are passed.
+   Aside from that, you can include as many other registers as you like.
+
+  ax,dx,cx,bx,si,di,bp,sp,st,st1,st2,st3,st4,st5,st6,st7,arg
+{  1, 1, 1, 0, 0, 0, 0, 1, 1,  1,  1,  1,  1,  1,  1,  1,  1 }
+ */
+
+/*
+ * Note: GRUB is compiled with the options -mrtd and -mregparm=3.
+ *       So the first three arguments are passed in %eax, %edx, and %ecx,
+ *       respectively, and if a function has a fixed number of arguments
+ *       and the number if greater than three, the function must return
+ *       with "ret $N" where N is ((the number of arguments) - 3) * 4.
+ */
+
+/*
+ *  This is the area for all of the special variables.
+ */
+
+	.p2align	2	/* force 4-byte alignment */
+
+protstack:
+	.long	GRUB_MEMORY_MACHINE_PROT_STACK
+
+/*
+ * This is the Global Descriptor Table
+ *
+ *  An entry, a "Segment Descriptor", looks like this:
+ *
+ * 31          24         19   16                 7           0
+ * ------------------------------------------------------------
+ * |             | |B| |A|       | |   |1|0|E|W|A|            |
+ * | BASE 31..24 |G|/|L|V| LIMIT |P|DPL|  TYPE   | BASE 23:16 |  4
+ * |             | |D| |L| 19..16| |   |1|1|C|R|A|            |
+ * ------------------------------------------------------------
+ * |                             |                            |
+ * |        BASE 15..0           |       LIMIT 15..0          |  0
+ * |                             |                            |
+ * ------------------------------------------------------------
+ *
+ *  Note the ordering of the data items is reversed from the above
+ *  description.
+ */
+
+	.p2align	2	/* force 4-byte alignment */
+gdt:
+	.word	0, 0
+	.byte	0, 0, 0, 0
+
+	/* -- code segment --
+	 * base = 0x00000000, limit = 0xFFFFF (4 KiB Granularity), present
+	 * type = 32bit code execute/read, DPL = 0
+	 */
+	.word	0xFFFF, 0
+	.byte	0, 0x9A, 0xCF, 0
+
+	/* -- data segment --
+	 * base = 0x00000000, limit 0xFFFFF (4 KiB Granularity), present
+	 * type = 32 bit data read/write, DPL = 0 
+	 */
+	.word	0xFFFF, 0
+	.byte	0, 0x92, 0xCF, 0
+
+	/* -- 16 bit real mode CS --
+	 * base = 0x00000000, limit 0x0FFFF (1 B Granularity), present
+	 * type = 16 bit code execute/read only/conforming, DPL = 0
+	 */
+	.word	0xFFFF, 0
+	.byte	0, 0x9E, 0, 0
+
+	/* -- 16 bit real mode DS --
+	 * base = 0x00000000, limit 0x0FFFF (1 B Granularity), present
+	 * type = 16 bit data read/write, DPL = 0
+	 */
+	.word	0xFFFF, 0
+	.byte	0, 0x92, 0, 0
+
+
+/* this is the GDT descriptor */
+gdtdesc:
+	.word	0x27			/* limit */
+	.long	gdt			/* addr */
+
+/*
+ *  These next routine, "prot_to_real" is structured in a very
+ *  specific way.  Be very careful when changing it.
+ *
+ *  NOTE:  Use of it messes up %eax and %ebp.
+ */
+
+prot_to_real:
+	/* just in case, set GDT */
+	lgdt	gdtdesc
+
+	/* save the protected mode stack */
+	movl	%esp, %eax
+	movl	%eax, protstack
+
+	/* get the return address */
+	movl	(%esp), %eax
+	movl	%eax, GRUB_MEMORY_MACHINE_REAL_STACK
+
+	/* set up new stack */
+	movl	$GRUB_MEMORY_MACHINE_REAL_STACK, %eax
+	movl	%eax, %esp
+	movl	%eax, %ebp
+
+	/* set up segment limits */
+	movw	$GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG, %ax
+	movw	%ax, %ds
+	movw	%ax, %es
+	movw	%ax, %fs
+	movw	%ax, %gs
+	movw	%ax, %ss
+
+	/* this might be an extra step */
+	/* jump to a 16 bit segment */
+	ljmp	$GRUB_MEMORY_MACHINE_PSEUDO_REAL_CSEG, $tmpcseg
+
+tmpcseg:
+	.code16
+
+	/* clear the PE bit of CR0 */
+	movl	%cr0, %eax
+	andl 	$(~GRUB_MEMORY_MACHINE_CR0_PE_ON), %eax
+	movl	%eax, %cr0
+
+	/* flush prefetch queue, reload %cs */
+	DATA32	ljmp	$0, $realcseg
+
+realcseg:
+	/* we are in real mode now
+	 * set up the real mode segment registers : DS, SS, ES
+	 */
+	/* zero %eax */
+	xorl	%eax, %eax
+
+	movw	%ax, %ds
+	movw	%ax, %es
+	movw	%ax, %fs
+	movw	%ax, %gs
+	movw	%ax, %ss
+
+	/* restore interrupts */
+	sti
+
+	/* return on new stack! */
+	DATA32	ret
+
+	.code32

Re: [PATCH] split realmode and loader routines out of startup.S

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> This patch splits realmode and loader routines out of startup.S.  The idea
> is that the LinuxBIOS port can be adapted to share more code with the rest
> of GRUB instead of duplicating it.
>
> This is quite critical stuff, so even if the change seems trivial I'd suggest
> being careful, since I don't trust myself too much.  Of course, I've tested
> that it can still boot Linux and Multiboot (on qemu only).  Perhaps testing
> on real hardware would be appropiate (but I don't have this handy atm).

Neither have I.  Hopefully someone else?

> 2007-10-16  Robert Millan  <rmh@aybabtu.com>
>
> 	* kern/i386/loader.S: New file.
>
> 	* kern/i386/pc/startup.S (grub_linux_prot_size): Moved to ...
> 	* kern/i386/loader.S (grub_linux_prot_size): ... here.

I would say:

(...): Moved from here...
(...): ... to here.

Can you change that?

> 	* kern/i386/pc/startup.S (grub_linux_tmp_addr): Moved to ...
> 	* kern/i386/loader.S (grub_linux_tmp_addr): ... here.
> 	* kern/i386/pc/startup.S (grub_linux_real_addr): Moved to ...
> 	* kern/i386/loader.S (grub_linux_real_addr): ... here.
> 	* kern/i386/pc/startup.S (grub_linux_boot_zimage): Moved to ...
> 	* kern/i386/loader.S (grub_linux_boot_zimage): ... here.
> 	* kern/i386/pc/startup.S (grub_linux_boot_bzimage): Moved to ...
> 	* kern/i386/loader.S (grub_linux_boot_bzimage): ... here.
> 	* kern/i386/pc/startup.S (grub_multiboot_real_boot): Moved to ...
> 	* kern/i386/loader.S (grub_multiboot_real_boot): ... here.
> 	* kern/i386/pc/startup.S (grub_multiboot2_real_boot): Moved to ...
> 	* kern/i386/loader.S (grub_multiboot2_real_boot): ... here.
>
> 	* kern/i386/realmode.S: New file.
>
> 	* kern/i386/pc/startup.S (protstack): Moved to ...
> 	* kern/i386/realmode.S (protstack): ... here.
> 	* kern/i386/pc/startup.S (gdt): Moved to ...
> 	* kern/i386/realmode.S (gdt): ... here.
> 	* kern/i386/pc/startup.S (prot_to_real): Moved to ...
> 	* kern/i386/realmode.S (prot_to_real): ... here.
>
> 	* kern/i386/pc/startup.S: Include `kern/i386/loader.S' and
> 	`kern/i386/realmode.S'.

Why include?  Can't it be linked?  That's what a linker is for :-)

Please try to fix this.

--
Marco

Re: [PATCH] split realmode and loader routines out of startup.S

[Robert Millan]

On Wed, Oct 17, 2007 at 10:32:28AM +0200, Marco Gerards wrote:
> > 	* kern/i386/pc/startup.S (grub_linux_prot_size): Moved to ...
> > 	* kern/i386/loader.S (grub_linux_prot_size): ... here.
> 
> I would say:
> 
> (...): Moved from here...
> (...): ... to here.
> 
> Can you change that?

Ok (ACKed also for the next patch I already sent).

> > 	* kern/i386/pc/startup.S (grub_linux_tmp_addr): Moved to ...
> > 	* kern/i386/loader.S (grub_linux_tmp_addr): ... here.
> > 	* kern/i386/pc/startup.S (grub_linux_real_addr): Moved to ...
> > 	* kern/i386/loader.S (grub_linux_real_addr): ... here.
> > 	* kern/i386/pc/startup.S (grub_linux_boot_zimage): Moved to ...
> > 	* kern/i386/loader.S (grub_linux_boot_zimage): ... here.
> > 	* kern/i386/pc/startup.S (grub_linux_boot_bzimage): Moved to ...
> > 	* kern/i386/loader.S (grub_linux_boot_bzimage): ... here.
> > 	* kern/i386/pc/startup.S (grub_multiboot_real_boot): Moved to ...
> > 	* kern/i386/loader.S (grub_multiboot_real_boot): ... here.
> > 	* kern/i386/pc/startup.S (grub_multiboot2_real_boot): Moved to ...
> > 	* kern/i386/loader.S (grub_multiboot2_real_boot): ... here.
> >
> > 	* kern/i386/realmode.S: New file.
> >
> > 	* kern/i386/pc/startup.S (protstack): Moved to ...
> > 	* kern/i386/realmode.S (protstack): ... here.
> > 	* kern/i386/pc/startup.S (gdt): Moved to ...
> > 	* kern/i386/realmode.S (gdt): ... here.
> > 	* kern/i386/pc/startup.S (prot_to_real): Moved to ...
> > 	* kern/i386/realmode.S (prot_to_real): ... here.
> >
> > 	* kern/i386/pc/startup.S: Include `kern/i386/loader.S' and
> > 	`kern/i386/realmode.S'.
> 
> Why include?  Can't it be linked?  That's what a linker is for :-)

Linking required wrapping a lot of references with EXT_C() macro, so I opted
for #include (note this is done for lzo1x.S already).

Also, #including stuff from the wrong [1] place caused cpu faults in my tests.
With my current patch, code doesn't change much its location (real mode stuff
at the beginning, loader stuff at the end).

[1] Sorry, I'm not sure what wrong means here.  Maybe it has to do with GAS
    syntax magic affecting cross-include borders or with a weird hardcoded
    reference to a specific memory layout.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] split realmode and loader routines out of startup.S

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Wed, Oct 17, 2007 at 10:32:28AM +0200, Marco Gerards wrote:
>> > 	* kern/i386/pc/startup.S (grub_linux_prot_size): Moved to ...
>> > 	* kern/i386/loader.S (grub_linux_prot_size): ... here.
>> 
>> I would say:
>> 
>> (...): Moved from here...
>> (...): ... to here.
>> 
>> Can you change that?
>
> Ok (ACKed also for the next patch I already sent).

Great!

>> > 	* kern/i386/pc/startup.S (grub_linux_tmp_addr): Moved to ...
>> > 	* kern/i386/loader.S (grub_linux_tmp_addr): ... here.
>> > 	* kern/i386/pc/startup.S (grub_linux_real_addr): Moved to ...
>> > 	* kern/i386/loader.S (grub_linux_real_addr): ... here.
>> > 	* kern/i386/pc/startup.S (grub_linux_boot_zimage): Moved to ...
>> > 	* kern/i386/loader.S (grub_linux_boot_zimage): ... here.
>> > 	* kern/i386/pc/startup.S (grub_linux_boot_bzimage): Moved to ...
>> > 	* kern/i386/loader.S (grub_linux_boot_bzimage): ... here.
>> > 	* kern/i386/pc/startup.S (grub_multiboot_real_boot): Moved to ...
>> > 	* kern/i386/loader.S (grub_multiboot_real_boot): ... here.
>> > 	* kern/i386/pc/startup.S (grub_multiboot2_real_boot): Moved to ...
>> > 	* kern/i386/loader.S (grub_multiboot2_real_boot): ... here.
>> >
>> > 	* kern/i386/realmode.S: New file.
>> >
>> > 	* kern/i386/pc/startup.S (protstack): Moved to ...
>> > 	* kern/i386/realmode.S (protstack): ... here.
>> > 	* kern/i386/pc/startup.S (gdt): Moved to ...
>> > 	* kern/i386/realmode.S (gdt): ... here.
>> > 	* kern/i386/pc/startup.S (prot_to_real): Moved to ...
>> > 	* kern/i386/realmode.S (prot_to_real): ... here.
>> >
>> > 	* kern/i386/pc/startup.S: Include `kern/i386/loader.S' and
>> > 	`kern/i386/realmode.S'.
>> 
>> Why include?  Can't it be linked?  That's what a linker is for :-)
>
> Linking required wrapping a lot of references with EXT_C() macro, so I opted
> for #include (note this is done for lzo1x.S already).

Is using EXT_C a problem?  I don't know this code too well,
unfortunately :(

> Also, #including stuff from the wrong [1] place caused cpu faults in my tests.
> With my current patch, code doesn't change much its location (real mode stuff
> at the beginning, loader stuff at the end).

:-/

> [1] Sorry, I'm not sure what wrong means here.  Maybe it has to do with GAS
>     syntax magic affecting cross-include borders or with a weird hardcoded
>     reference to a specific memory layout.

I can't help you there.

--
Marco

Re: [PATCH] split realmode and loader routines out of startup.S

[Robert Millan]

On Wed, Oct 17, 2007 at 12:04:14PM +0200, Marco Gerards wrote:
> >
> > Linking required wrapping a lot of references with EXT_C() macro, so I opted
> > for #include (note this is done for lzo1x.S already).
> 
> Is using EXT_C a problem?  I don't know this code too well,
> unfortunately :(

No.  Maybe linking bring in other problems, but EXT_C per se isn't a big deal.
Though do we really want it?  It doesn't seem to be a problem for lzo1x.S.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] split realmode and loader routines out of startup.S

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Wed, Oct 17, 2007 at 12:04:14PM +0200, Marco Gerards wrote:
>> >
>> > Linking required wrapping a lot of references with EXT_C() macro, so I opted
>> > for #include (note this is done for lzo1x.S already).
>> 
>> Is using EXT_C a problem?  I don't know this code too well,
>> unfortunately :(
>
> No.  Maybe linking bring in other problems, but EXT_C per se isn't a big deal.
> Though do we really want it?  It doesn't seem to be a problem for lzo1x.S.

Including is not a big problem for me.  It should just not become a
common practise.  It's easy to change so I am fine with it.

--
Marco

Re: [PATCH] split realmode and loader routines out of startup.S

[Robert Millan]

Committed.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] split realmode and loader routines out of startup.S

[Jeroen Dekkers]

At Tue, 16 Oct 2007 23:09:07 +0200,
Robert Millan wrote:
> This patch splits realmode and loader routines out of startup.S.  The idea
> is that the LinuxBIOS port can be adapted to share more code with the rest
> of GRUB instead of duplicating it.
> 
> This is quite critical stuff, so even if the change seems trivial I'd suggest
> being careful, since I don't trust myself too much.  Of course, I've tested
> that it can still boot Linux and Multiboot (on qemu only).  Perhaps testing
> on real hardware would be appropiate (but I don't have this handy atm).
<SNIP>
> 	* kern/i386/pc/startup.S (protstack): Moved to ...
> 	* kern/i386/realmode.S (protstack): ... here.
> 	* kern/i386/pc/startup.S (gdt): Moved to ...
> 	* kern/i386/realmode.S (gdt): ... here.
> 	* kern/i386/pc/startup.S (prot_to_real): Moved to ...
> 	* kern/i386/realmode.S (prot_to_real): ... here.
> 
> 	* kern/i386/pc/startup.S: Include `kern/i386/loader.S' and
> 	`kern/i386/realmode.S'.

Why did you move prot_to_real to realmode.S, but not real_to_prot?
That seems a bit strange to me...


Jeroen Dekkers

Re: [PATCH] split realmode and loader routines out of startup.S

[Robert Millan]

On Fri, Jan 04, 2008 at 06:15:17PM +0100, Jeroen Dekkers wrote:
> At Tue, 16 Oct 2007 23:09:07 +0200,
> Robert Millan wrote:
> > This patch splits realmode and loader routines out of startup.S.  The idea
> > is that the LinuxBIOS port can be adapted to share more code with the rest
> > of GRUB instead of duplicating it.
> > 
> > This is quite critical stuff, so even if the change seems trivial I'd suggest
> > being careful, since I don't trust myself too much.  Of course, I've tested
> > that it can still boot Linux and Multiboot (on qemu only).  Perhaps testing
> > on real hardware would be appropiate (but I don't have this handy atm).
> <SNIP>
> > 	* kern/i386/pc/startup.S (protstack): Moved to ...
> > 	* kern/i386/realmode.S (protstack): ... here.
> > 	* kern/i386/pc/startup.S (gdt): Moved to ...
> > 	* kern/i386/realmode.S (gdt): ... here.
> > 	* kern/i386/pc/startup.S (prot_to_real): Moved to ...
> > 	* kern/i386/realmode.S (prot_to_real): ... here.
> > 
> > 	* kern/i386/pc/startup.S: Include `kern/i386/loader.S' and
> > 	`kern/i386/realmode.S'.
> 
> Why did you move prot_to_real to realmode.S, but not real_to_prot?
> That seems a bit strange to me...

Because on i386-pc, GRUB wants to do both things, but on i386-linuxbios,
you start in protected mode and only want to switch once to real mode when
booting Linux.  There's no way back AFAICT.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)