[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v2 1/3] x86/svm: Simplify svm_get_insn_len()



The existing __get_instruction_length_from_list() has a single user
which uses the list functionality.  That user however should be looking
specifically for INVD or WBINVD, as reported by the vmexit exit reason.

Modify svm_vmexit_do_invalidate_cache() to ask for the correct
instruction, and drop all list functionality from the helper.

Take the opportunity to rename it to svm_get_insn_len(), and drop the
IOIO length handling whch has never been used.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
CC: Brian Woods <brian.woods@xxxxxxx>

v2:
 * New
---
 xen/arch/x86/hvm/svm/emulate.c        | 65 ++++++++++++++++-------------------
 xen/arch/x86/hvm/svm/nestedsvm.c      |  9 ++---
 xen/arch/x86/hvm/svm/svm.c            | 34 +++++++++---------
 xen/include/asm-x86/hvm/svm/emulate.h |  9 +----
 4 files changed, 51 insertions(+), 66 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
index 3d04af0..3f695b9 100644
--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -83,13 +83,12 @@ static const struct {
     [INSTR_CPUID]   = { X86EMUL_OPC(0x0f, 0xa2) },
 };
 
-int __get_instruction_length_from_list(struct vcpu *v,
-        const enum instruction_index *list, unsigned int list_count)
+int svm_get_insn_len(struct vcpu *v, enum instruction_index insn)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     struct hvm_emulate_ctxt ctxt;
     struct x86_emulate_state *state;
-    unsigned long inst_len, j;
+    unsigned long nrip_len, emul_len;
     unsigned int modrm_rm, modrm_reg;
     int modrm_mod;
 
@@ -98,13 +97,10 @@ int __get_instruction_length_from_list(struct vcpu *v,
      * hardware.
      */
 #ifdef NDEBUG
-    if ( (inst_len = svm_nextrip_insn_length(v)) > MAX_INST_LEN )
-        gprintk(XENLOG_WARNING, "NRip reported inst_len %lu\n", inst_len);
-    else if ( inst_len != 0 )
-        return inst_len;
-
-    if ( vmcb->exitcode == VMEXIT_IOIO )
-        return vmcb->exitinfo2 - vmcb->rip;
+    if ( (nrip_len = svm_nextrip_insn_length(v)) > MAX_INST_LEN )
+        gprintk(XENLOG_WARNING, "NRip reported inst_len %lu\n", nrip_len);
+    else if ( nrip_len != 0 )
+        return nrip_len;
 #endif
 
     ASSERT(v == current);
@@ -114,46 +110,43 @@ int __get_instruction_length_from_list(struct vcpu *v,
     if ( IS_ERR_OR_NULL(state) )
         return 0;
 
-    inst_len = x86_insn_length(state, &ctxt.ctxt);
+    emul_len = x86_insn_length(state, &ctxt.ctxt);
     modrm_mod = x86_insn_modrm(state, &modrm_rm, &modrm_reg);
     x86_emulate_free_state(state);
+
 #ifndef NDEBUG
-    if ( vmcb->exitcode == VMEXIT_IOIO )
-        j = vmcb->exitinfo2 - vmcb->rip;
-    else
-        j = svm_nextrip_insn_length(v);
-    if ( j && j != inst_len )
+    nrip_len = svm_nextrip_insn_length(v);
+    if ( nrip_len && nrip_len != emul_len )
     {
         gprintk(XENLOG_WARNING, "insn-len[%02x]=%lu (exp %lu)\n",
-                ctxt.ctxt.opcode, inst_len, j);
-        return j;
+                ctxt.ctxt.opcode, nrip_len, emul_len);
+        return nrip_len;
     }
 #endif
 
-    for ( j = 0; j < list_count; j++ )
+    if ( (unsigned int)insn >= ARRAY_SIZE(opc_tab) )
     {
-        unsigned int instr = list[j];
-
-        if ( instr >= ARRAY_SIZE(opc_tab) )
-        {
-            ASSERT_UNREACHABLE();
-            break;
-        }
-        if ( opc_tab[instr].opcode == ctxt.ctxt.opcode )
-        {
-            if ( !opc_tab[instr].modrm.mod )
-                return inst_len;
-
-            if ( modrm_mod == opc_tab[instr].modrm.mod &&
-                 (modrm_rm & 7) == opc_tab[instr].modrm.rm &&
-                 (modrm_reg & 7) == opc_tab[instr].modrm.reg )
-                return inst_len;
-        }
+        gdprintk(XENLOG_ERR, "insn %d out of range\n", insn);
+        ASSERT_UNREACHABLE();
+        goto out;
+    }
+
+    if ( opc_tab[insn].opcode == ctxt.ctxt.opcode )
+    {
+        if ( !opc_tab[insn].modrm.mod )
+            return emul_len;
+
+        if ( modrm_mod == opc_tab[insn].modrm.mod &&
+             (modrm_rm & 7) == opc_tab[insn].modrm.rm &&
+             (modrm_reg & 7) == opc_tab[insn].modrm.reg )
+            return emul_len;
     }
 
     gdprintk(XENLOG_WARNING,
              "%s: Mismatch between expected and actual instruction: "
              "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
+
+ out:
     hvm_inject_hw_exception(TRAP_gp_fault, 0);
     return 0;
 }
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 9660202..35c1a04 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -743,8 +743,9 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
 
-    inst_len = __get_instruction_length(v, INSTR_VMRUN);
-    if (inst_len == 0) {
+    inst_len = svm_get_insn_len(v, INSTR_VMRUN);
+    if ( inst_len == 0 )
+    {
         svm->ns_vmexit.exitcode = VMEXIT_SHUTDOWN;
         return -1;
     }
@@ -1616,7 +1617,7 @@ void svm_vmexit_do_stgi(struct cpu_user_regs *regs, 
struct vcpu *v)
         return;
     }
 
-    if ( (inst_len = __get_instruction_length(v, INSTR_STGI)) == 0 )
+    if ( (inst_len = svm_get_insn_len(v, INSTR_STGI)) == 0 )
         return;
 
     nestedsvm_vcpu_stgi(v);
@@ -1637,7 +1638,7 @@ void svm_vmexit_do_clgi(struct cpu_user_regs *regs, 
struct vcpu *v)
         return;
     }
 
