x86emul: New return code for unimplemented instruction

Enforce the distinction between an instruction not implemented by the
emulator and the failure to emulate that instruction by defining a new
return code, X86EMUL_UNIMPLEMENTED.

This value should only be returned by the core emulator when a valid
opcode is found but the execution logic for that instruction is missing.
It should NOT be returned by any of the x86_emulate_ops callbacks.

e.g. hvm_process_io_intercept should not return X86EMUL_UNIMPLEMENTED.
The return value of this function depends on either the return code of
one of the hvm_io_ops handlers (read/write) or the value returned by
hvm_copy_guest_from_phys / hvm_copy_to_guest_phys.

Similary, none of this functions should return X86EMUL_UNIMPLEMENTED.
 - hvm_io_intercept
 - hvmemul_do_io
 - hvm_send_buffered_ioreq
 - hvm_send_ioreq
 - hvm_broadcast_ioreq
 - hvmemul_do_io_buffer
 - hvmemul_validate

Also the behavior of hvm_emulate_one_insn and vmx_realmode_emulate_one
was modified to generate an Invalid Opcode trap when X86EMUL_UNRECOGNIZED
is returned by the emulator instead of just crash the domain.

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index cc874ce64ae329540a69aaf42785a86efffeca42..385fe1e30be5e2ced3efbb37a256dc1a9624fa97 100644 (file)
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -284,10 +284,15 @@ static int hvmemul_do_io(
         }
         break;
     }
+    case X86EMUL_UNIMPLEMENTED:
+        ASSERT_UNREACHABLE();
+        /* Fall-through */
     default:
         BUG();
     }
 
+    ASSERT(rc != X86EMUL_UNIMPLEMENTED);
+
     if ( rc != X86EMUL_OKAY )
         return rc;
 
@@ -313,6 +318,9 @@ static int hvmemul_do_io_buffer(
 
     rc = hvmemul_do_io(is_mmio, addr, reps, size, dir, df, 0,
                        (uintptr_t)buffer);
+
+    ASSERT(rc != X86EMUL_UNIMPLEMENTED);
+
     if ( rc == X86EMUL_UNHANDLEABLE && dir == IOREQ_READ )
         memset(buffer, 0xff, size);
 
@@ -405,6 +413,8 @@ static int hvmemul_do_io_addr(
     rc = hvmemul_do_io(is_mmio, addr, &count, size, dir, df, 1,
                        ram_gpa);
 
+    ASSERT(rc != X86EMUL_UNIMPLEMENTED);
+
     if ( rc == X86EMUL_OKAY )
         v->arch.hvm_vcpu.hvm_io.mmio_retry = (count < *reps);
 
@@ -2045,6 +2055,7 @@ int hvm_emulate_one_mmio(unsigned long mfn, unsigned long gla)
     switch ( rc )
     {
     case X86EMUL_UNHANDLEABLE:
+    case X86EMUL_UNIMPLEMENTED:
         hvm_dump_emulation_state(XENLOG_G_WARNING, "MMCFG", &ctxt);
         break;
     case X86EMUL_EXCEPTION:
@@ -2102,6 +2113,7 @@ void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int trapnr,
          * consistent with X86EMUL_RETRY.
          */
         return;
+    case X86EMUL_UNIMPLEMENTED:
     case X86EMUL_UNHANDLEABLE:
         hvm_dump_emulation_state(XENLOG_G_DEBUG, "Mem event", &ctx);
         hvm_inject_hw_exception(trapnr, errcode);

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 887f0e556c5eea1df2001991793cb016cfcee8f7..b0a46d7f7203bf38059c3da82e1175695f8e6ed6 100644 (file)
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3735,6 +3735,7 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
     switch ( hvm_emulate_one(&ctxt) )
     {
     case X86EMUL_UNHANDLEABLE:
+    case X86EMUL_UNIMPLEMENTED:
         hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
         break;
     case X86EMUL_EXCEPTION:

diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 1ddcaba52e3ab9b3a9f3d16525e452b95e4bcfc3..508e28ff4fd3be62c210c9572f043dd68c30b93e 100644 (file)
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -99,6 +99,11 @@ bool hvm_emulate_one_insn(hvm_emulate_validate_t *validate, const char *descr)
         hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt);
         return false;
 
+    case X86EMUL_UNRECOGNIZED:
+        hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt);
+        hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
+        break;
+
     case X86EMUL_EXCEPTION:
         hvm_inject_event(&ctxt.ctxt.event);
         break;

diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
index 12d43ad98cca63570c43ba4c13adc1d13d9b23ac..b73fc80c9ca2dc1d57e5d931c4ca705f417d784d 100644 (file)
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -112,6 +112,15 @@ void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt)
         goto fail;
     }
 
