From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: buggy emulate_int_real Date: Tue, 12 Apr 2011 22:51:12 +0200 Message-ID: <4DA4BB40.2010201@web.de> References: <20110408210900.GA26787@hallyn.com> <4DA16AA1.7010108@redhat.com> <20110412075319.GA28696@hallyn.com> <4DA406F8.4090701@redhat.com> <20110412141226.GA6766@hallyn.com> <4DA45ED0.90601@redhat.com> <4DA472E5.9060807@siemens.com> <20110412183158.GA29423@hallyn.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigA8CA6FC8C24111448A625939" Cc: Avi Kivity , KVM mailing list To: "Serge E. Hallyn" Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:52834 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932362Ab1DLUvS (ORCPT ); Tue, 12 Apr 2011 16:51:18 -0400 In-Reply-To: <20110412183158.GA29423@hallyn.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigA8CA6FC8C24111448A625939 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2011-04-12 20:31, Serge E. Hallyn wrote: > Quoting Jan Kiszka (jan.kiszka@siemens.com): >> On 2011-04-12 16:16, Avi Kivity wrote: >>> On 04/12/2011 05:12 PM, Serge E. Hallyn wrote: >>>> Quoting Avi Kivity (avi@redhat.com): >>>>> On 04/12/2011 10:53 AM, Serge E. Hallyn wrote: >>>>> >Quoting Avi Kivity (avi@redhat.com): >>>>> >> On 04/09/2011 12:09 AM, Serge E. Hallyn wrote: >>>>> >> >Hi, >>>>> >> > >>>>> >> >at https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/7= 47090, it was >>>>> >> >found that emulate_int_real() sometimes pushes the wrong eip= when doing a >>>>> >> >int. Whereas with non-kvm qemu we push the next instruction= after the >>>>> >> >int, with kvm we push the addr of the instruction itself. >>>>> >> > >>>>> >> >>>>> >> >>>>> >> The code says: >>>>> >> >>>>> >> c->src.val =3D c->eip; >>>>> >> emulate_push(ctxt, ops); >>>>> >> rc =3D writeback(ctxt, ops); >>>>> >> if (rc !=3D X86EMUL_CONTINUE) >>>>> >> return rc; >>>>> >> >>>>> >> which appears to be the address of the next instruction from = my >>>>> >> reading of the code (see how insn_fetch() increments c->eip).= >>>>> > >>>>> >Nevertheless removing commits >>>>> > >>>>> > a92601bb707f6f49fd5563ef3d09928e70cc222e >>>>> > 63995653ade16deacaea5b49ceaf6376314593ac >>>>> > 6e154e56b4d7a6a28c54f0984e13d3f8defc4755 >>>>> > >>>>> >changes the eip value being pushed. If you look at >>>>> >a92601bb707f6f49fd5563ef3d09928e70cc222e, you see: >>>>> > >>>>> > if (vmx->rmode.vm86_active) { >>>>> >- vmx->rmode.irq.pending =3D true; >>>>> >- vmx->rmode.irq.vector =3D nr; >>>>> >- vmx->rmode.irq.rip =3D kvm_rip_read(vcpu); >>>>> >- if (kvm_exception_is_soft(nr)) >>>>> >- vmx->rmode.irq.rip +=3D >>>>> >- vmx->vcpu.arch.event_exit_inst_le= n; >>>>> >- intr_info |=3D INTR_TYPE_SOFT_INTR; >>>>> >- vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr_info)= ; >>>>> >- vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 1); >>>>> >- kvm_rip_write(vcpu, vmx->rmode.irq.rip - 1); >>>>> >+ if (kvm_inject_realmode_interrupt(vcpu, nr) !=3D = EMULATE_DONE) >>>>> >+ kvm_make_request(KVM_REQ_TRIPLE_FAULT, vc= pu); >>>>> > return; >>>>> > } >>>>> > >>>>> >but kvm_inject_realmode_interrupt() does not appear to increment >>>>> >vmx->rmode.irq.rip anywhere, as the code being replaced does. >>>>> >>>>> Ah, I see now. There are two cases, hard interrupt and soft >>>>> interrupts. I guess hard interrupts are handled fine, and the >>>>> failing case is >>>>> >>>>> guest executes INTn instruction in guest mode >>>>> vmx intercepts a page fault (say due to access to the IDT or the= stack) >>>>> kvm notes that a soft interrupt was in progress (vmx_complete_in= terrupts) >>>>> kvm handles the exception >>>>> reinject the interrupt while reentering the guest >>>>> >>>>> so we do need something like >>>>> >>>>> if (soft) >>>>> vcpu->arch.emulate_ctxt.eip +=3D inst_len; >>>>> >>>>> in kvm_inject_realmode_interrupt(). >>>> >>>> Oops, right. Disregard last email pls :) >>>> >>>> So is 'kvm_exception_is_soft(irq)' a reliable check? >>>> >>> >>> No, need to check vcpu->arch.interrupt.soft instead. Not sure about = >>> kvm_exception_is_soft(). Jan? >> >> Jumping late on this, I don't understand the question. Reliable /wrt w= hat? >=20 > As to whether we are supposed to increment eip or not. =46rom a brief refresh-reading, I would say if (interrupt.soft || kvm_exception_is_soft(nr)) increment_eip_by_inst_len but only for those interrupts/exceptions which were raised by the triggering instructions, _not_ for exceptions raise while processing them (e.g. a page fault while accessing the IDT). Jan --------------enigA8CA6FC8C24111448A625939 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk2ku0MACgkQitSsb3rl5xRZtACfY/NFcCN2hUsRk33fJVJxDsNw KPYAn32L0901VxEDLcKjteZFtJx8t0JO =eK/e -----END PGP SIGNATURE----- --------------enigA8CA6FC8C24111448A625939--