From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: Regression after "Remove support for reporting coalesced APIC IRQs" Date: Thu, 20 Jun 2013 23:01:54 +0200 Message-ID: <51C36DC2.4040508@web.de> References: <20130606085352.GZ4725@redhat.com> <20130620114745.GH5832@redhat.com> <51C361AA.8040705@web.de> <20130620202926.GB26747@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="----enig2NPLSWRRHEHCIASURDBHN" Cc: kvm@vger.kernel.org, yongjie.ren@intel.com To: Gleb Natapov Return-path: Received: from mout.web.de ([212.227.17.11]:56494 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757555Ab3FTVCB (ORCPT ); Thu, 20 Jun 2013 17:02:01 -0400 In-Reply-To: <20130620202926.GB26747@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2NPLSWRRHEHCIASURDBHN Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-06-20 22:29, Gleb Natapov wrote: > 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. Fortunat= ely >>>> further investigation showed that it is not really related to removi= ng >>>> APIC timer interrupt reinjection and the real problem is that we can= not >>>> assume that __apic_accept_irq() always injects interrupts like the p= atch >>>> does because the function skips interrupt injection if APIC is disab= led. >>>> This misreporting screws RTC interrupt tracking, so further RTC inte= rrupt >>>> are stopped to be injected. The simplest solution that I see is to r= evert >>>> 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 sinc= e 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. OK, so we may have to enhance kvm_apic_set_irq. Or implement the apic_enabled check properly in __apic_accept_irq (though only kvm_apic_set_irq will have a use for it). The (historic) check looks very strange, misplaced, and carries a comment that is probably also outdated. The check used to protect the only functioning delivery mode, but then was left in place, ignoring all the other modes. Yes, that's not directly related to this regression. We can revert first and then rework this interface. But the latter should definitely be done as the revert will make the interface even worse IMHO. Jan ------enig2NPLSWRRHEHCIASURDBHN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlHDbcIACgkQitSsb3rl5xTtUgCfd03O0TPq85xQuP/9RrTtdnKa qm8AoIcrwGZ2R0pD3vsi3RoLHsepT67Q =CF4J -----END PGP SIGNATURE----- ------enig2NPLSWRRHEHCIASURDBHN--