Hello,
I'm sending my previous patches split into multiple patches and some more I've done since. This is not including the change of the / node name as it was suggested to preserve the original name on some other property but the name of that new property was not decided so I don't know what to do with it but that's also needed to get further with MorphOS.
Regards, BALATON Zoltan
On 06/03/14 18:33, BALATON Zoltan wrote:
Hello,
I'm sending my previous patches split into multiple patches and some more I've done since. This is not including the change of the / node name as it was suggested to preserve the original name on some other property but the name of that new property was not decided so I don't know what to do with it but that's also needed to get further with MorphOS.
Regards, BALATON Zoltan
Thanks these patches look great! I've been absolutely flat out with other work this week but I should have a chance to review them over the weekend.
ATB,
Mark.
On 06/03/14 18:33, BALATON Zoltan wrote:
Hello,
I'm sending my previous patches split into multiple patches and some more I've done since. This is not including the change of the / node name as it was suggested to preserve the original name on some other property but the name of that new property was not decided so I don't know what to do with it but that's also needed to get further with MorphOS.
Regards, BALATON Zoltan
The patches look good, and I don't see any regressions against my -M mac99 PPC images here so I've committed all but patch 4 of this series.
The reason I haven't applied patch 4 is because this is going to need review from someone on the PPC side (Alex/Andreas), although the basic concept seems fine. Thanks a lot for the patches!
ATB,
Mark.
On Sat, 8 Mar 2014, Mark Cave-Ayland wrote:
The patches look good, and I don't see any regressions against my -M mac99 PPC images here so I've committed all but patch 4 of this series.
The reason I haven't applied patch 4 is because this is going to need review from someone on the PPC side (Alex/Andreas), although the basic concept seems fine. Thanks a lot for the patches!
Thanks. I now have these patches in my tree that are not yet merged:
1. patch 4 you mentioned above
2. Change the device device name in openbios-devel/forth/device/tree.fs to device-tree Not sure what was the consensus of the review for this one.
3. In openbios-devel/libopenbios/client.c fix debug output: } 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]); Provided by Olivier Danet here: http://www.openfirmware.info/pipermail/openbios/2014-March/008146.html
4. A new one that adds tlb-sets, tlb-size and bus-frequency properties in openbios-devel/arch/ppc/qemu/init.c but only for G4 and I'm not sure if they are really needed but these fill in some values in MorphOS debug log.
What to do with these?
Regards, BALATON Zoltan
On 2014-Mar-7, 20:26 , BALATON Zoltan wrote:
[...] 2. Change the device device name in openbios-devel/forth/device/tree.fs to device-tree Not sure what was the consensus of the review for this one.
I missed the reason you wanted the name changed; I commented at the time that for IEEE 1275, the name property at the root node is supposed to be the platform name. Why do you want it called "device-tree", which would certainly be incorrect?
On 08/03/14 01:32, Tarl Neustaedter wrote:
On 2014-Mar-7, 20:26 , BALATON Zoltan wrote:
[...] 2. Change the device device name in openbios-devel/forth/device/tree.fs to device-tree Not sure what was the consensus of the review for this one.
I missed the reason you wanted the name changed; I commented at the time that for IEEE 1275, the name property at the root node is supposed to be the platform name. Why do you want it called "device-tree", which would certainly be incorrect?
Believe it or not, it's actually what Apple use for the device name for the root node of their OF device trees: see http://web.archive.org/web/20090107145016/http://penguinppc.org/historical/d.... for an example.
Although I would also argue that using this method to detect the platform being used is a fairly brain-dead client behaviour too...
ATB,
Mark.
On 08/03/14 01:26, BALATON Zoltan wrote:
Thanks. I now have these patches in my tree that are not yet merged:
patch 4 you mentioned above
Change the device device name in openbios-devel/forth/device/tree.fs
to device-tree Not sure what was the consensus of the review for this one.
- In openbios-devel/libopenbios/client.c fix debug output:
} 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]); Provided by Olivier Danet here: http://www.openfirmware.info/pipermail/openbios/2014-March/008146.html
This patch looks fine, so I've just applied this to the main repository (with credit to Oliver).
- A new one that adds tlb-sets, tlb-size and bus-frequency properties
in openbios-devel/arch/ppc/qemu/init.c but only for G4 and I'm not sure if they are really needed but these fill in some values in MorphOS debug log.
What to do with these?
I would say do a separate patchset with patches 1 and 4 with CC to the PPC maintainers (Alex and Andreas) and make sure that they are happy with the changes.
Patch 2 is little more tricky; I'd be tempted to move the "OpenBIOS" version to another property in the root node of the tree and set it to "device-tree" for compatibility reasons. Then again I'm finding it quite hard to justify jumping through these sorts of hoops for an OS whose developers are so actively hostile :/
ATB,
Mark.
On Sat, 8 Mar 2014, Mark Cave-Ayland wrote:
This patch looks fine, so I've just applied this to the main repository (with credit to Oliver).
Thanks.
I would say do a separate patchset with patches 1 and 4 with CC to the PPC maintainers (Alex and Andreas) and make sure that they are happy with the changes.
OK, I will do so after I came up with a hopefully final version. I may need to change them still.
Patch 2 is little more tricky; I'd be tempted to move the "OpenBIOS" version to another property in the root node of the tree and set it to "device-tree" for compatibility reasons. Then again I'm finding it quite hard to justify
I don't mind which solution will be chosen as long as we can match the Apple behaviour on PPC. I see these alternatives:
1. My first version was to change it for all but this does not seem to be the right choice.
2. Mark's suggestion was to move the current value to some other property and change the name. If we choose this we need to decide the name of the new property. My suggestion for this was "vendor" but this is just a guess based on nothing and I'm open to other suggestions. If we agree on a name I can prepare a patch. Does the name of the root node actually matters to anyone on platforms other than Apple?
3. I may look into enclosing it in [IFDEF] CONFIG_PPC or even patching it up in qemu/init.c based on what is_apple returns but does it worth the hassle?
Regards, BALATON Zoltan
On Sat, 8 Mar 2014, Mark Cave-Ayland wrote:
I would say do a separate patchset with patches 1 and 4 with CC to the PPC maintainers (Alex and Andreas) and make sure that they are happy with the changes.
Here are my proposed changes to qemu/init.c. This is all in one patch currently that I can split into 3 or 4 separate patches if that preferred but before doing that I'd like to get your comments if other changes are needed. The patch contains the following changes:
- Remove unused clock_frequency from CPU info (these are now get via FW_CFG for a while) - Add tlb-sets and tlb-size instead (only for G4 for now because I don't know the correct values for other CPUs) - Add bus-frequency property to CPU - Rename the root node on Apple machines to match their conventions
Regards, BALATON Zoltan
Index: openbios-devel/arch/ppc/qemu/init.c =================================================================== --- openbios-devel/arch/ppc/qemu/init.c (revision 1277) +++ openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -44,7 +44,7 @@ int icache_size, dcache_size; int icache_sets, dcache_sets; int icache_block_size, dcache_block_size; - int clock_frequency; + int tlb_sets, tlb_size; void (*initfn)(const struct cpudef *cpu); };
@@ -269,6 +269,18 @@ push_str("i-cache-block-size"); fword("property");
+ if(cpu->tlb_size) { + PUSH(cpu->tlb_sets); + fword("encode-int"); + push_str("tlb-sets"); + fword("property"); + + PUSH(cpu->tlb_size); + fword("encode-int"); + push_str("tlb-size"); + fword("property"); + } + PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); fword("encode-int"); push_str("timebase-frequency"); @@ -279,6 +291,11 @@ push_str("clock-frequency"); fword("property");
+ PUSH(100 * 1000 * 1000); + fword("encode-int"); + push_str("bus-frequency"); + fword("property"); + push_str("running"); fword("encode-string"); push_str("state"); @@ -391,7 +408,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x07de2900, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values @@ -403,7 +421,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x07de2900, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values @@ -415,7 +434,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x07de2900, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values @@ -427,7 +447,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x14dc9380, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_750_init, }, { @@ -439,7 +460,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x14dc9380, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values @@ -451,7 +473,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x14dc9380, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values @@ -463,7 +486,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x14dc9380, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values @@ -475,7 +499,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x14dc9380, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values @@ -487,7 +512,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x14dc9380, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_750_init, }, { @@ -499,7 +525,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20, - .clock_frequency = 0x1dcd6500, + .tlb_sets = 0x40, + .tlb_size = 0x80, .initfn = cpu_g4_init, }, { @@ -511,7 +538,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80, - .clock_frequency = 0x5f5e1000, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_970_init, }, { // XXX find out real values @@ -523,7 +551,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80, - .clock_frequency = 0x1dcd6500, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_970_init, }, { @@ -535,7 +564,8 @@ .dcache_sets = 0x40, .icache_block_size = 0x80, .dcache_block_size = 0x80, - .clock_frequency = 0x629b4940, + .tlb_sets = 0x00, + .tlb_size = 0x00, .initfn = cpu_970_init, }, }; @@ -702,6 +732,12 @@ push_str("/"); fword("find-device");
+ /* Apple calls the root node device-tree */ + if (is_apple()) { + push_str("device-tree"); + fword("device-name"); + } + switch(machine_id) { case ARCH_HEATHROW: /* OldWorld */
On 09.03.14 01:13, BALATON Zoltan wrote:
On Sat, 8 Mar 2014, Mark Cave-Ayland wrote:
I would say do a separate patchset with patches 1 and 4 with CC to the PPC maintainers (Alex and Andreas) and make sure that they are happy with the changes.
Here are my proposed changes to qemu/init.c. This is all in one patch currently that I can split into 3 or 4 separate patches if that preferred but before doing that I'd like to get your comments if other changes are needed. The patch contains the following changes:
- Remove unused clock_frequency from CPU info (these are now get via
FW_CFG for a while)
- Add tlb-sets and tlb-size instead (only for G4 for now because I don't
know the correct values for other CPUs)
- Add bus-frequency property to CPU
- Rename the root node on Apple machines to match their conventions
Regards, BALATON Zoltan
Index: openbios-devel/arch/ppc/qemu/init.c
--- openbios-devel/arch/ppc/qemu/init.c (revision 1277) +++ openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -44,7 +44,7 @@ int icache_size, dcache_size; int icache_sets, dcache_sets; int icache_block_size, dcache_block_size;
- int clock_frequency;
- int tlb_sets, tlb_size; void (*initfn)(const struct cpudef *cpu); };
@@ -269,6 +269,18 @@ push_str("i-cache-block-size"); fword("property");
- if(cpu->tlb_size) {
PUSH(cpu->tlb_sets);fword("encode-int");push_str("tlb-sets");fword("property");PUSH(cpu->tlb_size);fword("encode-int");push_str("tlb-size");fword("property");
Please just find the correct values and add the properties to the cpu nodes always.
- }
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); fword("encode-int"); push_str("timebase-frequency");@@ -279,6 +291,11 @@ push_str("clock-frequency"); fword("property");
- PUSH(100 * 1000 * 1000);
- fword("encode-int");
- push_str("bus-frequency");
- fword("property");
I'm fairly sure this is wrong.
push_str("running"); fword("encode-string"); push_str("state");@@ -391,7 +408,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -403,7 +421,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -415,7 +434,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -427,7 +447,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, {@@ -439,7 +460,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -451,7 +473,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -463,7 +486,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -475,7 +499,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -487,7 +512,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, {@@ -499,7 +525,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x1dcd6500,
.tlb_sets = 0x40,.tlb_size = 0x80, .initfn = cpu_g4_init, }, {@@ -511,7 +538,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x5f5e1000,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, }, { // XXX find out real values@@ -523,7 +551,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x1dcd6500,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, }, {@@ -535,7 +564,8 @@ .dcache_sets = 0x40, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x629b4940,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, },@@ -702,6 +732,12 @@ push_str("/"); fword("find-device");
- /* Apple calls the root node device-tree */
- if (is_apple()) {
push_str("device-tree");fword("device-name");- }
Always? No difference between oldworld and newworld?
Alex
On Mon, 14 Apr 2014, Alexander Graf wrote:
On 09.03.14 01:13, BALATON Zoltan wrote:
On Sat, 8 Mar 2014, Mark Cave-Ayland wrote:
I would say do a separate patchset with patches 1 and 4 with CC to the PPC maintainers (Alex and Andreas) and make sure that they are happy with the changes.
Here are my proposed changes to qemu/init.c. This is all in one patch currently that I can split into 3 or 4 separate patches if that preferred but before doing that I'd like to get your comments if other changes are needed. The patch contains the following changes:
- Remove unused clock_frequency from CPU info (these are now get via
FW_CFG for a while)
- Add tlb-sets and tlb-size instead (only for G4 for now because I don't
know the correct values for other CPUs)
- Add bus-frequency property to CPU
- Rename the root node on Apple machines to match their conventions
Regards, BALATON Zoltan
Index: openbios-devel/arch/ppc/qemu/init.c
--- openbios-devel/arch/ppc/qemu/init.c (revision 1277) +++ openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -44,7 +44,7 @@ int icache_size, dcache_size; int icache_sets, dcache_sets; int icache_block_size, dcache_block_size;
- int clock_frequency;
- int tlb_sets, tlb_size; void (*initfn)(const struct cpudef *cpu); };
@@ -269,6 +269,18 @@ push_str("i-cache-block-size"); fword("property");
- if(cpu->tlb_size) {
PUSH(cpu->tlb_sets);fword("encode-int");push_str("tlb-sets");fword("property");PUSH(cpu->tlb_size);fword("encode-int");push_str("tlb-size");fword("property");Please just find the correct values and add the properties to the cpu nodes always.
Where can I find the correct values? Maybe QEMU should send it via FW_CFG because OpenBIOS only seems to have data for a few CPUs and most of them have a comment saying that the values are not correct anyway. But I could not find where is this data available in QEMU.
- }
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); fword("encode-int"); push_str("timebase-frequency");@@ -279,6 +291,11 @@ push_str("clock-frequency"); fword("property");
- PUSH(100 * 1000 * 1000);
- fword("encode-int");
- push_str("bus-frequency");
- fword("property");
I'm fairly sure this is wrong.
What is the good value then? (Some other data e.g. the clock frequency is also a made up value to make the clients happy.)
push_str("running"); fword("encode-string"); push_str("state");@@ -391,7 +408,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -403,7 +421,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -415,7 +434,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -427,7 +447,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, {@@ -439,7 +460,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -451,7 +473,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -463,7 +486,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -475,7 +499,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -487,7 +512,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, {@@ -499,7 +525,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x1dcd6500,
.tlb_sets = 0x40,.tlb_size = 0x80, .initfn = cpu_g4_init, }, {@@ -511,7 +538,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x5f5e1000,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, }, { // XXX find out real values@@ -523,7 +551,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x1dcd6500,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, }, {@@ -535,7 +564,8 @@ .dcache_sets = 0x40, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x629b4940,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, },@@ -702,6 +732,12 @@ push_str("/"); fword("find-device");
- /* Apple calls the root node device-tree */
- if (is_apple()) {
push_str("device-tree");fword("device-name");- }
Always? No difference between oldworld and newworld?
As far as I can tell looking at these device tree dumps: http://web.archive.org/web/20081120133742/http://penguinppc.org/historical/d... it seems to be always called like that also on old world machines.
Regards, BALATON Zoltan
On 14.04.14 22:20, BALATON Zoltan wrote:
On Mon, 14 Apr 2014, Alexander Graf wrote:
On 09.03.14 01:13, BALATON Zoltan wrote:
On Sat, 8 Mar 2014, Mark Cave-Ayland wrote:
I would say do a separate patchset with patches 1 and 4 with CC to the PPC maintainers (Alex and Andreas) and make sure that they are happy with the changes.
Here are my proposed changes to qemu/init.c. This is all in one patch currently that I can split into 3 or 4 separate patches if that preferred but before doing that I'd like to get your comments if other changes are needed. The patch contains the following changes:
- Remove unused clock_frequency from CPU info (these are now get via
FW_CFG for a while)
- Add tlb-sets and tlb-size instead (only for G4 for now because I
don't know the correct values for other CPUs)
- Add bus-frequency property to CPU
- Rename the root node on Apple machines to match their conventions
Regards, BALATON Zoltan
Index: openbios-devel/arch/ppc/qemu/init.c
--- openbios-devel/arch/ppc/qemu/init.c (revision 1277) +++ openbios-devel/arch/ppc/qemu/init.c (working copy) @@ -44,7 +44,7 @@ int icache_size, dcache_size; int icache_sets, dcache_sets; int icache_block_size, dcache_block_size;
- int clock_frequency;
- int tlb_sets, tlb_size; void (*initfn)(const struct cpudef *cpu); };
@@ -269,6 +269,18 @@ push_str("i-cache-block-size"); fword("property");
- if(cpu->tlb_size) {
PUSH(cpu->tlb_sets);fword("encode-int");push_str("tlb-sets");fword("property");PUSH(cpu->tlb_size);fword("encode-int");push_str("tlb-size");fword("property");Please just find the correct values and add the properties to the cpu nodes always.
Where can I find the correct values? Maybe QEMU should send it via FW_CFG because OpenBIOS only seems to have data for a few CPUs and most of them have a comment saying that the values are not correct anyway. But I could not find where is this data available in QEMU.
That'd work for me. I also wouldn't be opposed to move OpenBIOS to a scheme similar to SLOF and u-boot where we pass the device tree to firmware and use that instead of fw_cfg to enumerate devices in our system.
- }
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); fword("encode-int"); push_str("timebase-frequency");@@ -279,6 +291,11 @@ push_str("clock-frequency"); fword("property");
- PUSH(100 * 1000 * 1000);
- fword("encode-int");
- push_str("bus-frequency");
- fword("property");
I'm fairly sure this is wrong.
What is the good value then? (Some other data e.g. the clock frequency is also a made up value to make the clients happy.)
Don't we send the bus frequency via fw_cfg? I remember we had a pretty long thread about bus frequency and how it kept breaking Mac OS X.
push_str("running"); fword("encode-string"); push_str("state");@@ -391,7 +408,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -403,7 +421,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -415,7 +434,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x07de2900,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_604_init, }, { // XXX find out real values@@ -427,7 +447,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, {@@ -439,7 +460,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -451,7 +473,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -463,7 +486,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -475,7 +499,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, { // XXX find out real values@@ -487,7 +512,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x14dc9380,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_750_init, }, {@@ -499,7 +525,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x20, .dcache_block_size = 0x20,
.clock_frequency = 0x1dcd6500,
.tlb_sets = 0x40,.tlb_size = 0x80, .initfn = cpu_g4_init, }, {@@ -511,7 +538,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x5f5e1000,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, }, { // XXX find out real values@@ -523,7 +551,8 @@ .dcache_sets = 0x80, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x1dcd6500,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, }, {@@ -535,7 +564,8 @@ .dcache_sets = 0x40, .icache_block_size = 0x80, .dcache_block_size = 0x80,
.clock_frequency = 0x629b4940,
.tlb_sets = 0x00,.tlb_size = 0x00, .initfn = cpu_970_init, },@@ -702,6 +732,12 @@ push_str("/"); fword("find-device");
- /* Apple calls the root node device-tree */
- if (is_apple()) {
push_str("device-tree");fword("device-name");- }
Always? No difference between oldworld and newworld?
As far as I can tell looking at these device tree dumps: http://web.archive.org/web/20081120133742/http://penguinppc.org/historical/d...
it seems to be always called like that also on old world machines.
Ok :).
Alex
On Wed, 16 Apr 2014, Alexander Graf wrote:
@@ -269,6 +269,18 @@ push_str("i-cache-block-size"); fword("property");
- if(cpu->tlb_size) {
PUSH(cpu->tlb_sets);fword("encode-int");push_str("tlb-sets");fword("property");PUSH(cpu->tlb_size);fword("encode-int");push_str("tlb-size");fword("property");Please just find the correct values and add the properties to the cpu nodes always.
Where can I find the correct values? Maybe QEMU should send it via FW_CFG because OpenBIOS only seems to have data for a few CPUs and most of them have a comment saying that the values are not correct anyway. But I could not find where is this data available in QEMU.
That'd work for me. I also wouldn't be opposed to move OpenBIOS to a scheme similar to SLOF and u-boot where we pass the device tree to firmware and use that instead of fw_cfg to enumerate devices in our system.
The problem with this is that OpenBIOS is used for a lot more architectures than just mac emulation and such a change would also affect others (like sparc). Also the device tree is partly created in Forth and then patched in C so using a whole device tree from QEMU would need modifications to the Forth part as well. So this is no small task and one that's too much for me. If someone can take on this task it may be done but the most I can do is adding these to FW_CFG and even for that someone should tell me how to find these values in QEMU.
- }
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); fword("encode-int"); push_str("timebase-frequency");@@ -279,6 +291,11 @@ push_str("clock-frequency"); fword("property");
- PUSH(100 * 1000 * 1000);
- fword("encode-int");
- push_str("bus-frequency");
- fword("property");
I'm fairly sure this is wrong.
What is the good value then? (Some other data e.g. the clock frequency is also a made up value to make the clients happy.)
Don't we send the bus frequency via fw_cfg? I remember we had a pretty long thread about bus frequency and how it kept breaking Mac OS X.
Wasn't that the clock-frequency? That is sent via FW_CFG but the bus-frequency isn't and was just missing from the CPU properties.
Regards, BALATON Zoltan
On 16.04.14 18:35, BALATON Zoltan wrote:
On Wed, 16 Apr 2014, Alexander Graf wrote:
@@ -269,6 +269,18 @@ push_str("i-cache-block-size"); fword("property");
- if(cpu->tlb_size) {
PUSH(cpu->tlb_sets);fword("encode-int");push_str("tlb-sets");fword("property");PUSH(cpu->tlb_size);fword("encode-int");push_str("tlb-size");fword("property");Please just find the correct values and add the properties to the cpu nodes always.
Where can I find the correct values? Maybe QEMU should send it via FW_CFG because OpenBIOS only seems to have data for a few CPUs and most of them have a comment saying that the values are not correct anyway. But I could not find where is this data available in QEMU.
That'd work for me. I also wouldn't be opposed to move OpenBIOS to a scheme similar to SLOF and u-boot where we pass the device tree to firmware and use that instead of fw_cfg to enumerate devices in our system.
The problem with this is that OpenBIOS is used for a lot more architectures than just mac emulation and such a change would also affect others (like sparc). Also the device tree is partly created in Forth and then patched in C so using a whole device tree from QEMU would need modifications to the Forth part as well. So this is no small task and one that's too much for me. If someone can take on this task it may be done but the most I can do is adding these to FW_CFG and even for that someone should tell me how to find these values in QEMU.
- }
PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ)); fword("encode-int"); push_str("timebase-frequency");@@ -279,6 +291,11 @@ push_str("clock-frequency"); fword("property");
- PUSH(100 * 1000 * 1000);
- fword("encode-int");
- push_str("bus-frequency");
- fword("property");
I'm fairly sure this is wrong.
What is the good value then? (Some other data e.g. the clock frequency is also a made up value to make the clients happy.)
Don't we send the bus frequency via fw_cfg? I remember we had a pretty long thread about bus frequency and how it kept breaking Mac OS X.
Wasn't that the clock-frequency? That is sent via FW_CFG but the bus-frequency isn't and was just missing from the CPU properties.
Ah, it was the clock frequency vs tb frequency. Jeez - we have way too many of these.
Yes, please add it as a value in fw_cfg.
Alex
On Wed, 16 Apr 2014, Alexander Graf wrote:
Ah, it was the clock frequency vs tb frequency. Jeez - we have way too many of these.
Yes, please add it as a value in fw_cfg.
This is what's in qemu/include/hw/ppc/ppc.h:
#define FW_CFG_PPC_WIDTH (FW_CFG_ARCH_LOCAL + 0x00) #define FW_CFG_PPC_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01) #define FW_CFG_PPC_DEPTH (FW_CFG_ARCH_LOCAL + 0x02) #define FW_CFG_PPC_TBFREQ (FW_CFG_ARCH_LOCAL + 0x03) #define FW_CFG_PPC_CLOCKFREQ (FW_CFG_ARCH_LOCAL + 0x04) #define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05) #define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06) #define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07)
And this is what's in openbios-devel/include/arch/common/fw_cfg.h:
#define FW_CFG_PPC_WIDTH (FW_CFG_ARCH_LOCAL + 0x00) #define FW_CFG_PPC_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01) #define FW_CFG_PPC_DEPTH (FW_CFG_ARCH_LOCAL + 0x02) #define FW_CFG_PPC_TBFREQ (FW_CFG_ARCH_LOCAL + 0x03) #define FW_CFG_PPC_CLOCKFREQ (FW_CFG_ARCH_LOCAL + 0x04) #define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05) #define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06) #define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07) #define FW_CFG_PPC_NVRAM_ADDR (FW_CFG_ARCH_LOCAL + 0x08)
Should I add new FW_CFG values with a gap (or adding an unused FW_CFG_PPC_NVRAM_ADDR to qemu) or should I renumber these ti add FW_CFG_PPC_BUSFREQ grouped with the other FREQ values?
Regards, BALATON Zoltan
On 17.04.14 11:52, BALATON Zoltan wrote:
On Wed, 16 Apr 2014, Alexander Graf wrote:
Ah, it was the clock frequency vs tb frequency. Jeez - we have way too many of these.
Yes, please add it as a value in fw_cfg.
This is what's in qemu/include/hw/ppc/ppc.h:
#define FW_CFG_PPC_WIDTH (FW_CFG_ARCH_LOCAL + 0x00) #define FW_CFG_PPC_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01) #define FW_CFG_PPC_DEPTH (FW_CFG_ARCH_LOCAL + 0x02) #define FW_CFG_PPC_TBFREQ (FW_CFG_ARCH_LOCAL + 0x03) #define FW_CFG_PPC_CLOCKFREQ (FW_CFG_ARCH_LOCAL + 0x04) #define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05) #define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06) #define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07)
And this is what's in openbios-devel/include/arch/common/fw_cfg.h:
#define FW_CFG_PPC_WIDTH (FW_CFG_ARCH_LOCAL + 0x00) #define FW_CFG_PPC_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01) #define FW_CFG_PPC_DEPTH (FW_CFG_ARCH_LOCAL + 0x02) #define FW_CFG_PPC_TBFREQ (FW_CFG_ARCH_LOCAL + 0x03) #define FW_CFG_PPC_CLOCKFREQ (FW_CFG_ARCH_LOCAL + 0x04) #define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05) #define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06) #define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07) #define FW_CFG_PPC_NVRAM_ADDR (FW_CFG_ARCH_LOCAL + 0x08)
Should I add new FW_CFG values with a gap (or adding an unused FW_CFG_PPC_NVRAM_ADDR to qemu) or should I renumber these ti add FW_CFG_PPC_BUSFREQ grouped with the other FREQ values?
Just add BUSFREQ as 0x09 in both and leave a gap in QEMU's fw_cfg header.
Alex
-
Alexander Graf -
BALATON Zoltan -
Mark Cave-Ayland -
Tarl Neustaedter