[BUG/RFC] INIT IPI lost when VM starts

Commit Message

Herongguang (Stephen) March 21, 2017, 3:34 a.m. UTC

Let me clarify it more clearly. Time sequence is that qemu handles ‘query-cpus’ qmp command, vcpu 1 (and vcpu 0) got registers from kvm-kmod (qmp_query_cpus-> cpu_synchronize_state-> kvm_cpu_synchronize_state->
 > do_kvm_cpu_synchronize_state-> kvm_arch_get_registers), then vcpu 0 (BSP) sends INIT-SIPI to vcpu 1(AP). In kvm-kmod, vcpu 1’s pending_events’s KVM_APIC_INIT bit set.
Then vcpu 1 continue running, vcpu1 thread in qemu calls kvm_arch_put_registers-> kvm_put_vcpu_events, so KVM_APIC_INIT bit in vcpu 1’s pending_events got cleared, i.e., lost.

In kvm-kmod, except for pending_events, sipi_vector may also be overwritten., so I am not sure if there are other fields/registers in danger, i.e., those may be modified asynchronously with vcpu thread itself.

BTW, using a sleep like following can reliably reproduce this problem, if VM equipped with more than 2 vcpus and starting VM using libvirtd.


On 2017/3/20 22:21, Herongguang (Stephen) wrote:
> Hi,
> 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?
>

Patch

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 55865db..5099290 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2534,6 +2534,11 @@  static int kvm_put_vcpu_events(X86CPU *cpu, int level)
              KVM_VCPUEVENT_VALID_NMI_PENDING | KVM_VCPUEVENT_VALID_SIPI_VECTOR;
      }

+    if (CPU(cpu)->cpu_index == 1) {
+        fprintf(stderr, "vcpu 1 sleep!!!!\n");
+        sleep(10);
+    }
+
      return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, &events);
  }