+    if ( rc == X86EMUL_UNRECOGNIZED )
+    {
+        gdprintk(XENLOG_ERR, "Unrecognized insn.\n");
+        if ( curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE )
+            goto fail;
+
+        realmode_deliver_exception(TRAP_invalid_op, 0, hvmemul_ctxt);
+    }
+
     if ( rc == X86EMUL_EXCEPTION )
     {
         if ( unlikely(curr->domain->debugger_attached) &&

diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 8d4f2442b86280c8dbac5a2b89b4918e75061623..2557e21ebe1745a590438b425ce80fc17a85615f 100644 (file)
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3488,7 +3488,7 @@ static int sh_page_fault(struct vcpu *v,
      * would be a good unshadow hint. If we *do* decide to unshadow-on-fault
      * then it must be 'failable': we cannot require the unshadow to succeed.
      */
-    if ( r == X86EMUL_UNHANDLEABLE )
+    if ( r == X86EMUL_UNHANDLEABLE || r == X86EMUL_UNIMPLEMENTED )
     {
         perfc_incr(shadow_fault_emulate_failed);
 #if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index ff1a401bae3cc368e47502319520297bceb4b63e..a68676cf5fbeb470db05fe2badec6bc958ca0eb5 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -848,7 +848,8 @@ do{ asm volatile (                                                      \
                 stub.func);                                             \
         generate_exception_if(res_.fields.trapnr == EXC_UD, EXC_UD);    \
         domain_crash(current->domain);                                  \
-        goto cannot_emulate;                                            \
+        rc = X86EMUL_UNHANDLEABLE;                                      \
+        goto done;                                                      \
     }                                                                   \
 } while (0)
 #else
@@ -2585,7 +2586,7 @@ x86_decode(
                         d = twobyte_table[0x3a].desc;
                         break;
                     default:
-                        rc = X86EMUL_UNHANDLEABLE;
+                        rc = X86EMUL_UNRECOGNIZED;
                         goto done;
                     }
                 }
@@ -2599,7 +2600,7 @@ x86_decode(
                 }
                 else
                 {
-                    rc = X86EMUL_UNHANDLEABLE;
+                    rc = X86EMUL_UNRECOGNIZED;
                     goto done;
                 }
 
@@ -2879,7 +2880,7 @@ x86_decode(
 
     default:
         ASSERT_UNREACHABLE();
-        return X86EMUL_UNHANDLEABLE;
+        return X86EMUL_UNIMPLEMENTED;
     }
 
     if ( ea.type == OP_MEM )
