Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1483138
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: kvm-unit-test fail for split irqchip |
| Date | 2016-09-14 11:50 +0200 |
| Message-ID | <sheZk-7DT-11@gated-at.bofh.it> (permalink) |
| References | <sgTLc-11k-25@gated-at.bofh.it> <sh1fH-61g-5@gated-at.bofh.it> <sh2Ou-6TO-35@gated-at.bofh.it> <sh9wB-330-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 14/09/2016 05:57, Wanpeng Li wrote:
> 2016-09-14 4:43 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>>
>>
>> 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?
The macro would be defined like
#define TYPE_APIC "apic"
#define APIC(obj) \
OBJECT_CHECK(APICCommonState, (obj), TYPE_APIC)
With this change and without your other patch, you should get an
assertion failure in eventinj.flat (which would have pointed immediately
at the bug!).
Thanks,
Paolo
> Regards,
> Wanpeng Li
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-13 13:10 +0200
Re: kvm-unit-test fail for split irqchip Radim Krcmar <rkrcmar@redhat.com> - 2016-09-13 21:10 +0200
Re: kvm-unit-test fail for split irqchip Paolo Bonzini <pbonzini@redhat.com> - 2016-09-13 22:50 +0200
Re: kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-14 06:00 +0200
Re: kvm-unit-test fail for split irqchip Paolo Bonzini <pbonzini@redhat.com> - 2016-09-14 11:50 +0200
Re: kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-15 03:00 +0200
Re: kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-14 06:00 +0200
csiph-web