[Qemu-devel] Help needed testing on ppc

[Qemu-devel] Help needed testing on ppc

[BALATON Zoltan]

Hello,

As I got no reply on the qemu-ppc list so far I try here maybe there are 
some people who read this list but don't follow the ppc one.

I don't have the necessary hardware to do the testing needed for the patch 
below. Some context for the discussion can be found in this message: 
http://lists.nongnu.org/archive/html/qemu-ppc/2014-04/msg00277.html

It seems we have some code that contains instructions with a reserved bit set 
in an stwx instruction that works on real hardware but causes an invalid 
instruction exception on QEMU.

I'd appreciate some insight and help.

Regards,
BALATON Zoltan

On Thu, 17 Apr 2014, Programmingkid wrote:
> On Apr 17, 2014, at 3:16 AM, qemu-ppc-request@nongnu.org wrote:
>> On Wed, 16 Apr 2014, Alexander Graf wrote:
>>> On 16.04.14 12:24, BALATON Zoltan wrote:
>>>> On Tue, 15 Apr 2014, Alexander Graf wrote:
>>>>> Try to do the same with the _E macro. Be creative :)
>>>> 
>>>> This one did it:
>>>> 
>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>>>> index e3fcb03..d1e175e 100644
>>>> --- a/target-ppc/translate.c
>>>> +++ b/target-ppc/translate.c
>>>> @@ -10341,7 +10341,7 @@ GEN_HANDLER(stop##u, opc, 0xFF, 0xFF, 0x00000000,
>>>> type),
>>>> #define GEN_STUX(name, stop, opc2, opc3, type)
>>>> \
>>>> GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
>>>> #define GEN_STX_E(name, stop, opc2, opc3, type, type2)
>>>> \
>>>> -GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
>>>> +GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000000, type, type2),
>>>> #define GEN_STS(name, stop, op, type)
>>>> \
>>>> GEN_ST(name, stop, op | 0x20, type)
>>>> \
>>>> GEN_STU(name, stop, op | 0x21, type)
>>>> \
>>> 
>>> Cool. Could you please write a small program similar to the one I sent you
>>> that runs all of these instructions and checks that really none of them
>>> trigger a program interrupt on real hardware? We can then remove the 
>>> reserved
>>> 1 bit from the mask.
>> 
>> Would something like this work? (You should be able to change the
>> instruction to test at the 1: label.) I can't test it though without a PPC
>> machine.
>> 
>> #include <stdio.h>
>> 
>> int main(int argc, char **argv)
>> {
>>   register unsigned long r8 asm("r8");
>>   register unsigned long r9 asm("r9");
>>   register unsigned long r10 asm("r10");
>>   register unsigned long r11 asm("r11");
>>   register unsigned long r12 asm("r12");
>>   long val = 0;
>>
>>   r8 = 0;
>>   r9 = (long)&val;
>>   r10 = 0;
>>
>>   asm volatile("mfcr 8                \n\t"
>>                "bl 1f                 \n\t"
>>                "mfcr 11               \n\t"
>>                "mflr 0                \n\t"
>>                "lwz 8, 36(0)          \n\t"
>>                "ori 8, 8, 1           \n\t"
>>                "stw 8, 36(0)          \n\t"
>>                "mfcr 8                \n\t"
>>                "bl 1f                 \n\t"
>>                "mfcr 12               \n\t"
>>                "b 2f                  \n\t"
>>                "1: stwx 8, 9, 10      \n\t"
>>                "blr                   \n\t"
>>                "2:                    \n\t"
>>                 : "=r"(r8), "=r"(r11), "=r"(r12)
>>                 : "r"(r8), "r"(r9), "r"(r10)
>>                 : "cc");
>>
>>   printf("old cr  (mem):\t%#lx\n", val);
>>   printf("old cr  (reg):\t%#lx\n", r8);
>>   printf("new cr1 (reg):\t%#lx\n", r11);
>>   printf("new cr2 (reg):\t%#lx\n", r12);
>>
>>   return 0;
>> }
>> 
>> Regards,
>> BALATON Zoltan
> 
> Just tried out your program on a Macintosh with a G3 processor. It doesn't 
> compile under Mac OS X. Under Linux it crashes with a segmentation fault.

Re: [Qemu-devel] Help needed testing on ppc

[Tom Musta]

On 5/6/2014 5:03 AM, BALATON Zoltan wrote:
> Hello,
> 
> As I got no reply on the qemu-ppc list so far I try here maybe there are some people who read this list but don't follow the ppc one.
> 
> I don't have the necessary hardware to do the testing needed for the patch below. Some context for the discussion can be found in this message: http://lists.nongnu.org/archive/html/qemu-ppc/2014-04/msg00277.html
> 
> It seems we have some code that contains instructions with a reserved bit set in an stwx instruction that works on real hardware but causes an invalid instruction exception on QEMU.
> 
> I'd appreciate some insight and help.
> 
> Regards,
> BALATON Zoltan

This is a bit tricky.  You appear to have code that has a reserved bit set.

Early forms of the PowerPC ISA (circa 1998) said this:  "All reserved fields in instructions should be zero.  If they are not, the instruction form
is invalid. ...  Any attempt to execute an invalid form of an instruction will cause the system illegal instruction error handler to
be invoked or yield boundedly undefined results."   QEMU, as a general rule, meets this requirement by causing illegal instruction
exceptions.

More modern versions of the ISA (circa 2006) say this: "Reserved fields in instructions are ignored by the processor.  This is a requirement
in the Server environment and is being phased into the Embedded environment. ... To maximize compatibility with future architecture
extensions, software must ensure that reserved fields in instructions contain zero and that defined fields of instructions do not contain
reserved values."  Technically, QEMU does not comply with the requirement in the first sentence;  and MorpOS does not comply with the third.

The newer form of the ISA is compatible with the older one since ignoring reserved fields is a valid implementation of "boundedly undefined."

A few questions and comments:

(1) Why is MorphOS using this invalid instruction form?  Would it be easier to fix the OS rather than QEMU?  Is there some undocumented
processor behavior that the code is dependent upon (e.g. is it actually expected CR0 to be set?).

(2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not
addressed by the patch.  I think fixing only some will be a future source of confusion.

(3) The change risks breaking behavior on older designs which may very well have taken the illegal instruction interrupt.  Would it make more
sense to leave the masks as-is and instead make a single, isolated change in the decoder (gen_intermediate_code_internal).  This behavior
could be made conditional (configuration item?  processor family specific flag?).  Unfortunately, the masks also catch some invalid forms
that do not involve reserved fields (e.g. lq/stq to odd numbered registers).

(4) In general, modeling undefined behavior is a slippery slope.  I would much prefer to see the code fixed or justified before changing QEMU.

Re: [Qemu-devel] Help needed testing on ppc

[BALATON Zoltan]

On Tue, 6 May 2014, Tom Musta wrote:
> On 5/6/2014 5:03 AM, BALATON Zoltan wrote:
>> Hello,
>>
>> As I got no reply on the qemu-ppc list so far I try here maybe there 
>> are some people who read this list but don't follow the ppc one.
>>
>> I don't have the necessary hardware to do the testing needed for the 
>> patch below. Some context for the discussion can be found in this 
>> message: 
>> http://lists.nongnu.org/archive/html/qemu-ppc/2014-04/msg00277.html
>>
>> It seems we have some code that contains instructions with a reserved 
>> bit set in an stwx instruction that works on real hardware but causes 
>> an invalid instruction exception on QEMU.
>>
>> I'd appreciate some insight and help.
>>
>> Regards,
>> BALATON Zoltan
>
> This is a bit tricky.  You appear to have code that has a reserved bit 
> set.
>
> Early forms of the PowerPC ISA (circa 1998) said this:  "All reserved 
> fields in instructions should be zero.  If they are not, the instruction 
> form is invalid. ...  Any attempt to execute an invalid form of an 
> instruction will cause the system illegal instruction error handler to 
> be invoked or yield boundedly undefined results."  QEMU, as a general 
> rule, meets this requirement by causing illegal instruction exceptions.
>
> More modern versions of the ISA (circa 2006) say this: "Reserved fields 
> in instructions are ignored by the processor.  This is a requirement in 
> the Server environment and is being phased into the Embedded 
> environment. ... To maximize compatibility with future architecture 
> extensions, software must ensure that reserved fields in instructions 
> contain zero and that defined fields of instructions do not contain 
> reserved values."  Technically, QEMU does not comply with the 
> requirement in the first sentence;  and MorpOS does not comply with the 
> third.
>
> The newer form of the ISA is compatible with the older one since 
> ignoring reserved fields is a valid implementation of "boundedly 
> undefined."

Thanks for the exhaustive answer with definitive references. This is 
really very helpful.

> A few questions and comments:
>
> (1) Why is MorphOS using this invalid instruction form?  Would it be 
> easier to fix the OS rather than QEMU?

I don't know why is it used. I can ask the MorphOS developers but they did 
not seem to be too supportive so far and at least one of them expressed 
that they have no interest supporting other than their officially 
supported list of hardware at this time. So I assume it is easier to fix 
QEMU than MorphOS and if it works on a real Mac then it should also work 
on QEMU's emulation of that Mac hardware.

> Is there some undocumented processor behavior that the code is dependent 
> upon (e.g. is it actually expected CR0 to be set?).

This is what the testing was supposed to find out but MorphOS seems to run 
better with the quoted patch so I don't think it depends on any other 
undocumented behaviour other than ignoring reserved bits but I have no 
definitive answer.

> (2) Your patch makes some store instructions compliant with the most 
> recent ISAs but there are many other instructions that are not addressed 
> by the patch.  I think fixing only some will be a future source of 
> confusion.
>
> (3) The change risks breaking behavior on older designs which may very 
> well have taken the illegal instruction interrupt.  Would it make more 
> sense to leave the masks as-is and instead make a single, isolated 
> change in the decoder (gen_intermediate_code_internal).  This behavior 
> could be made conditional (configuration item?  processor family 
> specific flag?).  Unfortunately, the masks also catch some invalid forms 
> that do not involve reserved fields (e.g. lq/stq to odd numbered 
> registers).

I don't know this code very well so not sure I can follow your suggestion. 
Are you proposing that the invalid masks could be ignored globally in 
gen_intermediate_code_internal (around target-ppc/traslate.c:11444) based 
on some condition for all opcodes?

Since your quotes above show that QEMU does not implement the current 
specification and code relying on older behaviour would not run on newer 
processors so it's likely they will get fixed so I think the risk of 
breaking older designs is less than breaking software that rely on current 
specification so IMO it should be changed in QEMU if possible and only 
care about older designs when one is actually encountered.

> (4) In general, modeling undefined behavior is a slippery slope.  I 
> would much prefer to see the code fixed or justified before changing 
> QEMU.

I can try to ask on the MorphOS list but their previous answer to another 
question was that it works on the hardware they officially support.

Regards,
BALATON Zoltan

Re: [Qemu-devel] Help needed testing on ppc

[Tom Musta]

On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
> On Tue, 6 May 2014, Tom Musta wrote:
>> On 5/6/2014 5:03 AM, BALATON Zoltan wrote:
>>> I'd appreciate some insight and help.
[snip]
>> (1) Why is MorphOS using this invalid instruction form?  Would it be easier to fix the OS rather than QEMU?
> 
> I don't know why is it used. I can ask the MorphOS developers but they did not seem to be too supportive so far and at least one of them expressed that they have no interest supporting other than their officially supported list of hardware at this time. So
> I assume it is easier to fix QEMU than MorphOS and if it works on a real Mac then it should also work on QEMU's emulation of that Mac hardware.
> 
>> Is there some undocumented processor behavior that the code is dependent upon (e.g. is it actually expected CR0 to be set?).
> 
> This is what the testing was supposed to find out but MorphOS seems to run better with the quoted patch so I don't think it depends on any other undocumented behaviour other than ignoring reserved bits but I have no definitive answer.
> 

It still seems to me that setting a reserved instruction bit is an strange thing to do.  It would be nice to at least
have a justification from MorphOS.  It is possible that no one even knows the answer.

>> (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch.  I think fixing only some will be a future source of confusion.>>

Alex:  do you have an opinion on this?  Are you OK with changing masks for a few stores but not all instructions in general?

>> (3) The change risks breaking behavior on older designs which may very well have taken the illegal instruction interrupt.  Would it make more sense to leave the masks as-is and instead make a single, isolated change in the decoder
>> (gen_intermediate_code_internal).  This behavior could be made conditional (configuration item?  processor family specific flag?).  Unfortunately, the masks also catch some invalid forms that do not involve reserved fields (e.g. lq/stq to odd numbered
>> registers).
> 
> I don't know this code very well so not sure I can follow your suggestion. Are you proposing that the invalid masks could be ignored globally in gen_intermediate_code_internal (around target-ppc/traslate.c:11444) based on some condition for all opcodes?
> 

target-ppc/translate.c has a function named gen_intermediate_code_internal which does the decoding of the guest instructions.
Specifically it has this code:

 11434              }
 11435          } else {
 11436              uint32_t inval;
 11437
 11438              if (unlikely(handler->type & (PPC_SPE | PPC_SPE_SINGLE | PPC_SPE_DOUBLE) && Rc(ctx.opcode))) {
 11439                  inval = handler->inval2;
 11440              } else {
 11441                  inval = handler->inval1;
 11442              }
 11443
 11444              if (unlikely((ctx.opcode & inval) != 0)) {
 11445                  if (qemu_log_enabled()) {
 11446                      qemu_log("invalid bits: %08x for opcode: "
 11447                               "%02x - %02x - %02x (%08x) " TARGET_FMT_lx "\n",
 11448                               ctx.opcode & inval, opc1(ctx.opcode),
 11449                               opc2(ctx.opcode), opc3(ctx.opcode),
 11450                               ctx.opcode, ctx.nip - 4);
 11451                  }
 11452                  gen_inval_exception(ctxp, POWERPC_EXCP_INVAL_INVAL);
 11453                  break;
 11454              }
 11455          }

My observations are that (a) rather than fix individual masks (like your patch does), one could inhibit the detection of illegal bits
in one spot.  This behavior could be made dependent on something ... a configuration flag ... or it could be dependent on the processor
model.  So there is an opportunity to not change every PPC model because of an oddity in MorpOS.

> Since your quotes above show that QEMU does not implement the current specification and code relying on older behaviour would not run on newer processors so it's likely they will get fixed so I think the risk of breaking older designs is less than breaking
> software that rely on current specification so IMO it should be changed in QEMU if possible and only care about older designs when one is actually encountered.
> 

I agree with this argument except for the clause that says: "... and is being phased into the Embedded environment",
which still appears in the most recent ISA.  So the Book E folks my not be so ready to eliminate the reserved
bit masks.

>> (4) In general, modeling undefined behavior is a slippery slope.  I would much prefer to see the code fixed or justified before changing QEMU.
> 
> I can try to ask on the MorphOS list but their previous answer to another question was that it works on the hardware they officially support.

"Working" should not be confused for "correct"  :)

Re: [Qemu-devel] Help needed testing on ppc

[Alexander Graf]

On 05/07/2014 05:31 PM, Tom Musta wrote:
> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>> On Tue, 6 May 2014, Tom Musta wrote:
>>> On 5/6/2014 5:03 AM, BALATON Zoltan wrote:
>>>> I'd appreciate some insight and help.
> [snip]
>>> (1) Why is MorphOS using this invalid instruction form?  Would it be easier to fix the OS rather than QEMU?
>> I don't know why is it used. I can ask the MorphOS developers but they did not seem to be too supportive so far and at least one of them expressed that they have no interest supporting other than their officially supported list of hardware at this time. So
>> I assume it is easier to fix QEMU than MorphOS and if it works on a real Mac then it should also work on QEMU's emulation of that Mac hardware.
>>
>>> Is there some undocumented processor behavior that the code is dependent upon (e.g. is it actually expected CR0 to be set?).
>> This is what the testing was supposed to find out but MorphOS seems to run better with the quoted patch so I don't think it depends on any other undocumented behaviour other than ignoring reserved bits but I have no definitive answer.
>>
> It still seems to me that setting a reserved instruction bit is an strange thing to do.  It would be nice to at least
> have a justification from MorphOS.  It is possible that no one even knows the answer.
>
>>> (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch.  I think fixing only some will be a future source of confusion.>>
> Alex:  do you have an opinion on this?  Are you OK with changing masks for a few stores but not all instructions in general?

I would like to see someone just test all those load/store instructions 
on old CPUs and see whether they fault. If none faults, we should just 
be consistent and remove them for all. If say a 750 really only ignores 
the Rc bit for stwx for some reason we should just model it accordingly.


Alex

Re: [Qemu-devel] Help needed testing on ppc

[BALATON Zoltan]

On Wed, 7 May 2014, Alexander Graf wrote:
> On 05/07/2014 05:31 PM, Tom Musta wrote:
>> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>>> On Tue, 6 May 2014, Tom Musta wrote:
>>>> (2) Your patch makes some store instructions compliant with the most 
>>>> recent ISAs but there are many other instructions that are not addressed 
>>>> by the patch.  I think fixing only some will be a future source of 
>>>> confusion.>>
>> Alex:  do you have an opinion on this?  Are you OK with changing masks for 
>> a few stores but not all instructions in general?
>
> I would like to see someone just test all those load/store instructions on 
> old CPUs and see whether they fault. If none faults, we should just be 
> consistent and remove them for all. If say a 750 really only ignores the Rc 
> bit for stwx for some reason we should just model it accordingly.

To get some answers to this and other questions that are still open I've 
made a test program by stripping down yaboot and adding tests to it so 
that it should be possible to run from Open Firmware as a boot loader. It 
can be found here:

http://goliat.eik.bme.hu/~balaton/oftest/

The files there are:
* oftest - an ELF executable that you can put on some device OF can read
         and run it if it were a boot loader ( e.g. 0> boot hd0,0:\oftest )
* oftest.hfs.xz - the same file on an 800k HFS volume that can be put on
         e.g. a USB drive or CD then used as the previous one
* oftest-src.tar.xz - the source

When run from Open Firmware it should print some information about memory 
layout, MSR setting, stack location, BAT registers and test the stwx 
opcode with and without reserved bit which should help us understand 
better the differences between QEMU and real hardware. I could only test 
it on QEMU though.

I'd appreciate if you could run it on real hardware and take a picture of 
the output screen which you upload somewhere and send the URL to that (or 
if you cannot upload you can send the picture but in that case only to me 
not to the list please). If cannot be seen on the picture please also 
include the model of your machine that should appear at the beginning of 
the Open Firmware greeting line.

Regards,
BALATON Zoltan

Re: [Qemu-devel] Help needed testing on ppc

[BALATON Zoltan]

On Thu, 12 Jun 2014, BALATON Zoltan wrote:
> On Wed, 7 May 2014, Alexander Graf wrote:
>> On 05/07/2014 05:31 PM, Tom Musta wrote:
>>> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>>>> On Tue, 6 May 2014, Tom Musta wrote:
>>>>> (2) Your patch makes some store instructions compliant with the most 
>>>>> recent ISAs but there are many other instructions that are not addressed 
>>>>> by the patch.  I think fixing only some will be a future source of 
>>>>> confusion.>>
>>> Alex:  do you have an opinion on this?  Are you OK with changing masks for 
>>> a few stores but not all instructions in general?
>> 
>> I would like to see someone just test all those load/store instructions on 
>> old CPUs and see whether they fault. If none faults, we should just be 
>> consistent and remove them for all. If say a 750 really only ignores the Rc 
>> bit for stwx for some reason we should just model it accordingly.
>
> To get some answers to this and other questions that are still open I've made 
> a test program by stripping down yaboot and adding tests to it so that it 
> should be possible to run from Open Firmware as a boot loader. It can be 
> found here:
>
> http://goliat.eik.bme.hu/~balaton/oftest/
>
> The files there are:
> * oftest - an ELF executable that you can put on some device OF can read
>        and run it if it were a boot loader ( e.g. 0> boot hd0,0:\oftest )
> * oftest.hfs.xz - the same file on an 800k HFS volume that can be put on
>        e.g. a USB drive or CD then used as the previous one
> * oftest-src.tar.xz - the source
>
> When run from Open Firmware it should print some information about memory 
> layout, MSR setting, stack location, BAT registers and test the stwx opcode 
> with and without reserved bit which should help us understand better the 
> differences between QEMU and real hardware. I could only test it on QEMU 
> though.

I've got some results (but more are welcome) which can be seen here:

http://goliat.eik.bme.hu/~balaton/oftest/results/

The results show that the stwx instruction with reserved bit set does not 
change status bits and does not generate an exception on any CPU tested 
(G3 and G4) so it is most probably just ignored as we thought.

Regards,
BALATON Zoltan

Re: [Qemu-devel] Help needed testing on ppc

[Alexander Graf]

On 17.06.14 01:42, BALATON Zoltan wrote:
> On Thu, 12 Jun 2014, BALATON Zoltan wrote:
>> On Wed, 7 May 2014, Alexander Graf wrote:
>>> On 05/07/2014 05:31 PM, Tom Musta wrote:
>>>> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>>>>> On Tue, 6 May 2014, Tom Musta wrote:
>>>>>> (2) Your patch makes some store instructions compliant with the 
>>>>>> most recent ISAs but there are many other instructions that are 
>>>>>> not addressed by the patch.  I think fixing only some will be a 
>>>>>> future source of confusion.>>
>>>> Alex:  do you have an opinion on this?  Are you OK with changing 
>>>> masks for a few stores but not all instructions in general?
>>>
>>> I would like to see someone just test all those load/store 
>>> instructions on old CPUs and see whether they fault. If none faults, 
>>> we should just be consistent and remove them for all. If say a 750 
>>> really only ignores the Rc bit for stwx for some reason we should 
>>> just model it accordingly.
>>
>> To get some answers to this and other questions that are still open 
>> I've made a test program by stripping down yaboot and adding tests to 
>> it so that it should be possible to run from Open Firmware as a boot 
>> loader. It can be found here:
>>
>> http://goliat.eik.bme.hu/~balaton/oftest/
>>
>> The files there are:
>> * oftest - an ELF executable that you can put on some device OF can read
>>        and run it if it were a boot loader ( e.g. 0> boot 
>> hd0,0:\oftest )
>> * oftest.hfs.xz - the same file on an 800k HFS volume that can be put on
>>        e.g. a USB drive or CD then used as the previous one
>> * oftest-src.tar.xz - the source
>>
>> When run from Open Firmware it should print some information about 
>> memory layout, MSR setting, stack location, BAT registers and test 
>> the stwx opcode with and without reserved bit which should help us 
>> understand better the differences between QEMU and real hardware. I 
>> could only test it on QEMU though.
>
> I've got some results (but more are welcome) which can be seen here:
>
> http://goliat.eik.bme.hu/~balaton/oftest/results/
>
> The results show that the stwx instruction with reserved bit set does 
> not change status bits and does not generate an exception on any CPU 
> tested (G3 and G4) so it is most probably just ignored as we thought.

[adding qemu-ppc and tom to CC]

Tom already commented on this. Is there a pattern that matches all the 
indexed load/store instructions or is stwx a one-off?


Alex

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[BALATON Zoltan]

On Tue, 17 Jun 2014, Alexander Graf wrote:
> On 17.06.14 01:42, BALATON Zoltan wrote:
>> On Thu, 12 Jun 2014, BALATON Zoltan wrote:
>>> On Wed, 7 May 2014, Alexander Graf wrote:
>>>> On 05/07/2014 05:31 PM, Tom Musta wrote:
>>>>> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>>>>>> On Tue, 6 May 2014, Tom Musta wrote:
>>>>>>> (2) Your patch makes some store instructions compliant with the most 
>>>>>>> recent ISAs but there are many other instructions that are not 
>>>>>>> addressed by the patch.  I think fixing only some will be a future 
>>>>>>> source of confusion.>>
>>>>> Alex:  do you have an opinion on this?  Are you OK with changing masks 
>>>>> for a few stores but not all instructions in general?
>>>> 
>>>> I would like to see someone just test all those load/store instructions 
>>>> on old CPUs and see whether they fault. If none faults, we should just be 
>>>> consistent and remove them for all. If say a 750 really only ignores the 
>>>> Rc bit for stwx for some reason we should just model it accordingly.
>>> 
>>> To get some answers to this and other questions that are still open I've 
>>> made a test program by stripping down yaboot and adding tests to it so 
>>> that it should be possible to run from Open Firmware as a boot loader. It 
>>> can be found here:
>>> 
>>> http://goliat.eik.bme.hu/~balaton/oftest/
>>> 
>>> The files there are:
>>> * oftest - an ELF executable that you can put on some device OF can read
>>>        and run it if it were a boot loader ( e.g. 0> boot hd0,0:\oftest )
>>> * oftest.hfs.xz - the same file on an 800k HFS volume that can be put on
>>>        e.g. a USB drive or CD then used as the previous one
>>> * oftest-src.tar.xz - the source
>>> 
>>> When run from Open Firmware it should print some information about memory 
>>> layout, MSR setting, stack location, BAT registers and test the stwx 
>>> opcode with and without reserved bit which should help us understand 
>>> better the differences between QEMU and real hardware. I could only test 
>>> it on QEMU though.
>> 
>> I've got some results (but more are welcome) which can be seen here:
>> 
>> http://goliat.eik.bme.hu/~balaton/oftest/results/
>> 
>> The results show that the stwx instruction with reserved bit set does not 
>> change status bits and does not generate an exception on any CPU tested (G3 
>> and G4) so it is most probably just ignored as we thought.
>
> [adding qemu-ppc and tom to CC]
>
> Tom already commented on this. Is there a pattern that matches all the 
> indexed load/store instructions or is stwx a one-off?

Is this a question to whom? If to me I don't understand it.

Regards,
BALATON Zoltan

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[Alexander Graf]

On 17.06.14 11:34, BALATON Zoltan wrote:
> On Tue, 17 Jun 2014, Alexander Graf wrote:
>> On 17.06.14 01:42, BALATON Zoltan wrote:
>>> On Thu, 12 Jun 2014, BALATON Zoltan wrote:
>>>> On Wed, 7 May 2014, Alexander Graf wrote:
>>>>> On 05/07/2014 05:31 PM, Tom Musta wrote:
>>>>>> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>>>>>>> On Tue, 6 May 2014, Tom Musta wrote:
>>>>>>>> (2) Your patch makes some store instructions compliant with the 
>>>>>>>> most recent ISAs but there are many other instructions that are 
>>>>>>>> not addressed by the patch.  I think fixing only some will be a 
>>>>>>>> future source of confusion.>>
>>>>>> Alex:  do you have an opinion on this?  Are you OK with changing 
>>>>>> masks for a few stores but not all instructions in general?
>>>>>
>>>>> I would like to see someone just test all those load/store 
>>>>> instructions on old CPUs and see whether they fault. If none 
>>>>> faults, we should just be consistent and remove them for all. If 
>>>>> say a 750 really only ignores the Rc bit for stwx for some reason 
>>>>> we should just model it accordingly.
>>>>
>>>> To get some answers to this and other questions that are still open 
>>>> I've made a test program by stripping down yaboot and adding tests 
>>>> to it so that it should be possible to run from Open Firmware as a 
>>>> boot loader. It can be found here:
>>>>
>>>> http://goliat.eik.bme.hu/~balaton/oftest/
>>>>
>>>> The files there are:
>>>> * oftest - an ELF executable that you can put on some device OF can 
>>>> read
>>>>        and run it if it were a boot loader ( e.g. 0> boot 
>>>> hd0,0:\oftest )
>>>> * oftest.hfs.xz - the same file on an 800k HFS volume that can be 
>>>> put on
>>>>        e.g. a USB drive or CD then used as the previous one
>>>> * oftest-src.tar.xz - the source
>>>>
>>>> When run from Open Firmware it should print some information about 
>>>> memory layout, MSR setting, stack location, BAT registers and test 
>>>> the stwx opcode with and without reserved bit which should help us 
>>>> understand better the differences between QEMU and real hardware. I 
>>>> could only test it on QEMU though.
>>>
>>> I've got some results (but more are welcome) which can be seen here:
>>>
>>> http://goliat.eik.bme.hu/~balaton/oftest/results/
>>>
>>> The results show that the stwx instruction with reserved bit set 
>>> does not change status bits and does not generate an exception on 
>>> any CPU tested (G3 and G4) so it is most probably just ignored as we 
>>> thought.
>>
>> [adding qemu-ppc and tom to CC]
>>
>> Tom already commented on this. Is there a pattern that matches all 
>> the indexed load/store instructions or is stwx a one-off?
>
> Is this a question to whom? If to me I don't understand it.

stwx is part of a group of instructions. It's very rare that hardware 
only shows certain behavior (like ignore a reserved bit) for single 
instructions. Usually it happens on complete groups.


Alex

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[BALATON Zoltan]

On Tue, 17 Jun 2014, Alexander Graf wrote:
>>>> http://goliat.eik.bme.hu/~balaton/oftest/results/
>>>> 
>>>> The results show that the stwx instruction with reserved bit set does not 
>>>> change status bits and does not generate an exception on any CPU tested 
>>>> (G3 and G4) so it is most probably just ignored as we thought.
>>> 
>>> [adding qemu-ppc and tom to CC]
>>> 
>>> Tom already commented on this. Is there a pattern that matches all the 
>>> indexed load/store instructions or is stwx a one-off?
>> 
>> Is this a question to whom? If to me I don't understand it.
>
> stwx is part of a group of instructions. It's very rare that hardware only 
> shows certain behavior (like ignore a reserved bit) for single instructions. 
> Usually it happens on complete groups.

Who would know that? The test only tested stwx and I assume the same that 
this should not behave differently than any other instruction. Also this 
is the only instruction that was used with the set reserved bit in MorphOS 
and the patch ignoring reserved bits for this group of instructions that 
we discussed earlier seems to fix it. (There's another case with an 
altivec instruction with a similar failure but I did not look at that yet 
if that's a reserved bit too or something else.) As to why it's in MorphOS 
I don't know, I got no answer from them.

Regards,
BALATON Zoltan

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[Tom Musta]

On 6/17/2014 6:05 AM, BALATON Zoltan wrote:
> On Tue, 17 Jun 2014, Alexander Graf wrote:
>>>>> http://goliat.eik.bme.hu/~balaton/oftest/results/
>>>>>
>>>>> The results show that the stwx instruction with reserved bit set does not change status bits and does not generate an exception on any CPU tested (G3 and G4) so it is most probably just ignored as we thought.
>>>>
>>>> [adding qemu-ppc and tom to CC]
>>>>
>>>> Tom already commented on this. Is there a pattern that matches all the indexed load/store instructions or is stwx a one-off?
>>>
>>> Is this a question to whom? If to me I don't understand it.
>>
>> stwx is part of a group of instructions. It's very rare that hardware only shows certain behavior (like ignore a reserved bit) for single instructions. Usually it happens on complete groups.
> 
> Who would know that? The test only tested stwx and I assume the same that this should not behave differently than any other instruction. Also this is the only instruction that was used with the set reserved bit in MorphOS and the patch ignoring reserved
> bits for this group of instructions that we discussed earlier seems to fix it. (There's another case with an altivec instruction with a similar failure but I did not look at that yet if that's a reserved bit too or something else.) As to why it's in
> MorphOS I don't know, I got no answer from them.
> 
> Regards,
> BALATON Zoltan

I am looking at the test case source code and do not see how you are setting the reserved bit.  Maybe I am missing some cleverness in how the test is built?

#include <prom.h>

void stwxtest(void)
{
  unsigned int val, cr, cr1, cr2;

  prom_printf("Testing stwx reserved bit\n");

  val = 0;

  asm volatile("mfcr %0               \n\t"
               "bl 1f                 \n\t"
               "mfcr %1               \n\t"
               "mflr 10               \n\t"
               "lwz %0, 36(10)        \n\t"
               "ori %0, %0, 1         \n\t"
               "stw %0, 36(10)        \n\t"
               "mfcr %0               \n\t"
               "bl 1f                 \n\t"
               "mfcr %2               \n\t"
               "b 2f                  \n\t"
               "1: stwx %0, %4, %6    \n\t"          <<<<<<<<<<<<< just a normal stwx, right?
               "blr                   \n\t"
               "2:                    \n\t"
                : "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
                : "r"(&val), "m"(val), "r"(8)
                : "r8", "r9", "r10", "cc", "memory");

  prom_printf("old cr  (mem):\t%#x\n", val);
  prom_printf("old cr  (reg):\t%#x\n", cr);
  prom_printf("new cr1 (reg):\t%#x\n", cr1);
  prom_printf("new cr2 (reg):\t%#x\n", cr2);
}


But the objdump of your test binary does not show that it is set either:

00102784 <stwxtest>:
stwxtest():
  102784:       7c 08 02 a6     mflr    r0
  102788:       94 21 ff d0     stwu    r1,-48(r1)
  10278c:       3c 60 00 10     lis     r3,16
  102790:       38 63 32 29     addi    r3,r3,12841
  102794:       bf a1 00 24     stmw    r29,36(r1)
  102798:       90 01 00 34     stw     r0,52(r1)
  10279c:       4c c6 31 82     crclr   4*cr1+eq
  1027a0:       4b ff e2 75     bl      100a14 <prom_printf>
  1027a4:       7c 27 0b 78     mr      r7,r1
  1027a8:       39 20 00 00     li      r9,0
  1027ac:       95 27 00 08     stwu    r9,8(r7)
  1027b0:       38 c0 00 08     li      r6,8
  1027b4:       7f a0 00 26     mfcr    r29
  1027b8:       48 00 00 29     bl      1027e0 <stwxtest+0x5c>
  1027bc:       7f c0 00 26     mfcr    r30
  1027c0:       7d 48 02 a6     mflr    r10
  1027c4:       83 aa 00 24     lwz     r29,36(r10)
  1027c8:       63 bd 00 01     ori     r29,r29,1
  1027cc:       93 aa 00 24     stw     r29,36(r10)
  1027d0:       7f a0 00 26     mfcr    r29
  1027d4:       48 00 00 0d     bl      1027e0 <stwxtest+0x5c>
  1027d8:       7f e0 00 26     mfcr    r31
  1027dc:       48 00 00 0c     b       1027e8 <stwxtest+0x64>
  1027e0:       7f a7 31 2e     stwx    r29,r7,r6  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  1027e4:       4e 80 00 20     blr

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[BALATON Zoltan]

On Tue, 17 Jun 2014, Tom Musta wrote:
> I am looking at the test case source code and do not see how you are 
> setting the reserved bit.  Maybe I am missing some cleverness in how the 
> test is built?

Probably I should have written it more straight-forward but I wanted it to 
be possible to change it for other tests easily so it's a bit tricky. 
Basically I get the code location by a bl then fetching the link register:

>  asm volatile("mfcr %0               \n\t"
>               "bl 1f                 \n\t"
>               "mfcr %1               \n\t"
>               "mflr 10               \n\t"

and then set the bit with the next three lines after testing the normal 
case:

>               "lwz %0, 36(10)        \n\t"
>               "ori %0, %0, 1         \n\t"
>               "stw %0, 36(10)        \n\t"

Then test again with the bit set:

>               "mfcr %0               \n\t"
>               "bl 1f                 \n\t"
>               "mfcr %2               \n\t"

and exit:

>               "b 2f                  \n\t"
>               "1: stwx %0, %4, %6    \n\t"          <<<<<<<<<<<<< just a normal stwx, right?
>               "blr                   \n\t"
>               "2:                    \n\t"
>                : "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
>                : "r"(&val), "m"(val), "r"(8)
>                : "r8", "r9", "r10", "cc", "memory");
>
>  prom_printf("old cr  (mem):\t%#x\n", val);
>  prom_printf("old cr  (reg):\t%#x\n", cr);
>  prom_printf("new cr1 (reg):\t%#x\n", cr1);
>  prom_printf("new cr2 (reg):\t%#x\n", cr2);
> }
>
>
> But the objdump of your test binary does not show that it is set either:

It should show in a debugger the second time the stwx is called (it did 
for me).

Regards,
BALATON Zoltan

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[Tom Musta]

On 6/17/2014 10:17 AM, BALATON Zoltan wrote:
> On Tue, 17 Jun 2014, Tom Musta wrote:
>> I am looking at the test case source code and do not see how you are setting the reserved bit.  Maybe I am missing some cleverness in how the test is built?
> 
> Probably I should have written it more straight-forward but I wanted it to be possible to change it for other tests easily so it's a bit tricky. Basically I get the code location by a bl then fetching the link register:
> 
>>  asm volatile("mfcr %0               \n\t"
>>               "bl 1f                 \n\t"
>>               "mfcr %1               \n\t"
>>               "mflr 10               \n\t"
> 
> and then set the bit with the next three lines after testing the normal case:
> 
>>               "lwz %0, 36(10)        \n\t"
>>               "ori %0, %0, 1         \n\t"
>>               "stw %0, 36(10)        \n\t"
> 
> Then test again with the bit set:
> 
>>               "mfcr %0               \n\t"
>>               "bl 1f                 \n\t"
>>               "mfcr %2               \n\t"
> 
> and exit:
> 
>>               "b 2f                  \n\t"
>>               "1: stwx %0, %4, %6    \n\t"          <<<<<<<<<<<<< just a normal stwx, right?
>>               "blr                   \n\t"
>>               "2:                    \n\t"
>>                : "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
>>                : "r"(&val), "m"(val), "r"(8)
>>                : "r8", "r9", "r10", "cc", "memory");
>>
>>  prom_printf("old cr  (mem):\t%#x\n", val);
>>  prom_printf("old cr  (reg):\t%#x\n", cr);
>>  prom_printf("new cr1 (reg):\t%#x\n", cr1);
>>  prom_printf("new cr2 (reg):\t%#x\n", cr2);
>> }
>>
>>
>> But the objdump of your test binary does not show that it is set either:
> 
> It should show in a debugger the second time the stwx is called (it did for me).
> 
> Regards,
> BALATON Zoltan

There should be an icbi after the ori/stw sequence to ensure that the modified code gets into the instruction cache.

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[BALATON Zoltan]

On Wed, 18 Jun 2014, Tom Musta wrote:
> On 6/17/2014 10:17 AM, BALATON Zoltan wrote:
>> On Tue, 17 Jun 2014, Tom Musta wrote:
>>> I am looking at the test case source code and do not see how you are setting the reserved bit.  Maybe I am missing some cleverness in how the test is built?
>>
>> Probably I should have written it more straight-forward but I wanted it to be possible to change it for other tests easily so it's a bit tricky. Basically I get the code location by a bl then fetching the link register:
>>
>>>  asm volatile("mfcr %0               \n\t"
>>>               "bl 1f                 \n\t"
>>>               "mfcr %1               \n\t"
>>>               "mflr 10               \n\t"
>>
>> and then set the bit with the next three lines after testing the normal case:
>>
>>>               "lwz %0, 36(10)        \n\t"
>>>               "ori %0, %0, 1         \n\t"
>>>               "stw %0, 36(10)        \n\t"
>>
>> Then test again with the bit set:
>>
>>>               "mfcr %0               \n\t"
>>>               "bl 1f                 \n\t"
>>>               "mfcr %2               \n\t"
>>
>> and exit:
>>
>>>               "b 2f                  \n\t"
>>>               "1: stwx %0, %4, %6    \n\t"          <<<<<<<<<<<<< just a normal stwx, right?
>>>               "blr                   \n\t"
>>>               "2:                    \n\t"
>>>                : "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
>>>                : "r"(&val), "m"(val), "r"(8)
>>>                : "r8", "r9", "r10", "cc", "memory");
>>>
>>>  prom_printf("old cr  (mem):\t%#x\n", val);
>>>  prom_printf("old cr  (reg):\t%#x\n", cr);
>>>  prom_printf("new cr1 (reg):\t%#x\n", cr1);
>>>  prom_printf("new cr2 (reg):\t%#x\n", cr2);
>>> }
>>>
>>>
>>> But the objdump of your test binary does not show that it is set either:
>>
>> It should show in a debugger the second time the stwx is called (it did for me).
>>
>
> There should be an icbi after the ori/stw sequence to ensure that the 
> modified code gets into the instruction cache.

I've corrected the test accordingly and rerun on iMac,1. It did not change 
the stwx test results, the cr values are still the same.

Regards,
BALATON Zoltan

Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc

[Alexander Graf]

On 19.06.14 15:21, BALATON Zoltan wrote:
> On Wed, 18 Jun 2014, Tom Musta wrote:
>> On 6/17/2014 10:17 AM, BALATON Zoltan wrote:
>>> On Tue, 17 Jun 2014, Tom Musta wrote:
>>>> I am looking at the test case source code and do not see how you 
>>>> are setting the reserved bit. Maybe I am missing some cleverness in 
>>>> how the test is built?
>>>
>>> Probably I should have written it more straight-forward but I wanted 
>>> it to be possible to change it for other tests easily so it's a bit 
>>> tricky. Basically I get the code location by a bl then fetching the 
>>> link register:
>>>
>>>>  asm volatile("mfcr %0 \n\t"
>>>>               "bl 1f                 \n\t"
>>>>               "mfcr %1               \n\t"
>>>>               "mflr 10               \n\t"
>>>
>>> and then set the bit with the next three lines after testing the 
>>> normal case:
>>>
>>>>               "lwz %0, 36(10) \n\t"
>>>>               "ori %0, %0, 1         \n\t"
>>>>               "stw %0, 36(10)        \n\t"
>>>
>>> Then test again with the bit set:
>>>
>>>>               "mfcr %0 \n\t"
>>>>               "bl 1f                 \n\t"
>>>>               "mfcr %2               \n\t"
>>>
>>> and exit:
>>>
>>>>               "b 2f \n\t"
>>>>               "1: stwx %0, %4, %6    \n\t" <<<<<<<<<<<<< just a 
>>>> normal stwx, right?
>>>>               "blr                   \n\t"
>>>>               "2:                    \n\t"
>>>>                : "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
>>>>                : "r"(&val), "m"(val), "r"(8)
>>>>                : "r8", "r9", "r10", "cc", "memory");
>>>>
>>>>  prom_printf("old cr  (mem):\t%#x\n", val);
>>>>  prom_printf("old cr  (reg):\t%#x\n", cr);
>>>>  prom_printf("new cr1 (reg):\t%#x\n", cr1);
>>>>  prom_printf("new cr2 (reg):\t%#x\n", cr2);
>>>> }
>>>>
>>>>
>>>> But the objdump of your test binary does not show that it is set 
>>>> either:
>>>
>>> It should show in a debugger the second time the stwx is called (it 
>>> did for me).
>>>
>>
>> There should be an icbi after the ori/stw sequence to ensure that the 
>> modified code gets into the instruction cache.
>
> I've corrected the test accordingly and rerun on iMac,1. It did not 
> change the stwx test results, the cr values are still the same.

Great :). Now please check through all opcodes that get generated by the 
GEN_STUX, GEN_STX_E, GEN_LDUX and GEN_LDX_E helpers in translate.c and 
verify that the bit gets ignored on all of them. We can then easily  
just remove the reserved Rc bit on those instruction definitions 
generically and call it a day.


Alex

Re: [Qemu-devel] Help needed testing on ppc

[BALATON Zoltan]

On Wed, 7 May 2014, Tom Musta wrote:
> On 5/6/2014 6:17 PM, BALATON Zoltan wrote:
>> On Tue, 6 May 2014, Tom Musta wrote:
>>> On 5/6/2014 5:03 AM, BALATON Zoltan wrote:
>>>> I'd appreciate some insight and help.
> [snip]
>>> (1) Why is MorphOS using this invalid instruction form?  Would it be easier to fix the OS rather than QEMU?
>>
>> I don't know why is it used. I can ask the MorphOS developers but they did not seem to be too supportive so far and at least one of them expressed that they have no interest supporting other than their officially supported list of hardware at this time. So
>> I assume it is easier to fix QEMU than MorphOS and if it works on a real Mac then it should also work on QEMU's emulation of that Mac hardware.
>>
>>> Is there some undocumented processor behavior that the code is dependent upon (e.g. is it actually expected CR0 to be set?).
>>
>> This is what the testing was supposed to find out but MorphOS seems to run better with the quoted patch so I don't think it depends on any other undocumented behaviour other than ignoring reserved bits but I have no definitive answer.
>
> It still seems to me that setting a reserved instruction bit is an strange thing to do.  It would be nice to at least
> have a justification from MorphOS.  It is possible that no one even knows the answer.

I've tried to ask them about this but all I got as an answer was that 
running on QEMU is not supported so I take it that they don't know or 
won't tell. But it's also very unlikely they would change it in MorphOS so 
it seems it is not easier to fix in MorphOS.

>>> (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch.  I think fixing only some will be a future source of confusion.>>
>
> Alex:  do you have an opinion on this?  Are you OK with changing masks for a few stores but not all instructions in general?

I've got a bit further and came across another invalid instruction 
exception:

invalid bits: 02000000 for opcode: 1f - 16 - 0b (7e04caec) 1020574c

I don't know if this is another case of using reserved bits or something 
else though.

Regards,
BALATON Zoltan