Re: [Qemu-devel] [PATCH] pc: apic: fix touch LAPIC when irqchip is split

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-09-14 11:04+0800, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Add -kernel_irqchip=split
> ./x86-run x86/eventinj.flat
> 
> qemu-system-x86_64 -enable-kvm -machine kernel_irqchip=split -cpu host
> -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc
> none -serial stdio -device pci-testdev -kernel x86/eventinj.flat
> enabling apic
> paging enabled
> cr0 = 80010011
> cr3 = 7fff000
> cr4 = 20
> Sending vec 33 and 62 and mask one with TPR
> irq1 running
> irq1 running
> After 33/62 TPR test
> FAIL: TPR
> irq0 running
> irq0 running
> 
> Both irq1 and irq0 are executing twice.
> 
> kvm_entry: vcpu 0
> kvm_exit: reason MSR_WRITE rip 0x401f33 info 0 0
> kvm_apic: apic_write APIC_EOI = 0x0
> kvm_eoi: apicid 0 vector 62
> kvm_msr: msr_write 80b = 0x0
> kvm_entry: vcpu 0
> kvm_exit: reason PENDING_INTERRUPT rip 0x401f35 info 0 0
> kvm_userspace_exit: reason KVM_EXIT_IRQ_WINDOW_OPEN (7)
> kvm_inj_virq: irq 62
> kvm_entry: vcpu 0
> kvm_exit: reason IO_INSTRUCTION rip 0x4016ec info 3fd0008 0
> 
> From the trace we can see there is an interrupt window exit 
> after the first interrupt EOI(irq 62), and the same irq(62) 
> is injected duplicately after the interrupt window.
> 
> QEMU does KVM_INTERRUPT(62) ioctl after KVM exits with 
> KVM_EXIT_IRQ_WINDOW_OPEN, which QEMU requested while the
> guest was printing.  The printing calls
> 
> serial_update_irq() -> qemu_irq_lower() -> qemu_set_irq() ->
> gsi_handler() -> qemu_set_irq() -> pic_irq_request() ->
> apic_deliver_pic_intr() -> kvm_handle_interrupt()
> 
> kvm_handle_interrupt() does
> 
> 	interrupt_request |= CPU_INTERRUPT_HARD
> 
> which later calls cpu_get_pic_interrupt() in kvm_arch_pre_run(), 
> but that function uses stale information from APIC and injects 
> 62 again. If we synchronized the APIC, then the test would #GP, 
> because there would be no injectable interrupt in LAPIC or PIC, 
> so pic_read_irq() would return 15, thinking it was spurious.
> 
> This patch fix it by don't touch LAPIC if LAPIC is in kernel.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Suggested-by: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

(The code is horribly tangled, so I might have missed something.)