Re: Claim on IEEE1275

Re: Claim on IEEE1275

[Hollis Blanchard]

This patch implements the idea I described: only use "map" in virtual
mode, and report errors instead of treating them as fatal. It fixes
Linux loading on briQ (and most likely Pegasos). May I check it in?

2005-01-02  Hollis Blanchard  <hollis@penguinppc.org>
	* boot/powerpc/ieee1275/cmain.c (grub_ieee1275_realmode): New
	variable.
	(find_options): New function.
	(cmain): Call find_options.
	* include/grub/powerpc/ieee1275/ieee1275.h
	(grub_ieee1275_realmode): New extern.
	* kern/powerpc/ieee1275/openfw.c (grub_claimmap): Only call
	grub_map if grub_ieee1275_realmode is false. Print map failures,
	but return success.

Index: boot/powerpc/ieee1275/cmain.c
===================================================================
RCS file: /cvsroot/grub/grub2/boot/powerpc/ieee1275/cmain.c,v
retrieving revision 1.5
diff -u -p -r1.5 cmain.c
--- boot/powerpc/ieee1275/cmain.c	27 Dec 2004 13:46:20 -0000	1.5
+++ boot/powerpc/ieee1275/cmain.c	2 Jan 2005 17:40:57 -0000
@@ -37,8 +37,21 @@ struct module_info
 intptr_t (*grub_ieee1275_entry_fn) (void *);
 
 grub_uint32_t grub_ieee1275_flags;
+int grub_ieee1275_realmode;
 
 \f
+
+static void
+find_options (void)
+{
+  grub_ieee1275_phandle_t options;
+
+  grub_ieee1275_finddevice ("/options", &options);
+
+  grub_ieee1275_get_property (options, "real-mode?", &grub_ieee1275_realmode,
+			      sizeof (grub_ieee1275_realmode), 0);
+}
+
 /* Setup the argument vector and pass control over to the main
    function.  */
 void
@@ -72,6 +85,8 @@ cmain (uint32_t r3, uint32_t r4 __attrib
       grub_ieee1275_entry_fn = (intptr_t (*)(void *)) r5;
     }
 
+  find_options ();
+
   /* If any argument was passed to the kernel (us), they are
      put in the bootargs property of /chosen.  The string can
      be null (just the nul-character), so check that the size
Index: include/grub/powerpc/ieee1275/ieee1275.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/powerpc/ieee1275/ieee1275.h,v
retrieving revision 1.10
diff -u -p -r1.10 ieee1275.h
--- include/grub/powerpc/ieee1275/ieee1275.h	27 Dec 2004 13:46:20 -0000	1.10
+++ include/grub/powerpc/ieee1275/ieee1275.h	2 Jan 2005 17:40:58 -0000
@@ -66,6 +66,7 @@ typedef intptr_t grub_ieee1275_phandle_t
 
 extern intptr_t (*grub_ieee1275_entry_fn) (void *);
 extern grub_uint32_t grub_ieee1275_flags;
+extern int grub_ieee1275_realmode;
 
 /* Old World firmware fails seek when "dev:0" is opened.  */
 #define GRUB_IEEE1275_NO_PARTITION_0 0x1
