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

[Erik van der Kouwe <vdkouwe@cs.vu.nl>]

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