On Fri, 28 Feb 2014, BALATON Zoltan wrote:
On Thu, 27 Feb 2014, Mark Cave-Ayland wrote:
FWIW there is a known issue with PPC's next-property iterator being implemented differently to the standard (see Olivier's tentative patch at http://www.openfirmware.info/pipermail/openbios/2014-February/008141.html). It's likely that a broken device tree iteration could be the cause of the MorphOS bootloader not being able to locate a suitable console mode which could be related to your issue.
I think I did not hit this one yet at least the console problem is surely not caused by this but by MorphOS only supporting some specific Radeon GPUs that are found in the PPC Macs it is supporting. It is looking for them by their vendor-id:device-id and fails if not found, but tries to continue anyway.
While the next property iterator is not the cause of the console failed message it may very well be related to the hang in CPU detection later that I'm currently facing. I see the following openbios debug output with DEBUG_CIF enabled in libopenbios/client.c while MorphOS tries to detect the CPU:
finddevice("/cpus") = 0xfff4bbec child(0xfff4bbec) = 0xfff5194c nextprop(0xfff5194c, "", 0x07de7e30) = 1 0x07de7e30 6e __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ n getprop(0xfff5194c, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff5194c, "name", 0x07de7e30) = -1 child(0xfff5194c) = 0x00000000
and here it gives up and freezes. This is with Olivier's patch applied, which does not change anything in the above, I'm seeing this with or without this patch. It seems to be a case of nextprop failing which the patch does not correct. Any hints how to debug it further? I don't know how to test or trace forth so I'd need some help with this.
By the way, if it didn't fail to explore the properties of the CPU it seems to be looking for these on the CPU node:
altivec bus-frequency clock-frequency cpu-version data-streams d-cache-block-size d-cache-line-size d-cache-sets d-cache-size i-cache-block-size i-cache-line-size i-cache-sets i-cache-size performance-monitor reservation-granule-size timebase-frequency tlb-size tlb-sets
then it tries to go down the tree and explore l2-cache and l3-cache nodes (this is the last child call returning NULL in the above). I don't know which of these are optional and which are critical but looking at what QEMU provides now:
name "PowerPC,G4" device_type "cpu" cpu-version c0209 dcache-size 8000 icache-size 8000 dcache-sets 80 icache-sets 80 dcache-block-size 20 icache-block-size 20 timebase-frequency 3b9aca00 clock-frequency 0 state "running" reg 00000000 available 00004000 07c54000 07e10000 f80f0000 00000000 ffffffff translations 00001000 00003000 00001000 00000000 07c58000 001b8000 07c58000 00000000 fff00000 00100000 07f00000 00000000
I expect some problems with these too once we manage to make nextprop work. For a start frequency entries are missing or seem to be wrong.
Regards, BALATON Zoltan
On 01/03/2014 01:49, BALATON Zoltan wrote:
On Fri, 28 Feb 2014, BALATON Zoltan wrote:
On Thu, 27 Feb 2014, Mark Cave-Ayland wrote:
FWIW there is a known issue with PPC's next-property iterator being implemented differently to the standard (see Olivier's tentative patch at http://www.openfirmware.info/pipermail/openbios/2014-February/008141.html). It's likely that a broken device tree iteration could be the cause of the MorphOS bootloader not being able to locate a suitable console mode which could be related to your issue.
I think I did not hit this one yet at least the console problem is surely not caused by this but by MorphOS only supporting some specific Radeon GPUs that are found in the PPC Macs it is supporting. It is looking for them by their vendor-id:device-id and fails if not found, but tries to continue anyway.
While the next property iterator is not the cause of the console failed message it may very well be related to the hang in CPU detection later that I'm currently facing. I see the following openbios debug output with DEBUG_CIF enabled in libopenbios/client.c while MorphOS tries to detect the CPU:
finddevice("/cpus") = 0xfff4bbec child(0xfff4bbec) = 0xfff5194c nextprop(0xfff5194c, "", 0x07de7e30) = 1 0x07de7e30 6e __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ n getprop(0xfff5194c, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff5194c, "name", 0x07de7e30) = -1 child(0xfff5194c) = 0x00000000
and here it gives up and freezes. This is with Olivier's patch applied, which does not change anything in the above, I'm seeing this with or without this patch. It seems to be a case of nextprop failing which the patch does not correct. Any hints how to debug it further? I don't know how to test or trace forth so I'd need some help with this.
I am afraid the patch cannot change anything to that. The nextprop(..."name"...) behavior is very strange.
It is possible to invoke manually "nextprop" from the OpenBIOS prompt :
------------------------------------------------------------------- cd /cpus/PowerPC,G4
\ Empty string : Returns 1 "name"= first property " mmmmmmmmmmmm" drop dup " "(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
\ Nonexistant : Returns -1 "" " mmmmmmmmmmmm" drop dup " mmmmm"(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
\ Property : Returns 1 and the following one : "device_type" " mmmmmmmmmmmm" drop dup " name"(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
\ Last : Returns 0 "" " mmmmmmmmmmmm" drop dup " translations"(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
------------------------------------------------------------------- ... and it works!
Maybe you could try this :
Index: libopenbios/client.c =================================================================== --- libopenbios/client.c (révision 1269) +++ libopenbios/client.c (copie de travail) @@ -189,7 +189,7 @@ memdump(arg2pointer(pb->args[2]), MIN(pb->args[3], pb->args[pb->nargs])); } else if (strcmp(service, "nextprop") == 0) { printk(FMT_prom_arg "\n", pb->args[pb->nargs]); - memdump(arg2pointer(pb->args[2]), pb->args[pb->nargs]); + memdump(arg2pointer(pb->args[2]), 32); } else if (strcmp(service, "setprop") == 0) { printk(FMT_prom_arg "\n", pb->args[pb->nargs]); } else if (strcmp(service, "canon") == 0) { ===================================================================
It will not solve the problem, but it will show if nextprop changes the memory buffer.
"pb->args[pb->nargs]" is wrong, because the return value of nextprop is not the lenght of the property.
Finally, you can also try this :
Index: forth/system/ciface.fs =================================================================== --- forth/system/ciface.fs (révision 1269) +++ forth/system/ciface.fs (copie de travail) @@ -110,18 +110,7 @@ ( buf prev prev_len ) 0 3 pick c!
- \ verify that prev exists (overkill...) - dup if - 2dup r@ get-package-property if - r> 2drop 2drop -1 exit - else - 2drop - then - then - - ( buf prev prev_len ) - - r> next-property if + r> next-property if ( buf name name_len ) dup 1+ -rot ci-strcpy drop 1 else =================================================================== (or next-property-std above if you applied my previous patch)
Maybe it will change something.
Olivier
Hello,
On Sun, 2 Mar 2014, Olivier Danet wrote:
It is possible to invoke manually "nextprop" from the OpenBIOS prompt :
cd /cpus/PowerPC,G4
\ Empty string : Returns 1 "name"= first property " mmmmmmmmmmmm" drop dup " "(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
\ Nonexistant : Returns -1 "" " mmmmmmmmmmmm" drop dup " mmmmm"(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
\ Property : Returns 1 and the following one : "device_type" " mmmmmmmmmmmm" drop dup " name"(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
\ Last : Returns 0 "" " mmmmmmmmmmmm" drop dup " translations"(00)" drop ?active-package " nextprop" client-call-iface . cr . dup cstrlen cr type cr
... and it works!
These commands work but when called by the boot loader it fails.
Maybe you could try this :
memdump(arg2pointer(pb->args[2]), pb->args[pb->nargs]);
memdump(arg2pointer(pb->args[2]), 32);
With this patch it shows that it returns -1 and an empty string:
finddevice("/cpus") = 0xfff4bbe8 child(0xfff4bbe8) = 0xfff51948 nextprop(0xfff51948, "", 0x07de7e30) = 1 0x07de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff51948, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff51948, "name", 0x07de7e30) = -1 0x07de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ child(0xfff51948) = 0x00000000
Finally, you can also try this :
Index: forth/system/ciface.fs
--- forth/system/ciface.fs (révision 1269) +++ forth/system/ciface.fs (copie de travail) @@ -110,18 +110,7 @@ ( buf prev prev_len ) 0 3 pick c!
- \ verify that prev exists (overkill...)
- dup if
- 2dup r@ get-package-property if
r> 2drop 2drop -1 exit- else
2drop- then
- then
- ( buf prev prev_len )
- r> next-property if
- r> next-property if ( buf name name_len ) dup 1+ -rot ci-strcpy drop 1 else
=================================================================== (or next-property-std above if you applied my previous patch)
Maybe it will change something.
It only changes that now it returns 0 but still an empty string:
finddevice("/cpus") = 0xfff4bba8 child(0xfff4bba8) = 0xfff51908 nextprop(0xfff51908, "", 0x07de7e30) = 1 0x07de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff51908, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff51908, "name", 0x07de7e30) = 0 0x07de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ child(0xfff51908) = 0x00000000
Any more ideas?
Thanks, BALATON Zoltan
On 01/03/14 00:49, BALATON Zoltan wrote:
While the next property iterator is not the cause of the console failed message it may very well be related to the hang in CPU detection later that I'm currently facing. I see the following openbios debug output with DEBUG_CIF enabled in libopenbios/client.c while MorphOS tries to detect the CPU:
finddevice("/cpus") = 0xfff4bbec child(0xfff4bbec) = 0xfff5194c nextprop(0xfff5194c, "", 0x07de7e30) = 1 0x07de7e30 6e __ __ __ __ __ __ __ __ __ __ __ __ __ __ __ n getprop(0xfff5194c, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff5194c, "name", 0x07de7e30) = -1 child(0xfff5194c) = 0x00000000
and here it gives up and freezes. This is with Olivier's patch applied, which does not change anything in the above, I'm seeing this with or without this patch. It seems to be a case of nextprop failing which the patch does not correct. Any hints how to debug it further? I don't know how to test or trace forth so I'd need some help with this.
Okay I see what's happening here - the reason Olivier's patch doesn't make a difference is because the MorphOS bootloader is calling the nextprop word via the client interface rather than calling next-property directly by passing a Forth string into OpenBIOS.
If you take a look at section 6.3.2.2 of the IEEE1275 specification then you will find how the nextprop call should work; it's functionality is basically an implementation of the vanilla next-property word with a few extras. You can see the code for this in forth/system/ciface.fs within the OpenBIOS source.
Now the fact that nextprop returns -1 indicates that the following check has failed:
\ verify that prev exists (overkill...) dup if 2dup r@ get-package-property if r> 2drop 2drop -1 exit else 2drop then then
This means that the client requested the next property after "name" but OpenBIOS's get-package-property couldn't find "name" and so returns -1 back to the caller as shown in your trace.
If I had to guess as to why the "name" property couldn't be found by nextprop, it would be that MorphOS is returning the property "name" returned by the previous call to getprop in the subsequent nextprop call but with an extra NULL (0) byte at the end. However you'll need to dig a bit further to see if this is the case or not.
Fortunately OpenBIOS has an in-built source debugger so you should be able to step through the code doing something like this:
- Launch qemu-system-ppc with -prom-env 'auto-boot?=false' -nographic (this disables auto boot and turns off the VGA console)
- Setup debugging the nextprop word using something like this:
0 > cd /openprom/client-services ok 0 > debug nextprop Stepper keys: <space>/<enter> Up Down Trace Rstack Forth ok 0 > cd / ok 0 > boot
You should find that OpenBIOS boots your image and then breaks on nextprop, allowing you to step through the function examining the stack (and input string) to see what is going wrong.
By the way, if it didn't fail to explore the properties of the CPU it seems to be looking for these on the CPU node:
altivec bus-frequency clock-frequency cpu-version data-streams d-cache-block-size d-cache-line-size d-cache-sets d-cache-size i-cache-block-size i-cache-line-size i-cache-sets i-cache-size performance-monitor reservation-granule-size timebase-frequency tlb-size tlb-sets
then it tries to go down the tree and explore l2-cache and l3-cache nodes (this is the last child call returning NULL in the above). I don't know which of these are optional and which are critical but looking at what QEMU provides now:
name "PowerPC,G4" device_type "cpu" cpu-version c0209 dcache-size 8000 icache-size 8000 dcache-sets 80 icache-sets 80 dcache-block-size 20 icache-block-size 20 timebase-frequency 3b9aca00 clock-frequency 0 state "running" reg 00000000 available 00004000 07c54000 07e10000 f80f0000 00000000 ffffffff translations 00001000 00003000 00001000 00000000 07c58000 001b8000 07c58000 00000000 fff00000 00100000 07f00000 00000000
I expect some problems with these too once we manage to make nextprop work. For a start frequency entries are missing or seem to be wrong.
That's highly likely; most of the existing properties have probably been added by enabling DEBUG_CIF as you have done and then copying the values from the example device trees as required. So let's get nextprop fixed first and then we can figure out what is needed later.
ATB,
Mark.
On Sun, 2 Mar 2014, Mark Cave-Ayland wrote:
Now the fact that nextprop returns -1 indicates that the following check has failed:
\ verify that prev exists (overkill...) dup if 2dup r@ get-package-property if r> 2drop 2drop -1 exit else 2drop then then
This means that the client requested the next property after "name" but OpenBIOS's get-package-property couldn't find "name" and so returns -1 back to the caller as shown in your trace.
If I remove this check it returns 0 and an empty string. With debug nextprop (and the above check removed + Olivier's patch) I get the following:
finddevice("/cpus") = 0xfff4bba8 child(0xfff4bba8) = 0xfff51908 nextprop(0xfff51908, "", 0x07de7e30) =
: nextprop ( 7de7e30 7de7e30 fff51908 ) fff4a06c: >r R: ( fff49af0 fff49bec fff383a4 fff4aa8c fff3dba4 0 fff3db54 fffb4214 fff4cb08 0 3 0 fff35164 fffb4214 fff38584 7c58522 100 0 15 16 fff38500 fff37dfc fffb4214 f ff49224 fff448c0 fffb4214 fffb4214 fff4a8b0 fff4a7ac 0 4 c fff35164 fffb4214 ) ( 7de7e30 7de7e30 ) fff4a070: dup ( 7de7e30 7de7e30 7de7e30 ) fff4a074: 0= ( 7de7e30 7de7e30 0 ) fff4a078: do?branch ( 7de7e30 7de7e30 ) fff4a08c: dup ( 7de7e30 7de7e30 7de7e30 ) fff4a090: cstrlen ( 7de7e30 7de7e30 0 ) fff4a094: 0 ( 7de7e30 7de7e30 0 0 ) fff4a098: 3 ( 7de7e30 7de7e30 0 0 3 ) fff4a09c: pick ( 7de7e30 7de7e30 0 0 7de7e30 ) fff4a0a0: c! ( 7de7e30 7de7e30 0 ) fff4a0a4: r> ( 7de7e30 7de7e30 0 fff51908 ) fff4a0a8: next-property-std ( 7de7e30 fff5197c 4 ffffffff ) fff4a0ac: do?branch ( 7de7e30 fff5197c 4 ) fff4a0b4: dup ( 7de7e30 fff5197c 4 4 ) fff4a0b8: 1+ ( 7de7e30 fff5197c 4 5 ) fff4a0bc: -rot ( 7de7e30 5 fff5197c 4 ) fff4a0c0: ci-strcpy ( 4 ) fff4a0c4: drop ( Empty ) fff4a0c8: 1 ( 1 ) fff4a0cc: dobranch ( 1 ) fff4a0dc: (semis) [ Finished nextprop ] 1
0x07de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff51908, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff51908, "name", 0x07de7e30) =
: nextprop ( 7de7e30 7de7e30 fff51908 ) fff4a06c: >r R: ( fff49af0 fff49bec fff383a4 fff4aa8c fff3dba4 0 fff3db54 fffb4214 fff4cb08 0 3 0 fff35164 fffb4214 fff38584 7c58522 100 0 15 16 fff38500 fff37dfc fffb4214 f ff49224 fff448c0 fffb4214 fffb4214 fff4a8b0 fff4a7ac 0 4 c fff35164 fffb4214 ) ( 7de7e30 7de7e30 ) fff4a070: dup ( 7de7e30 7de7e30 7de7e30 ) fff4a074: 0= ( 7de7e30 7de7e30 0 ) fff4a078: do?branch ( 7de7e30 7de7e30 ) fff4a08c: dup ( 7de7e30 7de7e30 7de7e30 ) fff4a090: cstrlen ( 7de7e30 7de7e30 4 ) fff4a094: 0 ( 7de7e30 7de7e30 4 0 ) fff4a098: 3 ( 7de7e30 7de7e30 4 0 3 ) fff4a09c: pick ( 7de7e30 7de7e30 4 0 7de7e30 ) fff4a0a0: c! ( 7de7e30 7de7e30 4 ) fff4a0a4: r> ( 7de7e30 7de7e30 4 fff51908 ) fff4a0a8: next-property-std ( 7de7e30 0 ) fff4a0ac: do?branch ( 7de7e30 ) fff4a0d4: drop ( Empty ) fff4a0d8: 0 ( 0 ) fff4a0dc: (semis) [ Finished nextprop ] 0
0x07de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ child(0xfff51908) = 0x00000000
You should find that OpenBIOS boots your image and then breaks on nextprop, allowing you to step through the function examining the stack (and input string) to see what is going wrong.
I hope the above make sense to you and helps finding why it does not work.
Regards, BALATON Zoltan
On 03/03/14 13:05, BALATON Zoltan wrote:
On Sun, 2 Mar 2014, Mark Cave-Ayland wrote:
Now the fact that nextprop returns -1 indicates that the following check has failed:
\ verify that prev exists (overkill...) dup if 2dup r@ get-package-property if r> 2drop 2drop -1 exit else 2drop then then
This means that the client requested the next property after "name" but OpenBIOS's get-package-property couldn't find "name" and so returns -1 back to the caller as shown in your trace.
If I remove this check it returns 0 and an empty string. With debug nextprop (and the above check removed + Olivier's patch) I get the following:
(cut)
I hope the above make sense to you and helps finding why it does not work.
Ah that's why this looks a little strange to me. Can you try with just plain SVN trunk (without Olivier's patch) to make sure that doesn't have an effect too?
The important part is to view the incoming parameters to the function; from your output you show this:
: nextprop ( 7de7e30 7de7e30 fff51908 )
Compare this with the stack diagram in ciface.fs which shows this:
: nextprop ( buf prev phandle -- 1|0|-1 )
According to the IEEE1275 specification, prev should be a NULL-terminated string of the previous property ("name" in this case) while buf should be a buffer into which the next property name is copied.
During debug, you can press "f" to drop to a Forth console and then display memory using the dump command. For example the following will show you what's in the incoming buffer:
7de7e30 20 dump
You can then type "resume" in order to return to the debugger.
I wonder if the problem could be caused by the incoming property address and the outgoing property address being the same? It may be that MorphOS relies on an implementation-specific behaviour that means this just happens to work on PPC Open Firmware.
ATB,
Mark.
On Mon, 3 Mar 2014, Mark Cave-Ayland wrote:
Ah that's why this looks a little strange to me. Can you try with just plain SVN trunk (without Olivier's patch) to make sure that doesn't have an effect too?
Here it is with SVN trunk with only DEBUG_CIF and the memdump length changed for nextprop in debug code:
nextprop(0xfff517e4, "name", 0x07de7e30) =
: nextprop ( 7de7e30 7de7e30 fff517e4 ) fff49f08: >r 3 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 fff517e4 ) fff49f08: >r ( 7de7e30 7de7e30 ) fff49f0c: dup ( 7de7e30 7de7e30 7de7e30 ) fff49f10: 0= ( 7de7e30 7de7e30 0 ) fff49f14: do?branch ( 7de7e30 7de7e30 ) fff49f28: dup ( 7de7e30 7de7e30 7de7e30 ) fff49f2c: cstrlen 3 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 7de7e30 ) fff49f2c: cstrlen ( 7de7e30 7de7e30 4 ) fff49f30: 0 3 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 4 ) fff49f30: 0 ( 7de7e30 7de7e30 4 0 ) fff49f34: 3 ( 7de7e30 7de7e30 4 0 3 ) fff49f38: pick ( 7de7e30 7de7e30 4 0 7de7e30 ) fff49f3c: c! 5 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 5 > resume ok ( 7de7e30 7de7e30 4 0 7de7e30 ) fff49f3c: c! ( 7de7e30 7de7e30 4 ) fff49f40: dup 3 > 7de7e30 20 dump 7de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 4 ) fff49f40: dup ( 7de7e30 7de7e30 4 4 ) fff49f44: do?branch ( 7de7e30 7de7e30 4 ) fff49f4c: 2dup ( 7de7e30 7de7e30 4 7de7e30 4 ) fff49f50: r@ ( 7de7e30 7de7e30 4 7de7e30 4 fff517e4 ) fff49f54: get-package-property ( 7de7e30 7de7e30 4 ffffffff ) fff49f58: do?branch ( 7de7e30 7de7e30 4 ) fff49f60: r> ( 7de7e30 7de7e30 4 fff517e4 ) fff49f64: 2drop ( 7de7e30 7de7e30 ) fff49f68: 2drop ( Empty ) fff49f6c: -1 ( ffffffff ) fff49f70: exit -1
0x07de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ child(0xfff517e4) = 0x00000000
I wonder if the problem could be caused by the incoming property address and the outgoing property address being the same? It may be that MorphOS relies on an implementation-specific behaviour that means this just happens to work on PPC Open Firmware.
It can be. It looks like that the buffer is cleared before it gets used by later code. Do you have any idea how to fix this?
Regards, BALATON Zoltan
On 03/03/14 15:35, BALATON Zoltan wrote:
On Mon, 3 Mar 2014, Mark Cave-Ayland wrote:
Ah that's why this looks a little strange to me. Can you try with just plain SVN trunk (without Olivier's patch) to make sure that doesn't have an effect too?
Here it is with SVN trunk with only DEBUG_CIF and the memdump length changed for nextprop in debug code:
nextprop(0xfff517e4, "name", 0x07de7e30) =
: nextprop ( 7de7e30 7de7e30 fff517e4 ) fff49f08: >r 3 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 fff517e4 ) fff49f08: >r ( 7de7e30 7de7e30 ) fff49f0c: dup ( 7de7e30 7de7e30 7de7e30 ) fff49f10: 0= ( 7de7e30 7de7e30 0 ) fff49f14: do?branch ( 7de7e30 7de7e30 ) fff49f28: dup ( 7de7e30 7de7e30 7de7e30 ) fff49f2c: cstrlen 3 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 7de7e30 ) fff49f2c: cstrlen ( 7de7e30 7de7e30 4 ) fff49f30: 0 3 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 4 ) fff49f30: 0 ( 7de7e30 7de7e30 4 0 ) fff49f34: 3 ( 7de7e30 7de7e30 4 0 3 ) fff49f38: pick ( 7de7e30 7de7e30 4 0 7de7e30 ) fff49f3c: c! 5 > 7de7e30 20 dump 7de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 5 > resume ok ( 7de7e30 7de7e30 4 0 7de7e30 ) fff49f3c: c! ( 7de7e30 7de7e30 4 ) fff49f40: dup 3 > 7de7e30 20 dump
Bingo! This is the problem: "0 3 pick c!" writes a 0 byte into the return buffer before calling get-package-property which is why it always fails when buf == prev (as prev always becomes an empty string and so never matches).
7de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 7de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ ok 3 > resume ok ( 7de7e30 7de7e30 4 ) fff49f40: dup ( 7de7e30 7de7e30 4 4 ) fff49f44: do?branch ( 7de7e30 7de7e30 4 ) fff49f4c: 2dup ( 7de7e30 7de7e30 4 7de7e30 4 ) fff49f50: r@ ( 7de7e30 7de7e30 4 7de7e30 4 fff517e4 ) fff49f54: get-package-property ( 7de7e30 7de7e30 4 ffffffff ) fff49f58: do?branch ( 7de7e30 7de7e30 4 ) fff49f60: r> ( 7de7e30 7de7e30 4 fff517e4 ) fff49f64: 2drop ( 7de7e30 7de7e30 ) fff49f68: 2drop ( Empty ) fff49f6c: -1 ( ffffffff ) fff49f70: exit -1
0x07de7e30 00 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 .ame............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ child(0xfff517e4) = 0x00000000
I wonder if the problem could be caused by the incoming property address and the outgoing property address being the same? It may be that MorphOS relies on an implementation-specific behaviour that means this just happens to work on PPC Open Firmware.
It can be. It looks like that the buffer is cleared before it gets used by later code. Do you have any idea how to fix this?
Can you try the attached patch? I think what the "0 3 pick c!" line was supposed to do was set an empty string as the fallback return value in buf for nextprop.
According to the IEEE1275 specification, return values of -1 and 0 should return an empty string, so what I've done is move the write of the 0 length string into buf to the two points in code just before nextprop exits with these return codes.
ATB,
Mark.
On Mon, 3 Mar 2014, Mark Cave-Ayland wrote:
Can you try the attached patch? I think what the "0 3 pick c!" line was supposed to do was set an empty string as the fallback return value in buf for nextprop.
See my other message where I came up with a patch identical to half of yours and it seems to work.
According to the IEEE1275 specification, return values of -1 and 0 should return an empty string, so what I've done is move the write of the 0 length string into buf to the two points in code just before nextprop exits with these return codes.
OK, I didn't care about the -1 case so your patch should be the correct one.
Thanks, BALATON Zoltan
On 03/03/14 16:33, BALATON Zoltan wrote:
On Mon, 3 Mar 2014, Mark Cave-Ayland wrote:
Can you try the attached patch? I think what the "0 3 pick c!" line was supposed to do was set an empty string as the fallback return value in buf for nextprop.
See my other message where I came up with a patch identical to half of yours and it seems to work.
According to the IEEE1275 specification, return values of -1 and 0 should return an empty string, so what I've done is move the write of the 0 length string into buf to the two points in code just before nextprop exits with these return codes.
OK, I didn't care about the -1 case so your patch should be the correct one.
Yes indeed, it looks as if our patches have crossed in the mail. I've just tested all my PPC/SPARC64 images and see no regressions so I've committed my version of the patch to SVN trunk. Thanks a lot for the bug report and time spent tracking this done!
ATB,
Mark.
On Mon, 3 Mar 2014, BALATON Zoltan wrote:
It can be. It looks like that the buffer is cleared before it gets used by later code. Do you have any idea how to fix this?
OK, how about this patch:
Index: forth/system/ciface.fs =================================================================== --- forth/system/ciface.fs (revision 1270) +++ forth/system/ciface.fs (working copy) @@ -108,7 +108,6 @@ dup 0= if 0 else dup cstrlen then
( buf prev prev_len ) - 0 3 pick c!
\ verify that prev exists (overkill...) dup if @@ -126,7 +125,7 @@ dup 1+ -rot ci-strcpy drop 1 else ( buf ) - drop 0 + 0 swap c! 0 then ;
with this it gets further (and then fails like before, assumably because of missing/wrong properties but it seems to get over the nextprop issue at least). Here is the debug log I get now with SVN trunk and the above patch (without Olivier's):
finddevice("/uni-n") = 0xfff512f4 finddevice("/cpus") = 0xfff4ba7c child(0xfff4ba7c) = 0xfff517dc nextprop(0xfff517dc, "", 0x07de7e30) = 1 0x07de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff517dc, "name", 0x07de7e30) = 1 0x07de7e30 64 65 76 69 63 65 5f 74 79 70 65 00 00 00 00 00 device_type..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "device_type", 0x07de7df0, 64) = 4 0x07de7df0 63 70 75 00 __ __ __ __ __ __ __ __ __ __ __ __ cpu. nextprop(0xfff517dc, "device_type", 0x07de7e30) = 1 0x07de7e30 63 70 75 2d 76 65 72 73 69 6f 6e 00 00 00 00 00 cpu-version..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "cpu-version", 0x07de7df0, 64) = 4 0x07de7df0 00 0c 02 09 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "cpu-version", 0x07de7e30) = 1 0x07de7e30 64 63 61 63 68 65 2d 73 69 7a 65 00 00 00 00 00 dcache-size..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "dcache-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 80 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "dcache-size", 0x07de7e30) = 1 0x07de7e30 69 63 61 63 68 65 2d 73 69 7a 65 00 00 00 00 00 icache-size..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "icache-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 80 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "icache-size", 0x07de7e30) = 1 0x07de7e30 64 63 61 63 68 65 2d 73 65 74 73 00 00 00 00 00 dcache-sets..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "dcache-sets", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 80 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "dcache-sets", 0x07de7e30) = 1 0x07de7e30 69 63 61 63 68 65 2d 73 65 74 73 00 00 00 00 00 icache-sets..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "icache-sets", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 80 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "icache-sets", 0x07de7e30) = 1 0x07de7e30 64 63 61 63 68 65 2d 62 6c 6f 63 6b 2d 73 69 7a dcache-block-siz 0x07de7e40 65 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e............... getprop(0xfff517dc, "dcache-block-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 20 __ __ __ __ __ __ __ __ __ __ __ __ ... nextprop(0xfff517dc, "dcache-block-size", 0x07de7e30) = 1 0x07de7e30 69 63 61 63 68 65 2d 62 6c 6f 63 6b 2d 73 69 7a icache-block-siz 0x07de7e40 65 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e............... getprop(0xfff517dc, "icache-block-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 20 __ __ __ __ __ __ __ __ __ __ __ __ ... nextprop(0xfff517dc, "icache-block-size", 0x07de7e30) = 1 0x07de7e30 74 69 6d 65 62 61 73 65 2d 66 72 65 71 75 65 6e timebase-frequen 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "timebase-frequency", 0x07de7df0, 64) = 4 0x07de7df0 3b 9a ca 00 __ __ __ __ __ __ __ __ __ __ __ __ ;... nextprop(0xfff517dc, "timebase-frequency", 0x07de7e30) = 1 0x07de7e30 63 6c 6f 63 6b 2d 66 72 65 71 75 65 6e 63 79 00 clock-frequency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "clock-frequency", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "clock-frequency", 0x07de7e30) = 1 0x07de7e30 73 74 61 74 65 00 66 72 65 71 75 65 6e 63 79 00 state.frequency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "state", 0x07de7df0, 64) = 8 0x07de7df0 72 75 6e 6e 69 6e 67 00 __ __ __ __ __ __ __ __ running. nextprop(0xfff517dc, "state", 0x07de7e30) = 1 0x07de7e30 72 65 67 00 65 00 66 72 65 71 75 65 6e 63 79 00 reg.e.frequency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "reg", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "reg", 0x07de7e30) = 1 0x07de7e30 61 76 61 69 6c 61 62 6c 65 00 75 65 6e 63 79 00 available.uency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "available", 0x07de7df0, 64) = 40 0x07de7df0 00 00 40 00 00 3f c0 00 00 68 72 4c 07 5d 0d b4 ..@..?...hrL.].. 0x07de7e00 07 e1 00 00 78 1f 00 00 81 00 00 00 7e f0 00 00 ....x.......~... 0x07de7e10 00 00 00 00 ff ff ff ff __ __ __ __ __ __ __ __ ........ nextprop(0xfff517dc, "available", 0x07de7e30) = 1 0x07de7e30 74 72 61 6e 73 6c 61 74 69 6f 6e 73 00 63 79 00 translations.cy. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "translations", 0x07de7df0, 64) = 80 0x07de7df0 00 00 10 00 00 00 30 00 00 00 10 00 00 00 00 00 ......0......... 0x07de7e00 00 40 00 00 00 28 80 00 00 40 00 00 00 00 00 02 .@...(...@...... 0x07de7e10 07 c5 80 00 00 1b 80 00 07 c5 80 00 00 00 00 00 ................ 0x07de7e20 80 00 00 00 01 00 00 00 80 00 00 00 00 00 00 6a ...............j nextprop(0xfff517dc, "translations", 0x07de7e30) = 0 0x07de7e30 00 72 61 6e 73 6c 61 74 69 6f 6e 73 00 63 79 00 .ranslations.cy. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. child(0xfff517dc) = 0x00000000
Regards, BALATON Zoltan
On 03/03/14 16:20, BALATON Zoltan wrote:
(cut)
with this it gets further (and then fails like before, assumably because of missing/wrong properties but it seems to get over the nextprop issue at least). Here is the debug log I get now with SVN trunk and the above patch (without Olivier's):
finddevice("/uni-n") = 0xfff512f4 finddevice("/cpus") = 0xfff4ba7c child(0xfff4ba7c) = 0xfff517dc nextprop(0xfff517dc, "", 0x07de7e30) = 1 0x07de7e30 6e 61 6d 65 00 00 00 00 00 00 00 00 00 00 00 00 name............ 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "name", 0x07de7df0, 64) = 11 0x07de7df0 50 6f 77 65 72 50 43 2c 47 34 00 __ __ __ __ __ PowerPC,G4. nextprop(0xfff517dc, "name", 0x07de7e30) = 1 0x07de7e30 64 65 76 69 63 65 5f 74 79 70 65 00 00 00 00 00 device_type..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "device_type", 0x07de7df0, 64) = 4 0x07de7df0 63 70 75 00 __ __ __ __ __ __ __ __ __ __ __ __ cpu. nextprop(0xfff517dc, "device_type", 0x07de7e30) = 1 0x07de7e30 63 70 75 2d 76 65 72 73 69 6f 6e 00 00 00 00 00 cpu-version..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "cpu-version", 0x07de7df0, 64) = 4 0x07de7df0 00 0c 02 09 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "cpu-version", 0x07de7e30) = 1 0x07de7e30 64 63 61 63 68 65 2d 73 69 7a 65 00 00 00 00 00 dcache-size..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "dcache-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 80 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "dcache-size", 0x07de7e30) = 1 0x07de7e30 69 63 61 63 68 65 2d 73 69 7a 65 00 00 00 00 00 icache-size..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "icache-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 80 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "icache-size", 0x07de7e30) = 1 0x07de7e30 64 63 61 63 68 65 2d 73 65 74 73 00 00 00 00 00 dcache-sets..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "dcache-sets", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 80 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "dcache-sets", 0x07de7e30) = 1 0x07de7e30 69 63 61 63 68 65 2d 73 65 74 73 00 00 00 00 00 icache-sets..... 0x07de7e40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ getprop(0xfff517dc, "icache-sets", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 80 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "icache-sets", 0x07de7e30) = 1 0x07de7e30 64 63 61 63 68 65 2d 62 6c 6f 63 6b 2d 73 69 7a dcache-block-siz 0x07de7e40 65 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e............... getprop(0xfff517dc, "dcache-block-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 20 __ __ __ __ __ __ __ __ __ __ __ __ ... nextprop(0xfff517dc, "dcache-block-size", 0x07de7e30) = 1 0x07de7e30 69 63 61 63 68 65 2d 62 6c 6f 63 6b 2d 73 69 7a icache-block-siz 0x07de7e40 65 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e............... getprop(0xfff517dc, "icache-block-size", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 20 __ __ __ __ __ __ __ __ __ __ __ __ ... nextprop(0xfff517dc, "icache-block-size", 0x07de7e30) = 1 0x07de7e30 74 69 6d 65 62 61 73 65 2d 66 72 65 71 75 65 6e timebase-frequen 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "timebase-frequency", 0x07de7df0, 64) = 4 0x07de7df0 3b 9a ca 00 __ __ __ __ __ __ __ __ __ __ __ __ ;... nextprop(0xfff517dc, "timebase-frequency", 0x07de7e30) = 1 0x07de7e30 63 6c 6f 63 6b 2d 66 72 65 71 75 65 6e 63 79 00 clock-frequency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "clock-frequency", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "clock-frequency", 0x07de7e30) = 1 0x07de7e30 73 74 61 74 65 00 66 72 65 71 75 65 6e 63 79 00 state.frequency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "state", 0x07de7df0, 64) = 8 0x07de7df0 72 75 6e 6e 69 6e 67 00 __ __ __ __ __ __ __ __ running. nextprop(0xfff517dc, "state", 0x07de7e30) = 1 0x07de7e30 72 65 67 00 65 00 66 72 65 71 75 65 6e 63 79 00 reg.e.frequency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "reg", 0x07de7df0, 64) = 4 0x07de7df0 00 00 00 00 __ __ __ __ __ __ __ __ __ __ __ __ .... nextprop(0xfff517dc, "reg", 0x07de7e30) = 1 0x07de7e30 61 76 61 69 6c 61 62 6c 65 00 75 65 6e 63 79 00 available.uency. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "available", 0x07de7df0, 64) = 40 0x07de7df0 00 00 40 00 00 3f c0 00 00 68 72 4c 07 5d 0d b4 ..@..?...hrL.].. 0x07de7e00 07 e1 00 00 78 1f 00 00 81 00 00 00 7e f0 00 00 ....x.......~... 0x07de7e10 00 00 00 00 ff ff ff ff __ __ __ __ __ __ __ __ ........ nextprop(0xfff517dc, "available", 0x07de7e30) = 1 0x07de7e30 74 72 61 6e 73 6c 61 74 69 6f 6e 73 00 63 79 00 translations.cy. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. getprop(0xfff517dc, "translations", 0x07de7df0, 64) = 80 0x07de7df0 00 00 10 00 00 00 30 00 00 00 10 00 00 00 00 00 ......0......... 0x07de7e00 00 40 00 00 00 28 80 00 00 40 00 00 00 00 00 02 .@...(...@...... 0x07de7e10 07 c5 80 00 00 1b 80 00 07 c5 80 00 00 00 00 00 ................ 0x07de7e20 80 00 00 00 01 00 00 00 80 00 00 00 00 00 00 6a ...............j nextprop(0xfff517dc, "translations", 0x07de7e30) = 0 0x07de7e30 00 72 61 6e 73 6c 61 74 69 6f 6e 73 00 63 79 00 .ranslations.cy. 0x07de7e40 63 79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 cy.............. child(0xfff517dc) = 0x00000000
Yes indeed, this looks much better - please let us know how you get on further with your MorphOS work! :)
ATB,
Mark.
On Mon, 3 Mar 2014, Mark Cave-Ayland wrote:
Yes indeed, this looks much better - please let us know how you get on further with your MorphOS work! :)
Here is some progress. It turns out that I get much farther with the patch at the end of this message. This corrects the names of CPU cache parameters that seem to be missing a dash compared to the device tree dump I've referred to before and this is why MorphOS could not find them while looking for them. Apart from that I also had to change the model string to a newer one that is supported by MorphOS for it to continue and it seems that it looks for the name of the / node and expects it to be called device-tree on a Mac so I had to change this too. With these it gets over its initial boot and starts to initialise devices (I also got some messages on the debug serial port). Then it hits an exception again that seems to be due to some memory corruption like the 0x80 issue right at the beginning that prevents it from booting still. This time it may be a null pointer issue but I'll need to debug it further to know what's happening. This is how far it gets now:
IN: 0x00441bc8: lwz r0,0(r4) 0x00441bcc: stw r0,0(r3) 0x00441bd0: lwz r9,4(r4) 0x00441bd4: stw r9,4(r3) 0x00441bd8: lwz r0,8(r4) 0x00441bdc: stw r0,8(r3) 0x00441be0: lwz r9,12(r4) 0x00441be4: stw r9,12(r3) 0x00441be8: lwz r0,16(r4) 0x00441bec: stw r0,16(r3) 0x00441bf0: lwz r9,20(r4) 0x00441bf4: stw r9,20(r3) 0x00441bf8: lwz r0,24(r4) 0x00441bfc: stw r0,24(r3) 0x00441c00: lwz r9,28(r4) 0x00441c04: stw r9,28(r3) 0x00441c08: addi r4,r4,32 0x00441c0c: addi r3,r3,32 0x00441c10: bdnz+ 0x441bc8
Raise exception at 00441bcc => 00000002 (00) IN: 0x00000300: b 0xffffc3a0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffc3a0 0 IN: 0xffffc3a0: .long 0x0
Raise exception at ffffc3a4 => 00000006 (21) IN: 0x00000700: mtsprg 2,r2 0x00000704: li r2,7 0x00000708: b 0xffffe0f0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffe0f0 0 IN: 0xffffe0f0: .long 0x0
Raise exception at ffffe0f4 => 00000006 (21) Raise exception at ffffe0f4 => 00000006 (21)
Something seems to overwrite the vector at 0x300 (which was set to 0x238c before this point) but the new value seems to point to the wrong place.
What do you think about the patch below?
Regards, BALATON Zoltan
Index: arch/ppc/qemu/init.c =================================================================== --- arch/ppc/qemu/init.c (revision 1271) +++ arch/ppc/qemu/init.c (working copy) @@ -241,32 +241,32 @@
PUSH(cpu->dcache_size); fword("encode-int"); - push_str("dcache-size"); + push_str("d-cache-size"); fword("property");
PUSH(cpu->icache_size); fword("encode-int"); - push_str("icache-size"); + push_str("i-cache-size"); fword("property");
PUSH(cpu->dcache_sets); fword("encode-int"); - push_str("dcache-sets"); + push_str("d-cache-sets"); fword("property");
PUSH(cpu->icache_sets); fword("encode-int"); - push_str("icache-sets"); + push_str("i-cache-sets"); fword("property");
PUSH(cpu->dcache_block_size); fword("encode-int"); - push_str("dcache-block-size"); + push_str("d-cache-block-size"); fword("property");
PUSH(cpu->icache_block_size); fword("encode-int"); - push_str("icache-block-size"); + push_str("i-cache-block-size"); fword("property");
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); @@ -745,12 +745,12 @@
/* model */
- push_str("PowerMac2,1"); + push_str("PowerMac3,1"); fword("model");
/* compatible */
- push_str("PowerMac2,1"); + push_str("PowerMac3,1"); fword("encode-string"); push_str("MacRISC"); fword("encode-string"); Index: forth/device/tree.fs =================================================================== --- forth/device/tree.fs (revision 1271) +++ forth/device/tree.fs (working copy) @@ -11,7 +11,7 @@
\ root node new-device - " OpenBiosTeam,OpenBIOS" device-name + " device-tree" device-name 1 encode-int " #address-cells" property : open true ; : close ;
On Tue, 4 Mar 2014, BALATON Zoltan wrote:
Raise exception at 00441bcc => 00000002 (00) IN: 0x00000300: b 0xffffc3a0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffc3a0 0 IN: 0xffffc3a0: .long 0x0
Raise exception at ffffc3a4 => 00000006 (21) IN: 0x00000700: mtsprg 2,r2 0x00000704: li r2,7 0x00000708: b 0xffffe0f0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffe0f0 0 IN: 0xffffe0f0: .long 0x0
Raise exception at ffffe0f4 => 00000006 (21) Raise exception at ffffe0f4 => 00000006 (21)
Something seems to overwrite the vector at 0x300 (which was set to 0x238c before this point) but the new value seems to point to the wrong place.
It seems that this is happening when MorphOS tries to install its own exception handlers but something is going wrong during this. What I think it does is copying a block of memory with the exception handler vectors and then it tries to fix up the jumps in it to point to somewhere but either the fixup is not correct or the handlers are not where they are expected to be. I'm currently out of ideas how to debug this further.
Regards, BALATON Zoltan
On 04/03/14 18:27, BALATON Zoltan wrote:
On Tue, 4 Mar 2014, BALATON Zoltan wrote:
Raise exception at 00441bcc => 00000002 (00) IN: 0x00000300: b 0xffffc3a0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffc3a0 0 IN: 0xffffc3a0: .long 0x0
Raise exception at ffffc3a4 => 00000006 (21) IN: 0x00000700: mtsprg 2,r2 0x00000704: li r2,7 0x00000708: b 0xffffe0f0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffe0f0 0 IN: 0xffffe0f0: .long 0x0
Raise exception at ffffe0f4 => 00000006 (21) Raise exception at ffffe0f4 => 00000006 (21)
Something seems to overwrite the vector at 0x300 (which was set to 0x238c before this point) but the new value seems to point to the wrong place.
It seems that this is happening when MorphOS tries to install its own exception handlers but something is going wrong during this. What I think it does is copying a block of memory with the exception handler vectors and then it tries to fix up the jumps in it to point to somewhere but either the fixup is not correct or the handlers are not where they are expected to be. I'm currently out of ideas how to debug this further.
Yeah, it's quite tricky without source :/
One trick I managed with the Solaris kernel was to copy the binary (an ELF executable) from the ISO and then load up QEMU with the gdbstub, e.g. with -s -S and use a cross-gdb against the ELF image.
Because the image still had symbols I could extract them using objdump and then set breakpoints at different symbol names to see how far I was getting; plus if I got stuck in a loop then I could often get a clue by using "bt" in gdb to get the call stack and use the symbol names to guess what was happening.
Probably the easiest way though is to try and engage with the MorphOS people on their mailing list and see if someone is willing to give you pointers based on execution addresses (or function names if objdump works) as to what could be the problem.
ATB,
Mark.
On Tue, 4 Mar 2014, Mark Cave-Ayland wrote:
Yeah, it's quite tricky without source :/
One trick I managed with the Solaris kernel was to copy the binary (an ELF executable) from the ISO and then load up QEMU with the gdbstub, e.g. with -s -S and use a cross-gdb against the ELF image.
Because the image still had symbols I could extract them using objdump and then set breakpoints at different symbol names to see how far I was getting; plus if I got stuck in a loop then I could often get a clue by using "bt" in gdb to get the call stack and use the symbol names to guess what was happening.
This is what I was doing too but it had no symbols so gdb does not help too much and I can only follow addresses and string offsets which makes it more difficult. Looking at it some more I believe these memory management exceptions are not supposed to happen at all and MorphOS does not expect them to happen. It only enables the MSR_DR and MSR_IR bits after it has set up the handlers and before that it only enables MSR_ME and MSR_FP at the start. I think it does not expect the firmware to enable or use these memory management exceptions. Can someone with access to a PPC Mac verify what bits of the MSR are enabled when the boot executable is started? If these exceptions don't happen on a Mac until enabled then it would explain why it can write to 0x80 without problems and why it does not crash during installing exception vectors.
I've tried to turn off these bits in call_elf before the executable is called but this causes problems in client callbacks which hang with this change. Then I also tried to additionally reenable the bits in of_client_callback like this:
Index: arch/ppc/qemu/start.S =================================================================== --- arch/ppc/qemu/start.S (revision 1271) +++ arch/ppc/qemu/start.S (working copy) @@ -521,13 +521,13 @@ LOAD_REG_IMMEDIATE(r5, of_client_callback) // r5 = callback li r6,0 // r6 = address of client program arguments (unused) li r7,0 // r7 = length of client program arguments (unused) - li r0,MSR_FP | MSR_ME | MSR_DR | MSR_IR + li r0,MSR_FP | MSR_ME MTMSRD(r0) blrl
#ifdef CONFIG_PPC64 /* Restore SF bit */ - LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_DR | MSR_IR) + LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME) MTMSRD(r0) #endif LOAD_REG_IMMEDIATE(r8, saved_stack) // restore stack pointer @@ -541,10 +541,10 @@
#ifdef __powerpc64__ #define STKOFF STACKFRAME_MINSIZE -#define SAVE_SPACE 320 +#define SAVE_SPACE 328 #else #define STKOFF 8 -#define SAVE_SPACE 144 +#define SAVE_SPACE 148 #endif GLOBL(of_client_callback):
@@ -614,6 +614,12 @@ PPC_STL r30, (STKOFF + 31 * ULONG_SIZE)(r1) PPC_STL r31, (STKOFF + 32 * ULONG_SIZE)(r1)
+ /* temporarily enable memory management */ + mfmsr r2 + PPC_STL r2, (STKOFF + 33 * ULONG_SIZE)(r1) + ori r2,r2,MSR_DR | MSR_IR + MTMSRD(r2) + #ifdef CONFIG_PPC64 LOAD_REG_IMMEDIATE(r2, of_client_interface) ld r2, 8(r2) @@ -652,6 +658,8 @@
/* restore ctr, cr and xer */
+ PPC_LL r2, (STKOFF + 33 * ULONG_SIZE)(r1) + MTMSRD(r2) PPC_LL r2, (STKOFF + 3 * ULONG_SIZE)(r1) mtctr r2 PPC_LL r2, (STKOFF + 4 * ULONG_SIZE)(r1)
but this causes a fatal error during return from the callback:
qemu: fatal: Trying to execute code outside RAM or ROM at 0x60000000
NIP 60000000 LR 60000000 CTR 00000000 XER 00000000 MSR 00000000 HID0 00000000 HF 00000000 idx 1 TB 00000000 1676840346 DECR 2618127007 GPR00 0000000000000000 0000000060000000 0000000000000000 0000000000000000 GPR04 000000004bfffffc 0000000000000000 0000000000000000 0000000000000000 GPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000 CR 00000000 [ - - - - - - - - ] RES ffffffff FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPSCR 00000000 SRR0 fff0dac4 SRR1 00003030 PVR 000c0209 VRSAVE 00000000 SPRG0 07e00000 SPRG1 ffffff6c SPRG2 22000042 SPRG3 00000000 SPRG4 00000000 SPRG5 00000000 SPRG6 00000000 SPRG7 00000000 SDR1 07e00000
which is strange but I'm not that sure about my PPC assembly.
Probably the easiest way though is to try and engage with the MorphOS people on their mailing list and see if someone is willing to give you pointers based on execution addresses (or function names if objdump works) as to what could be the problem.
I'll try to contact them and see what they say.
Regards, BALATON Zoltan
On Mar 5, 2014, Programmingkid wrote:
Is there a reason why the model is being changed? I'm worried this change might break compatibility with other OS's.
Which OSs in particular? These two models are from the same era and quite similar to each other. See here:
https://www.everymac.com/ultimate-mac-lookup/?search_keywords=PowerMac2,1 https://www.everymac.com/ultimate-mac-lookup/?search_keywords=PowerMac3,1
QEMU emulates a G4 CPU for which PowerMac3,1 is a better match and also this is supported by MorphOS while the iMac with a G3 that the previous model referred to is not. I don't think it matters for other OSs but if you know about one please tell us.
Regards, BALATON Zoltan
On Wed, 5 Mar 2014, BALATON Zoltan wrote:
Which OSs in particular? These two models are from the same era and quite similar to each other. See here:
https://www.everymac.com/ultimate-mac-lookup/?search_keywords=PowerMac2,1 https://www.everymac.com/ultimate-mac-lookup/?search_keywords=PowerMac3,1
Here's another more detailed comparison of the Kihei and Sawtooth boards corresponding to these model IDs:
http://www.macinfo.de/hardware/boards-g3.html
It also shows they are almost identical so I think it's safe to make this change unless you know about a specific OS which supports PowerMac2,1 but not 3,1. I only know about the opposite with MorphOS.
Regards, BALATON Zoltan
On Mar 5, 2014, at 10:45 AM, BALATON Zoltan wrote:
On Wed, 5 Mar 2014, BALATON Zoltan wrote:
Which OSs in particular? These two models are from the same era and quite similar to each other. See here:
https://www.everymac.com/ultimate-mac-lookup/?search_keywords=PowerMac2,1 https://www.everymac.com/ultimate-mac-lookup/?search_keywords=PowerMac3,1
Here's another more detailed comparison of the Kihei and Sawtooth boards corresponding to these model IDs:
http://www.macinfo.de/hardware/boards-g3.html
It also shows they are almost identical so I think it's safe to make this change unless you know about a specific OS which supports PowerMac2,1 but not 3,1. I only know about the opposite with MorphOS.
Regards, BALATON Zoltan
Sounds like a good change. Do you have other OS's like Debian Linux, or Mac OS X you can test on your version of OpenBIOS?
On 04/03/14 01:22, BALATON Zoltan wrote:
Here is some progress. It turns out that I get much farther with the patch at the end of this message. This corrects the names of CPU cache parameters that seem to be missing a dash compared to the device tree dump I've referred to before and this is why MorphOS could not find them while looking for them. Apart from that I also had to change the model string to a newer one that is supported by MorphOS for it to continue and it seems that it looks for the name of the / node and expects it to be called device-tree on a Mac so I had to change this too. With these it gets over its initial boot and starts to initialise devices (I also got some messages on the debug serial port). Then it hits an exception again that seems to be due to some memory corruption like the 0x80 issue right at the beginning that prevents it from booting still. This time it may be a null pointer issue but I'll need to debug it further to know what's happening. This is how far it gets now:
IN: 0x00441bc8: lwz r0,0(r4) 0x00441bcc: stw r0,0(r3) 0x00441bd0: lwz r9,4(r4) 0x00441bd4: stw r9,4(r3) 0x00441bd8: lwz r0,8(r4) 0x00441bdc: stw r0,8(r3) 0x00441be0: lwz r9,12(r4) 0x00441be4: stw r9,12(r3) 0x00441be8: lwz r0,16(r4) 0x00441bec: stw r0,16(r3) 0x00441bf0: lwz r9,20(r4) 0x00441bf4: stw r9,20(r3) 0x00441bf8: lwz r0,24(r4) 0x00441bfc: stw r0,24(r3) 0x00441c00: lwz r9,28(r4) 0x00441c04: stw r9,28(r3) 0x00441c08: addi r4,r4,32 0x00441c0c: addi r3,r3,32 0x00441c10: bdnz+ 0x441bc8
Raise exception at 00441bcc => 00000002 (00) IN: 0x00000300: b 0xffffc3a0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffc3a0 0 IN: 0xffffc3a0: .long 0x0
Raise exception at ffffc3a4 => 00000006 (21) IN: 0x00000700: mtsprg 2,r2 0x00000704: li r2,7 0x00000708: b 0xffffe0f0
invalid/unsupported opcode: 00 - 00 - 00 (00000000) ffffe0f0 0 IN: 0xffffe0f0: .long 0x0
Raise exception at ffffe0f4 => 00000006 (21) Raise exception at ffffe0f4 => 00000006 (21)
Something seems to overwrite the vector at 0x300 (which was set to 0x238c before this point) but the new value seems to point to the wrong place.
What do you think about the patch below?
Regards, BALATON Zoltan
Index: arch/ppc/qemu/init.c
--- arch/ppc/qemu/init.c (revision 1271) +++ arch/ppc/qemu/init.c (working copy) @@ -241,32 +241,32 @@
PUSH(cpu->dcache_size); fword("encode-int");
- push_str("dcache-size");
- push_str("d-cache-size");
fword("property");
PUSH(cpu->icache_size); fword("encode-int");
- push_str("icache-size");
- push_str("i-cache-size");
fword("property");
PUSH(cpu->dcache_sets); fword("encode-int");
- push_str("dcache-sets");
- push_str("d-cache-sets");
fword("property");
PUSH(cpu->icache_sets); fword("encode-int");
- push_str("icache-sets");
- push_str("i-cache-sets");
fword("property");
PUSH(cpu->dcache_block_size); fword("encode-int");
- push_str("dcache-block-size");
- push_str("d-cache-block-size");
fword("property");
PUSH(cpu->icache_block_size); fword("encode-int");
- push_str("icache-block-size");
- push_str("i-cache-block-size");
fword("property");
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); @@ -745,12 +745,12 @@
/* model */
- push_str("PowerMac2,1");
- push_str("PowerMac3,1");
fword("model");
/* compatible */
- push_str("PowerMac2,1");
- push_str("PowerMac3,1");
fword("encode-string"); push_str("MacRISC"); fword("encode-string"); Index: forth/device/tree.fs =================================================================== --- forth/device/tree.fs (revision 1271) +++ forth/device/tree.fs (working copy) @@ -11,7 +11,7 @@
\ root node new-device
- " OpenBiosTeam,OpenBIOS" device-name
- " device-tree" device-name
1 encode-int " #address-cells" property : open true ; : close ;
These patches basically look okay, although would it be possible to submit them as separate patches? The reason for this is to allow bisection e.g. in the case that renaming the CPU properties suddenly means a guest OS can find them, and it then breaks as a result.
- The CPU property renaming patch looks good
- The PowerMac model name change is stylistically fine, however I don't know enough about PPC to know whether bumping the model from PowerMac2 to PowerMac3 violates the -M mac99 QEMU machine - Alex Graf is probably the person to ask about this one
- The root node rename is stylistically okay, however it's a pretty stupid thing to do to locate the root node of the tree based upon it's name (which *isn't* given in the spec) - finddevice("/") is your friend. Perhaps the best way here is to move the OpenBIOS property to a different name so that it can be detected another way if required?
ATB,
Mark.
On 2014-Mar-4, 17:41 , Mark Cave-Ayland wrote:
- The root node rename is stylistically okay, however it's a pretty
stupid thing to do to locate the root node of the tree based upon it's name (which *isn't* given in the spec) - finddevice("/") is your friend. Perhaps the best way here is to move the OpenBIOS property to a different name so that it can be detected another way if required?
The name of the root node should be the platform name. E.g., on modern Sun machines, it contains "sun4v-platform" (since we don't want guests being able to tell one virtual machine from another). On older machines, it contained something like "SUNW,Ultra-5".
On Tue, 4 Mar 2014, Mark Cave-Ayland wrote:
These patches basically look okay, although would it be possible to submit them as separate patches? The reason for this is to allow bisection e.g. in the case that renaming the CPU properties suddenly means a guest OS can find them, and it then breaks as a result.
I can split them or you could take the parts and commit them separately.
- The CPU property renaming patch looks good
This can be taken as is. Do I need to resubmit as a separate patch?
- The PowerMac model name change is stylistically fine, however I don't know
enough about PPC to know whether bumping the model from PowerMac2 to PowerMac3 violates the -M mac99 QEMU machine - Alex Graf is probably the person to ask about this one
Since QEMU now says to emulate a G4 CPU I believe PowerMac3,1 is a better match as the PowerMac2,1 had a G3 CPU according to everymac.com. I hope Alex or someone who is more knowledgeable about Mac hardware can chime in and confirm/correct this. If this is OK this part can be taken as is too.
- The root node rename is stylistically okay, however it's a pretty stupid
thing to do to locate the root node of the tree based upon it's name (which *isn't* given in the spec) - finddevice("/") is your friend.
I agree this looks like a bug or too much assumption in MorphOS but I'm not sure they are willing to change and this is simple to correct in openbios. What it does is actually call finddevice("/") but then does getprop("name") to see if this is called "device-tree" to find out if it is running on a Mac where it also gets the "model" property. Otherwise it skips this and only tries to get "CODEGEN,vendor" and "revision" properties that seem to exist on the bplan boards it runs on apart from Macintosh hardware. I've found a device tree dump of a bplan board, here it is for comparison:
http://mega-tokyo.com/blog/index.php/site/grins_and_giggles
Perhaps the best way here is to move the OpenBIOS property to a different name so that it can be detected another way if required?
What would be a good name for it? I'd say "vendor" but I don't know if there is any standard or clashes with any other implementations. Shall I submit a patch with this or I'm also OK with you modifying it as you see fit.
Regards, BALATON Zoltan
On Mar 4, 2014, at 5:41 PM, Mark Cave-Ayland wrote:
/* model */
- push_str("PowerMac2,1");
- push_str("PowerMac3,1");
fword("model");
/* compatible */
- push_str("PowerMac2,1");
- push_str("PowerMac3,1");
fword("encode-string"); push_str("MacRISC"); fword("encode-string"); Index: forth/device/tree.fs
- The PowerMac model name change is stylistically fine, however I don't know enough about PPC to know whether bumping the model from PowerMac2 to PowerMac3 violates the -M mac99 QEMU machine - Alex Graf is probably the person to ask about this one
Is there a reason why the model is being changed? I'm worried this change might break compatibility with other OS's.
-
BALATON Zoltan -
Mark Cave-Ayland -
Olivier Danet -
Programmingkid -
Tarl Neustaedter