All of lore.kernel.org
 help / color / mirror / Atom feed
From: BALATON Zoltan <balaton@eik.bme.hu>
To: Tom Musta <tommusta@gmail.com>
Cc: qemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>
Subject: Re: [Qemu-devel] Help needed testing on ppc
Date: Wed, 7 May 2014 01:17:39 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LMD.2.02.1405070022550.23536@jedlik.phy.bme.hu> (raw)
In-Reply-To: <5368D385.7050900@gmail.com>

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

  reply	other threads:[~2014-05-06 23:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-06 10:03 [Qemu-devel] Help needed testing on ppc BALATON Zoltan
2014-05-06 12:20 ` Tom Musta
2014-05-06 23:17   ` BALATON Zoltan [this message]
2014-05-07 15:31     ` Tom Musta
2014-05-07 16:59       ` Alexander Graf
2014-06-12  0:49         ` BALATON Zoltan
2014-06-16 23:42           ` BALATON Zoltan
2014-06-17  8:42             ` Alexander Graf
2014-06-17  9:34               ` [Qemu-devel] [Qemu-ppc] " BALATON Zoltan
2014-06-17  9:37                 ` Alexander Graf
2014-06-17 11:05                   ` BALATON Zoltan
2014-06-17 11:54                     ` Tom Musta
2014-06-17 15:17                       ` BALATON Zoltan
2014-06-18 12:40                         ` Tom Musta
2014-06-19 13:21                           ` BALATON Zoltan
2014-06-23 17:07                             ` Alexander Graf
2014-05-20 23:55       ` [Qemu-devel] " BALATON Zoltan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LMD.2.02.1405070022550.23536@jedlik.phy.bme.hu \
    --to=balaton@eik.bme.hu \
    --cc=agraf@suse.de \
    --cc=qemu-devel@nongnu.org \
    --cc=tommusta@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.