Re: [PATCH] kvm: x86: fix a race condition result to lost INIT

From: Wanpeng Li
Date: Sun Jul 30 2017 - 07:54:23 EST


2017-07-30 19:33 GMT+08:00 <peng.hao2@xxxxxxxxxx>:
>
>
>
>
>>2017-07-30 18:42 GMT+08:00 <peng.hao2@xxxxxxxxxx>:
>
>>>> 2017-07-31 0:24 GMT+08:00 Peng Hao <peng.hao2@xxxxxxxxxx>:
>>>
>>>> > when SMP VM start, AP may lost INIT because of receiving INIT between
>>>> > kvm_vcpu_ioctl_x86_get/set_vcpu_events.
>>>> >
>>>> > vcpu 0 vcpu 1
>>>> > kvm_vcpu_ioctl_x86_get_vcpu_events
>>>> > events->smi.latched_init=0
>>>> > send INIT to vcpu1
>>>> > set vcpu1's pending_events
>>>> > kvm_vcpu_ioctl_x86_set_vcpu_events
>>>> > events->smi.latched_init == 0
>>>> > clear INIT in pending_events
>>>> > I don't think it need set/clear kernel state according to userspace's
>>>> > info.
>>>> >
>>>> > Signed-off-by: Peng Hao <peng.hao2@xxxxxxxxxx>
>>>> > ---
>>>> > arch/x86/kvm/x86.c | 6 ------
>>>> > 1 file changed, 6 deletions(-)
>>>> >
>>>> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>>> > index 6c7266f..393a7b7 100644
>>>> > --- a/arch/x86/kvm/x86.c
>>>> > +++ b/arch/x86/kvm/x86.c
>>>> > @@ -3157,12 +3157,6 @@ static int
>>>> > kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
>>>> > vcpu->arch.hflags |= HF_SMM_INSIDE_NMI_MASK;
>>>> > else
>>>> > vcpu->arch.hflags &= ~HF_SMM_INSIDE_NMI_MASK;
>>>> > - if (lapic_in_kernel(vcpu)) {
>>>> > - if (events->smi.latched_init)
>>>> > - set_bit(KVM_APIC_INIT,
>>>> > &vcpu->arch.apic->pending_events);
>>>
>>>> This is not correct, you will lose the INIT after live migration. I
>>>> just send out another patch to fix it. Thanks for the report.
>>>
>>> yes, you're right. I should modify on qemu not kernel.
>
>>There is a patch here to fix it in kvm, https://lkml.org/lkml/2017/7/30/72
>
>>>
>
> I was affected by sipi, kernel never report valid sipi to user space,

How you afftected by SIPI? In addition, you reported the lost of INIT.
Is it observed by your workload or by code review?

Regards,
Wanpeng Li

>
> we don't care whether sipi is lost after migration. Why?
>
>>>> > - else
>>>> > - clear_bit(KVM_APIC_INIT,
>>>> > &vcpu->arch.apic->pending_events);
>>>> > - }
>>>> > }
>
>
>