From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Herongguang (Stephen)" Subject: Re: [BUG/RFC] INIT IPI lost when VM starts Date: Thu, 6 Apr 2017 09:47:21 +0800 Message-ID: <58E59E29.7070305@huawei.com> References: <58CFE56E.9090303@huawei.com> <135b6663-b65c-b5ba-d3a8-b2a2127a07fa@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit To: Paolo Bonzini , , , , , "kvm@vger.kernel.org" , , "weidong.huang@huawei.com >> Huangweidong (C)" Return-path: Received: from szxga01-in.huawei.com ([45.249.212.187]:5305 "EHLO dggrg01-dlp.huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1756623AbdDFBsU (ORCPT ); Wed, 5 Apr 2017 21:48:20 -0400 In-Reply-To: <135b6663-b65c-b5ba-d3a8-b2a2127a07fa@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 > > . >