On 25/10/2017 09:26, Zhuangyanying wrote:
> From: Xiangyou Xie <xiexiangyou@xxxxxxxxxx>
>
> When the guest is using the pv-spinlock, and there is a race of vcpu1 decides to HALT and vcpu2 releasing the lock, the vcpu2 might fail to wake up vcpu1.
> In the case of PV spinlock, the vcpu2 executes the following to wake up vcpu:
> case APIC_DM_REMRD:
> result = 1;
> vcpu->arch.pv.pv_unhalted = 1;
> kvm_make_request(KVM_REQ_EVENT, vcpu);
> kvm_vcpu_kick(vcpu);
> break;
> Due to lack of smp_mb before kvm_vcpu_kick, the changes of pv_unhalted
> and vcpu->requests may be delayed to be after the operation of kvm_vcpu_kick.
Since Linux 4.7 (commit 2e4682ba2ed7, "KVM: add missing memory barrier
in kvm_{make,check}_request", 2016-04-20), kvm_make_request has a smp_wmb().
However, even before that commit "set_bit" implicitly has a barrier on
x86, so there is no effect from the patch below.
Paolo
> On the other part, if the vcpu1 is just to execute the prepare_to_wait, vcpu2's kvm_vcpu_kick will have no effect, but vcpu1 fails to pass the kvm_vcpu_check_block() and as a result get blocked.
> for (;;) {
> //------->when vcpu2 do the kvm_vcpu_kick, vcpu1 has not reached here
> prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
>
> if (kvm_vcpu_check_block(vcpu) < 0)
> break;
>
> waited = true;
> schedule();
> }
>
> Adding an smp_mb() before vcpu2 doing the kvm_vcpu_kick works.
>
> Signed-off-by: Xiangyou Xie<xiexiangyou@xxxxxxxxxx>
> Signed-off-by: Liuxiaojian <liuxiaojian6@xxxxxxxxxx>
> ---
> arch/x86/kvm/lapic.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 69c5612..d37c0fd 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -987,6 +987,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> result = 1;
> vcpu->arch.pv.pv_unhalted = 1;
> kvm_make_request(KVM_REQ_EVENT, vcpu);
> + smp_mb();
> kvm_vcpu_kick(vcpu);
> break;
>
>
