PATCH v2: Rework CIF interface to handle parameters in reverse order

[Mark Cave-Ayland]

Hi everyone,

Here is a second version of the patch which attempts to reverse the 
argument order of the CIF Forth words so that they can be called 
correctly from both Forth and C.

The patch is fairly straightforward; probably the only unexpected part 
is the need to rename the existing /openprom/client-services "claim" and 
"release" words to "cif-claim" and "cif-release" respectively. This is 
because we need to use the "claim" and "release" words in 
forth/system/ciface.fs to reverse the argument order before calling the 
real underlying words.

I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and 
with this version of the patch, the CIF words are called correctly in 
both cases. I'd like to apply this reasonably soon, so please can people 
test on their setups to make sure it doesn't break anything?

Signed-off-by: Mark Cave-Ayland

ATB,

Mark.

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Laurent Vivier]

Le dimanche 29 novembre 2009 à 13:30 +0000, Mark Cave-Ayland a écrit :
> Hi everyone,
>
> Here is a second version of the patch which attempts to reverse the 
> argument order of the CIF Forth words so that they can be called 
> correctly from both Forth and C.
>
> The patch is fairly straightforward; probably the only unexpected part 
> is the need to rename the existing /openprom/client-services "claim" and 
> "release" words to "cif-claim" and "cif-release" respectively. This is 
> because we need to use the "claim" and "release" words in 
> forth/system/ciface.fs to reverse the argument order before calling the 
> real underlying words.
>
> I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and 
> with this version of the patch, the CIF words are called correctly in 
> both cases. I'd like to apply this reasonably soon, so please can people 
> test on their setups to make sure it doesn't break anything?

I doesn't work with:

debian-40r6-powerpc-netinst.iso
debian-500-powerpc-netinst.iso
Fedora-10-ppc-netinst.iso
Fedora-11-Preview-ppc-netinst.iso
openSUSE-11.1-NET-ppc.iso
OpenBSD 4.6

Laurent

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Laurent Vivier]

Le dimanche 29 novembre 2009 à 22:46 +0100, Laurent Vivier a écrit :
> Le dimanche 29 novembre 2009 à 13:30 +0000, Mark Cave-Ayland a écrit :
> > Hi everyone,
> >
> > Here is a second version of the patch which attempts to reverse the 
> > argument order of the CIF Forth words so that they can be called 
> > correctly from both Forth and C.
> >
> > The patch is fairly straightforward; probably the only unexpected part 
> > is the need to rename the existing /openprom/client-services "claim" and 
> > "release" words to "cif-claim" and "cif-release" respectively. This is 
> > because we need to use the "claim" and "release" words in 
> > forth/system/ciface.fs to reverse the argument order before calling the 
> > real underlying words.
> >
> > I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and 
> > with this version of the patch, the CIF words are called correctly in 
> > both cases. I'd like to apply this reasonably soon, so please can people 
> > test on their setups to make sure it doesn't break anything?
>
> I doesn't work with:
>
> debian-40r6-powerpc-netinst.iso
> debian-500-powerpc-netinst.iso
> Fedora-10-ppc-netinst.iso
> Fedora-11-Preview-ppc-netinst.iso
> openSUSE-11.1-NET-ppc.iso
> OpenBSD 4.6

Sorry, I didn't test the good patch. V2 works well with all of them.

Regards,
Laurent

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Laurent Vivier]

Le dimanche 29 novembre 2009 à 13:30 +0000, Mark Cave-Ayland a écrit :
> Hi everyone,
>
> Here is a second version of the patch which attempts to reverse the 
> argument order of the CIF Forth words so that they can be called 
> correctly from both Forth and C.
>
> The patch is fairly straightforward; probably the only unexpected part 
> is the need to rename the existing /openprom/client-services "claim" and 
> "release" words to "cif-claim" and "cif-release" respectively. This is 
> because we need to use the "claim" and "release" words in 
> forth/system/ciface.fs to reverse the argument order before calling the 
> real underlying words.
>
> I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and 
> with this version of the patch, the CIF words are called correctly in 
> both cases. I'd like to apply this reasonably soon, so please can people 
> test on their setups to make sure it doesn't break anything?
>
> Signed-off-by: Mark Cave-Ayland

Just one question about the patch: Why do you rename "claim" and
"release" to "cif-claim" and "cif-release" ?
Moreover, I think it is not compliant with OpenFirmware specification
(IEEE 1295).

Regards,
Laurent

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Andreas Färber]