@@ -4193,7 +4194,7 @@ x86_emulate(
                 break;
             case 4: /* fldenv - TODO */
                 state->fpu_ctrl = true;
-                goto cannot_emulate;
+                goto unimplemented_insn;
             case 5: /* fldcw m2byte */
                 state->fpu_ctrl = true;
                 if ( (rc = ops->read(ea.mem.seg, ea.mem.off, &src.val,
@@ -4204,7 +4205,7 @@ x86_emulate(
                 break;
             case 6: /* fnstenv - TODO */
                 state->fpu_ctrl = true;
-                goto cannot_emulate;
+                goto unimplemented_insn;
             case 7: /* fnstcw m2byte */
                 state->fpu_ctrl = true;
                 emulate_fpu_insn_memdst("fnstcw", dst.val);
@@ -4440,7 +4441,7 @@ x86_emulate(
             case 4: /* frstor - TODO */
             case 6: /* fnsave - TODO */
                 state->fpu_ctrl = true;
-                goto cannot_emulate;
+                goto unimplemented_insn;
             case 7: /* fnstsw m2byte */
                 state->fpu_ctrl = true;
                 emulate_fpu_insn_memdst("fnstsw", dst.val);
@@ -5199,7 +5200,7 @@ x86_emulate(
 #undef _GRP7
 
         default:
-            goto cannot_emulate;
+            goto unimplemented_insn;
         }
         break;
     }
@@ -6197,7 +6198,7 @@ x86_emulate(
                 /* vpsll{w,d} $imm8,{x,y}mm,{x,y}mm */
             break;
         default:
-            goto cannot_emulate;
+            goto unrecognized_insn;
         }
     simd_0f_shift_imm:
         generate_exception_if(ea.type != OP_REG, EXC_UD);
@@ -6245,7 +6246,7 @@ x86_emulate(
         case 6: /* psllq $imm8,mm */
             goto simd_0f_shift_imm;
         }
-        goto cannot_emulate;
+        goto unrecognized_insn;
 
     case X86EMUL_OPC_66(0x0f, 0x73):
     case X86EMUL_OPC_VEX_66(0x0f, 0x73):
@@ -6261,7 +6262,7 @@ x86_emulate(
                 /* vpslldq $imm8,{x,y}mm,{x,y}mm */
             goto simd_0f_shift_imm;
         }
-        goto cannot_emulate;
+        goto unrecognized_insn;
 
     case X86EMUL_OPC(0x0f, 0x77):        /* emms */
     case X86EMUL_OPC_VEX(0x0f, 0x77):    /* vzero{all,upper} */
@@ -6325,7 +6326,7 @@ x86_emulate(
         case 0: /* extrq $imm8,$imm8,xmm */
             break;
         default:
-            goto cannot_emulate;
+            goto unrecognized_insn;
         }
         /* fall through */
     case X86EMUL_OPC_F2(0x0f, 0x78):     /* insertq $imm8,$imm8,xmm,xmm */
@@ -6520,7 +6521,7 @@ x86_emulate(
                 goto done;
             break;
         default:
-            goto cannot_emulate;
+            goto unimplemented_insn;
         }
         break;
 
@@ -6536,7 +6537,7 @@ x86_emulate(
             vcpu_must_have(avx);
             goto stmxcsr;
         }
-        goto cannot_emulate;
+        goto unrecognized_insn;
 
     case X86EMUL_OPC_F3(0x0f, 0xae): /* Grp15 */
         fail_if(modrm_mod != 3);
@@ -6779,10 +6780,10 @@ x86_emulate(
             switch ( modrm_reg & 7 )
             {
             default:
-                goto cannot_emulate;
+                goto unrecognized_insn;
 
-#ifdef HAVE_GAS_RDRAND
             case 6: /* rdrand */
+#ifdef HAVE_GAS_RDRAND
                 generate_exception_if(rep_prefix(), EXC_UD);
                 host_and_vcpu_must_have(rdrand);
                 dst = ea;
@@ -6807,6 +6808,8 @@ x86_emulate(
                 if ( carry )
                     _regs.eflags |= X86_EFLAGS_CF;
                 break;
+#else
+                goto unimplemented_insn;
 #endif
 
             case 7: /* rdseed / rdpid */
@@ -7361,7 +7364,7 @@ x86_emulate(
             host_and_vcpu_must_have(bmi1);
             break;
         default:
-            goto cannot_emulate;
+            goto unrecognized_insn;
         }
 
         generate_exception_if(vex.l, EXC_UD);
@@ -7672,7 +7675,7 @@ x86_emulate(
             host_and_vcpu_must_have(tbm);
             break;
         default:
-            goto cannot_emulate;
+            goto unrecognized_insn;
         }
 
     xop_09_rm_rv:
@@ -7706,7 +7709,7 @@ x86_emulate(
             host_and_vcpu_must_have(tbm);
             goto xop_09_rm_rv;
         }
-        goto cannot_emulate;
+        goto unrecognized_insn;
 
     case X86EMUL_OPC_XOP(0a, 0x10): /* bextr imm,r/m,r */
     {
@@ -7738,8 +7741,11 @@ x86_emulate(
     }
 
     default:
-    cannot_emulate:
-        rc = X86EMUL_UNHANDLEABLE;
+    unimplemented_insn:
+        rc = X86EMUL_UNIMPLEMENTED;
+        goto done;
+    unrecognized_insn:
+        rc = X86EMUL_UNRECOGNIZED;
         goto done;
     }
 
@@ -7791,7 +7797,8 @@ x86_emulate(
                 if ( (d & DstMask) != DstMem )
                 {
                     ASSERT_UNREACHABLE();
-                    goto cannot_emulate;
+                    rc = X86EMUL_UNHANDLEABLE;
+                    goto done;
                 }
                 break;
             }

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 4ddf11148781acc92301ac6b96ee9fda2187eb0d..0c8c80ad5a79807bf6042cbd40eb60f269788ea8 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -133,6 +133,23 @@ struct x86_emul_fpu_aux {
   * Undefined behavior when used anywhere else.
   */
 #define X86EMUL_DONE           4
+ /*
+  * Current instruction is not implemented by the emulator.
+  * This value should only be returned by the core emulator when a valid
+  * opcode is found but the execution logic for that instruction is missing.
+  * It should NOT be returned by any of the x86_emulate_ops callbacks.
+  */
+#define X86EMUL_UNIMPLEMENTED  5
+ /*
+  * The current instruction's opcode is not valid.
+  * If this error code is returned by a function, an #UD trap should be
+  * raised by the final consumer of it.
+  *
+  * TODO: For the moment X86EMUL_UNRECOGNIZED and X86EMUL_UNIMPLEMENTED
+  * can be used interchangeably therefore raising an #UD trap is not
+  * strictly expected for now.
+ */
+#define X86EMUL_UNRECOGNIZED   X86EMUL_UNIMPLEMENTED
 
 /* FPU sub-types which may be requested via ->get_fpu(). */
 enum x86_emulate_fpu_type {