-    if ( (inst_len = __get_instruction_length(v, INSTR_CLGI)) == 0 )
+    if ( (inst_len = svm_get_insn_len(v, INSTR_CLGI)) == 0 )
         return;
 
     nestedsvm_vcpu_clgi(v);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 40937bf..f8b7e8b 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2244,7 +2244,7 @@ static void svm_do_msr_access(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
     bool rdmsr = curr->arch.hvm.svm.vmcb->exitinfo1 == 0;
-    int rc, inst_len = __get_instruction_length(
+    int rc, inst_len = svm_get_insn_len(
         curr, rdmsr ? INSTR_RDMSR : INSTR_WRMSR);
 
     if ( inst_len == 0 )
@@ -2272,7 +2272,7 @@ static void svm_vmexit_do_hlt(struct vmcb_struct *vmcb,
 {
     unsigned int inst_len;
 
-    if ( (inst_len = __get_instruction_length(current, INSTR_HLT)) == 0 )
+    if ( (inst_len = svm_get_insn_len(current, INSTR_HLT)) == 0 )
         return;
     __update_guest_eip(regs, inst_len);
 
@@ -2283,7 +2283,7 @@ static void svm_vmexit_do_rdtsc(struct cpu_user_regs 
*regs)
 {
     unsigned int inst_len;
 
-    if ( (inst_len = __get_instruction_length(current, INSTR_RDTSC)) == 0 )
+    if ( (inst_len = svm_get_insn_len(current, INSTR_RDTSC)) == 0 )
         return;
     __update_guest_eip(regs, inst_len);
 
@@ -2294,7 +2294,7 @@ static void svm_vmexit_do_pause(struct cpu_user_regs 
*regs)
 {
     unsigned int inst_len;
 
-    if ( (inst_len = __get_instruction_length(current, INSTR_PAUSE)) == 0 )
+    if ( (inst_len = svm_get_insn_len(current, INSTR_PAUSE)) == 0 )
         return;
     __update_guest_eip(regs, inst_len);
 
@@ -2361,7 +2361,7 @@ svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
     unsigned int inst_len;
     struct page_info *page;
 
-    if ( (inst_len = __get_instruction_length(v, INSTR_VMLOAD)) == 0 )
+    if ( (inst_len = svm_get_insn_len(v, INSTR_VMLOAD)) == 0 )
         return;
 
     if ( !nsvm_efer_svm_enabled(v) ) 
@@ -2396,7 +2396,7 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
     unsigned int inst_len;
     struct page_info *page;
 
-    if ( (inst_len = __get_instruction_length(v, INSTR_VMSAVE)) == 0 )
+    if ( (inst_len = svm_get_insn_len(v, INSTR_VMSAVE)) == 0 )
         return;
 
     if ( !nsvm_efer_svm_enabled(v) ) 
@@ -2464,13 +2464,11 @@ static void svm_wbinvd_intercept(void)
         flush_all(FLUSH_CACHE);
 }
 
-static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
+static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs,
+                                           bool invld)
 {
-    static const enum instruction_index list[] = { INSTR_INVD, INSTR_WBINVD };
-    int inst_len;
+    int inst_len = svm_get_insn_len(current, invld ? INSTR_INVD : 
INSTR_WBINVD);
 
-    inst_len = __get_instruction_length_from_list(
-        current, list, ARRAY_SIZE(list));
     if ( inst_len == 0 )
         return;
 
@@ -2745,7 +2743,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
             else
             {
                 trap_type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
-                inst_len = __get_instruction_length(v, INSTR_ICEBP);
+                inst_len = svm_get_insn_len(v, INSTR_ICEBP);
             }
 
             rc = hvm_monitor_debug(regs->rip,
@@ -2762,7 +2760,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
         break;
 
     case VMEXIT_EXCEPTION_BP:
-        inst_len = __get_instruction_length(v, INSTR_INT3);
+        inst_len = svm_get_insn_len(v, INSTR_INT3);
 
         if ( inst_len == 0 )
              break;
@@ -2853,7 +2851,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
 
     case VMEXIT_INVD:
     case VMEXIT_WBINVD:
-        svm_vmexit_do_invalidate_cache(regs);
+        svm_vmexit_do_invalidate_cache(regs, exit_reason == VMEXIT_INVD);
         break;
 
     case VMEXIT_TASK_SWITCH: {
@@ -2882,7 +2880,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
 
     case VMEXIT_CPUID:
     {
-        unsigned int inst_len = __get_instruction_length(v, INSTR_CPUID);
+        unsigned int inst_len = svm_get_insn_len(v, INSTR_CPUID);
         int rc = 0;
 
         if ( inst_len == 0 )
@@ -2938,14 +2936,14 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
             hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
             break;
         }
-        if ( (inst_len = __get_instruction_length(v, INSTR_INVLPGA)) == 0 )
+        if ( (inst_len = svm_get_insn_len(v, INSTR_INVLPGA)) == 0 )
             break;
         svm_invlpga_intercept(v, regs->rax, regs->ecx);
         __update_guest_eip(regs, inst_len);
         break;
 
     case VMEXIT_VMMCALL:
-        if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
+        if ( (inst_len = svm_get_insn_len(v, INSTR_VMCALL)) == 0 )
             break;
         BUG_ON(vcpu_guestmode);
         HVMTRACE_1D(VMMCALL, regs->eax);
@@ -3001,7 +2999,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
     case VMEXIT_XSETBV:
         if ( vmcb_get_cpl(vmcb) )
             hvm_inject_hw_exception(TRAP_gp_fault, 0);
-        else if ( (inst_len = __get_instruction_length(v, INSTR_XSETBV)) &&
+        else if ( (inst_len = svm_get_insn_len(v, INSTR_XSETBV)) &&
                   hvm_handle_xsetbv(regs->ecx, msr_fold(regs)) == X86EMUL_OKAY 
)
             __update_guest_eip(regs, inst_len);
         break;
diff --git a/xen/include/asm-x86/hvm/svm/emulate.h 
b/xen/include/asm-x86/hvm/svm/emulate.h
index 3de8236..1d062d2 100644
--- a/xen/include/asm-x86/hvm/svm/emulate.h
+++ b/xen/include/asm-x86/hvm/svm/emulate.h
@@ -44,14 +44,7 @@ enum instruction_index {
 
 struct vcpu;
 
-int __get_instruction_length_from_list(
-    struct vcpu *, const enum instruction_index *, unsigned int list_count);
-
-static inline int __get_instruction_length(
-    struct vcpu *v, enum instruction_index instr)
-{
-    return __get_instruction_length_from_list(v, &instr, 1);
-}
+int svm_get_insn_len(struct vcpu *v, enum instruction_index instr);
 
 #endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.