Redefining GRUB_MOD_GAP in init.c

Redefining GRUB_MOD_GAP in init.c

[Pavel Roskin]

Hi Robert,

I'm concerned about your latest commit:

        * kern/powerpc/ieee1275/init.c (grub_arch_modules_addr): Skip
        `GRUB_MOD_GAP' for platforms in which it's not defined.

The algorithm for calculating the start of the modules should be exactly
the same in grub-mkimage and in the core.  Any fallbacks should be
coherent if possible.  Therefore, I think it would be better to define
GRUB_MOD_GAP only in header files.

It's hard for me to understand why kern/powerpc/ieee1275/init.c can be
used on any platform other than PowerPC.  I was assuming that my changes
would not affect other platforms.

Also, I don't see how GRUB_MOD_GAP would not be defined to 0x8000 for
i386-ieee1275 platform considering that
include/grub/i386/ieee1275/kernel.h simply includes
include/grub/powerpc/ieee1275/kernel.h

I suggest that i386-ieee1275 stops using any files for PowerPC.
kernel.h is not big, so it shouldn't be a problem to copy it.  As for
init.c, it should be either copied or moved to a more suitable place.

-- 
Regards,
Pavel Roskin

Re: Redefining GRUB_MOD_GAP in init.c

[Robert Millan]

On Sun, Jan 27, 2008 at 10:25:32PM -0500, Pavel Roskin wrote:
> Hi Robert,
> 
> I'm concerned about your latest commit:
> 
>         * kern/powerpc/ieee1275/init.c (grub_arch_modules_addr): Skip
>         `GRUB_MOD_GAP' for platforms in which it's not defined.
> 
> The algorithm for calculating the start of the modules should be exactly
> the same in grub-mkimage and in the core.  Any fallbacks should be
> coherent if possible.  Therefore, I think it would be better to define
> GRUB_MOD_GAP only in header files.

Sorry, I should've really sent this for review;  it wasn't such an obvious fix
as you pointed out.  I'll get that moved to headers as you suggest.

> Also, I don't see how GRUB_MOD_GAP would not be defined to 0x8000 for
> i386-ieee1275 platform considering that
> include/grub/i386/ieee1275/kernel.h simply includes
> include/grub/powerpc/ieee1275/kernel.h

Sounds strange.. I'm wondering that myself.  Will check..

> It's hard for me to understand why kern/powerpc/ieee1275/init.c can be
> used on any platform other than PowerPC.  I was assuming that my changes
> would not affect other platforms.
> 
> [...]
> I suggest that i386-ieee1275 stops using any files for PowerPC.
> kernel.h is not big, so it shouldn't be a problem to copy it.  As for
> init.c, it should be either copied or moved to a more suitable place.

I think it's safe to say that the only part of kern/powerpc/ieee1275/init.c
that only works on PowerPC is its name ;-)

Maybe it's time we move out those generic files under powerpc directory.  I
propose moving the following:

kern/powerpc/ieee1275/init.c
kern/powerpc/ieee1275/cmain.c
kern/powerpc/ieee1275/openfw.c
loader/powerpc/ieee1275/multiboot2.c

Does that seem fine?

-- 
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 /.)

GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Mon, Jan 28, 2008 at 10:09:10AM +0100, Robert Millan wrote:
> > Also, I don't see how GRUB_MOD_GAP would not be defined to 0x8000 for
> > i386-ieee1275 platform considering that
> > include/grub/i386/ieee1275/kernel.h simply includes
> > include/grub/powerpc/ieee1275/kernel.h
> 
> Sounds strange.. I'm wondering that myself.  Will check..

Ah, right.  The problem is not on i386-ieee1275 but on i386-linuxbios,
which uses util/elf/grub-mkimage.c.

So, we define GRUB_MOD_GAP for every port that wants to use ELF ?  Or we
put the fallback only in grub-mkimage.c ?  I don't like either of the
solutions very much :-(

What do you suggest?

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Pavel Roskin]

On Mon, 2008-01-28 at 10:30 +0100, Robert Millan wrote:
> On Mon, Jan 28, 2008 at 10:09:10AM +0100, Robert Millan wrote:
> > > Also, I don't see how GRUB_MOD_GAP would not be defined to 0x8000 for
> > > i386-ieee1275 platform considering that
> > > include/grub/i386/ieee1275/kernel.h simply includes
> > > include/grub/powerpc/ieee1275/kernel.h
> > 
> > Sounds strange.. I'm wondering that myself.  Will check..
> 
> Ah, right.  The problem is not on i386-ieee1275 but on i386-linuxbios,
> which uses util/elf/grub-mkimage.c.

Sorry, I didn't realize that i386-linuxbios was affected by my patch.

> So, we define GRUB_MOD_GAP for every port that wants to use ELF ?  Or we
> put the fallback only in grub-mkimage.c ?  I don't like either of the
> solutions very much :-(
> 
> What do you suggest?

I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
needed, it could be removed across the board.

-- 
Regards,
Pavel Roskin

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

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

On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
> > What do you suggest?
> 
> I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
> needed, it could be removed across the board.

Ok, does this seem fine?

-- 
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: tmp.diff --]
[-- Type: text/x-diff, Size: 2518 bytes --]

2008-01-27  Robert Millan  <rmh@aybabtu.com>

	* include/grub/i386/linuxbios/kernel.h (GRUB_MOD_GAP): New macro.
	* kern/powerpc/ieee1275/init.c (grub_arch_modules_addr): Remove
	`ifndef GRUB_MOD_GAP' hack.
	* util/elf/grub-mkimage.c (add_segments): Likewise.

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/i386/linuxbios/kernel.h ./include/grub/i386/linuxbios/kernel.h
--- ../grub2/include/grub/i386/linuxbios/kernel.h	2007-10-31 23:35:12.000000000 +0100
+++ ./include/grub/i386/linuxbios/kernel.h	2008-01-28 17:25:50.000000000 +0100
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2005,2006,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2005,2006,2007,2008  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
@@ -23,4 +23,7 @@
 
 #define GRUB_MOD_ALIGN 0x1000
 
+/* Non-zero value is only needed for some IEEE-1275 platforms.  */
+#define GRUB_MOD_GAP 0
+
 #endif /* ! GRUB_KERNEL_MACHINE_HEADER */
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/powerpc/ieee1275/init.c ./kern/powerpc/ieee1275/init.c
--- ../grub2/kern/powerpc/ieee1275/init.c	2008-01-28 00:11:01.000000000 +0100
+++ ./kern/powerpc/ieee1275/init.c	2008-01-28 17:23:11.000000000 +0100
@@ -242,10 +242,5 @@ grub_get_rtc (void)
 grub_addr_t
 grub_arch_modules_addr (void)
 {
-/* Only needed for Apple hardware (therefore, powerpc).  */
-#ifndef GRUB_MOD_GAP
-#define GRUB_MOD_GAP 0
-#endif
-
   return ALIGN_UP(_end + GRUB_MOD_GAP, GRUB_MOD_ALIGN);
 }
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/util/elf/grub-mkimage.c ./util/elf/grub-mkimage.c
--- ../grub2/util/elf/grub-mkimage.c	2008-01-28 10:01:11.000000000 +0100
+++ ./util/elf/grub-mkimage.c	2008-01-28 17:24:32.000000000 +0100
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2004,2005,2006,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2004,2005,2006,2007,2008  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
@@ -217,11 +217,6 @@ add_segments (char *dir, FILE *out, int 
     {
       grub_addr_t modbase;
 
-/* Only needed for Apple hardware (therefore, powerpc).  */
-#ifndef GRUB_MOD_GAP
-#define GRUB_MOD_GAP 0
-#endif
-
       /* Place modules just after grub segment.  */
       modbase = ALIGN_UP(grub_end + GRUB_MOD_GAP, GRUB_MOD_ALIGN);
 

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Pavel Roskin]

On Mon, 2008-01-28 at 17:29 +0100, Robert Millan wrote:
> On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
> > > What do you suggest?
> > 
> > I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
> > needed, it could be removed across the board.
> 
> Ok, does this seem fine?

Looks good to me.  Thanks!

-- 
Regards,
Pavel Roskin

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Mon, Jan 28, 2008 at 11:34:07AM -0500, Pavel Roskin wrote:
> 
> On Mon, 2008-01-28 at 17:29 +0100, Robert Millan wrote:
> > On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
> > > > What do you suggest?
> > > 
> > > I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
> > > needed, it could be removed across the board.
> > 
> > Ok, does this seem fine?
> 
> Looks good to me.  Thanks!

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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
>> > What do you suggest?
>> 
>> I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
>> needed, it could be removed across the board.
>
> Ok, does this seem fine?

Am I missing something, or didn't you explain what GRUB_MOD_GAP is
supposed to do...?  To be honest, I do not know what it is about.

--
Marco

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Tue, Jan 29, 2008 at 09:38:31AM +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> > On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
> >> > What do you suggest?
> >> 
> >> I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
> >> needed, it could be removed across the board.
> >
> > Ok, does this seem fine?
> 
> Am I missing something, or didn't you explain what GRUB_MOD_GAP is
> supposed to do...?  To be honest, I do not know what it is about.

Pavel did.  Check previous mails.

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Tue, Jan 29, 2008 at 09:38:31AM +0100, Marco Gerards wrote:
>> Robert Millan <rmh@aybabtu.com> writes:
>> 
>> > On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
>> >> > What do you suggest?
>> >> 
>> >> I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
>> >> needed, it could be removed across the board.
>> >
>> > Ok, does this seem fine?
>> 
>> Am I missing something, or didn't you explain what GRUB_MOD_GAP is
>> supposed to do...?  To be honest, I do not know what it is about.
>
> Pavel did.  Check previous mails.

I just see him raising his concerns.  Or do I have to look at some
other thread?

--
Marco

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Tue, Jan 29, 2008 at 10:32:36AM +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> > On Tue, Jan 29, 2008 at 09:38:31AM +0100, Marco Gerards wrote:
> >> Robert Millan <rmh@aybabtu.com> writes:
> >> 
> >> > On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
> >> >> > What do you suggest?
> >> >> 
> >> >> I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
> >> >> needed, it could be removed across the board.
> >> >
> >> > Ok, does this seem fine?
> >> 
> >> Am I missing something, or didn't you explain what GRUB_MOD_GAP is
> >> supposed to do...?  To be honest, I do not know what it is about.
> >
> > Pavel did.  Check previous mails.
> 
> I just see him raising his concerns.  Or do I have to look at some
> other thread?

"Testing on PowerMac G4" for the long explanation.  A short explanation was
committed in a C comment before GRUB_MOD_GAP definition.

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Pavel Roskin]

On Tue, 2008-01-29 at 09:38 +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> > On Mon, Jan 28, 2008 at 08:44:51AM -0500, Pavel Roskin wrote:
> >> > What do you suggest?
> >> 
> >> I would prefer to have GRUB_MOD_GAP is all headers.  Once it's not
> >> needed, it could be removed across the board.
> >
> > Ok, does this seem fine?
> 
> Am I missing something, or didn't you explain what GRUB_MOD_GAP is
> supposed to do...?  To be honest, I do not know what it is about.

If it's not there, PowerMac OpenFirmware would not load the grub image
made by grub-mkimage.  Instead, it will report "CLAIM failed".

The modules use to be loaded at 3 Mb.  Then they were moved directly
after the core (kernel.elf), which created the problem.  Search for the
shortest gap that doesn't result in "CLAIM failed" indicated that it's
0x8000 (32k).  I tried adding more code to kernel.elf, and the gap of
32k was still sufficient.

I assume that if the result of grub-mkimage is a valid ELF file, like
kernel.elf, "CLAIM failed" would not happen.

-- 
Regards,
Pavel Roskin

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Wed, Jan 30, 2008 at 04:42:25PM -0500, Pavel Roskin wrote:
> 
> I assume that if the result of grub-mkimage is a valid ELF file, like
> kernel.elf, "CLAIM failed" would not happen.

But you don't have to assume.  Did you try booting kernel.elf directly?

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Pavel Roskin]

On Wed, 2008-01-30 at 23:03 +0100, Robert Millan wrote:
> On Wed, Jan 30, 2008 at 04:42:25PM -0500, Pavel Roskin wrote:
> > 
> > I assume that if the result of grub-mkimage is a valid ELF file, like
> > kernel.elf, "CLAIM failed" would not happen.
> 
> But you don't have to assume.  Did you try booting kernel.elf directly?

Yes.  It was fine.  No "CLAIM failed".

The only problem is that is was useless because it could not load any
module without "apple" and "hfs".

I only said "I assume" because there is a subtle difference between a
"valid ELF file" and "indistinguishable for kernel.elf".  It should be
valid and acceptable to the OpenFirmware, and the later is a black box
to me.

-- 
Regards,
Pavel Roskin

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Wed, Jan 30, 2008 at 05:15:03PM -0500, Pavel Roskin wrote:
> 
> On Wed, 2008-01-30 at 23:03 +0100, Robert Millan wrote:
> > On Wed, Jan 30, 2008 at 04:42:25PM -0500, Pavel Roskin wrote:
> > > 
> > > I assume that if the result of grub-mkimage is a valid ELF file, like
> > > kernel.elf, "CLAIM failed" would not happen.
> > 
> > But you don't have to assume.  Did you try booting kernel.elf directly?
> 
> Yes.  It was fine.  No "CLAIM failed".
> 
> The only problem is that is was useless because it could not load any
> module without "apple" and "hfs".
> 
> I only said "I assume" because there is a subtle difference between a
> "valid ELF file" and "indistinguishable for kernel.elf".  It should be
> valid and acceptable to the OpenFirmware, and the later is a black box
> to me.

Uhm.. I wonder what'd happen if you pre-link all the stuff into kernel.elf.

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Pavel Roskin]

On Wed, 2008-01-30 at 23:26 +0100, Robert Millan wrote:
> > I only said "I assume" because there is a subtle difference between a
> > "valid ELF file" and "indistinguishable for kernel.elf".  It should be
> > valid and acceptable to the OpenFirmware, and the later is a black box
> > to me.
> 
> Uhm.. I wonder what'd happen if you pre-link all the stuff into kernel.elf.

I tried to prelink "apple" and "hfs" by changing the makefiles.  The
functionality wasn't available.  It makes sense, because the core
doesn't know that the modules are linked.  The modules are never
registered with the core.

It would be nice to have some linker trick to tell the code about the
prelinked modules.  That could make grub-mkimage a very thin wrapper
around "ld -r", perhaps written as a shell script.

-- 
Regards,
Pavel Roskin

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Marco Gerards]

Pavel Roskin <proski@gnu.org> writes:

> On Wed, 2008-01-30 at 23:26 +0100, Robert Millan wrote:
>> > I only said "I assume" because there is a subtle difference between a
>> > "valid ELF file" and "indistinguishable for kernel.elf".  It should be
>> > valid and acceptable to the OpenFirmware, and the later is a black box
>> > to me.
>> 
>> Uhm.. I wonder what'd happen if you pre-link all the stuff into kernel.elf.
>
> I tried to prelink "apple" and "hfs" by changing the makefiles.  The
> functionality wasn't available.  It makes sense, because the core
> doesn't know that the modules are linked.  The modules are never
> registered with the core.
>
> It would be nice to have some linker trick to tell the code about the
> prelinked modules.  That could make grub-mkimage a very thin wrapper
> around "ld -r", perhaps written as a shell script.

We have a trick for grub-emu, a list of init functions that have to be
called is created, a function that does this is created and it is
called when grub-emu is started.

You could do this manually when testing.

--
Marco

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Wed, Jan 30, 2008 at 05:48:14PM -0500, Pavel Roskin wrote:
> On Wed, 2008-01-30 at 23:26 +0100, Robert Millan wrote:
> > > I only said "I assume" because there is a subtle difference between a
> > > "valid ELF file" and "indistinguishable for kernel.elf".  It should be
> > > valid and acceptable to the OpenFirmware, and the later is a black box
> > > to me.
> > 
> > Uhm.. I wonder what'd happen if you pre-link all the stuff into kernel.elf.
> 
> I tried to prelink "apple" and "hfs" by changing the makefiles.  The
> functionality wasn't available.  It makes sense, because the core
> doesn't know that the modules are linked.  The modules are never
> registered with the core.

You can easily work around that by removing the GRUB_MOD_INIT() wrap and
calling the functions manually.

However, to determine if the "CLAIM failed" problem also affects "sane" elven,
you don't need to initialize these modules, just link them in.

> It would be nice to have some linker trick to tell the code about the
> prelinked modules.  That could make grub-mkimage a very thin wrapper
> around "ld -r", perhaps written as a shell script.

Sure it would.  Shouldn't be too hard, geninit.sh does half of what you want
already.

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Yoshinori K. Okuji]

On Thursday 31 January 2008 11:47, Robert Millan wrote:
> > It would be nice to have some linker trick to tell the code about the
> > prelinked modules.  That could make grub-mkimage a very thin wrapper
> > around "ld -r", perhaps written as a shell script.
>
> Sure it would.  Shouldn't be too hard, geninit.sh does half of what you
> want already.

What does it mean? Do you want to use ld from grub-mkimage?

Okuji

Re: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Robert Millan]

On Thu, Jan 31, 2008 at 12:38:01PM +0100, Yoshinori K. Okuji wrote:
> On Thursday 31 January 2008 11:47, Robert Millan wrote:
> > > It would be nice to have some linker trick to tell the code about the
> > > prelinked modules.  That could make grub-mkimage a very thin wrapper
> > > around "ld -r", perhaps written as a shell script.
> >
> > Sure it would.  Shouldn't be too hard, geninit.sh does half of what you
> > want already.
> 
> What does it mean? Do you want to use ld from grub-mkimage?

No, not grub-mkimage.  We were talking about the possibility that a C file
can be used as a module or linked in kernel.elf just by changing the makefiles,
without need to modify the code itself (to get grub_foo_init() working).

(Note that, ia64 patch just submitted uses ld in grub-install;  have a look
if you're concerned about that)

-- 
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: GRUB_MOD_GAP for non-ieee1275 ELF platforms

[Yoshinori K. Okuji]

On Thursday 31 January 2008 13:47, Robert Millan wrote:
> On Thu, Jan 31, 2008 at 12:38:01PM +0100, Yoshinori K. Okuji wrote:
> > On Thursday 31 January 2008 11:47, Robert Millan wrote:
> > > > It would be nice to have some linker trick to tell the code about the
> > > > prelinked modules.  That could make grub-mkimage a very thin wrapper
> > > > around "ld -r", perhaps written as a shell script.
> > >
> > > Sure it would.  Shouldn't be too hard, geninit.sh does half of what you
> > > want already.
> >
> > What does it mean? Do you want to use ld from grub-mkimage?
>
> No, not grub-mkimage.  We were talking about the possibility that a C file
> can be used as a module or linked in kernel.elf just by changing the
> makefiles, without need to modify the code itself (to get grub_foo_init()
> working).
>
> (Note that, ia64 patch just submitted uses ld in grub-install;  have a look
> if you're concerned about that)

Ah, ok. Understood. :)

Thanks,
Okuji

Re: Redefining GRUB_MOD_GAP in init.c

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Sun, Jan 27, 2008 at 10:25:32PM -0500, Pavel Roskin wrote:
>> Hi Robert,
>> 
>> I'm concerned about your latest commit:
>> 
>>         * kern/powerpc/ieee1275/init.c (grub_arch_modules_addr): Skip
>>         `GRUB_MOD_GAP' for platforms in which it's not defined.
>> 
>> The algorithm for calculating the start of the modules should be exactly
>> the same in grub-mkimage and in the core.  Any fallbacks should be
>> coherent if possible.  Therefore, I think it would be better to define
>> GRUB_MOD_GAP only in header files.
>
> Sorry, I should've really sent this for review;  it wasn't such an obvious fix
> as you pointed out.  I'll get that moved to headers as you suggest.
>
>> Also, I don't see how GRUB_MOD_GAP would not be defined to 0x8000 for
>> i386-ieee1275 platform considering that
>> include/grub/i386/ieee1275/kernel.h simply includes
>> include/grub/powerpc/ieee1275/kernel.h
>
> Sounds strange.. I'm wondering that myself.  Will check..
>
>> It's hard for me to understand why kern/powerpc/ieee1275/init.c can be
>> used on any platform other than PowerPC.  I was assuming that my changes
>> would not affect other platforms.
>> 
>> [...]
>> I suggest that i386-ieee1275 stops using any files for PowerPC.
>> kernel.h is not big, so it shouldn't be a problem to copy it.  As for
>> init.c, it should be either copied or moved to a more suitable place.
>
> I think it's safe to say that the only part of kern/powerpc/ieee1275/init.c
> that only works on PowerPC is its name ;-)
>
> Maybe it's time we move out those generic files under powerpc directory.  I
> propose moving the following:
>
> kern/powerpc/ieee1275/init.c
> kern/powerpc/ieee1275/cmain.c
> kern/powerpc/ieee1275/openfw.c
> loader/powerpc/ieee1275/multiboot2.c

Move it where?

--
Marco

Re: Redefining GRUB_MOD_GAP in init.c

[Robert Millan]

On Tue, Jan 29, 2008 at 09:36:41AM +0100, Marco Gerards wrote:
> >
> > Maybe it's time we move out those generic files under powerpc directory.  I
> > propose moving the following:
> >
> > kern/powerpc/ieee1275/init.c
> > kern/powerpc/ieee1275/cmain.c
> > kern/powerpc/ieee1275/openfw.c
> > loader/powerpc/ieee1275/multiboot2.c
> 
> Move it where?

To s,powerpc/,,g

-- 
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: Redefining GRUB_MOD_GAP in init.c

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Tue, Jan 29, 2008 at 09:36:41AM +0100, Marco Gerards wrote:
>> >
>> > Maybe it's time we move out those generic files under powerpc directory.  I
>> > propose moving the following:
>> >
>> > kern/powerpc/ieee1275/init.c
>> > kern/powerpc/ieee1275/cmain.c
>> > kern/powerpc/ieee1275/openfw.c
>> > loader/powerpc/ieee1275/multiboot2.c
>> 
>> Move it where?
>
> To s,powerpc/,,g

Fine for me...

It would be nice if we could switch to SVN or so soon.  SVN is
available on savannah and works almost the same as CVS.  I know there
are better systems to work with, but this is a no brainer I hope.
Moving files is no fun with CVS :-(

What was the outcome of the previous discussions on this?  All these
threads explode with mails on which system is the best, because of
that I ignore the thread and nothing happens... :-)

--
Marco

CVS (Re: Redefining GRUB_MOD_GAP in init.c)

[Robert Millan]

On Tue, Jan 29, 2008 at 10:21:59AM +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> > On Tue, Jan 29, 2008 at 09:36:41AM +0100, Marco Gerards wrote:
> >> >
> >> > Maybe it's time we move out those generic files under powerpc directory.  I
> >> > propose moving the following:
> >> >
> >> > kern/powerpc/ieee1275/init.c
> >> > kern/powerpc/ieee1275/cmain.c
> >> > kern/powerpc/ieee1275/openfw.c
> >> > loader/powerpc/ieee1275/multiboot2.c
> >> 
> >> Move it where?
> >
> > To s,powerpc/,,g
> 
> Fine for me...
> 
> It would be nice if we could switch to SVN or so soon.  SVN is
> available on savannah and works almost the same as CVS.  I know there
> are better systems to work with, but this is a no brainer I hope.
> Moving files is no fun with CVS :-(

I thought SVN wasn't (properly) supported by Savannah.

> What was the outcome of the previous discussions on this?  All these
> threads explode with mails on which system is the best, because of
> that I ignore the thread and nothing happens... :-)

Some people wanted GIT, and Okuji objected, following a long, boring discussion
about the advantages and disadvantages of GIT.

