From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: Regression after "Remove support for reporting coalesced APIC IRQs" Date: Thu, 20 Jun 2013 23:29:26 +0300 Message-ID: <20130620202926.GB26747@redhat.com> References: <20130606085352.GZ4725@redhat.com> <20130620114745.GH5832@redhat.com> <51C361AA.8040705@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, yongjie.ren@intel.com To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:36174 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161140Ab3FTU3j (ORCPT ); Thu, 20 Jun 2013 16:29:39 -0400 Content-Disposition: inline In-Reply-To: <51C361AA.8040705@web.de> Sender: kvm-owner@vger.kernel.org List-ID: 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.