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

[Marco Gerards]

Robert Millan <address@hidden> 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  <address@hidden>
>
>       * 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