On 08/31/2009 04:39 PM, Mohammed Gamal wrote:
local_irq_enable();
preempt_enable();
These are now wrong, since handle_invalid_exit() is called with interrupts
and preemption enabled.
Do you mean vmx_handle_exit() ?
Yes.
/*
@@ -3405,9 +3412,12 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
/* If we need to emulate an MMIO from handle_invalid_guest_state
* we just return 0 */
if (vmx->emulation_required&& emulate_invalid_guest_state) {
- if (guest_state_valid(vcpu))
+ if (guest_state_valid(vcpu)) {
vmx->emulation_required = 0;
- return vmx->invalid_state_emulation_result !=
EMULATE_DO_MMIO;
+ return vmx->invalid_state_emulation_result !=
EMULATE_DO_MMIO;
This looks fishy. Can't say exactly why but vmx_handle_exit() should only
depend on the current guest execution, not the previous guest execution.
I still can't quite get the problem here!
The design of the main loop is that you can have a save/restore cycle
(or live migration) after __vcpu_run(). So abything in __vcpu_run() and
the function it calls can only depend on state set previously in
__vcpu_run(), or on state that is loaded from KVM_SET_REGS and similar
ioctls.
In this case I think vmx->invalid_state_emulation_result is not set
previously to its use within __vcpu_run().
/* Access CR3 don't cause VMExit in paging mode, so we need
@@ -3603,12 +3613,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
if (unlikely(!cpu_has_virtual_nmis()&& vmx->soft_vnmi_blocked))
vmx->entry_time = ktime_get();
- /* Handle invalid guest state instead of entering VMX */
- if (vmx->emulation_required&& emulate_invalid_guest_state) {
- handle_invalid_guest_state(vcpu);
- return;
- }
-
Don't we still need to return here? Otherwise we attempt guest entry
needlessly.
But how would a vmexit be triggered (thus calling vmx_handle_exit() )?
vmx_handle_exit() will be called in any case.
I personally prefer if we can start emulation before attempting guest
entry, but how can we tell vmx_vcpu_run() to return to userspace if it
doesn't return a value, I don't feel that changing it to return a
value would be a wise thing to do too, no?
vmx_vcpu_run() can simply return, setting an internal vmx flag. Then
vmx_handle_exit() can notice the flag, emulate, and handle the result of
this emulation.
--
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
