From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: buggy emulate_int_real Date: Tue, 12 Apr 2011 11:02:00 +0300 Message-ID: <4DA406F8.4090701@redhat.com> References: <20110408210900.GA26787@hallyn.com> <4DA16AA1.7010108@redhat.com> <20110412075319.GA28696@hallyn.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: KVM mailing list To: "Serge E. Hallyn" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26256 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752503Ab1DLICM (ORCPT ); Tue, 12 Apr 2011 04:02:12 -0400 In-Reply-To: <20110412075319.GA28696@hallyn.com> Sender: kvm-owner@vger.kernel.org List-ID: 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/747090, 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 = c->eip; > > emulate_push(ctxt, ops); > > rc = writeback(ctxt, ops); > > if (rc != 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 = true; > - vmx->rmode.irq.vector = nr; > - vmx->rmode.irq.rip = kvm_rip_read(vcpu); > - if (kvm_exception_is_soft(nr)) > - vmx->rmode.irq.rip += > - vmx->vcpu.arch.event_exit_inst_len; > - intr_info |= 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) != EMULATE_DONE) > + kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu); > 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_interrupts) kvm handles the exception reinject the interrupt while reentering the guest so we do need something like if (soft) vcpu->arch.emulate_ctxt.eip += inst_len; in kvm_inject_realmode_interrupt(). -- error compiling committee.c: too many arguments to function