Re: buggy emulate_int_real

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/12/2011 04:57 PM, Serge E. Hallyn wrote:
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?

Possibly.

For instance vmx_inject_nmi() didn't do it at all,
and the other two callers only did it for a soft interrupt.

NMIs are always hard.

Perhaps have a new helper for this.

--
error compiling committee.c: too many arguments to function

--
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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux