Re: help with powerpc asm (moving grub_ieee1275_entry_fn initialization to crt0.S)

[Robert Millan <rmh@aybabtu.com>]

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

On Thu, Jan 17, 2008 at 12:30:19PM -0500, Pavel Roskin wrote:
> 
> Oh, I didn't realize that kern/powerpc/ieee1275/cmain.c is actually used
> by the i386-ieee1275 build.  That's misleading!  Perhaps it should be
> moved to kern/ieee1275/

Yes.  I had this in mind, but I think it's better to wait untill the i386
port is complete before we start moving files.

> Anyway, Sparc code doesn't use cmain() in any way.  It starts
> immediately with the function _start() written in C.  So you shouldn't
> worry about breaking Sparc.

Ah, I see.  I didn't expect this.

> Please feel free to integrate the PowerPC part of my patch into your
> patch, as it indeed needs to be committed at once.

Ok.  I'm attaching the complete patch.

> > Another option would be to have just one param.  This would be the cleanest
> > on i386, since the call from OFW already uses -mregparm=1 for the callback
> > address, but I don't know about powerpc.
> 
> I would prefer that we don't choose global CFLAGS to adjust one
> function.  There is a "regparm" attribute that can be used on cmain() on
> i386 architecture.

The CFLAGS for i386-ieee1275 are already set to regparm.  This makes it easier
to interact with OFW in both ways, since call backs also use %eax as first
(and only) parameter.

In fact, our current code seems to assume that grub_ieee1275_entry_fn() always
has the same calling conventions than GRUB.  Because of this, I thought it'd
be consistent to assume the same for our _start routine.

However, powerpc and sparc64 seem to disagree with i386 on which is the
parameter that ought to be used to initialize grub_ieee1275_entry_fn.  I
suppose it's better to move this out of cmain, then..

> But if you want to keep cmain() shared between architectures, then the
> cleanest approach would be to take care of grub_ieee1275_entry_fn before
> cmain() is called.  It doesn't have to be in assembly.  Sparc can still
> use its _start function to set grub_ieee1275_entry_fn and then call
> cmain().

Ok.  That sounds good.

> Arguments are transmitted in registers r3, r4 and r5.  Nothing is passed
> on the stack.  r5 has the data we want to be in grub_ieee1275_entry_fn.
> 
> The first instruction takes the high 16 bits of the
> grub_ieee1275_entry_fn address and put is to register 9.  Register 9 is
> the favorite choice for memory operations, as it has no special purpose
> and doesn't need to be preserved.
> 
> The second instruction places the contents of register 5 into an address
> pointed to by the register 9 plus the lower 16 bits of the 
> grub_ieee1275_entry_fn address.
> 
> A 32-bit constant cannot be loaded in one operation, since all PowerPC
> commands are 32-bit, and it simply won't fit one command.

I see..

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


	PowerPC changes provided by Pavel Roskin.

	* kern/powerpc/ieee1275/cmain.c (cmain): Don't take any arguments.
	* kern/powerpc/ieee1275/crt0.S: Store r5 in grub_ieee1275_entry_fn,
	don't rely on cmain() doing it.
	* kern/i386/ieee1275/startup.S (_start): Store %eax in
	grub_ieee1275_entry_fn, don't rely on cmain() doing it.

diff -ur grub2/kern/i386/ieee1275/startup.S cmain/kern/i386/ieee1275/startup.S
--- grub2/kern/i386/ieee1275/startup.S	2008-01-15 21:05:44.000000000 +0100
+++ cmain/kern/i386/ieee1275/startup.S	2008-01-17 20:05:51.000000000 +0100
@@ -34,5 +34,5 @@
 
 start:
 _start:
-	movl %eax, %ecx
+	movl %eax, EXT_C(grub_ieee1275_entry_fn)
 	jmp EXT_C(cmain)
diff -ur grub2/kern/powerpc/ieee1275/cmain.c cmain/kern/powerpc/ieee1275/cmain.c
--- grub2/kern/powerpc/ieee1275/cmain.c	2008-01-15 17:14:32.000000000 +0100
+++ cmain/kern/powerpc/ieee1275/cmain.c	2008-01-17 20:05:11.000000000 +0100
@@ -1,7 +1,7 @@
 /* cmain.c - Startup code for the PowerPC.  */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003,2004,2005,2006,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,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
@@ -102,12 +102,10 @@
     }
 }
 
-void cmain (uint32_t r3, uint32_t r4, uint32_t r5);
+void cmain (void);
 void
-cmain (UNUSED uint32_t r3, UNUSED uint32_t r4, uint32_t r5)
+cmain (void)
 {
-  grub_ieee1275_entry_fn = (int (*)(void *)) r5;
-
   grub_ieee1275_finddevice ("/chosen", &grub_ieee1275_chosen);
 
   grub_ieee1275_find_options ();
diff -ur grub2/kern/powerpc/ieee1275/crt0.S cmain/kern/powerpc/ieee1275/crt0.S
--- grub2/kern/powerpc/ieee1275/crt0.S	2007-07-22 01:32:27.000000000 +0200
+++ cmain/kern/powerpc/ieee1275/crt0.S	2008-01-17 20:05:04.000000000 +0100
@@ -1,7 +1,7 @@
 /* crt0.S - Startup code for the PowerPC.  */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003,2004,2005,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,2004,2005,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
@@ -38,5 +38,9 @@
 2:	stwu	2, 4(6) /* We know r2 is already 0 from above.  */
 	bdnz	2b
 
+	/* Store r5 in grub_ieee1275_entry_fn.  */
+	lis	9, grub_ieee1275_entry_fn@ha
+	stw	5, grub_ieee1275_entry_fn@l(9)
+
 	bl	cmain
 1:	b	1b