From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v4] kvm: Use a bitmap for tracking used GSIs Date: Mon, 18 May 2009 14:36:41 +0300 Message-ID: <4A114849.604@redhat.com> References: <1242217702.4786.59.camel@2710p.home> <4A0ABEA8.6030103@redhat.com> <1242219343.4786.66.camel@2710p.home> <4A0AC453.2000907@redhat.com> <1242220276.4786.67.camel@2710p.home> <20090513135502.GA1405@redhat.com> <1242224129.9456.6.camel@lappy> <1242225238.9456.9.camel@lappy> <1242256055.9456.326.camel@lappy> <4A1077F9.8040604@redhat.com> <20090518111246.GB3037@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alex Williamson , kvm@vger.kernel.org, sheng.yang@intel.com To: "Michael S. Tsirkin" Return-path: Received: from mx2.redhat.com ([66.187.237.31]:38661 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753366AbZERLgp (ORCPT ); Mon, 18 May 2009 07:36:45 -0400 In-Reply-To: <20090518111246.GB3037@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Michael S. Tsirkin wrote: > On Sun, May 17, 2009 at 11:47:53PM +0300, Avi Kivity wrote: > >> Alex Williamson wrote: >> >>> On Wed, 2009-05-13 at 08:33 -0600, Alex Williamson wrote: >>> >>> >>>> On Wed, 2009-05-13 at 08:15 -0600, Alex Williamson wrote: >>>> >>>> >>>>> On Wed, 2009-05-13 at 16:55 +0300, Michael S. Tsirkin wrote: >>>>> >>>>> >>>>>> Very surprising: I haven't seen any driver disable MSI expect on device >>>>>> destructor path. Is this a linux guest? >>>>>> >>>>>> >>>>> Yes, Debian 2.6.26 kernel. I'll check it it behaves the same on newer >>>>> upstream kernels and try to figure out why it's doing it. >>>>> >>>>> >>>> Updating the guest to 2.6.29 seems to fix the interrupt toggling. So >>>> it's either something in older kernels or something debian introduced, >>>> but that seems unlikely. >>>> >>>> >>> For the curious, this was fixed prior to 2.6.27-rc1 by this: >>> >>> commit ce6fce4295ba727b36fdc73040e444bd1aae64cd >>> Author: Matthew Wilcox >>> Date: Fri Jul 25 15:42:58 2008 -0600 >>> >>> PCI MSI: Don't disable MSIs if the mask bit isn't supported >>> David Vrabel has a device which generates an interrupt storm on >>> the INTx >>> pin if we disable MSI interrupts altogether. Masking interrupts is only >>> a performance optimisation, so we can ignore the request to mask the >>> interrupt. >>> >>> It looks like without the maskbit attribute on MSI, the default way to >>> mask an MSI interrupt was to toggle the MSI enable bit. This was >>> introduced in 58e0543e8f355b32f0778a18858b255adb7402ae, so it's lifespan >>> was probably 2.6.21 - 2.6.26. >>> >>> >>> >> On the other hand, if the host device supports this maskbit attribute, >> we might want to support it. I'm not sure exactly how though. >> >> If we trap msi entry writes, we're inviting the guest to exit every time >> it wants to disable interrupts. If we don't, we're inviting spurious >> interrupts, which will cause unwanted exits and injections. >> > > Avi, I think you are mixing up MSI and MSI-X. MSI does not have any tables: > all changes go through configuration writes, which AFAIK we always trap. > Isn't that right? > Right. > On the other hand, in MSI-X mask bit is mandatory, not optional > so we'll have to support it for assigned devices at some point. > > If we are worried about speed of masking/unmasking MSI-X interrupts for > assigned devices (older kernels used to mask them, recent kernels leave > this to drivers) we will probably need to have MSI-X support in the > kernel, and have kernel examine the mask bit before injecting the > interrupt, just like real devices do. > Yes. Let's actually quantify this though. Several times per second doesn't qualify. >> Maybe we ought to let the guest write to the msi tables without >> trapping, and in the injection logic do something like >> >> msi_entry = *msi_entry_ptr; >> mb(); >> if (msi_entry != msi->last_msi_entry) >> msi_reconfigure(); >> if (msi_enabled(msi_entry)) >> insert_the_needle(); >> > > I don't really understand why do you need the reconfigure > and tracking last msi entry here. > The msi entry can change the vector and delivery mode, no? This way we can cache some stuff instead of decoding it each time. For example, if the entry points at a specific vcpu, we can cache the vcpu pointer, instead of searching for the apic ID. -- error compiling committee.c: too many arguments to function