Quoting Avi Kivity (avi@xxxxxxxxxx): > On 04/12/2011 10:53 AM, Serge E. Hallyn wrote: > >Quoting Avi Kivity (avi@xxxxxxxxxx): > >> 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(). But not always, so perhaps it would be better to still do this at the callers? For instance vmx_inject_nmi() didn't do it at all, and the other two callers only did it for a soft interrupt. -serge -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
