The filll word fills memory addresses with a value. It's different from fill (2 l's) because filll fills the specified memory addresses with 32-bit values.
Index: trunk/openbios-devel/arch/ppc/qemu/init.c =================================================================== --- trunk/openbios-devel/arch/ppc/qemu/init.c (revision 1075) +++ trunk/openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -613,6 +613,25 @@ fword("finish-device"); }
+ +// filll ( addr len num -- ) +// Fills an address in memory with 32-bit values +static void filll(void) +{ + u32 * location; + u32 length, fillNumber, x; + + fillNumber = POP(); + length = POP(); + location = (u32 *) cell2pointer(POP()); + + for(x = 0; x <= length; x++) + { + location[x] = fillNumber; + } +} + + void arch_of_init(void) { @@ -879,4 +898,5 @@
bind_func("platform-boot", boot); bind_func("(go)", go); + bind_func("filll", filll); }
On 03/12/12 20:24, Programmingkid wrote:
The filll word fills memory addresses with a value. It's different from fill (2 l's) because filll fills the specified memory addresses with 32-bit values.
First up, this is a much better version of the patch. Comments included inline below:
Index: trunk/openbios-devel/arch/ppc/qemu/init.c
--- trunk/openbios-devel/arch/ppc/qemu/init.c (revision 1075) +++ trunk/openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -613,6 +613,25 @@ fword("finish-device"); }
+// filll ( addr len num -- ) +// Fills an address in memory with 32-bit values
OpenBIOS uses C /* ... */ comments rather than C++ // comments.
+static void filll(void) +{
- u32 * location;
- u32 length, fillNumber, x;
- fillNumber = POP();
- length = POP();
- location = (u32 *) cell2pointer(POP());
- for(x = 0; x<= length; x++)
- {
location[x] = fillNumber;- }
Compare the formatting of your for() statement above with the others in the file - you should use the existing style in your patch.
Also as per my comment on your last version of the patch, you need to use target_long() as per the existing code in lstore.
+}
- void arch_of_init(void) {
@@ -879,4 +898,5 @@
bind_func("platform-boot", boot); bind_func("(go)", go);
- bind_func("filll", filll); }
ATB,
Mark.
On Dec 3, 2012, at 3:54 PM, Mark Cave-Ayland wrote:
- for(x = 0; x<= length; x++)
- {
location[x] = fillNumber;- }
Compare the formatting of your for() statement above with the others in the file - you should use the existing style in your patch.
I was never a fan of that style, but ok.
Also as per my comment on your last version of the patch, you need to use target_long() as per the existing code in lstore.
Do you mean write_long()? This is what lstore uses. Why use this function as oppose to the code I have now?
On 03/12/12 21:05, Programmingkid wrote:
Compare the formatting of your for() statement above with the others in the file - you should use the existing style in your patch.
I was never a fan of that style, but ok.
In general, writing patches is like repairing holes in a plaster wall - the aim is to blend your patch in with what is already there so it's impossible to work out where the patch begins/ends without reading the SVN log.
Also as per my comment on your last version of the patch, you need to use target_long() as per the existing code in lstore.
Do you mean write_long()? This is what lstore uses. Why use this function as oppose to the code I have now?
Oh sorry - yes, you are correct, it is write_long(). Mainly for two reasons: 1) to preserve style and 2) ensure that cross-compilation works correctly.
ATB,
Mark.
On Dec 3, 2012, at 4:17 PM, Mark Cave-Ayland wrote:
On 03/12/12 21:05, Programmingkid wrote:
Compare the formatting of your for() statement above with the others in the file - you should use the existing style in your patch.
I was never a fan of that style, but ok.
In general, writing patches is like repairing holes in a plaster wall - the aim is to blend your patch in with what is already there so it's impossible to work out where the patch begins/ends without reading the SVN log.
Also as per my comment on your last version of the patch, you need to use target_long() as per the existing code in lstore.
Do you mean write_long()? This is what lstore uses. Why use this function as oppose to the code I have now?
Oh sorry - yes, you are correct, it is write_long(). Mainly for two reasons: 1) to preserve style and 2) ensure that cross-compilation works correctly.
Ok, I finally figured out how to include cross.h. Does this code look good?
Index: trunk/openbios-devel/arch/ppc/qemu/init.c =================================================================== --- trunk/openbios-devel/arch/ppc/qemu/init.c (revision 1075) +++ trunk/openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -34,6 +34,7 @@ #define NO_QEMU_PROTOS #include "arch/common/fw_cfg.h" #include "arch/ppc/processor.h" +#include "../../../kernel/cross.h"
#define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
@@ -613,6 +614,24 @@ fword("finish-device"); }
+ +/* filll ( addr len num -- ) */ +/* Fills an address in memory with 32-bit values */ +static void filll(void) +{ + u32 * location; + u32 length, fillNumber, x; + + fillNumber = POP(); + length = POP(); + location = (u32 *) cell2pointer(POP()); + + for(x = 0; x <= length; x++) { + write_long(location++, fillNumber); + } +} + + void arch_of_init(void) { @@ -879,4 +898,5 @@
bind_func("platform-boot", boot); bind_func("(go)", go); + bind_func("filll", filll); }
On 04/12/12 00:55, Programmingkid wrote:
Ok, I finally figured out how to include cross.h. Does this code look good?
That looks much better. However if kernel/cross.h has be included in this way then it looks like this is an abstraction that shouldn't be broken...
(goes and looks)
Ha so the definition of write_long() looks like this:
#define write_long(addr, value) {*(u32 *)(addr)=(value);}
Okay in that case I think out of the two methods, your first version is more preferable to ensure that the kernel abstraction remains. I'll do some more testing, a little tidying, and queue it up for commit if it looks good.
ATB,
Mark.
On 04/12/12 21:53, Mark Cave-Ayland wrote:
Okay in that case I think out of the two methods, your first version is more preferable to ensure that the kernel abstraction remains. I'll do some more testing, a little tidying, and queue it up for commit if it looks good.
I've just committed a revised version of your patch to SVN. Note that I also fixed a bug whereby you were writing one too many long words into memory. Thanks again for the patch.
ATB,
Mark.
-
Mark Cave-Ayland -
Programmingkid