KVM: x86 emulator: fix in/out emulation.

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>

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index bd46929..679245c 100644 (file)
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -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,

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b99cec1..776d3e2 100644 (file)
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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);

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 594574d..2d095ce 100644 (file)
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -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,
@@ -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;
@@ -2823,14 +2821,10 @@ special_insn:
                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;
@@ -2855,25 +2849,29 @@ special_insn:
                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;

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index abbc3f9..e9f7961 100644 (file)
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -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)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 87b3c68..1cceca1 100644 (file)
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -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

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f69854c..6624ad1 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3404,6 +3404,86 @@ emul_write:
        return emulator_write_emulated(addr, new, bytes, vcpu);
 }
 
+static int kernel_pio(struct kvm_vcpu *vcpu, void *pd)
+{
+       /* TODO: String I/O for in kernel device */
+       int r;
+
+       if (vcpu->arch.pio.in)
+               r = kvm_io_bus_read(vcpu->kvm, KVM_PIO_BUS, vcpu->arch.pio.port,
+                                   vcpu->arch.pio.size, pd);
+       else
+               r = kvm_io_bus_write(vcpu->kvm, KVM_PIO_BUS,
+                                    vcpu->arch.pio.port, vcpu->arch.pio.size,
+                                    pd);
+       return r;
+}
+
+
+static int emulator_pio_in_emulated(int size, unsigned short port, void *val,
+                            unsigned int count, struct kvm_vcpu *vcpu)
+{
+       if (vcpu->arch.pio.cur_count)
+               goto data_avail;
+
+       trace_kvm_pio(1, port, size, 1);
+
+       vcpu->arch.pio.port = port;
+       vcpu->arch.pio.in = 1;
+       vcpu->arch.pio.string = 0;
+       vcpu->arch.pio.down = 0;
+       vcpu->arch.pio.rep = 0;
+       vcpu->arch.pio.count = vcpu->arch.pio.cur_count = count;
+       vcpu->arch.pio.size = size;
+
+       if (!kernel_pio(vcpu, vcpu->arch.pio_data)) {
+       data_avail:
+               memcpy(val, vcpu->arch.pio_data, size * count);
+               vcpu->arch.pio.cur_count = 0;
+               return 1;
+       }
+
+       vcpu->run->exit_reason = KVM_EXIT_IO;
+       vcpu->run->io.direction = KVM_EXIT_IO_IN;
+       vcpu->run->io.size = size;
+       vcpu->run->io.data_offset = KVM_PIO_PAGE_OFFSET * PAGE_SIZE;
+       vcpu->run->io.count = count;
+       vcpu->run->io.port = port;
+
+       return 0;
+}
+
+static int emulator_pio_out_emulated(int size, unsigned short port,
+                             const void *val, unsigned int count,
+                             struct kvm_vcpu *vcpu)
+{
+       trace_kvm_pio(0, port, size, 1);
+
+       vcpu->arch.pio.port = port;
+       vcpu->arch.pio.in = 0;
+       vcpu->arch.pio.string = 0;
+       vcpu->arch.pio.down = 0;
+       vcpu->arch.pio.rep = 0;
+       vcpu->arch.pio.count = vcpu->arch.pio.cur_count = count;
+       vcpu->arch.pio.size = size;
+
+       memcpy(vcpu->arch.pio_data, val, size * count);
+
+       if (!kernel_pio(vcpu, vcpu->arch.pio_data)) {
+               vcpu->arch.pio.cur_count = 0;
+               return 1;
+       }
+
+       vcpu->run->exit_reason = KVM_EXIT_IO;
+       vcpu->run->io.direction = KVM_EXIT_IO_OUT;
+       vcpu->run->io.size = size;
+       vcpu->run->io.data_offset = KVM_PIO_PAGE_OFFSET * PAGE_SIZE;
+       vcpu->run->io.count = count;
+       vcpu->run->io.port = port;
+
+       return 0;
+}
+
 static unsigned long get_segment_base(struct kvm_vcpu *vcpu, int seg)
 {
        return kvm_x86_ops->get_segment_base(vcpu, seg);
@@ -3597,6 +3677,8 @@ static struct x86_emulate_ops emulate_ops = {
        .read_emulated       = emulator_read_emulated,
        .write_emulated      = emulator_write_emulated,
        .cmpxchg_emulated    = emulator_cmpxchg_emulated,
+       .pio_in_emulated     = emulator_pio_in_emulated,
+       .pio_out_emulated    = emulator_pio_out_emulated,
        .get_cached_descriptor = emulator_get_cached_descriptor,
        .set_cached_descriptor = emulator_set_cached_descriptor,
        .get_segment_selector = emulator_get_segment_selector,
@@ -3704,6 +3786,12 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
        if (vcpu->arch.pio.string)
                return EMULATE_DO_MMIO;
 
+       if (vcpu->arch.pio.cur_count && !vcpu->arch.pio.string) {
+               if (!vcpu->arch.pio.in)
+                       vcpu->arch.pio.cur_count = 0;
+               return EMULATE_DO_MMIO;
+       }
+
        if (r || vcpu->mmio_is_write) {
                run->exit_reason = KVM_EXIT_MMIO;
                run->mmio.phys_addr = vcpu->mmio_phys_addr;
@@ -3760,43 +3848,36 @@ int complete_pio(struct kvm_vcpu *vcpu)
        int r;
        unsigned long val;
 
-       if (!io->string) {
-               if (io->in) {
-                       val = kvm_register_read(vcpu, VCPU_REGS_RAX);
-                       memcpy(&val, vcpu->arch.pio_data, io->size);
-                       kvm_register_write(vcpu, VCPU_REGS_RAX, val);
-               }
-       } else {
-               if (io->in) {
-                       r = pio_copy_data(vcpu);
-                       if (r)
-                               goto out;
-               }
+       if (io->in) {
+               r = pio_copy_data(vcpu);
+               if (r)
+                       goto out;
+       }
 
-               delta = 1;
-               if (io->rep) {
-                       delta *= io->cur_count;
-                       /*
-                        * The size of the register should really depend on
-                        * current address size.
-                        */
-                       val = kvm_register_read(vcpu, VCPU_REGS_RCX);
-                       val -= delta;
-                       kvm_register_write(vcpu, VCPU_REGS_RCX, val);
-               }
-               if (io->down)
-                       delta = -delta;
-               delta *= io->size;
-               if (io->in) {
-                       val = kvm_register_read(vcpu, VCPU_REGS_RDI);
-                       val += delta;
-                       kvm_register_write(vcpu, VCPU_REGS_RDI, val);
-               } else {
-                       val = kvm_register_read(vcpu, VCPU_REGS_RSI);
-                       val += delta;
-                       kvm_register_write(vcpu, VCPU_REGS_RSI, val);
-               }
+       delta = 1;
+       if (io->rep) {
+               delta *= io->cur_count;
+               /*
+                * The size of the register should really depend on
+                * current address size.
+                */
+               val = kvm_register_read(vcpu, VCPU_REGS_RCX);
+               val -= delta;
+               kvm_register_write(vcpu, VCPU_REGS_RCX, val);
+       }
+       if (io->down)
+               delta = -delta;
+       delta *= io->size;
+       if (io->in) {
+               val = kvm_register_read(vcpu, VCPU_REGS_RDI);
+               val += delta;
+               kvm_register_write(vcpu, VCPU_REGS_RDI, val);
+       } else {
+               val = kvm_register_read(vcpu, VCPU_REGS_RSI);
+               val += delta;
+               kvm_register_write(vcpu, VCPU_REGS_RSI, val);
        }
+
 out:
        io->count -= io->cur_count;
        io->cur_count = 0;
@@ -3804,21 +3885,6 @@ out:
        return 0;
 }
 
-static int kernel_pio(struct kvm_vcpu *vcpu, void *pd)
-{
-       /* TODO: String I/O for in kernel device */
-       int r;
-
-       if (vcpu->arch.pio.in)
-               r = kvm_io_bus_read(vcpu->kvm, KVM_PIO_BUS, vcpu->arch.pio.port,
-                                   vcpu->arch.pio.size, pd);
-       else
-               r = kvm_io_bus_write(vcpu->kvm, KVM_PIO_BUS,
-                                    vcpu->arch.pio.port, vcpu->arch.pio.size,
-                                    pd);
-       return r;
-}
-
 static int pio_string_write(struct kvm_vcpu *vcpu)
 {
        struct kvm_pio_request *io = &vcpu->arch.pio;
@@ -3836,36 +3902,6 @@ static int pio_string_write(struct kvm_vcpu *vcpu)
        return r;
 }
 
-int kvm_emulate_pio(struct kvm_vcpu *vcpu, int in, int size, unsigned port)
-{
-       unsigned long val;
-
-       trace_kvm_pio(!in, port, size, 1);
-
-       vcpu->run->exit_reason = KVM_EXIT_IO;
-       vcpu->run->io.direction = in ? KVM_EXIT_IO_IN : KVM_EXIT_IO_OUT;
-       vcpu->run->io.size = vcpu->arch.pio.size = size;
-       vcpu->run->io.data_offset = KVM_PIO_PAGE_OFFSET * PAGE_SIZE;
-       vcpu->run->io.count = vcpu->arch.pio.count = vcpu->arch.pio.cur_count = 1;
-       vcpu->run->io.port = vcpu->arch.pio.port = port;
-       vcpu->arch.pio.in = in;
-       vcpu->arch.pio.string = 0;
-       vcpu->arch.pio.down = 0;
-       vcpu->arch.pio.rep = 0;
-
-       if (!vcpu->arch.pio.in) {
-               val = kvm_register_read(vcpu, VCPU_REGS_RAX);
-               memcpy(vcpu->arch.pio_data, &val, 4);
-       }
-
-       if (!kernel_pio(vcpu, vcpu->arch.pio_data)) {
-               complete_pio(vcpu);
-               return 1;
-       }
-       return 0;
-}
-EXPORT_SYMBOL_GPL(kvm_emulate_pio);
-
 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)
@@ -3931,6 +3967,16 @@ int kvm_emulate_pio_string(struct kvm_vcpu *vcpu, int in,
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_pio_string);
 
+int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port)
+{
+       unsigned long val = kvm_register_read(vcpu, VCPU_REGS_RAX);
+       int ret = emulator_pio_out_emulated(size, port, &val, 1, vcpu);
+       /* do not return to emulator after return from userspace */
+       vcpu->arch.pio.cur_count = 0;
+       return ret;
+}
+EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
+
 static void bounce_off(void *info)
 {
        /* nothing */
@@ -4661,9 +4707,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 
        if (vcpu->arch.pio.cur_count) {
                vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
-               r = complete_pio(vcpu);
+               if (!vcpu->arch.pio.string)
+                       r = emulate_instruction(vcpu, 0, 0, EMULTYPE_NO_DECODE);
+               else
+                       r = complete_pio(vcpu);
                srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
-               if (r)
+               if (r == EMULATE_DO_MMIO)
                        goto out;
        }
        if (vcpu->mmio_needed) {