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

["Herongguang (Stephen)" <herongguang.he@huawei.com>]

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/qemu-devel@nongnu.org/msg438675.html

> Thanks,
>
> paolo
>
> .
>