This patch differentiate the X86EMUL_* values returned from X86EMUL_* type functions. Note: During this work, we noticed some buggy return value checks in x86_emulate_insn(). See FIXME in this patch. Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@xxxxxxxxxxxxx> --- arch/x86/kvm/emulate.c | 73 +++++++++++++++++++++++++++++------------------- 1 files changed, 44 insertions(+), 29 deletions(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 9953f5b..d49e9de 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1693,7 +1693,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops) struct decode_cache *c = &ctxt->decode; unsigned int port; int io_dir_in; - int rc = 0; + int rc = X86EMUL_CONTINUE; ctxt->interruptibility = 0; @@ -1791,7 +1791,7 @@ special_insn: break; case 0x07: /* pop es */ rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0x08 ... 0x0d: @@ -1810,7 +1810,7 @@ special_insn: break; case 0x17: /* pop ss */ rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0x18 ... 0x1d: @@ -1822,7 +1822,7 @@ special_insn: break; case 0x1f: /* pop ds */ rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0x20 ... 0x25: @@ -1853,7 +1853,7 @@ special_insn: case 0x58 ... 0x5f: /* pop reg */ pop_instruction: rc = emulate_pop(ctxt, ops, &c->dst.val, c->op_bytes); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0x60: /* pusha */ @@ -1861,7 +1861,7 @@ special_insn: break; case 0x61: /* popa */ rc = emulate_popa(ctxt, ops); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0x63: /* movsxd */ @@ -2002,7 +2002,7 @@ special_insn: } case 0x8f: /* pop (sole member of Grp1a) */ rc = emulate_grp1a(ctxt, ops); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0x90: /* nop / xchg r8,rax */ @@ -2135,7 +2135,7 @@ special_insn: break; case 0xcb: /* ret far */ rc = emulate_ret_far(ctxt, ops); - if (rc) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0xd0 ... 0xd1: /* Grp2 */ @@ -2205,7 +2205,7 @@ special_insn: break; case 0xf6 ... 0xf7: /* Grp3 */ rc = emulate_grp3(ctxt, ops); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0xf8: /* clc */ @@ -2231,14 +2231,14 @@ special_insn: break; case 0xfe ... 0xff: /* Grp4/Grp5 */ rc = emulate_grp45(ctxt, ops); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; } writeback: rc = writeback(ctxt, ops); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; /* Commit shadow register state. */ @@ -2263,8 +2263,18 @@ twobyte_insn: if (c->modrm_mod != 3 || c->modrm_rm != 1) goto cannot_emulate; - rc = kvm_fix_hypercall(ctxt->vcpu); - if (rc) + /* FIXME: + * kvm_fix_hypercall() calls emulator_write_emulated() + * and if the return value is not X86EMUL_CONTINUE then + * returns -EFAULT, otherwise returns X86EMUL_CONTINUE. + * + * To handle the former case, original code just did + * goto done with rc = -EFAULT and passed the + * if (X86EMUL_UNHANDLEABLE) check. + * Instead of this, we just set rc to X86EMUL_CONTINUE. + */ + rc = X86EMUL_CONTINUE; + if (kvm_fix_hypercall(ctxt->vcpu)) goto done; /* Let the processor re-execute the fixed hypercall */ @@ -2275,7 +2285,7 @@ twobyte_insn: case 2: /* lgdt */ rc = read_descriptor(ctxt, ops, c->src.ptr, &size, &address, c->op_bytes); - if (rc) + if (rc != X86EMUL_CONTINUE) goto done; realmode_lgdt(ctxt->vcpu, size, address); /* Disable writeback. */ @@ -2285,8 +2295,9 @@ twobyte_insn: if (c->modrm_mod == 3) { switch (c->modrm_rm) { case 1: - rc = kvm_fix_hypercall(ctxt->vcpu); - if (rc) + /* FIXME: See above */ + rc = X86EMUL_CONTINUE; + if (kvm_fix_hypercall(ctxt->vcpu)) goto done; break; default: @@ -2296,7 +2307,7 @@ twobyte_insn: rc = read_descriptor(ctxt, ops, c->src.ptr, &size, &address, c->op_bytes); - if (rc) + if (rc != X86EMUL_CONTINUE) goto done; realmode_lidt(ctxt->vcpu, size, address); } @@ -2347,9 +2358,12 @@ twobyte_insn: case 0x21: /* mov from dr to reg */ if (c->modrm_mod != 3) goto cannot_emulate; - rc = emulator_get_dr(ctxt, c->modrm_reg, &c->regs[c->modrm_rm]); - if (rc) + if (emulator_get_dr(ctxt, c->modrm_reg, &c->regs[c->modrm_rm])) goto cannot_emulate; + /* FIXME: Next line is just for ensuring to pass + * the if (rc != X86EMUL_UNHANDLEABLE) test. + */ + rc = X86EMUL_CONTINUE; c->dst.type = OP_NONE; /* no writeback */ break; case 0x22: /* mov reg, cr */ @@ -2362,18 +2376,19 @@ twobyte_insn: case 0x23: /* mov from reg to dr */ if (c->modrm_mod != 3) goto cannot_emulate; - rc = emulator_set_dr(ctxt, c->modrm_reg, - c->regs[c->modrm_rm]); - if (rc) + if (emulator_set_dr(ctxt, c->modrm_reg, c->regs[c->modrm_rm])) goto cannot_emulate; + /* FIXME: Next line is just for ensuring to pass + * the if (rc != X86EMUL_UNHANDLEABLE) test. + */ + rc = X86EMUL_CONTINUE; c->dst.type = OP_NONE; /* no writeback */ break; case 0x30: /* wrmsr */ msr_data = (u32)c->regs[VCPU_REGS_RAX] | ((u64)c->regs[VCPU_REGS_RDX] << 32); - rc = kvm_set_msr(ctxt->vcpu, c->regs[VCPU_REGS_RCX], msr_data); - if (rc) { + if (kvm_set_msr(ctxt->vcpu, c->regs[VCPU_REGS_RCX], msr_data)) { kvm_inject_gp(ctxt->vcpu, 0); c->eip = kvm_rip_read(ctxt->vcpu); } @@ -2382,8 +2397,8 @@ twobyte_insn: break; case 0x32: /* rdmsr */ - rc = kvm_get_msr(ctxt->vcpu, c->regs[VCPU_REGS_RCX], &msr_data); - if (rc) { + if (kvm_get_msr(ctxt->vcpu, c->regs[VCPU_REGS_RCX], + &msr_data)) { kvm_inject_gp(ctxt->vcpu, 0); c->eip = kvm_rip_read(ctxt->vcpu); } else { @@ -2420,7 +2435,7 @@ twobyte_insn: break; case 0xa1: /* pop fs */ rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_FS); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0xa3: @@ -2439,7 +2454,7 @@ twobyte_insn: break; case 0xa9: /* pop gs */ rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_GS); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; break; case 0xab: @@ -2512,7 +2527,7 @@ twobyte_insn: break; case 0xc7: /* Grp9 (cmpxchg8b) */ rc = emulate_grp9(ctxt, ops, memop); - if (rc != 0) + if (rc != X86EMUL_CONTINUE) goto done; c->dst.type = OP_NONE; break; -- 1.6.3.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html