Index: kern/powerpc/ieee1275/openfw.c
===================================================================
RCS file: /cvsroot/grub/grub2/kern/powerpc/ieee1275/openfw.c,v
retrieving revision 1.6
diff -u -p -r1.6 openfw.c
--- kern/powerpc/ieee1275/openfw.c	27 Dec 2004 13:46:20 -0000	1.6
+++ kern/powerpc/ieee1275/openfw.c	2 Jan 2005 17:40:58 -0000
@@ -189,10 +189,9 @@ grub_claimmap (grub_addr_t addr, grub_si
 {
   if (grub_ieee1275_claim (addr, size, 0, 0))
     return -1;
-  if (grub_map (addr, addr, size, 0x00))
-    {
-      grub_ieee1275_release (addr, size);
-      return -1;
-    }
+
+  if ((! grub_ieee1275_realmode) && grub_map (addr, addr, size, 0x00))
+    grub_printf ("map failed: address 0x%x, size 0x%x\n", addr, size);
+
   return 0;
 }

Claim on IEEE1275

[Marco Gerards]

Hi,

Some minutes ago I have committed a patch that removes the stack
initialization code.  This seems to work perfectly fine for both the
PegasosII and the newworld mac.  This was required for loading linux
on the PegasosII.  Without this change linux crashed when decompressing
itself.  It's ok for me to restore this code when the problem is
found.  But at the moment this seems like a fine fix.

Only one problem remains for loading linux on the PegasosII.  The
grub_claimmap call fails because the map fails.  I think this is
because the firmware already maps the memory.

Hollis, is the map call really required?  We can better go back to
using grub_ieee1275_claim if it is not.  If it is required, do you
have an idea of how to detect this?

Thanks,
Marco

Re: Claim on IEEE1275

[Hollis Blanchard]

On Dec 28, 2004, at 5:02 PM, Marco Gerards wrote:
>
> Some minutes ago I have committed a patch that removes the stack
> initialization code.  This seems to work perfectly fine for both the
> PegasosII and the newworld mac.  This was required for loading linux
> on the PegasosII.  Without this change linux crashed when decompressing
> itself.  It's ok for me to restore this code when the problem is
> found.  But at the moment this seems like a fine fix.

I would still like to have some idea of the problem we're avoiding. We 
*were* using an 8KB stack (which seems like a lot). Now we use whatever 
firmware gave us. Firmware is required by spec (8.2.2 of the PowerPC 
IEEE1275 binding) to give us at least a 32KB stack, but of course we 
cannot guarantee that all implementations do this correctly. It is 
possible that Linux's gunzip code requires a stack greater than 8KB, 
and that would be good to know (and comment). Would you mind testing 
the old code with larger stack sizes and see if that matters?

I said earlier that I didn't think the old stack initialization code 
was correct, but now I think it may be ok. I have never seen use of the 
"la" mnemonic, but it might do what we want here.

> Only one problem remains for loading linux on the PegasosII.  The
> grub_claimmap call fails because the map fails.  I think this is
> because the firmware already maps the memory.

You can verify this by consulting the "translations" property of 
/chosen/mmu if I recall correctly.

> Hollis, is the map call really required?  We can better go back to
> using grub_ieee1275_claim if it is not.  If it is required, do you
> have an idea of how to detect this?

It is certainly required: how would you like it if you claimed some 
memory to write to but it was never mapped?

There is a "real-mode?" configuration variable, but I think that just 
specifies OF's translation mode, independent of the client's mode (see 
the IEEE1275 PowerPC and CHRP bindings). Knowing the value of that 
variable might be interesting but I wouldn't recommend changing it, as 
Apple firmware in particular is known to die a horrible death by 
mucking with one of those settings (I'm pretty sure it's "real-mode?").

-Hollis

Re: Claim on IEEE1275

[Marco Gerards]

Hollis Blanchard <hollis@penguinppc.org> writes:

> On Dec 28, 2004, at 5:02 PM, Marco Gerards wrote:
>>
>> Some minutes ago I have committed a patch that removes the stack
>> initialization code.  This seems to work perfectly fine for both the
>> PegasosII and the newworld mac.  This was required for loading linux
>> on the PegasosII.  Without this change linux crashed when decompressing
>> itself.  It's ok for me to restore this code when the problem is
>> found.  But at the moment this seems like a fine fix.
>
> I would still like to have some idea of the problem we're avoiding. We
> *were* using an 8KB stack (which seems like a lot). Now we use
> whatever firmware gave us. Firmware is required by spec (8.2.2 of the
> PowerPC IEEE1275 binding) to give us at least a 32KB stack, but of
> course we cannot guarantee that all implementations do this
> correctly. It is possible that Linux's gunzip code requires a stack
> greater than 8KB, and that would be good to know (and comment). Would
> you mind testing the old code with larger stack sizes and see if that
> matters?

I've done that before I removed it.  It did not help.  Perhaps the
problem is caused because stack is in .bss and it is not executable?

> I said earlier that I didn't think the old stack initialization code
> was correct, but now I think it may be ok. I have never seen use of
> the "la" mnemonic, but it might do what we want here.

Ok.  Hopefully Johan can comment on that, he wrote that code.

>> Only one problem remains for loading linux on the PegasosII.  The
>> grub_claimmap call fails because the map fails.  I think this is
>> because the firmware already maps the memory.
>
> You can verify this by consulting the "translations" property of
> /chosen/mmu if I recall correctly.

Ok.

>> Hollis, is the map call really required?  We can better go back to
>> using grub_ieee1275_claim if it is not.  If it is required, do you
>> have an idea of how to detect this?
>
> It is certainly required: how would you like it if you claimed some
> memory to write to but it was never mapped?

I assume it is mapped by claim.  Anyway, yaboot does not use map and
neither does linux, as it seems.

> There is a "real-mode?" configuration variable, but I think that just
> specifies OF's translation mode, independent of the client's mode (see
> the IEEE1275 PowerPC and CHRP bindings). Knowing the value of that
> variable might be interesting but I wouldn't recommend changing it, as
> Apple firmware in particular is known to die a horrible death by
> mucking with one of those settings (I'm pretty sure it's "real-mode?").

On my powerbook it is set to false, while it is set to true on the
pegasos.  So what do you suggest?  Using map when it is set to false?

Thanks,
Marco

Re: Claim on IEEE1275

[Hollis Blanchard]

On Dec 29, 2004, at 5:20 AM, Marco Gerards wrote:
>>> Hollis, is the map call really required?  We can better go back to
>>> using grub_ieee1275_claim if it is not.  If it is required, do you
>>> have an idea of how to detect this?
>>
>> It is certainly required: how would you like it if you claimed some
>> memory to write to but it was never mapped?
>
> I assume it is mapped by claim.

If claim also called map, we would not have needed to add an explicit 
map call. See also the IEEE1275 description of "claim":
	Claim does not automatically create an address translation for the 
allocated resource.
	See 3.6.5.

> Anyway, yaboot does not use map and neither does linux, as it seems.

Linux does not because it terminates OF and takes over the MMU itself.

As for yaboot, it does not run on Old World Macintosh, and described in 
the thread starting at 
http://lists.gnu.org/archive/html/grub-devel/2004-09/msg00014.html, I 
needed to use map on Old World. If it is *only* needed on Old World, 
then we could add another flag to grub_ieee1275_flags.

>> There is a "real-mode?" configuration variable, but I think that just
>> specifies OF's translation mode, independent of the client's mode (see
>> the IEEE1275 PowerPC and CHRP bindings). Knowing the value of that
>> variable might be interesting but I wouldn't recommend changing it, as
>> Apple firmware in particular is known to die a horrible death by
>> mucking with one of those settings (I'm pretty sure it's 
>> "real-mode?").
>
> On my powerbook it is set to false, while it is set to true on the
> pegasos.  So what do you suggest?  Using map when it is set to false?

You said that both the PowerBook and the Pegasos work without the map 
call, so if the briQ works too then I think we should use the approach 
described above.

-Hollis

Re: Claim on IEEE1275

[Marco Gerards]

Hollis Blanchard <hollis@penguinppc.org> writes:

> As for yaboot, it does not run on Old World Macintosh, and described
> in the thread starting at
> http://lists.gnu.org/archive/html/grub-devel/2004-09/msg00014.html, I
> needed to use map on Old World. If it is *only* needed on Old World,
> then we could add another flag to grub_ieee1275_flags.

I agree.

>>> There is a "real-mode?" configuration variable, but I think that just
>>> specifies OF's translation mode, independent of the client's mode (see
>>> the IEEE1275 PowerPC and CHRP bindings). Knowing the value of that
>>> variable might be interesting but I wouldn't recommend changing it, as
>>> Apple firmware in particular is known to die a horrible death by
>>> mucking with one of those settings (I'm pretty sure it's
>>> "real-mode?").
>>
>> On my powerbook it is set to false, while it is set to true on the
>> pegasos.  So what do you suggest?  Using map when it is set to false?
>
> You said that both the PowerBook and the Pegasos work without the map
> call, so if the briQ works too then I think we should use the approach
> described above.

Right.  When will you be able to test this?

Thanks,
Marco

Re: Claim on IEEE1275

[Hollis Blanchard]

On Dec 29, 2004, at 11:36 AM, Marco Gerards wrote:

> Hollis Blanchard <hollis@penguinppc.org> writes:
>
>> As for yaboot, it does not run on Old World Macintosh, and described
>> in the thread starting at
>> http://lists.gnu.org/archive/html/grub-devel/2004-09/msg00014.html, I
>> needed to use map on Old World. If it is *only* needed on Old World,
>> then we could add another flag to grub_ieee1275_flags.
>
> I agree.
>
>>>> There is a "real-mode?" configuration variable, but I think that 
>>>> just
>>>> specifies OF's translation mode, independent of the client's mode 
>>>> (see
>>>> the IEEE1275 PowerPC and CHRP bindings). Knowing the value of that
>>>> variable might be interesting but I wouldn't recommend changing it, 
>>>> as
>>>> Apple firmware in particular is known to die a horrible death by
>>>> mucking with one of those settings (I'm pretty sure it's
>>>> "real-mode?").
>>>
>>> On my powerbook it is set to false, while it is set to true on the
>>> pegasos.  So what do you suggest?  Using map when it is set to false?
>>
>> You said that both the PowerBook and the Pegasos work without the map
>> call, so if the briQ works too then I think we should use the approach
>> described above.
>
> Right.  When will you be able to test this?

I will be able to check Friday.

-Hollis

Re: Claim on IEEE1275

[Hollis Blanchard]

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

On Dec 29, 2004, at 11:11 AM, Hollis Blanchard wrote:
>
>>> There is a "real-mode?" configuration variable, but I think that just
>>> specifies OF's translation mode, independent of the client's mode 
>>> (see
>>> the IEEE1275 PowerPC and CHRP bindings). Knowing the value of that
>>> variable might be interesting but I wouldn't recommend changing it, 
>>> as
>>> Apple firmware in particular is known to die a horrible death by
>>> mucking with one of those settings (I'm pretty sure it's 
>>> "real-mode?").
>>
>> On my powerbook it is set to false, while it is set to true on the
>> pegasos.  So what do you suggest?  Using map when it is set to false?
>
> You said that both the PowerBook and the Pegasos work without the map 
> call, so if the briQ works too then I think we should use the approach 
> described above.

The briQ works without the map, and fails just like the Pegasos with it 
(which shouldn't be surprising given their firmware was written by the 
same people).

I've tested this patch (attached to prevent line-wrapping) on the briQ, 
but there's some other problem (not caused by the patch) that hangs 
after "boot". I'm not going to test Old World now, as there are some 
more fundamental problems there.

The real issue may be that Apple firmware (on your PowerBook) returns 
success when a mapping is requested for an existing translation, but 
Codegen firmware (on the briQ and Pegasos) returns an error. It would 
be interesting to re-implement the "suspend" command we talked about a 
while ago and examine the processor node's "translations" property.

-Hollis

2005-01-01  Hollis Blanchard	 <hollis@penguinppc.org>
	* boot/powerpc/ieee1275/cmain.c (cmain): Set the
	GRUB_IEEE1275_MAP_CLAIM bit in grub_ieee1275_flags.
	* include/grub/powerpc/ieee1275/ieee1275.h (GRUB_IEEE1275_MAP_CLAIM):
	New constant.
	* kern/powerpc/ieee1275/openfw.c (grub_claimmap): Only call grub_map 
if the
	GRUB_IEEE1275_MAP_CLAIM bit is set in grub_ieee1275_flags.


[-- Attachment #2: map-claim.diff --]
[-- Type: application/octet-stream, Size: 2211 bytes --]

? .vimrc
? build
? cscope.in.out
? cscope.out
? cscope.po.out
? map-claim.diff
Index: boot/powerpc/ieee1275/cmain.c
===================================================================
RCS file: /cvsroot/grub/grub2/boot/powerpc/ieee1275/cmain.c,v
retrieving revision 1.5
diff -u -p -r1.5 cmain.c
--- boot/powerpc/ieee1275/cmain.c	27 Dec 2004 13:46:20 -0000	1.5
+++ boot/powerpc/ieee1275/cmain.c	1 Jan 2005 19:53:29 -0000
@@ -56,7 +56,8 @@ cmain (uint32_t r3, uint32_t r4 __attrib
 
       grub_ieee1275_entry_fn = (intptr_t (*)(void *)) r3;
 
-      grub_ieee1275_flags = GRUB_IEEE1275_NO_PARTITION_0;
+      grub_ieee1275_flags = GRUB_IEEE1275_NO_PARTITION_0
+			    | GRUB_IEEE1275_MAP_CLAIM;
 
       /* Old World Open Firmware may use 4M-5M without claiming it.  */
       grub_ieee1275_claim (0x00400000, 0x00100000, 0, 0);
Index: include/grub/powerpc/ieee1275/ieee1275.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/powerpc/ieee1275/ieee1275.h,v
retrieving revision 1.10
diff -u -p -r1.10 ieee1275.h
--- include/grub/powerpc/ieee1275/ieee1275.h	27 Dec 2004 13:46:20 -0000	1.10
+++ include/grub/powerpc/ieee1275/ieee1275.h	1 Jan 2005 19:53:30 -0000
@@ -69,6 +69,8 @@ extern grub_uint32_t grub_ieee1275_flags
 
 /* Old World firmware fails seek when "dev:0" is opened.  */
 #define GRUB_IEEE1275_NO_PARTITION_0 0x1
+/* Is "map" required after "claim"?  */
+#define GRUB_IEEE1275_MAP_CLAIM 0x2
 
 \f
 
Index: kern/powerpc/ieee1275/openfw.c
===================================================================
RCS file: /cvsroot/grub/grub2/kern/powerpc/ieee1275/openfw.c,v
retrieving revision 1.6
diff -u -p -r1.6 openfw.c
--- kern/powerpc/ieee1275/openfw.c	27 Dec 2004 13:46:20 -0000	1.6
+++ kern/powerpc/ieee1275/openfw.c	1 Jan 2005 19:53:32 -0000
@@ -189,7 +189,9 @@ grub_claimmap (grub_addr_t addr, grub_si
 {
   if (grub_ieee1275_claim (addr, size, 0, 0))
     return -1;
-  if (grub_map (addr, addr, size, 0x00))
+
+  if ((grub_ieee1275_flags & GRUB_IEEE1275_MAP_CLAIM)
+      && grub_map (addr, addr, size, 0x00))
     {
       grub_ieee1275_release (addr, size);
       return -1;

Re: Claim on IEEE1275

[Hollis Blanchard]

On Jan 1, 2005, at 2:49 PM, Hollis Blanchard wrote:
>
> I've tested this patch (attached to prevent line-wrapping) on the 
> briQ, but there's some other problem (not caused by the patch) that 
> hangs after "boot". I'm not going to test Old World now, as there are 
> some more fundamental problems there.

It has been pointed out to me that a simpler fix would just be to 
ignore map failures. That sounds better to me... we *are* supposed to 
use map, but the first 32 MB or so are probably already mapped anyways 
on most systems.

Likely the error occurs because "map" is meaningless in real-mode, and 
Codegen chose to return an error in this case. I was told an 
implementation in real-mode *may* provide an mmu node, so things seem 
rather open-ended in this situation.

The patch I sent earlier may get us into trouble if we want to load 
things on New World that happen to fall outside the already-mapped 
region (like large ramdisks). I think a better plan would be:
1) only call map in virtual mode
2) *and* ignore map errors. We could report them as warnings in a 
verbose mode for remote debugging.

-Hollis

Re: Claim on IEEE1275

[Marco Gerards]

Hollis Blanchard <hollis@penguinppc.org> writes:

> It has been pointed out to me that a simpler fix would just be to
> ignore map failures. That sounds better to me... we *are* supposed to
> use map, but the first 32 MB or so are probably already mapped anyways
> on most systems.
>
> Likely the error occurs because "map" is meaningless in real-mode, and
> Codegen chose to return an error in this case. I was told an
> implementation in real-mode *may* provide an mmu node, so things seem
> rather open-ended in this situation.
>
> The patch I sent earlier may get us into trouble if we want to load
> things on New World that happen to fall outside the already-mapped
> region (like large ramdisks). I think a better plan would be:
> 1) only call map in virtual mode

Right.

> 2) *and* ignore map errors. We could report them as warnings in a
> verbose mode for remote debugging.

Ignore them in claimmap you mean, right?  But doesn't #1 fix the
problem already?

--
Marco