Re: kvm-unit-test fail for split irqchip

[Wanpeng Li]

2016-09-14 4:43 GMT+08:00 Paolo Bonzini <pbonzini@xxxxxxxxxx>:
>
>
> On 13/09/2016 21:01, Radim Krcmar wrote:
>> 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.
>>
>> I think the bug starts in pic_irq_request(), which should not touch
>> LAPIC.  The patch below makes it work (just the second hunk is
>> sufficient), but it's still far from sane code ...
>
> This makes sense.  Most of the functions exported by hw/intc/apic.c
> should only be used with a userspace APIC:
>
> 0000000000000b70 T apic_accept_pic_intr
> 00000000000010f0 T apic_deliver_irq
> 00000000000011e0 T apic_deliver_pic_intr
> 0000000000001310 T apic_get_interrupt
> 0000000000001270 T apic_poll_irq
> 0000000000000a40 T apic_sipi
>
> The patch is okay, though for consistency with other code I'd use
> !kvm_irqchip_in_kernel() rather than !kvm_irqchip_is_split().
>
> Wanpeng, can you do that,

Yeah, I just sent out a patch to fix the bug. Thanks for the long
discussion with me and thanks Radim's proposal. :)

> and change hw/intc/apic.c to use a new casting
> macro
>
>     APICCommonState *s = APIC(dev);
>
> instead of APIC_COMMON?
>

I'm not familiar with QOM too much, what APIC macro do you like?

Regards,
Wanpeng Li