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

[Herongguang (Stephen)]

On 2017/4/6 0:16, Paolo Bonzini wrote:
> On 20/03/2017 15:21, Herongguang (Stephen) wrote:
> > We encountered a problem that when a domain starts, seabios failed to
> > online a vCPU.
> >
> > After investigation, we found that the reason is in kvm-kmod,
> > KVM_APIC_INIT bit in
> > vcpu->arch.apic->pending_events was overwritten by qemu, and thus an
> > INIT IPI sent
> > to AP was lost. Qemu does this since libvirtd sends a ‘query-cpus’ qmp
> > command to qemu
> > on VM start.
> >
> > In qemu, qmp_query_cpus-> cpu_synchronize_state->
> > kvm_cpu_synchronize_state->
> > do_kvm_cpu_synchronize_state, qemu gets registers/vcpu_events from
> > kvm-kmod and
> > sets cpu->kvm_vcpu_dirty to true, and vcpu thread in qemu will call
> > kvm_arch_put_registers if cpu->kvm_vcpu_dirty is true, thus
> > pending_events is
> > overwritten by qemu.
> >
> > I think there is no need for qemu to set cpu->kvm_vcpu_dirty to true
> > after ‘query-cpus’,
> > and  kvm-kmod should not clear KVM_APIC_INIT unconditionally. And I am
> > not sure whether
> > it is OK for qemu to set cpu->kvm_vcpu_dirty in
> > do_kvm_cpu_synchronize_state in each caller.
> >
> > What’s your opinion?
> 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.

also see: https://www.mail-archive.com/address@hidden/msg438675.html

> Thanks,
>
> paolo
>
> .