I personaly think we should just ditch CVS, because CVS has these annoying
limitations, but not try to do something fancy.  SVN is the logical
evolution of CVS, so why not just do that.

We can always switch to GIT in the future if/when everybody agrees.

-- 
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: CVS (Re: Redefining GRUB_MOD_GAP in init.c)

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Tue, Jan 29, 2008 at 10:21:59AM +0100, Marco Gerards wrote:
>> Robert Millan <rmh@aybabtu.com> writes:
>> 
>> > On Tue, Jan 29, 2008 at 09:36:41AM +0100, Marco Gerards wrote:
>> >> >
>> >> > Maybe it's time we move out those generic files under powerpc directory.  I
>> >> > propose moving the following:
>> >> >
>> >> > kern/powerpc/ieee1275/init.c
>> >> > kern/powerpc/ieee1275/cmain.c
>> >> > kern/powerpc/ieee1275/openfw.c
>> >> > loader/powerpc/ieee1275/multiboot2.c
>> >> 
>> >> Move it where?
>> >
>> > To s,powerpc/,,g
>> 
>> Fine for me...
>> 
>> It would be nice if we could switch to SVN or so soon.  SVN is
>> available on savannah and works almost the same as CVS.  I know there
>> are better systems to work with, but this is a no brainer I hope.
>> Moving files is no fun with CVS :-(
>
> I thought SVN wasn't (properly) supported by Savannah.

I am not sure: http://savannah.gnu.org/svn/?group=phpgroupware

They seem to use it...

>> What was the outcome of the previous discussions on this?  All these
>> threads explode with mails on which system is the best, because of
>> that I ignore the thread and nothing happens... :-)
>
> Some people wanted GIT, and Okuji objected, following a long, boring discussion
> about the advantages and disadvantages of GIT.

