Re: [Qemu-devel] [BUG/RFC] INIT IPI lost when VM starts

[address@hidden]

2017-11-20 06:57+0000, Gonglei (Arei):
> Hi Paolo,
> 
> What's your opinion about this patch? We found it just before finishing 
> patches 
> for the past two days.

I think your case was fixed by f4ef19108608 ("KVM: X86: Fix loss of
pending INIT due to race"), but that patch didn't fix it perfectly, so
maybe you're hitting a similar case that happens in SMM ...

> > -----Original Message-----
> > From: address@hidden [mailto:address@hidden On
> > Behalf Of Herongguang (Stephen)
> > On 2017/4/6 0:16, Paolo Bonzini wrote:
> > > Hi Rongguang,
> > >
> > > sorry for the late response.
> > >
> > > Where exactly is KVM_APIC_INIT dropped?  kvm_get_mp_state does clear
> > the
> > > bit, but the result of the INIT is stored in mp_state.
> > 
> > It's dropped in KVM_SET_VCPU_EVENTS, see below.
> > 
> > >
> > > kvm_get_vcpu_events is called after kvm_get_mp_state; it retrieves
> > > KVM_APIC_INIT in events.smi.latched_init and kvm_set_vcpu_events passes
> > > it back.  Maybe it should ignore events.smi.latched_init if not in SMM,
> > > but I would like to understand the exact sequence of events.
> > 
> > time0:
> > vcpu1:
> > qmp_query_cpus-> cpu_synchronize_state-> kvm_cpu_synchronize_state->
> >  > do_kvm_cpu_synchronize_state(and set vcpu1's cpu->kvm_vcpu_dirty to
> > true)-> kvm_arch_get_registers(KVM_APIC_INIT bit in
> > vcpu->arch.apic->pending_events was not set)
> > 
> > time1:
> > vcpu0:
> > send INIT-SIPI to all AP->(in vcpu 0's
> > context)__apic_accept_irq(KVM_APIC_INIT bit in vcpu1's
> > arch.apic->pending_events is set)
> > 
> > time2:
> > vcpu1:
> > kvm_cpu_exec->(if cpu->kvm_vcpu_dirty is
> > true)kvm_arch_put_registers->kvm_put_vcpu_events(overwritten
> > KVM_APIC_INIT bit in vcpu->arch.apic->pending_events!)
> > 
> > So it's a race between vcpu1 get/put registers with kvm/other vcpus changing
> > vcpu1's status/structure fields in the mean time, I am in worry of if there 
> > are
> > other fields may be overwritten,
> > sipi_vector is one.

Fields that can be asynchronously written by other VCPUs (like SIPI,
NMI) must not be SET if other VCPUs were not paused since the last GET.
(Looking at the interface, we can currently lose pending SMI.)

INIT is one of the restricted fields, but the API unconditionally
couples SMM with latched INIT, which means that we can lose an INIT if
the VCPU is in SMM mode -- do you see SMM in kvm_vcpu_events?

Thanks.