From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sdfhu-000619-3O for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:49:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sdfhs-0004Tw-4l for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:49:17 -0400 Received: from mout.web.de ([212.227.17.11]:62736) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sdfhr-0004Th-Qn for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:49:16 -0400 Message-ID: <4FD47BA7.7030606@web.de> Date: Sun, 10 Jun 2012 12:49:11 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20120607143216.GA12638@redhat.com> <4FD0C459.7070104@web.de> <20120607162850.GA12884@redhat.com> <4FD0DAEE.2040405@web.de> <20120610095507.GD6250@redhat.com> <4FD47217.7020909@web.de> <20120610104120.GF6250@redhat.com> In-Reply-To: <20120610104120.GF6250@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigD446DB7D17878234573AFCDA" Subject: Re: [Qemu-devel] [PATCH 05/13] pci: Add pci_device_route_intx_to_irq List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Alex Williamson , qemu-devel This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigD446DB7D17878234573AFCDA Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2012-06-10 12:41, Michael S. Tsirkin wrote: > On Sun, Jun 10, 2012 at 12:08:23PM +0200, Jan Kiszka wrote: >> On 2012-06-10 11:55, Michael S. Tsirkin wrote: >>> On Thu, Jun 07, 2012 at 06:46:38PM +0200, Jan Kiszka wrote: >>>> On 2012-06-07 18:28, Michael S. Tsirkin wrote: >>>>> On Thu, Jun 07, 2012 at 05:10:17PM +0200, Jan Kiszka wrote: >>>>>> On 2012-06-07 16:32, Michael S. Tsirkin wrote: >>>>>>> On Mon, Jun 04, 2012 at 10:52:13AM +0200, Jan Kiszka wrote: >>>>>>>> @@ -1089,6 +1093,14 @@ static void pci_set_irq(void *opaque, int= irq_num, int level) >>>>>>>> pci_change_irq_level(pci_dev, irq_num, change); >>>>>>>> } >>>>>>>> =20 >>>>>>>> +PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int p= in) >>>>>>>> +{ >>>>>>>> + PCIBus *bus =3D dev->host_bus; >>>>>>>> + >>>>>>>> + assert(bus->route_intx_to_irq); >>>>>>>> + return bus->route_intx_to_irq(bus->irq_opaque, dev->host_in= tx_pin[pin]); >>>>>>>> +} >>>>>>>> + >>>>>>>> /***********************************************************/ >>>>>>>> /* monitor info on PCI */ >>>>>>>> =20 >>>>>>> >>>>>>> Just an idea: can devices cache this result, bypassing the >>>>>>> intx to irq lookup on data path? >>>>>> >>>>>> That lookup is part of set_irq which we don't bypass so far and wh= ere >>>>>> this is generally trivial. If we want to cache the effects of set_= irq as >>>>>> well, I guess things would become pretty complex (e.g. due to vmst= ate >>>>>> compatibility), and I'm unsure if it would buy us much. >>>>> >>>>> This is less for performance but more for making >>>>> everyone use the same infrastructure rather than >>>>> assigned devices being the weird case. >>>> >>>> Device assignment is weird. It bypasses all state updates as it does= not >>>> have to bother about migratability. >>>> >>>> Well, of course we could cache the host bridge routing result as wel= l, >>>> for every device. It would have to be in addition to host_intx_pin. = But >>>> the result would look pretty strange to me. >>>> >>>> In any case, I would prefer to do this, if at all, on top of this >>>> series, specifically as it will require to touch all host bridges. >>> >>> I'd like to ponder this a bit more then. >>> >>> If the claim is that device assignment is only needed for >>> piix anyway, then why not make it depend on piix *explicitly*? >>> Yes ugly but this will make it very easy to find and >>> address any missing pieces. >> >> Because it is conceptually independent of the PIIX, we will need it fo= r >> successors of that x86 chipset as well, and I won't add the ugly hack = of >> qemu-kvm upstream >=20 > So you look at an API and see it requires a route > callback. And you ask "why doesn't chipset X implement it"? > And the answer is "because it's only used by device assignment". > Which you will only know if you read this thread. So it's > a hack. And I'd rather have the hacks in device-assignment.c > than in pci.c even if the former are nastier. I don't share your view on this. It is surely _not_ a hack, specifically when compared to what we have so far and what could be done otherwise, e.g. hacking device-assignment and PIIX to make them cooperate (sorry, I would vote against such an attempt). This is just a partially used generic API. Any chipset not providing the required routing function will cause an assert once some tries to make use of it. So, what can I do to make this API more acceptable for you? Jan --------------enigD446DB7D17878234573AFCDA 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/Ue6cACgkQitSsb3rl5xQHeQCdG5omLqEPMEhRw0SIoc2SC/kN r1AAnjsQvjF+zxoREpiueymx9z8AO+Vf =cxMH -----END PGP SIGNATURE----- --------------enigD446DB7D17878234573AFCDA--