[Qemu-devel] Re: SVM emulation: EVENTINJ marked valid when a pagefault happens while issuing a software interrupt

[Erik van der Kouwe]

Hi,

> Be warned: Though my experience is already more than a year old, the SVM
> emulation in QEMU is most probably not yet rock-stable. Always check
> suspicious behavior against real hardware and/or the spec. [ As real
> hardware is everywhere, nesting works with KVM+SVM and is much faster,
> motivation to improve QEMU in this area is unfortunately limited. ]

Problem is: I'm compiling in Linux and testing in MINIX. Testing on the 
real hardware would require a reboot everytime. Moreover, it might screw 
up my system if I make bad mistakes (the MINIX filesystem is easily 
corrupted).

That said, I do aim to eventually test the real hardware. Plenty of 
virtualization capable hardware where I work, although unfortunately all 
Intel.

> > This issue is easy to work around by clearing the EVENTINJ field on each
> > #VMEXIT (and I have submitted a patch to that effect to the Palacios
> > people) and this approach is also found in KVM.
>
> /me does not find such clearing in KVM - what line(s) are you looking at?

Linux source tree (2.6.31-ubuntu), arch/x86/kvm/svm.c, end of function 
nested_svm_vmrun. Here event_inj and event_inj_err are copied from a 
different VMCB, effectively clearing the value set by the CPU. Maybe 
this isn't were I should have been looking though?

> > The relevant code is in target-i386/op_helper.c. The "handle_even_inj"
> > function sets the EVENTINJ field (called event_inf in the QEMU code) and
> > the helper_vmexit function copies that field into EXITINTINFO
> > (exit_int_info in the QEMU code). I believe (but once again, am not
> > certain) that the SVM documentation only says that this information
> > should be stored in EXITINTINFO.
>
> Yes, this also looks suspicious. handle_even_inj should not push the
> real (level 1) event to be injected into event_inj[_err] but into
> exit_int_info[_err] or some temporary fields from which the exit info is
> then loaded later on.

Yes, if this is indeed incorrect behaviour then this is what I would 
expect a fix to be like.

Thanks again,
Erik