Re: [Xen-devel] [PATCH 2/2] x86emul: adjust MOVSXD source operand handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/09/2019 07:34, Jan Beulich wrote:
> On 17.09.2019 19:17, Andrew Cooper wrote:
>> On 16/09/2019 10:48, Jan Beulich wrote:
>>> XED commit 1b2fd94425 ("Update MOVSXD to modern behavior") points out
>>> that as of SDM rev 064 MOVSXD is specified to read only 16 bits from
>>> memory (or register) when used without REX.W and with operand size
>>> override. Since the upper 16 bits of the value read won't be used
>>> anyway in this case, make the emulation uniformly follow this more
>>> compatible behavior when not emulating an AMD-like CPU, at the risk
>>> of missing an exception when emulating on/for older hardware (the
>>> boundary at SandyBridge noted in said commit looks questionable - I've
>>> observed the "new" behavior also on Westmere).
>> AMD documents this instruction has always using an 8 or 16bit source
>> operand.
> Have you mixed up MOVSX with MOVSXD? Both have separate pages in
> AMD's doc, but a common page in Intel's.

I had confused the two, yes.

I constructed an experiment using 66 6e 08, i.e.

movslq (%rax),%cx

according to objdump, and iterating backwards over the boundary to the
unmapped page at 0.

On a Rome system:

(d24) Ptr: 0000000000001000
(d24)  => c2c2
(d24) Ptr: 0000000000000fff
(d24) ******************************
(d24) PANIC: Unhandled exception at 0008:00000000001047a5
(d24) Vec 14 #PF[-d-sr-] %cr2 0000000000000fff
(d24) ******************************

Which also confirms the description which states that in the case of a
16 bit operand, no sign extension occurs.

I then tried the same test on an Intel Haswell system:

(d91) Ptr: 0000000000001000
(d91)  => c2c2
(d91) Ptr: 0000000000000fff
(d91) ******************************
(d91) PANIC: Unhandled exception at 0008:00000000001047a5
(d91) Vec 14 #PF[-d-sr-] %cr2 0000000000000fff
(d91) ******************************

So from this experimentation, I disbelieve the claim in XED, and and it
looks as if the Intel behaviour matches the AMD documentation.

Either way, I think further clarification from Intel is needed.

>
>> There are corner cases which we can't possibly reasonably cope with. 
>> e.g. It is model specific as to whether UD0 takes a ModRM byte or not,
>> and I'll note that the latest revision (3.31) of APM Vol2 clarifies in
>> Table 8-8:
>>
>> "This reflects the relative priority for faults encountered when
>> fetching the first byte of an instruction. In the fetching and decoding
>> of subsequent bytes of an instruction, if those bytes span the segment
>> limit or cross into a non-executable or not-present page, the fetch will
>> result in a #GP(0) fault or #PF as appropriate, preventing those bytes
>> from being accessed. However, if the instruction can be determined to be
>> invalid based just on the bytes preceding that boundary, a #UD fault may
>> take priority. This behavior is model-dependent."
>>
>> so we have no hope of getting model-accurate fault behaviour.
> How is UD0 relevant here?

to "there are model-specific corner cases which we can't possibly
reasonably cope with."

> And was the remainder of the above perhaps
> meant to be in response to the ARPL adjustment, described below? If
> so, I still wouldn't know what to take from it as far as this patch
> goes.

The ARPL bit is fine in isolation, and probably wants submitting in
isolation, given the conflicting evidence about MOVSXD.

If you do want to submit it individually, consider it R-by me to avoid
further latency.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel