From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40195) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Smuyx-0002RG-Bw for qemu-devel@nongnu.org; Thu, 05 Jul 2012 18:57:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Smuyu-0007Kc-UO for qemu-devel@nongnu.org; Thu, 05 Jul 2012 18:57:06 -0400 Received: from mout.web.de ([212.227.17.11]:64559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Smuyu-0007KL-KM for qemu-devel@nongnu.org; Thu, 05 Jul 2012 18:57:04 -0400 Message-ID: <4FF61BB5.5030502@web.de> Date: Fri, 06 Jul 2012 00:56:53 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <4FF56176.5000100@siemens.com> <20120705131758.GD30572@redhat.com> <4FF5A455.10205@siemens.com> <20120705204711.GH31257@redhat.com> In-Reply-To: <20120705204711.GH31257@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig0CEFDABC32283BD43A48D2DF" Subject: Re: [Qemu-devel] [PATCH] msix: Drop tracking of used vectors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig0CEFDABC32283BD43A48D2DF Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2012-07-05 22:47, Michael S. Tsirkin wrote: > On Thu, Jul 05, 2012 at 04:27:33PM +0200, Jan Kiszka wrote: >> On 2012-07-05 15:17, Michael S. Tsirkin wrote: >>> On Thu, Jul 05, 2012 at 11:42:14AM +0200, Jan Kiszka wrote: >>>> This optimization was once used in qemu-kvm to keep KVM route usage = low. >>>> But now we solved that problem via lazy updates. >>> >>> What if we are using vhost which AFAIK can't use the lazy path? >> >> It uses static vIRQs with irqfd, just as before. >=20 > Exactly. And the API is helpful in that case. Look at the code: this API serves no practical purpose anymore, not even for the irqfd scenario. I'm not touching the irqfd code while removing msix_entry_used. >=20 >>> >>>> It also tried to handle >>>> the case of vectors shared between different sources of the same dev= ice. >>>> However, this never really worked >>> >>> Interesting. Guests actually have support and it seems to work for me= =2E >>> What's the issue? >> >> The state of shared vectors is not tracked. Consider a device has two >> sources for the same vector and one source which raised the "line" >> before is deregistered, then we will leave the vector set. >=20 > In virtio vectors are registered/deregistered during setup > (before driver-ok), so this does not happen. =46rom that point of view, the current final msix_clr_pending in msix_vector_unuse is also useless. Then what is the remaining purpose of msix_entry_used? Also, we are not discussing a virtio API here but a generic MSI-X feature that affects every MSI-X using device. > Further, even in theory this might not be an issue since anyone sharing= > interrupts needs to deal with spurious events anyway. This is not about spurious interrupts, it's about the proper state of the PBA, something that should not contain any spurious states. >=20 > So is or is there not a bug in virtio when it shares vectors? There are deficits in virtio about how it handles potential vector sharing internally. But the use/unuse API will not be involved in making this more spec conforming. Nor are there other users behind the horizon. That's my point. >=20 >> What we rather need is to track the sharing at device level and report= >> the resulting state to the MSI-X layer. That's what the reduced API wi= ll >> allow. I don't think that vector sharing belongs to the generic layer.= >>> >>>> and will have to be addressed >>>> differently anyway. >>> >>> what's the plan to address this? >> >> As said above: Extend virtio to track their queue states, let it >> calculate the vector state and report that to the generic layer. >> >> If we really once identify another device model - besides virtio - tha= t >> does vector sharing, we can still try to model an optional extension t= o >> the MSI-X layer for such devices. But the majority of device models >> should not be bothered with such special cases. >> >>> >>>> So drop this obsolete interface. >>>> >>>> We still need interfaces to clear pending vectors though. Provide >>>> msix_clear_vector and msix_clear_all_vectors for this. >>>> >>>> Signed-off-by: Jan Kiszka >>> >>> Looks like if guest tries to use too many vectors it will have to swi= tch >>> to userspace virtio where previously we would report failure to guest= >>> and it would configure less vectors, which seems better. >> >> There is nothing in the current approach that tracks the availability = of >> vector. That's because it depends on the device if it considers a vect= or >> used when a single event source grabbed it or if it is able to share i= t >> internally. >=20 > So I'd like to see some code that shows how we avoid the regression in = the > above scenario, before we drop the API. Sorry, what regression? This patch does not change the behavior of virtio. Please review it and point out the contrary. >=20 >>> >>> I'd like to understand whether this patch is a pre-requisite for >>> anything or just a cleanup? >> >> It avoids spreading of redundant msix_vector_use/unuse to more devices= >> (next ones will be VFIO and kvm device assignment). >> >> Jan >=20 > Another less intrusive way is to add a flag that say > "device always uses all vectors". The existing API is the intrusive one, that's why I'm trying to remove it for a while now. Can you sketch a scenario where it helps to track vector usage via a central counter like it's done now? I simply don't see this. Therefore I see no point in keeping this infrastructure around - or even adding a flag to prolong its life. Jan --------------enig0CEFDABC32283BD43A48D2DF 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 Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/2G7kACgkQitSsb3rl5xQGcQCeNftMxiK75LlU2FdMLb1h71q3 U+oAoITvZW/2g6vSe3PYWi0c9Z3h2eGy =h4Kg -----END PGP SIGNATURE----- --------------enig0CEFDABC32283BD43A48D2DF--