Robert Millan wrote:
> Some comments on things that need polishing.  Some are more addressed at one
> of the two lists than the others, but feel free to join in either case.
>
> (also, if you feel this is off-topic in olpc-devel, feel free to ki^W let
> me know)
>
> btw, Mitch mentioned to me on IRC that the ELF loader on XO has some "weird"
> modifications to support Minix.  Is this documented somewhere?
>   

I was mistaken about Minix - there are no Minix-specific hacks in the 
ELF loader.  The only Minix support is a handler for the Minix 
filesystem layout, which doesn't affect the ELF loader.

The ELF loader has two special cases, which are not documented anywhere 
except the source code (cpu/x86/pc/olpc/linux.fth : ?memtest-elf-map-in )  :

a) It recognizes the memtest86 binary that is embedded in OFW, setting 
up some special mappings that memtest requires (or at least used to require)

b) It recognizes the ELF form of the Linux binary and sets up a mapping 
from virtual c0xx.xxxx to physical 00xx.xxxx .  Normally this isn't 
used, because we load the bzImage version of Linux.  But OFW can load 
the ELF portion of the Linux kernel, without the bzImage wrapper.  If 
you do it that way, OFW has access to the ELF symbol table and can thus 
resolve symbolic names for addresses inside Linux.

> On Sat, Jan 12, 2008 at 02:42:30PM +0100, Robert Millan wrote:
>   
>> +void
>> +grub_exit (void)
>> +{
>> +  /* Trap to Open Firmware.  */
>> +  /* FIXME.  */
>> +
>> +  for (;;);
>> +}
>>     
>
> We used to run "trap" insttruction on powerpc.  I assume for exitting via trap
> on i386 we need to generate an interrupt;  I'm just not sure which is the 
> right
> number for it.
>   

To exit to OFW, call the "exit()" client service.

To reboot, call "boot()" with an empty string as the single IN argument.

To power off, call "interpret()" with a cmd string of "power-off".

>   
>>    /* Load pre-loaded modules and free the space.  */
>>    grub_register_exported_symbols ();
>> -  grub_load_modules ();
>> +//  grub_load_modules ();
>>     
>
> This generates an out of bounds exception.  GRUB puts modules above _end
> (0x1000-aligned).  Is access to that address allowed by OFW ?
>   

The ELF loader will map in anything listed in a program header of type 
PT_LOAD , according to the vaddr, memsize, and filesize fields.  If the 
module area is covered by such a program header, they should be accessible.


>  
>   
>>    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);
>>     
>
> This OFW operates in what GRUB calls "real mode" (not to be confused with
> x86 realmode!), but /options/real-mode? doesn't exist.  Should we probe
> for the actual feature (/chosen/mmu IIRC), probe for firmware version, or
> #ifdef it for x86 CPUs?
>   

This OFW operates in virtual mode, but happens to have an identity 
mapping of low memory for convenience.

>   
>> -void cmain (uint32_t r3, uint32_t r4, uint32_t r5);
>>  void
>> -cmain (UNUSED uint32_t r3, UNUSED uint32_t r4, uint32_t r5)
>> +cmain (void)
>>  {
>> -  grub_ieee1275_entry_fn = (int (*)(void *)) r5;
>> -
>>     
>
> This was the only powerpc-specific part of that file, that I can tell.  I
> moved grub_ieee1275_entry_fn initialisation to startup.S.  I'd suggest doing
> the same on powerpc/sparc and move cmain.c out of powerpc/ directory.
>
>
>   
>> diff -x '*.mk' -x '*~' -x CVS -x .svn -x configure -x config.h.in -Nurp 
>> ../../grub2/kern/powerpc/ieee1275/init.c ./kern/powerpc/ieee1275/init.c
>> --- ../../grub2/kern/powerpc/ieee1275/init.c 2008-01-03 23:43:46.000000000 
>> +0100
>> +++ ./kern/powerpc/ieee1275/init.c   2008-01-12 03:26:06.000000000 +0100
>> @@ -52,15 +52,6 @@ grub_millisleep (grub_uint32_t ms)
>>    grub_millisleep_generic (ms);
>>  }
>>  
>> -void
>> -grub_exit (void)
>> -{
>> -  /* Trap to Open Firmware.  */
>> -  asm ("trap");
>> -
>> -  for (;;);
>> -}
>>     
>
> Same here.
>
>   
>>    /* Decode each entry and call `hook'.  */
>>    i = 0;
>> -  while (i < sizeof (available))
>> +  while (i < sizeof (available) && available[i])
>>     
>
> Here we were attempting to claim a region that starts at 0x0 and
> ends at INT_MAX !!  I suppose we just need to check for available property
> length.  Need to look more into this.
>
>   
>> -      address = available[i++];
>> +      address = grub_be_to_cpu32 (available[i++]);
>>        if (address_cells == 2)
>> -    address = (address << 32) | available[i++];
>> +    address = (address << 32) | grub_be_to_cpu32 (available[i++]);
>>  
>> -      size = available[i++];
>> +      size = grub_be_to_cpu32 (available[i++]);
>>        if (size_cells == 2)
>> -    size = (size << 32) | available[i++];
>> +    size = (size << 32) | grub_be_to_cpu32 (available[i++]);
>>     
>
> Integer properties are always in network byte order (thanks Mitch for
> clarifiing).
>
> I wonder if we should byteswap all callers of getprop or just mangle the
> result in the getprop function instead (the latter seems cleaner, but it 
> wasn't
> obvious how to do it without breaking non-integer properties).
>   

The kosher way to decode integer property values is with  (p[0] << 24) | 
(p[1] << 16) | (p[2] << 8) | p[3].  That works on all processors, 
independent of endianness and alignment requirements.

> Ah, and for some reason address_cells is set to a garbage number.  Might be
> a pointer dereference issue, will look into this.
>  
>   
>> --- ../../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,
>>      .setcursor = grub_ofconsole_setcursor,
>>      .refresh = grub_ofconsole_refresh,
>>      .flags = 0,
>>     
>
> Not really a porting issue, but when using serial terminal GRUB still tries to
> set white/black color on it, which becomes very ugly in my black/white
> gnome-terminal :-)
>
>   

_______________________________________________
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel

Reply via email to