Am 29.11.2009 um 23:09 schrieb Laurent Vivier:
> Le dimanche 29 novembre 2009 à 22:46 +0100, Laurent Vivier a écrit :
> > Le dimanche 29 novembre 2009 à 13:30 +0000, Mark Cave-Ayland a  
> > écrit :
> > > Hi everyone,
> > >
> > > Here is a second version of the patch which attempts to reverse the
> > > argument order of the CIF Forth words so that they can be called
> > > correctly from both Forth and C.
> > >
> > > The patch is fairly straightforward; probably the only unexpected  
> > > part
> > > is the need to rename the existing /openprom/client-services  
> > > "claim" and
> > > "release" words to "cif-claim" and "cif-release" respectively.  
> > > This is
> > > because we need to use the "claim" and "release" words in
> > > forth/system/ciface.fs to reverse the argument order before  
> > > calling the
> > > real underlying words.
> > >
> > > I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and
> > > with this version of the patch, the CIF words are called correctly  
> > > in
> > > both cases. I'd like to apply this reasonably soon, so please can  
> > > people
> > > test on their setups to make sure it doesn't break anything?
> >
> > [It works] with:
> >
> > debian-40r6-powerpc-netinst.iso
> > debian-500-powerpc-netinst.iso
> > Fedora-10-ppc-netinst.iso
> > Fedora-11-Preview-ppc-netinst.iso
> > openSUSE-11.1-NET-ppc.iso
> > OpenBSD 4.6

No regressions with these either:

debian-40r4a-powerpc-netinst.iso
Fedora-9-ppc-DVD.iso

some sparc32 Debian GNU/Linux 4.0: Linux debiansparc 2.6.18-6-sparc32  
#1 Thu May 8 07:15:15 UTC 2008 sparc

Andreas

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Nick Couchman]

> > > On 2009/11/29 at 06:30, Mark Cave-Ayland

wrote:
> Hi everyone,
>
> Here is a second version of the patch which attempts to reverse the 
> argument order of the CIF Forth words so that they can be called 
> correctly from both Forth and C.
>
> The patch is fairly straightforward; probably the only unexpected part 
> is the need to rename the existing /openprom/client-services "claim" and 
> "release" words to "cif-claim" and "cif-release" respectively. This is 
> because we need to use the "claim" and "release" words in 
> forth/system/ciface.fs to reverse the argument order before calling the 
> real underlying words.
>
> I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and 
> with this version of the patch, the CIF words are called correctly in 
> both cases. I'd like to apply this reasonably soon, so please can people 
> test on their setups to make sure it doesn't break anything?
>
> Signed-off-by: Mark Cave-Ayland

(Sorry if the patch wasn't supposed to fix this...)

After applying this patch still getting:

OpenBIOS for Sparc64
Configuration device id QEMU version 1 machine id 0
CPUs: 1 x SUNW,UltraSPARC-II
UUID: 00000000-0000-0000-0000-000000000000
claim isn't unique.
release isn't unique.
Welcome to OpenBIOS v1.0 built on Nov 30 2009 15:44
  Type 'help' for detailed information

0 > boot cdrom 
[sparc64] Booting file 'cdrom' with parameters ''
Not a bootable ELF image
Not a Linux kernel image
Not a bootable a.out image
Loading FCode image...
Loaded 7120 bytes
entry point is 0x4000
Evaluating FCode...
Unhandled Exception 0x0000000008000000
PC = 0x00000000ffd10e3c NPC = 0x00000000ffd10e40
Stopping execution

and GDB shows the error at:
(gdb) l *0x00000000ffd10e3c
0xffd10e3c is in cfetch (../include/openbios/stack.h:34).
29	typedef ucell phandle_t;
30	
31	
32	
33	
34	static inline void PUSH(ucell value) {
35		dstack[++dstackcnt] = (value);
36	}
37	static inline void PUSH_xt( xt_t xt ) { PUSH( (ucell)xt ); }
38	static inline void PUSH_ih( ihandle_t ih ) { PUSH( (ucell)ih ); }

Let me know if I can provide any additional output or debugging steps!
-Nick

--------
This e-mail may contain confidential and privileged material for the sole use of the intended recipient.  If this email is not intended for you, or you are not responsible for the delivery of this message to the intended recipient, please note that this message may contain SEAKR Engineering (SEAKR) Privileged/Proprietary Information.  In such a case, you are strictly prohibited from downloading, photocopying, distributing or otherwise using this message, its contents or attachments in any way.  If you have received this message in error, please notify us immediately by replying to this e-mail and delete the message from your mailbox.  Information contained in this message that does not relate to the business of SEAKR is neither endorsed by nor attributable to SEAKR.

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Blue Swirl]

On Sun, Nov 29, 2009 at 1:30 PM, Mark Cave-Ayland
 wrote:
