[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v3 1/3] x86/svm: Remove list functionality from __get_instruction_length_* infrastructure
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> Reviewed-by: 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 v3: * Deduplicate the calls to svm_nextrip_insn_length() --- xen/arch/x86/hvm/svm/emulate.c | 76 +++++++++++++++++------------------ xen/arch/x86/hvm/svm/nestedsvm.c | 9 +++-- xen/arch/x86/hvm/svm/svm.c | 39 +++++++++--------- xen/include/asm-x86/hvm/svm/emulate.h | 9 +---- 4 files changed, 61 insertions(+), 72 deletions(-) diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c index 4abeab8..7799908 100644 --- a/xen/arch/x86/hvm/svm/emulate.c +++ b/xen/arch/x86/hvm/svm/emulate.c @@ -84,28 +84,31 @@ 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) +/* + * First-gen SVM didn't have the NextRIP feature, meaning that when we take a + * fault-style vmexit, we have to decode the instruction stream to calculate + * how many bytes to move %rip forwards by. + * + * To double check the implementation, in debug builds, always compare the + * hardware reported instruction length (if available) with the result from + * x86_decode_insn(). + */ +unsigned 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; - /* - * In debug builds, always use x86_decode_insn() and compare with - * 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; + nrip_len = svm_nextrip_insn_length(v); - if ( vmcb->exitcode == VMEXIT_IOIO ) - return vmcb->exitinfo2 - vmcb->rip; +#ifdef NDEBUG + if ( nrip_len > 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); @@ -115,41 +118,34 @@ 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 ) + 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 ( 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; - } + ASSERT_UNREACHABLE(); + return 0; + } + + 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, 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 954822c..2584b90 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -2244,8 +2244,8 @@ 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( - curr, rdmsr ? INSTR_RDMSR : INSTR_WRMSR); + int rc, inst_len = svm_get_insn_len(curr, rdmsr ? INSTR_RDMSR + : INSTR_WRMSR); if ( inst_len == 0 ) return; @@ -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,6 @@ static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs, bool rdtscp) { struct vcpu *curr = current; const struct domain *currd = curr->domain; - enum instruction_index insn = rdtscp ? INSTR_RDTSCP : INSTR_RDTSC; unsigned int inst_len; if ( rdtscp && !currd->arch.cpuid->extd.rdtscp ) @@ -2292,7 +2291,8 @@ static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs, bool rdtscp) return; } - if ( (inst_len = __get_instruction_length(curr, insn)) == 0 ) + if ( (inst_len = svm_get_insn_len(curr, rdtscp ? INSTR_RDTSCP + : INSTR_RDTSC)) == 0 ) return; __update_guest_eip(regs, inst_len); @@ -2307,7 +2307,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); @@ -2374,7 +2374,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) ) @@ -2409,7 +2409,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) ) @@ -2477,13 +2477,12 @@ 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; + unsigned 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; @@ -2758,7 +2757,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, @@ -2775,7 +2774,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; @@ -2866,7 +2865,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: { @@ -2895,7 +2894,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 ) @@ -2951,14 +2950,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); @@ -3012,7 +3011,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 ca92abb..82359ec 100644 --- a/xen/include/asm-x86/hvm/svm/emulate.h +++ b/xen/include/asm-x86/hvm/svm/emulate.h @@ -45,14 +45,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); -} +unsigned 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |