Re: GRUB on OLPC / XO

[Marco Gerards <mgerards@xs4all.nl>]

Robert Millan <rmh@aybabtu.com> writes:

[...]

>> Great!  I'll just review the patch! :-)
>
> Please don't.  It's not meant for review, was just a proof of concept.  The
> patches I actually intend to get merged are being sent separately (see my later
> mails on this).
>
> Oh well, too late.. :-)

Whoops ;-)

>> > +COMMON_ASFLAGS	= -m32 -nostdinc -fno-builtin
>> > +COMMON_CFLAGS	= -ffreestanding -mrtd -mregparm=3
>> > +COMMON_LDFLAGS	= -nostdlib -static -lgcc
>> 
>> Why mregparm?  I don't think we need this for OF?  Do you call
>> assembler functions?
>
> OFW expects %eax to be the first parameter in callbacks.  Other than this,
> I assumed -mregparm is a useful size optimization.  Is that not it?  Why
> do we have it on i386-pc then?

On i386-pc the size restrictions are more important.  On OF the binary
is loaded from the filesystem.

>> >    /* Load pre-loaded modules and free the space.  */
>> >    grub_register_exported_symbols ();
>> > -  grub_load_modules ();
>> > +//  grub_load_modules ();
>> 
>> Why?
>
> I still don't know.  Memory corruption I think.  I'm investigating.

Ah :-)

>> > diff -x '*.mk' -x '*~' -x CVS -x .svn -x configure -x config.h.in -Nurp ../../grub2/kern/powerpc/ieee1275/cmain.c ./kern/powerpc/ieee1275/cmain.c
>> > --- ../../grub2/kern/powerpc/ieee1275/cmain.c	2007-12-30 09:52:05.000000000 +0100
>> > +++ ./kern/powerpc/ieee1275/cmain.c	2008-01-12 03:12:01.000000000 +0100
>> > @@ -58,7 +58,7 @@ grub_ieee1275_find_options (void)
>> >    grub_ieee1275_finddevice ("/options", &options);
>> >    rc = grub_ieee1275_get_property (options, "real-mode?", &realmode,
>> >  				   sizeof realmode, 0);
>> > -  if ((rc >= 0) && realmode)
>> > +//  if ((rc >= 0) && realmode)
>> >      grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_REAL_MODE);
>> 
>> Ehm?
>
> Nothing to see here.  Move along.... :-)

:-)

>> > -  while (i < sizeof (available))
>> > +  while (i < sizeof (available) && available[i])
>> >      {
>> >        grub_uint64_t address;
>> >        grub_uint64_t size;
>> >  
>> > -      address = available[i++];
>> > +      address = grub_be_to_cpu32 (available[i++]);
>> 
>> Why do you do this?  Isn't this information available in native byte order?
>
> See my other mail about IEEE-1275 and endianess.

Which subject?

>> > diff -x '*.mk' -x '*~' -x CVS -x .svn -x configure -x config.h.in -Nurp ../../grub2/term/ieee1275/ofconsole.c ./term/ieee1275/ofconsole.c
>> > --- ../../grub2/term/ieee1275/ofconsole.c	2007-12-25 12:10:47.000000000 +0100
>> > +++ ./term/ieee1275/ofconsole.c	2008-01-12 03:09:02.000000000 +0100
>> > @@ -369,9 +369,6 @@ static struct grub_term grub_ofconsole_t
>> >      .getwh = grub_ofconsole_getwh,
>> >      .gotoxy = grub_ofconsole_gotoxy,
>> >      .cls = grub_ofconsole_cls,
>> > -    .setcolorstate = grub_ofconsole_setcolorstate,
>> > -    .setcolor = grub_ofconsole_setcolor,
>> > -    .getcolor = grub_ofconsole_getcolor,
>> 
>> Why do you do this?
>
> GRUB still wants to set light-gray/black color when run over serial port
> (or OFW terminal which could pipe to serial port).  This is very ugly
> when your serial port is mapped to a black/white gnome-terminal.

I see.

--
Marco