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

2007-10-17 Thread Marco Gerards 
Robert Millan <[EMAIL PROTECTED]> 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  <[EMAIL PROTECTED]>
>
>   * 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



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

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

2007-10-17 Thread 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

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


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] split i386-generic part of i386/pc/loader.h

2007-10-17 Thread Robert Millan 

This patch splits the i386-generic part of i386/pc/loader.h into i386/loader.h.

Comments?

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)
2007-10-17  Robert Millan  <[EMAIL PROTECTED]>

	* include/grub/i386/loader.h: New file.

	* include/grub/i386/pc/loader.h (grub_linux_prot_size): Moved to ...
	* include/grub/i386/loader.h (grub_linux_prot_size): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_tmp_addr): Moved to ...
	* include/grub/i386/loader.h (grub_linux_tmp_addr): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_real_addr): Moved to ...
	* include/grub/i386/loader.h (grub_linux_real_addr): ... here.
	* include/grub/i386/pc/loader.h (grub_os_area_addr): Moved to ...
	* include/grub/i386/loader.h (grub_os_area_addr): ... here.
	* include/grub/i386/pc/loader.h (grub_os_area_size): Moved to ...
	* include/grub/i386/loader.h (grub_os_area_size): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_boot_zimage): Moved to ...
	* include/grub/i386/loader.h (grub_linux_boot_zimage): ... here.
	* include/grub/i386/pc/loader.h (grub_linux_boot_bzimage): Moved to ...
	* include/grub/i386/loader.h (grub_linux_boot_bzimage): ... here.
	* include/grub/i386/pc/loader.h (grub_multiboot_real_boot): Moved to ...
	* include/grub/i386/loader.h (grub_multiboot_real_boot): ... here.
	* include/grub/i386/pc/loader.h (grub_multiboot2_real_boot): Moved to ...
	* include/grub/i386/loader.h (grub_multiboot2_real_boot): ... here.
	* include/grub/i386/pc/loader.h (grub_rescue_cmd_linux): Moved to ...
	* include/grub/i386/loader.h (grub_rescue_cmd_linux): ... here.
	* include/grub/i386/pc/loader.h (grub_rescue_cmd_initrd): Moved to ...
	* include/grub/i386/loader.h (grub_rescue_cmd_initrd): ... here.

	* include/grub/i386/pc/loader.h: Include `grub/cpu/loader.h'.

diff -Nurp grub2/include/grub/i386/loader.h grub2.loader_h/include/grub/i386/loader.h
--- grub2/include/grub/i386/loader.h	1970-01-01 01:00:00.0 +0100
+++ grub2.loader_h/include/grub/i386/loader.h	2007-10-17 10:27:32.0 +0200
@@ -0,0 +1,48 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2002,2003,2004,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 .
+ */
+
+#ifndef GRUB_LOADER_CPU_HEADER
+#define GRUB_LOADER_CPU_HEADER	1
+
+#include 
+#include 
+#include 
+
+extern grub_uint32_t EXPORT_VAR(grub_linux_prot_size);
+extern char *EXPORT_VAR(grub_linux_tmp_addr);
+extern char *EXPORT_VAR(grub_linux_real_addr);
+extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
+extern grub_size_t EXPORT_VAR(grub_os_area_size);
+
+void EXPORT_FUNC(grub_linux_boot_zimage) (void) __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_linux_boot_bzimage) (void) __attribute__ ((noreturn));
+
+/* The asm part of the multiboot loader.  */
+void EXPORT_FUNC(grub_multiboot_real_boot) (grub_addr_t entry, 
+	struct grub_multiboot_info *mbi) 
+ __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_multiboot2_real_boot) (grub_addr_t entry,
+ struct grub_multiboot_info *mbi)
+ __attribute__ ((noreturn));
+
+/* It is necessary to export these functions, because normal mode commands
+   reuse rescue mode commands.  */
+void grub_rescue_cmd_linux (int argc, char *argv[]);
+void grub_rescue_cmd_initrd (int argc, char *argv[]);
+
+#endif /* ! GRUB_LOADER_CPU_HEADER */
diff -Nurp grub2/include/grub/i386/pc/loader.h grub2.loader_h/include/grub/i386/pc/loader.h
--- grub2/include/grub/i386/pc/loader.h	2007-07-25 21:29:23.0 +0200
+++ grub2.loader_h/include/grub/i386/pc/loader.h	2007-10-17 10:31:06.0 +0200
@@ -19,33 +19,10 @@
 #ifndef GRUB_LOADER_MACHINE_HEADER
 #define GRUB_LOADER_MACHINE_HEADER	1
 
-#include 
 #include 
-#include 
-
-extern grub_uint32_t EXPORT_VAR(grub_linux_prot_size);
-extern char *EXPORT_VAR(grub_linux_tmp_addr);
-extern char *EXPORT_VAR(grub_linux_real_addr);
-extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
-extern grub_size_t EXPORT_VAR(grub_os_area_size);
-
-void EXPORT_FUNC(grub_linux_boot_zimage) (void) __attribute__ ((noreturn));
-void EXPORT_FUNC(grub_linux_boot_bzimage) (void) __attribute__ ((noreturn));
+#include 
 
 /* This is an asm part of the chainloader.  */
 void EXPORT_FUNC(grub_chainloader_real_boot) (int drive, void *part_addr) __attribute__ ((noretu

Re: [PATCH] split i386-generic part of i386/pc/loader.h

2007-10-17 Thread Marco Gerards 
Robert Millan <[EMAIL PROTECTED]> writes:

Hi,

[...]

> 2007-10-17  Robert Millan  <[EMAIL PROTECTED]>
>
>   * include/grub/i386/loader.h: New file.
>
>   * include/grub/i386/pc/loader.h (grub_linux_prot_size): Moved to ...
>   * include/grub/i386/loader.h (grub_linux_prot_size): ... here.
>   * include/grub/i386/pc/loader.h (grub_linux_tmp_addr): Moved to ...
>   * include/grub/i386/loader.h (grub_linux_tmp_addr): ... here.
>   * include/grub/i386/pc/loader.h (grub_linux_real_addr): Moved to ...
>   * include/grub/i386/loader.h (grub_linux_real_addr): ... here.
>   * include/grub/i386/pc/loader.h (grub_os_area_addr): Moved to ...
>   * include/grub/i386/loader.h (grub_os_area_addr): ... here.
>   * include/grub/i386/pc/loader.h (grub_os_area_size): Moved to ...
>   * include/grub/i386/loader.h (grub_os_area_size): ... here.
>   * include/grub/i386/pc/loader.h (grub_linux_boot_zimage): Moved to ...
>   * include/grub/i386/loader.h (grub_linux_boot_zimage): ... here.
>   * include/grub/i386/pc/loader.h (grub_linux_boot_bzimage): Moved to ...
>   * include/grub/i386/loader.h (grub_linux_boot_bzimage): ... here.
>   * include/grub/i386/pc/loader.h (grub_multiboot_real_boot): Moved to ...
>   * include/grub/i386/loader.h (grub_multiboot_real_boot): ... here.
>   * include/grub/i386/pc/loader.h (grub_multiboot2_real_boot): Moved to 
> ...
>   * include/grub/i386/loader.h (grub_multiboot2_real_boot): ... here.
>   * include/grub/i386/pc/loader.h (grub_rescue_cmd_linux): Moved to ...
>   * include/grub/i386/loader.h (grub_rescue_cmd_linux): ... here.
>   * include/grub/i386/pc/loader.h (grub_rescue_cmd_initrd): Moved to ...
>   * include/grub/i386/loader.h (grub_rescue_cmd_initrd): ... here.

Same as the last patch:

(...): Move from here...
(...): ... to here.

btw, it is easier to do:

* include/grub/i386/pc/loader.h (grub_linux_prot_size)
(grub_linux_tmp_addr, grub_linux_real_addr): Moved from here...
* include/grub/i386/loader.h  (grub_linux_prot_size)
(grub_linux_tmp_addr, grub_linux_real_addr): ...to here.

Please have a close look how things like this were done in older
patches, to get a feeling on how to write this down.

--
Marco




___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] split i386-generic part of i386/pc/loader.h

2007-10-17 Thread Robert Millan 

Committed.

-- 
Robert Millan

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


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

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

2007-10-17 Thread Marco Gerards 
Robert Millan <[EMAIL PROTECTED]> 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



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

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

2007-10-17 Thread 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

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


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Implement grub_sleep() and grub_ticksleep()

2007-10-17 Thread Marco Gerards 
Robert Millan <[EMAIL PROTECTED]> writes:

> On Tue, Oct 16, 2007 at 08:46:16PM +0200, Marco Gerards wrote:
>> >
>> > That's what grub_ticksleep does.  grub_sleep() counts in seconds because
>> > I tried to mimic POSIX which seems to be a trend for grub_* functions.  I
>> > think it can be used for menu timeout although I didn't have time to look.
>> 
>> Right.  Although I do not like setting the time in
>> GRUB_TICKS_PER_SECOND for millisecond stuff, etc.  In that case
>> everyone has to implement the same functionality.
>
> Moved to grub_millisleep().

Good :)

>> > OTOH, this wouldn't be the first place in grub where __i386__ is tested ;-)
>> 
>> Oh?  Perhaps that code is wrong?
>
> Actually now that I check it's only in one file.  But the code is right 
> afaict.

What I mean is that this might be a wrong approach in this other file
as well.

>> >> > +
>> >> > +void
>> >> > +grub_ticksleep (grub_uint32_t ticks)
>> >> > +{
>> >> > +  grub_uint32_t end_at;
>> >> > +  end_at = grub_get_rtc () + ticks;
>> >> > +  while (grub_get_rtc () < end_at)
>> >> > +grub_cpu_idle ();
>> >> > +}
>> >> 
>> >> Why do you recreate this for every arch?  This seems portable as long
>> >> as you can sleep a bit from time to time.
>> >
>> > What if a platform provides a sleep-like mechanism, but not a get_rtc-like
>> > one?  You can implement sleep around get_rtc easily, but not the other way
>> > around.  This is the case for LB (simply because grub_get_rtc is not
>> > implemented yet), but it could also happen on platforms that are designed
>> > not to provide it or are just buggy.
>> 
>> Well, I have no objections to this approach.
>
> Ok.  I made it a bit better by implementing grub_millisleep_generic in
> kern/misc.c and making each port just use that, having the option to do it
> their way if preferred.

Great.

>> Are you sure init.c is
>> the right place?
>
> Mostly.  I've observed that for code that doesn't obviously belong somewhere
> else, it tends to be in init.c if it's in C and startup.S if it's in asm (in
> i386/pc/startup.S it actually gets to the extreme, since only a small part of
> it is used for "startup" as such).
>
> So I think init.c is fine.

Ok.

> 2007-10-15  Robert Millan  <[EMAIL PROTECTED]>
>
>   * include/grub/time.h: New file.
>   * include/grub/i386/time.h: Likewise.
>   * include/grub/powerpc/time.h: Likewise.
>   * include/grub/sparc64/time.h: Likewise.
>
>   * include/grub/i386/pc/time.h (KERNEL_TIME_HEADER): Rename all
>   instances to ...
>   (KERNEL_MACHINE_TIME_HEADER): ... this.
>   * include/grub/powerpc/ieee1275/time.h (KERNEL_TIME_HEADER): Rename all
>   instances to ...
>   (KERNEL_MACHINE_TIME_HEADER): ... this.
>   * include/grub/sparc64/ieee1275/time.h (KERNEL_TIME_HEADER): Rename all
>   instances to ...
>   (KERNEL_MACHINE_TIME_HEADER): ... this.
>
>   * kern/i386/efi/init.c: Include `grub/time.h'.

 is preferred.

[...]

> +void
> +grub_millisleep_generic (grub_uint32_t ms)
> +{
> +  grub_uint32_t time;
> +  int i;
> +
> +  for (; ms > 0; ms -= TICK_DURATION_IN_MS)
> +/* wait for the lowest fraction of milliseconds we can (rounded up) */
> +for (i = 0; i < TICK_DURATION_IN_MS; i++)
> +  {
> + /* wait for next tick */
> + time = grub_get_rtc ();
> + while (time == grub_get_rtc ())
> +   grub_cpu_idle ();
> +  }
> +}

The problem with this is when TICK_DURATION_IN_MS is not very
accurate.  I think you can be more accurate if you use
TICKS_PER_SECOND and use it to calculate the total amount of ticks to
wait.  This will avoid rounding problems.  Accuracy is not always that
important, but I prefer to have it, if we can.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

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

2007-10-17 Thread Marco Gerards 
Robert Millan <[EMAIL PROTECTED]> 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



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] split i386-generic part of i386/pc/loader.h

2007-10-17 Thread Vesa Jääskeläinen 
Robert Millan wrote:
> This patch splits the i386-generic part of i386/pc/loader.h into 
> i386/loader.h.
> 
> Comments?

Why did you move stuff from i386/pc to i386 ?

I do see a point on running grub2 on non-pc i386...


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] ELF program header

