On 18/02/2016 16:43, Radim Krčmář wrote: > 2016-02-18 15:51+0100, Paolo Bonzini: >> On 18/02/2016 15:18, Radim Krčmář wrote: >>> KVM just has to make sure that targeted VCPUs notice the interrupt, >>> which means to kick (wake up) VCPUs that don't have IsRunning set. >>> There is no need to do anything with running VCPUs, because they >>> - are in guest mode and noticed the doorbell >>> - are in host mode, where they will >>> 1) VMRUN as fast as they can because the VCPU didn't want to halt >>> (and IRR is handled on VMRUN) >>> 2) check IRR after unsetting IsRunning and goto (1) if there are >>> pending interrupts. (RFC doesn't do this, which is another bug) >> >> This is not necessary. IsRunning is only cleared at vcpu_put time. > > It's not necessary if we are being preempted, but it is necessary to > clear IsRunning earlier when we are going to block (i.e. after a halt). > >> The >> next KVM_RUN will look at IRR (kvm_arch_vcpu_runnable), if necessary set >> the mp_state to KVM_MP_STATE_RUNNABLE, and do the VMRUN. > > We're not always going to exit to userspace. I think the following > order of actions could result in a stuck VM: > > (The main idea is that VCPU targets another VCPU between its last check > for IRR and clearing of IsRunning.) > > 1) vcpu0 has set IsRunning and is in guest mode. > 2) vcpu0 executes HLT and exits guest mode. > 3) vcpu0 doesn't have any pending interrupts or other stuff that would > make kvm_arch_vcpu_runnable() return true. > 4) vcpu0 runs kvm_vcpu_block() and gets to call schedule(). > 5) *vcpu1* sends IPI to vcpu0. IsRunning is set on vcpu0, so AVIC > doesn't exit. A doorbell is sent, but it does nothing. > 6) vcpu0 runs schedule() and clears IsRunning in a callback. You're completely right. When the VCPU is halting, the preempt notifier's sched_out callback is too late to clear IsRunning; you need to do that before the last time you check for IRR. Patch 9 is okay, but it is also necessary to clear IsRunning in kvm_arch_vcpu_blocking and set it in kvm_arch_vcpu_unblocking. In addition, vcpu_put/vcpu_load should not modify IsRunning between kvm_arch_vcpu_blocking and kvm_arch_vcpu_unblocking. Do you agree? Paolo -- 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