> Hi everyone,
>
> Here is a second version of the patch which attempts to reverse the argument
> order of the CIF Forth words so that they can be called correctly from both
> Forth and C.
>
> The patch is fairly straightforward; probably the only unexpected part is
> the need to rename the existing /openprom/client-services "claim" and
> "release" words to "cif-claim" and "cif-release" respectively. This is
> because we need to use the "claim" and "release" words in
> forth/system/ciface.fs to reverse the argument order before calling the real
> underlying words.
>
> I've tested this on a FC12 PPC CD as well as my SPARC64 Milax CD and with
> this version of the patch, the CIF words are called correctly in both cases.
> I'd like to apply this reasonably soon, so please can people test on their
> setups to make sure it doesn't break anything?
>
> Signed-off-by: Mark Cave-Ayland

This passes also my tests.

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Mark Cave-Ayland]

Laurent Vivier wrote:
> > I doesn't work with:
> >
> > debian-40r6-powerpc-netinst.iso
> > debian-500-powerpc-netinst.iso
> > Fedora-10-ppc-netinst.iso
> > Fedora-11-Preview-ppc-netinst.iso
> > openSUSE-11.1-NET-ppc.iso
> > OpenBSD 4.6
>
> Sorry, I didn't test the good patch. V2 works well with all of them.

Thanks Laurent.

ATB,

Mark.

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Mark Cave-Ayland]

Laurent Vivier wrote:
> Just one question about the patch: Why do you rename "claim" and
> "release" to "cif-claim" and "cif-release" ?
> Moreover, I think it is not compliant with OpenFirmware specification
> (IEEE 1295).

Right. The reason for this is that all of the C memory management 
routines are in Forth order, and so if they are being called via CIF 
then the argument order needs to be reversed.

At the moment, the memory management routines are registered in 2 
places: within the MMU/memory package (called from Forth) and within the 
  CIF package. So what I did was add claim/release words back into 
forth/system/ciface.fs to swap the argument order and then pass onto the 
correct function.

However: the PPC and Sparc architectures use different C functions when 
called from CIF; hence I registered copies of whichever functions were 
used for that architecture as "cif-claim" and "cif-release" and then 
call these fixed names from "claim" and "release" in 
forth/system/ciface.fs. Does that help?

ATB,

Mark.

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Mark Cave-Ayland]

Blue Swirl wrote:
> This passes also my tests.

Great - thanks for testing. Assuming yourself/Laurent are happy with my 
explanations for "cif-claim" and "cif-release", I'll apply this over the 
  next day or so.

Many thanks,

Mark.

PATCH v2: Rework CIF interface to handle parameters in reverse order

[Mark Cave-Ayland]

Nick Couchman wrote:
> (Sorry if the patch wasn't supposed to fix this...)
>
> After applying this patch still getting:
>
> OpenBIOS for Sparc64
> Configuration device id QEMU version 1 machine id 0
> CPUs: 1 x SUNW,UltraSPARC-II
> UUID: 00000000-0000-0000-0000-000000000000
> claim isn't unique.
> release isn't unique.
> Welcome to OpenBIOS v1.0 built on Nov 30 2009 15:44
>   Type 'help' for detailed information
>
> 0 > boot cdrom 
> [sparc64] Booting file 'cdrom' with parameters ''
> Not a bootable ELF image
> Not a Linux kernel image
> Not a bootable a.out image
> Loading FCode image...
> Loaded 7120 bytes
> entry point is 0x4000
> Evaluating FCode...
> Unhandled Exception 0x0000000008000000
> PC = 0x00000000ffd10e3c NPC = 0x00000000ffd10e40
> Stopping execution
>
> and GDB shows the error at:
> (gdb) l *0x00000000ffd10e3c
> 0xffd10e3c is in cfetch (../include/openbios/stack.h:34).
> 29	typedef ucell phandle_t;
> 30	
> 31	
> 32	
> 33	
> 34	static inline void PUSH(ucell value) {
> 35		dstack[++dstackcnt] = (value);
> 36	}
> 37	static inline void PUSH_xt( xt_t xt ) { PUSH( (ucell)xt ); }
> 38	static inline void PUSH_ih( ihandle_t ih ) { PUSH( (ucell)ih ); }
>
> Let me know if I can provide any additional output or debugging steps!
> -Nick

Well, it got things a bit further but there is still another bug 
present. In this case the backtrace is useless because it's a Forth 
issue rather than a C issue - it's often more useful to step through in 
Forth first to see which word causes the breakage rather than post a 
backtrace from within the Forth engine.

In this case, I did a little bit of detective work and found that the 
branch offset calculations are wrong for certain combinations of nested 
b(<mark) and bbranch. I'm fairly sure I can fix this, I just need to 
find a moment to come up with a fix and test it. Oh, and I'll apply the 
above patch too.

ATB,

Mark.