Yes, other people want bzr, others want bzr, etc, etc.  I do not
care.  This way we never switch, which is even worse.

> I personaly think we should just ditch CVS, because CVS has these annoying
> limitations, but not try to do something fancy.  SVN is the logical
> evolution of CVS, so why not just do that.

That's exactly what I was thinking ;-).  I was bothered by the
limitation.  And that we cannot discuss changing to svn first :-)

So what now?

> We can always switch to GIT in the future if/when everybody agrees.

Right!

--
Marco

Re: CVS (Re: Redefining GRUB_MOD_GAP in init.c)

[Yoshinori K. Okuji]

On Tuesday 29 January 2008 12:15, Marco Gerards wrote:
> > I thought SVN wasn't (properly) supported by Savannah.
>
> I am not sure: http://savannah.gnu.org/svn/?group=phpgroupware
>
> They seem to use it...

All I can say is that SVN does not appear in the feature list.

> That's exactly what I was thinking ;-).  I was bothered by the
> limitation.  And that we cannot discuss changing to svn first :-)

I've never objected to SVN. The only problem is that Savannah does not still 
support it (officially, at least).

Okuji

Re: CVS (Re: Redefining GRUB_MOD_GAP in init.c)

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

[...]

> I personaly think we should just ditch CVS, because CVS has these annoying
> limitations, but not try to do something fancy.  SVN is the logical
> evolution of CVS, so why not just do that.

