|
[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 |