All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 04/17] x86emul: complete decoding of two-byte instructions
Date: Mon, 26 Sep 2016 01:34:02 -0600	[thread overview]
Message-ID: <57E8EB8A0200007800112518@prv-mh.provo.novell.com> (raw)
In-Reply-To: <83155336-c974-2f07-7362-a0451ffb3e4b@citrix.com>

>>> On 23.09.16 at 18:34, <andrew.cooper3@citrix.com> 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@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2016-09-26  7:34 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-08 12:58 [PATCH 00/17] x86: split insn emulator decode and execution Jan Beulich
2016-09-08 13:04 ` [PATCH 01/17] x86emul: split instruction decoding from execution Jan Beulich
2016-09-09 18:35   ` Andrew Cooper
2016-09-12  7:20     ` Jan Beulich
2016-09-08 13:07 ` [PATCH 02/17] x86emul: fetch all insn bytes during the decode phase Jan Beulich
2016-09-13 18:44   ` Andrew Cooper
2016-09-14  9:55     ` Jan Beulich
2016-09-23 14:48       ` Andrew Cooper
2016-09-23 15:04         ` Jan Beulich
2016-09-08 13:08 ` [PATCH 04/17] x86emul: track only rIP in emulator state Jan Beulich
2016-09-08 13:23   ` Jan Beulich
2016-09-08 13:09 ` [PATCH 03/17] " Jan Beulich
2016-09-13 19:09   ` Andrew Cooper
2016-09-14  9:58     ` Jan Beulich
2016-09-08 13:10 ` [PATCH 04/17] x86emul: complete decoding of two-byte instructions Jan Beulich
2016-09-14 14:22   ` Andrew Cooper
2016-09-14 15:05     ` Jan Beulich
2016-09-23 16:34       ` Andrew Cooper
2016-09-26  7:34         ` Jan Beulich [this message]
2016-09-27 13:28           ` Andrew Cooper
2016-09-27 13:51             ` Jan Beulich
2016-09-08 13:11 ` [PATCH 05/17] x86emul: add XOP decoding Jan Beulich
2016-09-14 16:11   ` Andrew Cooper
2016-09-14 16:21     ` Jan Beulich
2016-09-23 17:01       ` Andrew Cooper
2016-09-08 13:12 ` [PATCH 06/17] x86emul: add EVEX decoding Jan Beulich
2016-09-14 17:05   ` Andrew Cooper
2016-09-15  6:26     ` Jan Beulich
2016-09-08 13:13 ` [PATCH 07/17] x86emul: move x86_execute() common epilogue code Jan Beulich
2016-09-08 13:28   ` Jan Beulich
2016-09-14 17:13   ` Andrew Cooper
2016-09-08 13:14 ` [PATCH 08/17] x86emul: generate and make use of canonical opcode representation Jan Beulich
2016-09-14 17:30   ` Andrew Cooper
2016-09-15  6:43     ` Jan Beulich
2016-09-27 14:03       ` Andrew Cooper
2016-09-28  7:24         ` Jan Beulich
2016-09-08 13:14 ` [PATCH 09/17] SVM: use generic instruction decoding Jan Beulich
2016-09-14 17:56   ` Andrew Cooper
2016-09-15  6:55     ` Jan Beulich
2016-09-27 13:42       ` Andrew Cooper
2016-09-27 13:56         ` Jan Beulich
2016-09-27 15:53           ` Andrew Cooper
2016-09-08 13:16 ` [PATCH 10/17] x86/32on64: use generic instruction decoding for call gate emulation Jan Beulich
2016-09-08 13:17 ` [PATCH 11/17] x86/PV: split out dealing with CRn from privileged instruction handling Jan Beulich
2016-09-08 13:17 ` [PATCH 12/17] x86/PV: split out dealing with DRn " Jan Beulich
2016-09-08 13:18 ` [PATCH 13/17] x86/PV: split out dealing with MSRs " Jan Beulich
2016-09-08 13:18 ` [PATCH 14/17] x86emul: support XSETBV Jan Beulich
2016-09-08 13:19 ` [PATCH 15/17] x86emul: sort opcode 0f01 special case switch() statement Jan Beulich
2016-09-08 13:20 ` [PATCH 16/17] x86/PV: use generic emulator for privileged instruction handling Jan Beulich
2016-09-08 13:21 ` [PATCH 17/17] x86emul: don't assume a memory operand Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57E8EB8A0200007800112518@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.