GNU Arch is supported already:
https://savannah.gnu.org/maintenance/GNUArch

It seems subversion is not officially supported yet:
https://savannah.gnu.org/maintenance/WhenSvN

--
Marco

Re: CVS (Re: Redefining GRUB_MOD_GAP in init.c)

[Pavel Roskin]

On Tue, 2008-01-29 at 12:25 +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> [...]
> 
> > I personaly think we should just ditch CVS, because CVS has these annoying
> > limitations, but not try to do something fancy.  SVN is the logical
> > evolution of CVS, so why not just do that.
> 
> GNU Arch is supported already:
> https://savannah.gnu.org/maintenance/GNUArch
> 
> It seems subversion is not officially supported yet:
> https://savannah.gnu.org/maintenance/WhenSvN

Sorry for being late to the discussion.  Subversion is clearly superior
to CVS, but the difference between CVS and Subversion in terms of
project organization is insignificant, at least for a project like GRUB.

Considering that Subversion is not supported by Savannah, I think it's
not worth it to move the repository to another site just to use
Subversion.  

A commit that moves a file from one place to another will be understood
by the converters as a move.

As for git, I made a mirror at git://repo.or.cz/grub2.git that is
updated by a cron job every 2 hours.   I make all my patches in my local
git repository.

git can import from Subversion too.

-- 
Regards,
Pavel Roskin