Skip to content

Commit

Permalink
KVM: x86 emulator: fix in/out emulation.
Browse files Browse the repository at this point in the history
in/out emulation is broken now. The breakage is different depending
on where IO device resides. If it is in userspace emulator reports
emulation failure since it incorrectly interprets kvm_emulate_pio()
return value. If IO device is in the kernel emulation of 'in' will do
nothing since kvm_emulate_pio() stores result directly into vcpu
registers, so emulator will overwrite result of emulation during
commit of shadowed register.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
  • Loading branch information
Gleb Natapov authored and Avi Kivity committed May 17, 2010
1 parent d927112 commit cf8f70b
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 133 deletions.
7 changes: 7 additions & 0 deletions arch/x86/include/asm/kvm_emulate.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ struct x86_emulate_ops {
const void *new,
unsigned int bytes,
struct kvm_vcpu *vcpu);

int (*pio_in_emulated)(int size, unsigned short port, void *val,
unsigned int count, struct kvm_vcpu *vcpu);

int (*pio_out_emulated)(int size, unsigned short port, const void *val,
unsigned int count, struct kvm_vcpu *vcpu);

bool (*get_cached_descriptor)(struct desc_struct *desc,
int seg, struct kvm_vcpu *vcpu);
void (*set_cached_descriptor)(struct desc_struct *desc,
Expand Down
3 changes: 1 addition & 2 deletions arch/x86/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,7 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);

struct x86_emulate_ctxt;

int kvm_emulate_pio(struct kvm_vcpu *vcpu, int in,
int size, unsigned port);
int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port);
int kvm_emulate_pio_string(struct kvm_vcpu *vcpu, int in,
int size, unsigned long count, int down,
gva_t address, int rep, unsigned port);
Expand Down
50 changes: 24 additions & 26 deletions arch/x86/kvm/emulate.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,13 @@ static u32 opcode_table[256] = {
0, 0, 0, 0, 0, 0, 0, 0,
/* 0xE0 - 0xE7 */
0, 0, 0, 0,
ByteOp | SrcImmUByte, SrcImmUByte,
ByteOp | SrcImmUByte, SrcImmUByte,
ByteOp | SrcImmUByte | DstAcc, SrcImmUByte | DstAcc,
ByteOp | SrcImmUByte | DstAcc, SrcImmUByte | DstAcc,
/* 0xE8 - 0xEF */
SrcImm | Stack, SrcImm | ImplicitOps,
SrcImmU | Src2Imm16 | No64, SrcImmByte | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
SrcNone | ByteOp | DstAcc, SrcNone | DstAcc,
SrcNone | ByteOp | DstAcc, SrcNone | DstAcc,
/* 0xF0 - 0xF7 */
0, 0, 0, 0,
ImplicitOps | Priv, ImplicitOps, Group | Group3_Byte, Group | Group3,
Expand Down Expand Up @@ -2426,8 +2426,6 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
u64 msr_data;
unsigned long saved_eip = 0;
struct decode_cache *c = &ctxt->decode;
unsigned int port;
int io_dir_in;
int rc = X86EMUL_CONTINUE;

ctxt->interruptibility = 0;
Expand Down Expand Up @@ -2823,14 +2821,10 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
break;
case 0xe4: /* inb */
case 0xe5: /* in */
port = c->src.val;
io_dir_in = 1;
goto do_io;
goto do_io_in;
case 0xe6: /* outb */
case 0xe7: /* out */
port = c->src.val;
io_dir_in = 0;
goto do_io;
goto do_io_out;
case 0xe8: /* call (near) */ {
long int rel = c->src.val;
c->src.val = (unsigned long) c->eip;
Expand All @@ -2855,25 +2849,29 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
break;
case 0xec: /* in al,dx */
case 0xed: /* in (e/r)ax,dx */
port = c->regs[VCPU_REGS_RDX];
io_dir_in = 1;
goto do_io;
c->src.val = c->regs[VCPU_REGS_RDX];
do_io_in:
c->dst.bytes = min(c->dst.bytes, 4u);
if (!emulator_io_permited(ctxt, ops, c->src.val, c->dst.bytes)) {
kvm_inject_gp(ctxt->vcpu, 0);
goto done;
}
if (!ops->pio_in_emulated(c->dst.bytes, c->src.val,
&c->dst.val, 1, ctxt->vcpu))
goto done; /* IO is needed */
break;
case 0xee: /* out al,dx */
case 0xef: /* out (e/r)ax,dx */
port = c->regs[VCPU_REGS_RDX];
io_dir_in = 0;
do_io:
if (!emulator_io_permited(ctxt, ops, port,
(c->d & ByteOp) ? 1 : c->op_bytes)) {
c->src.val = c->regs[VCPU_REGS_RDX];
do_io_out:
c->dst.bytes = min(c->dst.bytes, 4u);
if (!emulator_io_permited(ctxt, ops, c->src.val, c->dst.bytes)) {
kvm_inject_gp(ctxt->vcpu, 0);
goto done;
}
if (kvm_emulate_pio(ctxt->vcpu, io_dir_in,
(c->d & ByteOp) ? 1 : c->op_bytes,
port) != 0) {
c->eip = saved_eip;
goto cannot_emulate;
}
ops->pio_out_emulated(c->dst.bytes, c->src.val, &c->dst.val, 1,
ctxt->vcpu);
c->dst.type = OP_NONE; /* Disable writeback. */
break;
case 0xf4: /* hlt */
ctxt->vcpu->arch.halt_request = 1;
Expand Down
20 changes: 7 additions & 13 deletions arch/x86/kvm/svm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1494,29 +1494,23 @@ static int shutdown_interception(struct vcpu_svm *svm)

static int io_interception(struct vcpu_svm *svm)
{
struct kvm_vcpu *vcpu = &svm->vcpu;
u32 io_info = svm->vmcb->control.exit_info_1; /* address size bug? */
int size, in, string;
unsigned port;

++svm->vcpu.stat.io_exits;

svm->next_rip = svm->vmcb->control.exit_info_2;

string = (io_info & SVM_IOIO_STR_MASK) != 0;

if (string) {
if (emulate_instruction(&svm->vcpu,
0, 0, 0) == EMULATE_DO_MMIO)
return 0;
return 1;
}

in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
if (string || in)
return !(emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO);

port = io_info >> 16;
size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT;

svm->next_rip = svm->vmcb->control.exit_info_2;
skip_emulated_instruction(&svm->vcpu);
return kvm_emulate_pio(&svm->vcpu, in, size, port);

return kvm_fast_pio_out(vcpu, size, port);
}

static int nmi_interception(struct vcpu_svm *svm)
Expand Down
18 changes: 8 additions & 10 deletions arch/x86/kvm/vmx.c
Original file line number Diff line number Diff line change
Expand Up @@ -2985,22 +2985,20 @@ static int handle_io(struct kvm_vcpu *vcpu)
int size, in, string;
unsigned port;

++vcpu->stat.io_exits;
exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
string = (exit_qualification & 16) != 0;
in = (exit_qualification & 8) != 0;

if (string) {
if (emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO)
return 0;
return 1;
}
++vcpu->stat.io_exits;

size = (exit_qualification & 7) + 1;
in = (exit_qualification & 8) != 0;
port = exit_qualification >> 16;
if (string || in)
return !(emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO);

port = exit_qualification >> 16;
size = (exit_qualification & 7) + 1;
skip_emulated_instruction(vcpu);
return kvm_emulate_pio(vcpu, in, size, port);

return kvm_fast_pio_out(vcpu, size, port);
}

static void
Expand Down
Loading

0 comments on commit cf8f70b

Please sign in to comment.