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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/09/16 16:05, Jan Beulich wrote:
> > > > On 14.09.16 at 16:22, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel