[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 23.09.16 at 18:34, <andrew.cooper3@xxxxxxxxxx> wrote:
> It would be helpful if you listed all of the decoding modified.
>  From the looks of things, the instructions changed are:

I don't see the point: If any of them got proper emulation added,
I'd agree. But with the purpose of the patch being to simply add
correct decoding for _all_ instructions in this group, it is quite
obvious that everything gets modified which so far didn't have
sufficient information for decoding. What exactly those
instructions do becomes of interest once we add actual emulation.

> 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.

You can view it either way, and for our purposes it is clearly easier
this way: The static tables are really mainly decoding helpers, and
all three groups (0F0F, 0F38, and 0F3A) have the nice property
that their operands are sufficiently uniform across the actual
opcodes. Hence the static tables better treat them as individual
opcodes (or else we'd have to introduce further tables with - for
each one of them - all entries identical), while actual emulation
(once added) would of course need to distinguish the various

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

Why? As mentioned elsewhere I think the (otherwise benign)
ImplicitOps (as well as the individual DstImplicit and SrcImplicit)
serve as documentation: Opcodes we actually handle have them
specified, whereas opcodes getting decoded but not emulated
don't. See the MOVQ and MOVD patches in the other series, which
add ImplicitOps to the table entries they add emulation for.

(The one corner case here would be operations without any
operands, but that's only the two forms of NOP, and I think we
can accept them to not fit this model.)


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.