[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 operations. > 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.) Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |