Re: [PATCH 04/17] x86emul: complete decoding of two-byte instructions

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 14/09/16 16:05, Jan Beulich wrote:
>>>> On 14.09.16 at 16:22, <andrew.cooper3@citrix.com> wrote:
>> On 08/09/16 14:10, Jan Beulich wrote:
>>> This way we can at least size (and e.g. skip) them if needed, and we
>>> also won't raise the wrong fault due to not having read all relevant
>>> bytes.
>> What faults are you referring to?  #UD vs #GP from hitting the %cs limit?
> Or #PF.
>
>>> This at once adds correct raising of #UD for the three "ud<n>" flavors
>>> (Intel names only "ud2", but AMD names all three of them in their
>>> opcode maps), as that may make a difference to callers compared to
>>> getting back X86EMUL_UNHANDLEABLE.
>> Definitely a good improvement.  I have been meaning to do this for a while.
>>
>> Intel does references 0FB9 in a footnote in the opcode map, but I can't
>> see any mention of 0FFF at all.
> Check AMD's.
>
>>> Note on opcodes 0FA6 and 0FA7: These are VIA's PadLock instructions,
>>> which have a ModRM like byte where only register forms are valid. I.e.
>>> we could also use SrcImmByte there, but ModRM is more likely to be
>>> correct for a hypothetical extension allowing non-register operations.
>> Won't the use of ModRM possibly cause us to read too much if it end up
>> with SIB and displacement encoding?  OTOH, do we really care?
> That's why I've added that paragraph: I'd be fine either way, but I
> do think the intention is a ModRM byte. Which is then also in line with
> these opcodes' uses in early 386 and 486 processors (xbts/ibts/
> cmpxchg).
>
>>> Note on opcode 0FB8: I think we're safe to ignore JMPE (which doesn't
>>> take a ModRM byte, but an immediate).
>> It took a while to find out what this instruction is.  Mind indicating
>> that it is Itanium-specific in the commit message?
> Sure.
>
>> POPCNT, the aliased instruction takes a full ModRM byte with no space to
>> distinguish.
> Well, distinguishing them is possible in principle, as by the time we
> process bytes past the main opcode one we already know whether
> an F3 prefix was present. I simply think it's not worth trying to do
> so.

It would be helpful if you listed all of the decoding modified.

 From the looks of things, the instructions changed are:

LAR, LSL, CLTS, UD2, FEMMS, 3DNow!, a bunch of SSE instructions, RDPMC, 
GETSEC, more SSE/MMX, RSM, POPCNT, UD1, yet more SSE,


A couple of other misc points:

What is the point of having 0F3A specified with 
DstReg|SrcImmByte|ModRM?  Being a prefix, it shouldn't be treated like a 
plain operation.

0F6F was previously ImplicitOps|ModRM, but looks like it should be ModRM 
like the rest of 0F6x.  0F7F, 0FC7 and 0FE7 similarly.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel