From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: buggy emulate_int_real Date: Tue, 12 Apr 2011 13:31:59 -0500 Message-ID: <20110412183158.GA29423@hallyn.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , KVM mailing list To: Jan Kiszka Return-path: Received: from hrndva-omtalb.mail.rr.com ([71.74.56.123]:56161 "EHLO hrndva-omtalb.mail.rr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758317Ab1DLScG (ORCPT ); Tue, 12 Apr 2011 14:32:06 -0400 Content-Disposition: inline In-Reply-To: <4DA472E5.9060807@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: 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/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(). > >> > >> 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 what? As to whether we are supposed to increment eip or not. thanks, -serge