Re: Regression after "Remove support for reporting coalesced APIC IRQs"

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jun 20, 2013 at 10:10:18PM +0200, Jan Kiszka wrote:
> On 2013-06-20 13:47, Gleb Natapov wrote:
> > Jan ping, are you OK with what I proposed below?
> > 
> > On Thu, Jun 06, 2013 at 11:53:52AM +0300, Gleb Natapov wrote:
> >> Hi Jan,
> >>
> >> I bisected [1] to f1ed0450a5fac7067590317cbf027f566b6ccbca. Fortunately
> >> further investigation showed that it is not really related to removing
> >> APIC timer interrupt reinjection and the real problem is that we cannot
> >> assume that __apic_accept_irq() always injects interrupts like the patch
> >> does because the function skips interrupt injection if APIC is disabled.
> >> This misreporting screws RTC interrupt tracking, so further RTC interrupt
> >> are stopped to be injected. The simplest solution that I see is to revert
> >> most of the commit and only leave APIC timer interrupt reinjection.
> 
> I'm not understanding the precise error yet and how __apic_accept_irq
> should be (properly) involved in its solution. Which code path depend on
> the information that the APIC is enabled?
> 
RTC interrupt injection tracking in virt/kvm/ioapic.c depends on
accurate information about which vcpus interrupt was injected into since it
expects EOI from each vcpu before injection next RTC interrupt. Since
now kvm_apic_set_irq() reports interrupt as injected for vcpus with
disabled apic the logic breaks because EOI will never happen.

> The point is that preserving the return value of __apic_accept_irq, just
> redefining it to "delivery_mode != APIC_DM_FIXED || apic_enabled()"
> creates a pretty ugly interface, no? Can't we address the specific issue
> of the RTC at a different level?
> 
Do not see how.

--
			Gleb.