[PATCH] RFC: Change ofmem_common.c to set memory translation properties by reference

[Mark Cave-Ayland]

Hi folks,

The attached patch changes OFMEM so that instead of allocating new space 
within the Forth dictionary every time the /memory and /virtual-memory 
available/translations nodes are updated, we simply change the property 
to point directly to a static buffer. This has the effect of saving 
substantial amounts of memory during OpenSolaris 10 boot (in fact the 
final dictionary size after boot is now < 256K once again).

Blue/Andreas: please could you take a look at this patch and make sure 
it doesn't break anything in your SPARC64/PPC tests?

On the plus side, with this patch applied Milax gets to the end of its 
natural boot without crashing giving the following output:

OpenBIOS for Sparc64
Configuration device id QEMU version 1 machine id 0
kernel cmdline
CPUs: 1 x SUNW,UltraSPARC-IIi
UUID: 00000000-0000-0000-0000-000000000000
Welcome to OpenBIOS v1.0 built on Oct 14 2010 20:18
   Type 'help' for detailed information
Trying cdrom:f...
Not a bootable ELF image
Not a bootable a.out image

Loading FCode image...
Loaded 7084 bytes
entry point is 0x4000
Ignoring failed claim for va 1000000 memsz bf34e!
Ignoring failed claim for va 1402000 memsz 303b3!
Ignoring failed claim for va 1800000 memsz 60a30!

Jumping to entry point 00000000010071d8 for type 0000000000000001...
switching to new context: entry point 0x10071d8 stack 0x00000000ffe06b49
warning:interpret: exception -13 caught
SunOS Release 5.11 Version MilaX_0.3.2 64-bit
Copyright 1983-2008 Sun Microsystems, Inc.  All rights reserved.
Use is subject to license terms.
spacex@:interpret: exception -13 caught
kdbg-words:interpret: exception -13 caught
cb-r/w:interpret: exception -13 caught
(Can't load tod module) EXIT
-1 >

Does anyone know what the tod modules does? Is it Time Of Day (i.e. we 
are missing some kind of hardware clock emulation?)

ATB,

Mark.

[Artyom Tarasenko]

On Thu, Oct 14, 2010 at 10:32 PM, Mark Cave-Ayland
 wrote:
> Hi folks,
>
> The attached patch changes OFMEM so that instead of allocating new space
> within the Forth dictionary every time the /memory and /virtual-memory
> available/translations nodes are updated, we simply change the property to
> point directly to a static buffer. This has the effect of saving substantial
> amounts of memory during OpenSolaris 10 boot (in fact the final dictionary
> size after boot is now < 256K once again).
>
> Blue/Andreas: please could you take a look at this patch and make sure it
> doesn't break anything in your SPARC64/PPC tests?
>
> On the plus side, with this patch applied Milax gets to the end of its
> natural boot without crashing giving the following output:
>
> OpenBIOS for Sparc64
> Configuration device id QEMU version 1 machine id 0
> kernel cmdline
> CPUs: 1 x SUNW,UltraSPARC-IIi
> UUID: 00000000-0000-0000-0000-000000000000
> Welcome to OpenBIOS v1.0 built on Oct 14 2010 20:18
>  Type 'help' for detailed information
> Trying cdrom:f...
> Not a bootable ELF image
> Not a bootable a.out image
>
> Loading FCode image...
> Loaded 7084 bytes
> entry point is 0x4000
> Ignoring failed claim for va 1000000 memsz bf34e!
> Ignoring failed claim for va 1402000 memsz 303b3!
> Ignoring failed claim for va 1800000 memsz 60a30!
>
> Jumping to entry point 00000000010071d8 for type 0000000000000001...
> switching to new context: entry point 0x10071d8 stack 0x00000000ffe06b49
> warning:interpret: exception -13 caught
> SunOS Release 5.11 Version MilaX_0.3.2 64-bit
> Copyright 1983-2008 Sun Microsystems, Inc.  All rights reserved.
> Use is subject to license terms.
> spacex@:interpret: exception -13 caught
> kdbg-words:interpret: exception -13 caught
> cb-r/w:interpret: exception -13 caught
> (Can't load tod module) EXIT
> -1 >

Cool! Congrats!
> Does anyone know what the tod modules does? Is it Time Of Day (i.e. we are
> missing some kind of hardware clock emulation?)

I think so. Actually m48t59 is emulated in sparc32, but not connected
in case of sparc64.

[Artyom Tarasenko]

On Thu, Oct 14, 2010 at 10:46 PM, Artyom Tarasenko  wrote:
> On Thu, Oct 14, 2010 at 10:32 PM, Mark Cave-Ayland
>  wrote:
> > Hi folks,
> >
> > The attached patch changes OFMEM so that instead of allocating new space
> > within the Forth dictionary every time the /memory and /virtual-memory
> > available/translations nodes are updated, we simply change the property to
> > point directly to a static buffer. This has the effect of saving substantial
> > amounts of memory during OpenSolaris 10 boot (in fact the final dictionary
> > size after boot is now < 256K once again).
> >
> > Blue/Andreas: please could you take a look at this patch and make sure it
> > doesn't break anything in your SPARC64/PPC tests?
> >
> > On the plus side, with this patch applied Milax gets to the end of its
> > natural boot without crashing giving the following output:
> >
> > OpenBIOS for Sparc64
> > Configuration device id QEMU version 1 machine id 0
> > kernel cmdline
> > CPUs: 1 x SUNW,UltraSPARC-IIi
> > UUID: 00000000-0000-0000-0000-000000000000
> > Welcome to OpenBIOS v1.0 built on Oct 14 2010 20:18
> >  Type 'help' for detailed information
> > Trying cdrom:f...
> > Not a bootable ELF image
> > Not a bootable a.out image
> >
> > Loading FCode image...
> > Loaded 7084 bytes
> > entry point is 0x4000
> > Ignoring failed claim for va 1000000 memsz bf34e!
> > Ignoring failed claim for va 1402000 memsz 303b3!
> > Ignoring failed claim for va 1800000 memsz 60a30!
> >
> > Jumping to entry point 00000000010071d8 for type 0000000000000001...
> > switching to new context: entry point 0x10071d8 stack 0x00000000ffe06b49
> > warning:interpret: exception -13 caught
> > SunOS Release 5.11 Version MilaX_0.3.2 64-bit
> > Copyright 1983-2008 Sun Microsystems, Inc.  All rights reserved.
> > Use is subject to license terms.
> > spacex@:interpret: exception -13 caught
> > kdbg-words:interpret: exception -13 caught
> > cb-r/w:interpret: exception -13 caught
> > (Can't load tod module) EXIT
> > -1 >
>
> Cool! Congrats!
> > Does anyone know what the tod modules does? Is it Time Of Day (i.e. we are
> > missing some kind of hardware clock emulation?)
>
> I think so. Actually m48t59 is emulated in sparc32, but not connected
> in case of sparc64.

The referencing comment in the opensolaris source:
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/sun4u/os/f...

* Appropriate tod module will be dynamically selected while booting
* based on finding a device tree node with a "device_type" property value
* of "tod". If such a node describing tod is not found, for backward
* compatibility, a node with a "name" property value of "eeprom" and
* "model" property value of "mk48t59" will be used. Failing to find a
* node matching either of the above criteria will result in no tod module
* being selected; this will cause the boot process to halt

On the real U30 machine:

ok cd /pci@1f,4000/ebus@1/eeprom@14,0
ok .properties
address                  fffa4000
reg                      00000014 00000000 00002000
model                    mk48t59
name                     eeprom
ok

[Mark Cave-Ayland]

Artyom Tarasenko wrote:
> > I think so. Actually m48t59 is emulated in sparc32, but not connected
> > in case of sparc64.
>
> The referencing comment in the opensolaris source:
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/sun4u/os/f...
> Appropriate tod module will be dynamically selected while booting
> based on finding a device tree node with a "device_type" property value
> of "tod". If such a node describing tod is not found, for backward
> compatibility, a node with a "name" property value of "eeprom" and
> "model" property value of "mk48t59" will be used. Failing to find a
> node matching either of the above criteria will result in no tod module
> being selected; this will cause the boot process to halt
>
> On the real U30 machine:
>
> ok cd /pci@1f,4000/ebus@1/eeprom@14,0
> ok .properties
> address                  fffa4000
> reg                      00000014 00000000 00002000
> model                    mk48t59
> name                     eeprom
> ok

Oh, that's interesting. If I look in drivers/obio.c then I can see an 
eeprom device named "mk48t08" rather than "mk48t59". Is this a typo, or 
does it need a new explicit device defined in pci.c's ebus_config_cb() 
somewhere?

If this is the case, it appears that qemu-system-sparc64 is set to use 
mc146818rtc rather m48t59 and so there is going to have to be a qemu 
build patch required here too.

ATB,

Mark.

[Blue Swirl]

On Fri, Oct 15, 2010 at 1:43 PM, Mark Cave-Ayland
 wrote:
> Artyom Tarasenko wrote:
> > > I think so. Actually m48t59 is emulated in sparc32, but not connected
> > > in case of sparc64.
> >
> > The referencing comment in the opensolaris source:
> >
> > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/sun4u/os/f...
> > Appropriate tod module will be dynamically selected while booting
> > based on finding a device tree node with a "device_type" property value
> > of "tod". If such a node describing tod is not found, for backward
> > compatibility, a node with a "name" property value of "eeprom" and
> > "model" property value of "mk48t59" will be used. Failing to find a
> > node matching either of the above criteria will result in no tod module
> > being selected; this will cause the boot process to halt
> >
> > On the real U30 machine:
> >
> > ok cd /pci@1f,4000/ebus@1/eeprom@14,0
> > ok .properties
> > address                  fffa4000
> > reg                      00000014 00000000 00002000
> > model                    mk48t59
> > name                     eeprom
> > ok
>
> Oh, that's interesting. If I look in drivers/obio.c then I can see an eeprom
> device named "mk48t08" rather than "mk48t59". Is this a typo, or does it
> need a new explicit device defined in pci.c's ebus_config_cb() somewhere?

mk48t59 is a newer version of mk48t08, there is alarm etc.
> If this is the case, it appears that qemu-system-sparc64 is set to use
> mc146818rtc rather m48t59 and so there is going to have to be a qemu build
> patch required here too.

Actually, while mc146818rtc is linked in, it's not used (there are
some machines that have similar device, so that's why it still hangs
around). m48t59 is on ebus.

Maybe it's time to pick a specific real machine type instead of 'Sun4u
platform'. Something that is not too far from what we have now, with
PC-like hardware. Is Netra-T1 OK?

[Artyom Tarasenko]

On Fri, Oct 15, 2010 at 7:57 PM, Blue Swirl  wrote:
> Maybe it's time to pick a specific real machine type instead of 'Sun4u
> platform'. Something that is not too far from what we have now, with
> PC-like hardware. Is Netra-T1 OK?

Just curious, why not U5, U10 or U30?

[Blue Swirl]

On Sat, Oct 16, 2010 at 7:09 PM, Artyom Tarasenko  wrote:
> On Fri, Oct 15, 2010 at 7:57 PM, Blue Swirl  wrote:
> > Maybe it's time to pick a specific real machine type instead of 'Sun4u
> > platform'. Something that is not too far from what we have now, with
> > PC-like hardware. Is Netra-T1 OK?
>
> Just curious, why not U5, U10 or U30?

U5 could be indeed better, since there's a display (ATYFB). I don't
have a device tree for Ultra 10. U30 has a Psycho bridge instead of
Sabre and FFB display. Otherwise all four look pretty similar.

[Mark Cave-Ayland]

Blue Swirl wrote:
> > If this is the case, it appears that qemu-system-sparc64 is set to use
> > mc146818rtc rather m48t59 and so there is going to have to be a qemu build
> > patch required here too.
>
> Actually, while mc146818rtc is linked in, it's not used (there are
> some machines that have similar device, so that's why it still hangs
> around). m48t59 is on ebus.
>
> Maybe it's time to pick a specific real machine type instead of 'Sun4u
> platform'. Something that is not too far from what we have now, with
> PC-like hardware. Is Netra-T1 OK?

To be honest, I'm not really bothered as long as Solaris 9 and above 
will work, and I can migrate an existing drive image from physical to 
virtual (though this wouldn't necessarily have to be with the same drivers).

ATB,

Mark.

[Artyom Tarasenko]

On Fri, Oct 15, 2010 at 3:43 PM, Mark Cave-Ayland
 wrote:
> Artyom Tarasenko wrote:
> > > I think so. Actually m48t59 is emulated in sparc32, but not connected
> > > in case of sparc64.
> >
> > The referencing comment in the opensolaris source:
> >
> > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/sun4u/os/f...
> > Appropriate tod module will be dynamically selected while booting
> > based on finding a device tree node with a "device_type" property value
> > of "tod". If such a node describing tod is not found, for backward
> > compatibility, a node with a "name" property value of "eeprom" and
> > "model" property value of "mk48t59" will be used. Failing to find a
> > node matching either of the above criteria will result in no tod module
> > being selected; this will cause the boot process to halt
> >
> > On the real U30 machine:
> >
> > ok cd /pci@1f,4000/ebus@1/eeprom@14,0
> > ok .properties
> > address                  fffa4000
> > reg                      00000014 00000000 00002000
> > model                    mk48t59
> > name                     eeprom
> > ok
>
> Oh, that's interesting. If I look in drivers/obio.c then I can see an eeprom
> device named "mk48t08" rather than "mk48t59". Is this a typo, or does it
> need a new explicit device defined in pci.c's ebus_config_cb() somewhere?

Have you meanwhile tried to change the name to "mk48t59" ? I think
changing the chip in qemu is not neccessary (and even if it is the
name in OpenBIOS has to be changed anyway).

[Mark Cave-Ayland]

On 04/05/11 08:57, Artyom Tarasenko wrote:
> > > > I think so. Actually m48t59 is emulated in sparc32, but not connected
> > > > in case of sparc64.
> > >
> > > The referencing comment in the opensolaris source:
> > >
> > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/sun4u/os/f...
> > > Appropriate tod module will be dynamically selected while booting
> > > based on finding a device tree node with a "device_type" property value
> > > of "tod". If such a node describing tod is not found, for backward
> > > compatibility, a node with a "name" property value of "eeprom" and
> > > "model" property value of "mk48t59" will be used. Failing to find a
> > > node matching either of the above criteria will result in no tod module
> > > being selected; this will cause the boot process to halt
> > >
> > > On the real U30 machine:
> > >
> > > ok cd /pci@1f,4000/ebus@1/eeprom@14,0
> > > ok .properties
> > > address                  fffa4000
> > > reg                      00000014 00000000 00002000
> > > model                    mk48t59
> > > name                     eeprom
> > > ok
> >
> > Oh, that's interesting. If I look in drivers/obio.c then I can see an eeprom
> > device named "mk48t08" rather than "mk48t59". Is this a typo, or does it
> > need a new explicit device defined in pci.c's ebus_config_cb() somewhere?
>
> Have you meanwhile tried to change the name to "mk48t59" ? I think
> changing the chip in qemu is not neccessary (and even if it is the
> name in OpenBIOS has to be changed anyway).

If Blue's subsequent comment about it already being wired up to the ebus 
is correct, then yes, probably renaming might work if the device 
registers are mapped correctly. Alas this was the last time I touched 
SPARC64 before switching to SPARC32, and so I never really looked into this.

ATB,

Mark.

[Artyom Tarasenko]

On Wed, May 4, 2011 at 11:19 AM, Mark Cave-Ayland
 wrote:
> On 04/05/11 08:57, Artyom Tarasenko wrote:
> > > > > I think so. Actually m48t59 is emulated in sparc32, but not connected
> > > > > in case of sparc64.
> > > >
> > > > The referencing comment in the opensolaris source:
> > > >
> > > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/sun4u/os/f...
> > > > Appropriate tod module will be dynamically selected while booting
> > > > based on finding a device tree node with a "device_type" property
> > > >
> > > > value
> > > > of "tod". If such a node describing tod is not found, for backward
> > > > compatibility, a node with a "name" property value of "eeprom" and
> > > > "model" property value of "mk48t59" will be used. Failing to find a
> > > > node matching either of the above criteria will result in no tod
> > > >
> > > > module
> > > > being selected; this will cause the boot process to halt
> > > >
> > > > On the real U30 machine:
> > > >
> > > > ok cd /pci@1f,4000/ebus@1/eeprom@14,0
> > > > ok .properties
> > > > address                  fffa4000
> > > > reg                      00000014 00000000 00002000
> > > > model                    mk48t59
> > > > name                     eeprom
> > > > ok
> > >
> > > Oh, that's interesting. If I look in drivers/obio.c then I can see an
> > > eeprom
> > > device named "mk48t08" rather than "mk48t59". Is this a typo, or does it
> > > need a new explicit device defined in pci.c's ebus_config_cb() somewhere?
> >
> > Have you meanwhile tried to change the name to "mk48t59" ? I think
> > changing the chip in qemu is not neccessary (and even if it is the
> > name in OpenBIOS has to be changed anyway).
>
> If Blue's subsequent comment about it already being wired up to the ebus is
> correct, then yes, probably renaming might work if the device registers are
> mapped correctly.

Yes, Blue is right (and I've managed to correct my wrong statement one
day before he did),
the device is connected via ebus.
> Alas this was the last time I touched SPARC64 before
> switching to SPARC32, and so I never really looked into this.
>
> ATB,
>
> Mark.
>
> Sirius Labs: http://www.siriusit.co.uk/labs

[Artyom Tarasenko]

On Thu, Oct 14, 2010 at 10:46 PM, Artyom Tarasenko  wrote:
> Actually m48t59 is emulated in sparc32, but not connected
> in case of sparc64.

Wrong. It is connected to isa (ebus), nevermind.

[PATCH] RFC: Change ofmem_common.c to set memory translation properties by reference

[Andreas Färber]

Mark,

Am 14.10.2010 um 22:32 schrieb Mark Cave-Ayland:
> The attached patch changes OFMEM so that instead of allocating new  
> space within the Forth dictionary every time the /memory and / 
> virtual-memory available/translations nodes are updated, we simply  
> change the property to point directly to a static buffer. This has  
> the effect of saving substantial amounts of memory during  
> OpenSolaris 10 boot (in fact the final dictionary size after boot is  
> now < 256K once again).
>
> Blue/Andreas: please could you take a look at this patch and make  
> sure it doesn't break anything in your SPARC64/PPC tests?

It looks as if none of us actually tested or ack'ed it yet, all  
answers were about sparc progress and devices only...
Me for one couldn't save your inline patch and my mailer is known to  
cause whitespace damage, and git-am wasn't able to handle the mbox  
file since your SVN diff is missing the a/ directory level expected by  
Git.

If you're asking for comments from us, please be more patient. I'm  
still playing with Blue's slightly older libgcc patch, and some  
trivial ones of mine (e.g., CONFIG_RTAS or wrong return) are awaiting  
review/committing longer than yours. Please at least let us know in  
advance how long you're gonna wait, so that we have a chance to send  
in incomplete review comments even if still untested.

I like the general optimization idea, but I found one typo s/fix/fit/  
and I was wondering whether the property setting function really needs  
to live inside ofmem_common.c or whether we might want to make  
set_property_nocopy() out of it for general use.
I also had some style questions related to the inconsistent use of  
spacing inside braces. Some ppc code even has a wild mix of tabs and 8  
spaces, which is unfortunate when many other projects including QEMU  
use an indentation of 4. We're lacking a CODING_STYLE document imo, to  
look up which way it's supposed to be.

BR,
Andreas

P.S. I have local patches to prepare ofmem_release() inside  
ofmem_common.c.

[Blue Swirl]

On Sat, Oct 16, 2010 at 3:18 PM, Andreas Färber  wrote:
> Mark,
>
> Am 14.10.2010 um 22:32 schrieb Mark Cave-Ayland:
> > The attached patch changes OFMEM so that instead of allocating new space
> > within the Forth dictionary every time the /memory and /virtual-memory
> > available/translations nodes are updated, we simply change the property to
> > point directly to a static buffer. This has the effect of saving substantial
> > amounts of memory during OpenSolaris 10 boot (in fact the final dictionary
> > size after boot is now < 256K once again).
> >
> > Blue/Andreas: please could you take a look at this patch and make sure it
> > doesn't break anything in your SPARC64/PPC tests?
>
> It looks as if none of us actually tested or ack'ed it yet, all answers were
> about sparc progress and devices only...
> Me for one couldn't save your inline patch and my mailer is known to cause
> whitespace damage, and git-am wasn't able to handle the mbox file since your
> SVN diff is missing the a/ directory level expected by Git.
>
> If you're asking for comments from us, please be more patient. I'm still
> playing with Blue's slightly older libgcc patch, and some trivial ones of
> mine (e.g., CONFIG_RTAS or wrong return) are awaiting review/committing
> longer than yours. Please at least let us know in advance how long you're
> gonna wait, so that we have a chance to send in incomplete review comments
> even if still untested.

I'll give them a spin.
> I like the general optimization idea, but I found one typo s/fix/fit/ and I
> was wondering whether the property setting function really needs to live
> inside ofmem_common.c or whether we might want to make set_property_nocopy()
> out of it for general use.
> I also had some style questions related to the inconsistent use of spacing
> inside braces. Some ppc code even has a wild mix of tabs and 8 spaces, which
> is unfortunate when many other projects including QEMU use an indentation of
> 4. We're lacking a CODING_STYLE document imo, to look up which way it's
> supposed to be.

I'd vote for QEMU style because of common developer base and even
small amount of shared code.

[Mark Cave-Ayland]

Andreas Färber wrote:
> > Blue/Andreas: please could you take a look at this patch and make sure 
> > it doesn't break anything in your SPARC64/PPC tests?
>
> It looks as if none of us actually tested or ack'ed it yet, all answers 
> were about sparc progress and devices only...
> Me for one couldn't save your inline patch and my mailer is known to 
> cause whitespace damage, and git-am wasn't able to handle the mbox file 
> since your SVN diff is missing the a/ directory level expected by Git.
>
> If you're asking for comments from us, please be more patient. I'm still 
> playing with Blue's slightly older libgcc patch, and some trivial ones 
> of mine (e.g., CONFIG_RTAS or wrong return) are awaiting 
> review/committing longer than yours. Please at least let us know in 
> advance how long you're gonna wait, so that we have a chance to send in 
> incomplete review comments even if still untested.

I feel your pain when it comes to having multiple patchsets on the go at 
once. For a while, I ended up with about 3 simultaneous sets working 
various things that had bugs, which when I fixed had other other bugs...

In terms of the patch comments, maybe I could have waited longer. 
Generally the OpenBIOS code is still being actively developed, and so 
the few people actively working on the code generally push on with what 
they are doing, and as long as people don't introduce regressions then 
no-one complains too loudly.

A few times when I've been unsure about a patch, I've posted it to the 
list for comment and people have been fairly quick in the past to 
criticise any bad points, so mostly I accept lack of response that 
people are generally happy with it. Perhaps now with more people working 
on the code we should reconsider this? I also did a complete set of 
testing on my PPC test images and didn't see any regressions there, so I 
felt it was reasonably safe...
> I like the general optimization idea, but I found one typo s/fix/fit/ 
> and I was wondering whether the property setting function really needs 
> to live inside ofmem_common.c or whether we might want to make 
> set_property_nocopy() out of it for general use.

Yes possibly, although I think strings/integers need to be encoded (and 
it is the encoding process that makes the copy of the input) whereas 
natively the properties work using byte arrays and so we can just set 
the property to point directly to them.
> I also had some style questions related to the inconsistent use of 
> spacing inside braces. Some ppc code even has a wild mix of tabs and 8 
> spaces, which is unfortunate when many other projects including QEMU use 
> an indentation of 4. We're lacking a CODING_STYLE document imo, to look 
> up which way it's supposed to be.

Yes, maybe we should start thinking more about CODING_STYLE...
> BR,
> Andreas
>
> P.S. I have local patches to prepare ofmem_release() inside ofmem_common.c.

Interesting - these would be very useful :)

ATB,

Mark.