| Message ID | 20110730180334.2f50c753.takuya.yoshikawa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/30/2011 12:03 PM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp> > > Return EMULATION_OK/FAILED consistently. Also treat instruction fetch > errors, not restricted to X86EMUL_UNHANDLEABLE, as EMULATION_FAILED; > although this cannot happen in practice, the current logic will continue > the emulation even if the decoder fails to fetch the instruction. In fact it can happen in practice, but not through normal usage. For example, one vcpu can execute an instruction which traps into the emulator, while another vcpu changes the page tables to make the instruction unfetchable.
On Sun, 31 Jul 2011 11:48:40 +0300 Avi Kivity <avi@redhat.com> wrote: > On 07/30/2011 12:03 PM, Takuya Yoshikawa wrote: > > From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp> > > > > Return EMULATION_OK/FAILED consistently. Also treat instruction fetch > > errors, not restricted to X86EMUL_UNHANDLEABLE, as EMULATION_FAILED; > > although this cannot happen in practice, the current logic will continue > > the emulation even if the decoder fails to fetch the instruction. > > In fact it can happen in practice, but not through normal usage. For > example, one vcpu can execute an instruction which traps into the > emulator, while another vcpu changes the page tables to make the > instruction unfetchable. > Really virtualization specific! I need to think about multiple vcpus more. Thanks, Takuya
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index ae8d23c..0453c07 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -3373,7 +3373,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len) break; #endif default: - return -1; + return EMULATION_FAILED; } ctxt->op_bytes = def_op_bytes; @@ -3465,7 +3465,7 @@ done_prefixes: break; case Prefix: if (ctxt->rep_prefix && op_prefix) - return X86EMUL_UNHANDLEABLE; + return EMULATION_FAILED; simd_prefix = op_prefix ? 0x66 : ctxt->rep_prefix; switch (simd_prefix) { case 0x00: opcode = opcode.u.gprefix->pfx_no; break; @@ -3475,7 +3475,7 @@ done_prefixes: } break; default: - return X86EMUL_UNHANDLEABLE; + return EMULATION_FAILED; } ctxt->d &= ~GroupMask; @@ -3488,10 +3488,10 @@ done_prefixes: /* Unrecognised? */ if (ctxt->d == 0 || (ctxt->d & Undefined)) - return -1; + return EMULATION_FAILED; if (!(ctxt->d & VendorSpecific) && ctxt->only_vendor_specific_insn) - return -1; + return EMULATION_FAILED; if (mode == X86EMUL_MODE_PROT64 && (ctxt->d & Stack)) ctxt->op_bytes = 8; @@ -3683,7 +3683,7 @@ done: if (memopp && memopp->type == OP_MEM && ctxt->rip_relative) memopp->addr.mem.ea += ctxt->_eip; - return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK; + return (rc != X86EMUL_CONTINUE) ? EMULATION_FAILED : EMULATION_OK; } static bool string_insn_completed(struct x86_emulate_ctxt *ctxt) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 6cb353c..baa427a 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4833,7 +4833,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, trace_kvm_emulate_insn_start(vcpu); ++vcpu->stat.insn_emulation; - if (r) { + if (r != EMULATION_OK) { if (emulation_type & EMULTYPE_TRAP_UD) return EMULATE_FAIL; if (reexecute_instruction(vcpu, cr2))