xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: xen-devel <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: [PATCH 02/17] x86emul: fetch all insn bytes during the decode phase
Date: Thu, 08 Sep 2016 07:07:50 -0600	[thread overview]
Message-ID: <57D17EC6020000780010D14C@prv-mh.provo.novell.com> (raw)
In-Reply-To: <57D17C78020000780010D127@prv-mh.provo.novell.com>

[-- Attachment #1: Type: text/plain, Size: 6670 bytes --]

This way we can offer to callers the service of just sizing
instructions, and we also can better guarantee not to raise the wrong
fault due to not having read all relevant bytes.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -129,8 +129,8 @@ static const opcode_desc_t opcode_table[
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps, ImplicitOps,
     /* 0xA0 - 0xA7 */
-    ByteOp|DstEax|SrcImplicit|Mov, DstEax|SrcImplicit|Mov,
-    ByteOp|ImplicitOps|Mov, ImplicitOps|Mov,
+    ByteOp|DstEax|SrcMem|Mov, DstEax|SrcMem|Mov,
+    ByteOp|DstMem|SrcEax|Mov, DstMem|SrcEax|Mov,
     ByteOp|ImplicitOps|Mov, ImplicitOps|Mov,
     ByteOp|ImplicitOps, ImplicitOps,
     /* 0xA8 - 0xAF */
@@ -1602,6 +1602,45 @@ struct x86_emulate_state {
 #define _regs (state->regs)
 
 static int
+x86_decode_base(
+    struct x86_emulate_state *state,
+    struct x86_emulate_ctxt *ctxt,
+    const struct x86_emulate_ops *ops)
+{
+    int rc = X86EMUL_OKAY;
+
+    switch ( state->opcode )
+    {
+    case 0x9a: /* call (far, absolute) */
+    case 0xea: /* jmp (far, absolute) */
+        generate_exception_if(mode_64bit(), EXC_UD, -1);
+
+        imm1 = insn_fetch_bytes(op_bytes);
+        imm2 = insn_fetch_type(uint16_t);
+        break;
+
+    case 0xa0: case 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
+    case 0xa2: case 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
+        /* Source EA is not encoded via ModRM. */
+        ea.mem.off = insn_fetch_bytes(ad_bytes);
+        break;
+
+    case 0xb8 ... 0xbf: /* mov imm{16,32,64},r{16,32,64} */
+        if ( op_bytes == 8 ) /* Fetch more bytes to obtain imm64. */
+            imm1 = ((uint32_t)imm1 |
+                    ((uint64_t)insn_fetch_type(uint32_t) << 32));
+        break;
+
+    case 0xc8: /* enter imm16,imm8 */
+        imm2 = insn_fetch_type(uint8_t);
+        break;
+    }
+
+ done:
+    return rc;
+}
+
+static int
 x86_decode(
     struct x86_emulate_state *state,
     struct x86_emulate_ctxt *ctxt,
@@ -1994,10 +2033,29 @@ x86_decode(
     state->opcode = b;
     state->desc = d;
 
+    switch ( ext )
+    {
+    case ext_none:
+        rc = x86_decode_base(state, ctxt, ops);
+        break;
+
+    case ext_0f:
+    case ext_0f38:
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        return X86EMUL_UNHANDLEABLE;
+    }
+
  done:
     return rc;
 }
 
+/* No insn fetching past this point. */
+#undef insn_fetch_bytes
+#undef insn_fetch_type
+
 int
 x86_emulate(
     struct x86_emulate_ctxt *ctxt,
@@ -2560,6 +2618,8 @@ x86_emulate(
     case 0xc6 ... 0xc7: /* mov (sole member of Grp11) */
         generate_exception_if((modrm_reg & 7) != 0, EXC_UD, -1);
     case 0x88 ... 0x8b: /* mov */
+    case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
+    case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
         dst.val = src.val;
         break;
 
@@ -2644,18 +2704,13 @@ x86_emulate(
 
     case 0x9a: /* call (far, absolute) */ {
         struct segment_register reg;
-        uint16_t sel;
-        uint32_t eip;
 
-        generate_exception_if(mode_64bit(), EXC_UD, -1);
+        ASSERT(!mode_64bit());
         fail_if(ops->read_segment == NULL);
 
-        eip = insn_fetch_bytes(op_bytes);
-        sel = insn_fetch_type(uint16_t);
-
         if ( (rc = ops->read_segment(x86_seg_cs, &reg, ctxt)) ||
-             (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) ||
-             (validate_far_branch(&cs, eip),
+             (rc = load_seg(x86_seg_cs, imm2, 0, &cs, ctxt, ops)) ||
+             (validate_far_branch(&cs, imm1),
               rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
                               &reg.sel, op_bytes, ctxt)) ||
              (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
@@ -2663,7 +2718,7 @@ x86_emulate(
              (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) )
             goto done;
 
-        _regs.eip = eip;
+        _regs.eip = imm1;
         break;
     }
 
@@ -2706,23 +2761,6 @@ x86_emulate(
         ((uint8_t *)&_regs.eax)[1] = (_regs.eflags & 0xd7) | 0x02;
         break;
 
-    case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
-        /* Source EA is not encoded via ModRM. */
-        dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-        if ( (rc = read_ulong(ea.mem.seg, insn_fetch_bytes(ad_bytes),
-                              &dst.val, dst.bytes, ctxt, ops)) != 0 )
-            goto done;
-        break;
-
-    case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
-        /* Destination EA is not encoded via ModRM. */
-        dst.type  = OP_MEM;
-        dst.mem.seg = ea.mem.seg;
-        dst.mem.off = insn_fetch_bytes(ad_bytes);
-        dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-        dst.val   = (unsigned long)_regs.eax;
-        break;
-
     case 0xa4 ... 0xa5: /* movs */ {
         unsigned long nr_reps = get_rep_prefix();
         dst.bytes = (d & ByteOp) ? 1 : op_bytes;
@@ -2840,9 +2878,6 @@ x86_emulate(
         break;
 
     case 0xb8 ... 0xbf: /* mov imm{16,32,64},r{16,32,64} */
-        if ( dst.bytes == 8 ) /* Fetch more bytes to obtain imm64 */
-            src.val = ((uint32_t)src.val |
-                       ((uint64_t)insn_fetch_type(uint32_t) << 32));
         dst.reg = decode_register(
             (b & 7) | ((rex_prefix & 1) << 3), &_regs, 0);
         dst.val = src.val;
@@ -2906,7 +2941,7 @@ x86_emulate(
         goto les;
 
     case 0xc8: /* enter imm16,imm8 */ {
-        uint8_t depth = insn_fetch_type(uint8_t) & 31;
+        uint8_t depth = imm2 & 31;
         int i;
 
         dst.type = OP_REG;
@@ -3627,17 +3662,12 @@ x86_emulate(
         jmp_rel((int32_t)src.val);
         break;
 
-    case 0xea: /* jmp (far, absolute) */ {
-        uint16_t sel;
-        uint32_t eip;
-        generate_exception_if(mode_64bit(), EXC_UD, -1);
-        eip = insn_fetch_bytes(op_bytes);
-        sel = insn_fetch_type(uint16_t);
-        if ( (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) ||
-             (rc = commit_far_branch(&cs, eip)) )
+    case 0xea: /* jmp (far, absolute) */
+        ASSERT(!mode_64bit());
+        if ( (rc = load_seg(x86_seg_cs, imm2, 0, &cs, ctxt, ops)) ||
+             (rc = commit_far_branch(&cs, imm1)) )
             goto done;
         break;
-    }
 
     case 0xf1: /* int1 (icebp) */
         src.val = EXC_DB;



[-- Attachment #2: x86emul-decode-base.patch --]
[-- Type: text/plain, Size: 6723 bytes --]

x86emul: fetch all insn bytes during the decode phase

This way we can offer to callers the service of just sizing
instructions, and we also can better guarantee not to raise the wrong
fault due to not having read all relevant bytes.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -129,8 +129,8 @@ static const opcode_desc_t opcode_table[
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps, ImplicitOps,
     /* 0xA0 - 0xA7 */
-    ByteOp|DstEax|SrcImplicit|Mov, DstEax|SrcImplicit|Mov,
-    ByteOp|ImplicitOps|Mov, ImplicitOps|Mov,
+    ByteOp|DstEax|SrcMem|Mov, DstEax|SrcMem|Mov,
+    ByteOp|DstMem|SrcEax|Mov, DstMem|SrcEax|Mov,
     ByteOp|ImplicitOps|Mov, ImplicitOps|Mov,
     ByteOp|ImplicitOps, ImplicitOps,
     /* 0xA8 - 0xAF */
@@ -1602,6 +1602,45 @@ struct x86_emulate_state {
 #define _regs (state->regs)
 
 static int
+x86_decode_base(
+    struct x86_emulate_state *state,
+    struct x86_emulate_ctxt *ctxt,
+    const struct x86_emulate_ops *ops)
+{
+    int rc = X86EMUL_OKAY;
+
+    switch ( state->opcode )
+    {
+    case 0x9a: /* call (far, absolute) */
+    case 0xea: /* jmp (far, absolute) */
+        generate_exception_if(mode_64bit(), EXC_UD, -1);
+
+        imm1 = insn_fetch_bytes(op_bytes);
+        imm2 = insn_fetch_type(uint16_t);
+        break;
+
+    case 0xa0: case 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
+    case 0xa2: case 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
+        /* Source EA is not encoded via ModRM. */
+        ea.mem.off = insn_fetch_bytes(ad_bytes);
+        break;
+
+    case 0xb8 ... 0xbf: /* mov imm{16,32,64},r{16,32,64} */
+        if ( op_bytes == 8 ) /* Fetch more bytes to obtain imm64. */
+            imm1 = ((uint32_t)imm1 |
+                    ((uint64_t)insn_fetch_type(uint32_t) << 32));
+        break;
+
+    case 0xc8: /* enter imm16,imm8 */
+        imm2 = insn_fetch_type(uint8_t);
+        break;
+    }
+
+ done:
+    return rc;
+}
+
+static int
 x86_decode(
     struct x86_emulate_state *state,
     struct x86_emulate_ctxt *ctxt,
@@ -1994,10 +2033,29 @@ x86_decode(
     state->opcode = b;
     state->desc = d;
 
+    switch ( ext )
+    {
+    case ext_none:
+        rc = x86_decode_base(state, ctxt, ops);
+        break;
+
+    case ext_0f:
+    case ext_0f38:
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        return X86EMUL_UNHANDLEABLE;
+    }
+
  done:
     return rc;
 }
 
+/* No insn fetching past this point. */
+#undef insn_fetch_bytes
+#undef insn_fetch_type
+
 int
 x86_emulate(
     struct x86_emulate_ctxt *ctxt,
@@ -2560,6 +2618,8 @@ x86_emulate(
     case 0xc6 ... 0xc7: /* mov (sole member of Grp11) */
         generate_exception_if((modrm_reg & 7) != 0, EXC_UD, -1);
     case 0x88 ... 0x8b: /* mov */
+    case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
+    case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
         dst.val = src.val;
         break;
 
@@ -2644,18 +2704,13 @@ x86_emulate(
 
     case 0x9a: /* call (far, absolute) */ {
         struct segment_register reg;
-        uint16_t sel;
-        uint32_t eip;
 
-        generate_exception_if(mode_64bit(), EXC_UD, -1);
+        ASSERT(!mode_64bit());
         fail_if(ops->read_segment == NULL);
 
-        eip = insn_fetch_bytes(op_bytes);
-        sel = insn_fetch_type(uint16_t);
-
         if ( (rc = ops->read_segment(x86_seg_cs, &reg, ctxt)) ||
-             (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) ||
-             (validate_far_branch(&cs, eip),
+             (rc = load_seg(x86_seg_cs, imm2, 0, &cs, ctxt, ops)) ||
+             (validate_far_branch(&cs, imm1),
               rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
                               &reg.sel, op_bytes, ctxt)) ||
              (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
@@ -2663,7 +2718,7 @@ x86_emulate(
              (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) )
             goto done;
 
-        _regs.eip = eip;
+        _regs.eip = imm1;
         break;
     }
 
@@ -2706,23 +2761,6 @@ x86_emulate(
         ((uint8_t *)&_regs.eax)[1] = (_regs.eflags & 0xd7) | 0x02;
         break;
 
-    case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
-        /* Source EA is not encoded via ModRM. */
-        dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-        if ( (rc = read_ulong(ea.mem.seg, insn_fetch_bytes(ad_bytes),
-                              &dst.val, dst.bytes, ctxt, ops)) != 0 )
-            goto done;
-        break;
-
-    case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
-        /* Destination EA is not encoded via ModRM. */
-        dst.type  = OP_MEM;
-        dst.mem.seg = ea.mem.seg;
-        dst.mem.off = insn_fetch_bytes(ad_bytes);
-        dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-        dst.val   = (unsigned long)_regs.eax;
-        break;
-
     case 0xa4 ... 0xa5: /* movs */ {
         unsigned long nr_reps = get_rep_prefix();
         dst.bytes = (d & ByteOp) ? 1 : op_bytes;
@@ -2840,9 +2878,6 @@ x86_emulate(
         break;
 
     case 0xb8 ... 0xbf: /* mov imm{16,32,64},r{16,32,64} */
-        if ( dst.bytes == 8 ) /* Fetch more bytes to obtain imm64 */
-            src.val = ((uint32_t)src.val |
-                       ((uint64_t)insn_fetch_type(uint32_t) << 32));
         dst.reg = decode_register(
             (b & 7) | ((rex_prefix & 1) << 3), &_regs, 0);
         dst.val = src.val;
@@ -2906,7 +2941,7 @@ x86_emulate(
         goto les;
 
     case 0xc8: /* enter imm16,imm8 */ {
-        uint8_t depth = insn_fetch_type(uint8_t) & 31;
+        uint8_t depth = imm2 & 31;
         int i;
 
         dst.type = OP_REG;
@@ -3627,17 +3662,12 @@ x86_emulate(
         jmp_rel((int32_t)src.val);
         break;
 
-    case 0xea: /* jmp (far, absolute) */ {
-        uint16_t sel;
-        uint32_t eip;
-        generate_exception_if(mode_64bit(), EXC_UD, -1);
-        eip = insn_fetch_bytes(op_bytes);
-        sel = insn_fetch_type(uint16_t);
-        if ( (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) ||
-             (rc = commit_far_branch(&cs, eip)) )
+    case 0xea: /* jmp (far, absolute) */
+        ASSERT(!mode_64bit());
+        if ( (rc = load_seg(x86_seg_cs, imm2, 0, &cs, ctxt, ops)) ||
+             (rc = commit_far_branch(&cs, imm1)) )
             goto done;
         break;
-    }
 
     case 0xf1: /* int1 (icebp) */
         src.val = EXC_DB;

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

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

  parent reply	other threads:[~2016-09-08 13:07 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 ` Jan Beulich [this message]
2016-09-13 18:44   ` [PATCH 02/17] x86emul: fetch all insn bytes during the decode phase 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
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=57D17EC6020000780010D14C@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).