From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v2 25/30] KVM: x86 emulator: fix in/out emulation. Date: Sun, 14 Mar 2010 18:54:11 +0200 Message-ID: <4B9D14B3.2020002@redhat.com> References: <1268583675-3101-1-git-send-email-gleb@redhat.com> <1268583675-3101-26-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:14237 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759508Ab0CNQyM (ORCPT ); Sun, 14 Mar 2010 12:54:12 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2EGsCOk013288 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 14 Mar 2010 12:54:12 -0400 Received: from cleopatra.tlv.redhat.com (cleopatra.tlv.redhat.com [10.35.255.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o2EGsBdT030837 for ; Sun, 14 Mar 2010 12:54:12 -0400 In-Reply-To: <1268583675-3101-26-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 03/14/2010 06:21 PM, Gleb Natapov wrote: > 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. > > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 8f5e4c8..344e17b 100644 > --- 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, > REX prefix shouldn't expand DstAcc to 64 bits here. Might cause problems further down in the pipeline.+ > 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); > whitespace damage. > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 3c5ffa2..37c6178 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3351,6 +3351,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 */ > ISTR this is actually needed for something. Is this a regression? -- error compiling committee.c: too many arguments to function