2007-10-17 Thread Stefan Reinauer 
* Robert Millan <[EMAIL PROTECTED]> [071014 14:00]:
> > We tried the same but the patch was a lot bigger, trying to actually
> > leave space in the beginning. Your patch might be enough, that would be
> > really nice.
> 
> That's the question.  Do we really have to add more space, or is that space
> already allocated for us?

Ok, I am not exactly sure how to correctly adapt to the latest
grub-mkimage changes, so please bear that the attached patch is against
Patrick's GSoC version of grub-mkimage.c

With this patch attached, grub-mkimage produces an image that works when
being loaded as payload from LinuxBIOS version 2.

Your smaller version of the patch unfortunately did not work for me.

Flames and suggestions please! 

Best wishes, 

Stefan


--- grub-mkimage.c.orig	2007-10-05 20:33:50.0 +
+++ grub-mkimage.c	2007-10-15 18:20:03.0 +
@@ -104,7 +104,7 @@
   FILE *in;
   char *kernel_path;
   grub_addr_t grub_end = 0;
-  off_t phdroff;
+  off_t phdroff, pstart;
   int i;
 
   /* Read ELF header.  */
@@ -115,18 +115,27 @@
 
   grub_util_read_at (&ehdr, sizeof (ehdr), 0, in);
   
+  /* The +1 is to leave a hole for the program header for extra modules. */
   phdrs = xmalloc (grub_le_to_cpu16 (ehdr.e_phentsize)
-		   * (grub_le_to_cpu16 (ehdr.e_phnum) + 2));
+		   * (grub_le_to_cpu16 (ehdr.e_phnum) + 1));
+  
+  /* Append entire segment table to the file.  */
+  //phdroff = ALIGN_UP (grub_util_get_fp_size (out), sizeof (long));
+  phdroff = ALIGN_UP (sizeof(ehdr), sizeof (long));
+  grub_util_write_image_at (phdrs, grub_le_to_cpu16 (ehdr.e_phentsize)
+			*(grub_le_to_cpu16 (ehdr.e_phnum) + 1), phdroff,
+			out);
+
   /* Copy all existing segments.  */
   grub_util_info ("%u segments", grub_le_to_cpu16 (ehdr.e_phnum));
+  pstart = grub_util_get_fp_size (out);
+  /* set up first phdr offset */
   for (i = 0; i < grub_le_to_cpu16 (ehdr.e_phnum); i++)
 {
   char *segment_img;
   grub_size_t segment_end;
-
   phdr = phdrs + i;
-
-  /* Read segment header.  */
+ /* Read segment header.  */
   grub_util_read_at (phdr, sizeof (Elf32_Phdr),
 			 (grub_le_to_cpu32 (ehdr.e_phoff)
 			  + (i * grub_le_to_cpu16 (ehdr.e_phentsize))),
@@ -146,9 +155,11 @@
   
   grub_util_read_at (segment_img, grub_le_to_cpu32 (phdr->p_filesz),
 			 grub_le_to_cpu32 (phdr->p_offset), in);
-  grub_util_write_image_at (segment_img, grub_le_to_cpu32 (phdr->p_filesz),
+ phdr->p_offset = grub_cpu_to_le32(pstart);
+ grub_util_write_image_at (segment_img, grub_le_to_cpu32 (phdr->p_filesz),
 grub_le_to_cpu32 (phdr->p_offset), out);
 
+  pstart += phdr->p_filesz;
   free (segment_img);
 }
 
@@ -176,9 +187,9 @@
   ehdr.e_shstrndx = 0;
 
   /* Append entire segment table to the file.  */
-  phdroff = ALIGN_UP (grub_util_get_fp_size (out), sizeof (long));
+  // phdroff = ALIGN_UP (grub_util_get_fp_size (out), sizeof (long));
   grub_util_write_image_at (phdrs, grub_le_to_cpu16 (ehdr.e_phentsize)
-			* grub_le_to_cpu16 (ehdr.e_phnum), phdroff,
+			* (grub_le_to_cpu16 (ehdr.e_phnum)), phdroff,
 			out);
 
   /* Write ELF header.  */
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] split i386-generic part of i386/pc/loader.h

2007-10-17 Thread Robert Millan 
On Wed, Oct 17, 2007 at 09:17:54PM +0300, Vesa Jääskeläinen wrote:
> Robert Millan wrote:
> > This patch splits the i386-generic part of i386/pc/loader.h into 
> > i386/loader.h.
> > 
> > Comments?
> 
> Why did you move stuff from i386/pc to i386 ?
> 
> I do see a point on running grub2 on non-pc i386...

It's for LinuxBIOS.

-- 
Robert Millan

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


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel