lkml.org 
[lkml]   [2016]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC
From
Date


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

\
 
 \ /
  Last update: 2016-02-18 17:21    [W:0.121 / U:1.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site