From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59982) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SdhYW-0000wB-Pm for qemu-devel@nongnu.org; Sun, 10 Jun 2012 08:47:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SdhYU-0003sW-Sc for qemu-devel@nongnu.org; Sun, 10 Jun 2012 08:47:44 -0400 Received: from mout.web.de ([212.227.17.11]:58033) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SdhYU-0003sS-JA for qemu-devel@nongnu.org; Sun, 10 Jun 2012 08:47:42 -0400 Message-ID: <4FD4976C.4070902@web.de> Date: Sun, 10 Jun 2012 14:47:40 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20120610094840.GC6250@redhat.com> <4FD47156.3020200@web.de> <20120610103352.GE6250@redhat.com> <4FD47A75.4020706@web.de> <20120610111149.GJ6250@redhat.com> <4FD48277.60704@web.de> <20120610113922.GO6250@redhat.com> <4FD48E70.3080406@web.de> <20120610121628.GA7852@redhat.com> <4FD493FF.5030907@web.de> <20120610124207.GC7852@redhat.com> In-Reply-To: <20120610124207.GC7852@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig4ACCE1A25AB42BFF2CD61BC2" Subject: Re: [Qemu-devel] [PATCH 06/13] pci: Add INTx routing notifier 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) --------------enig4ACCE1A25AB42BFF2CD61BC2 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2012-06-10 14:42, Michael S. Tsirkin wrote: > On Sun, Jun 10, 2012 at 02:33:03PM +0200, Jan Kiszka wrote: >> On 2012-06-10 14:16, Michael S. Tsirkin wrote: >>> On Sun, Jun 10, 2012 at 02:09:20PM +0200, Jan Kiszka wrote: >>>> On 2012-06-10 13:39, Michael S. Tsirkin wrote: >>>>> It's OK to use recursion but when done through a callback >>>>> like this it's unreadable. >>>> >>>> Isn't the alternative poking into foreign bridge device states for t= heir >>>> secondary buses? >>> >>> pci_set_bus_intx_routing does this already. >> >> True. OK, I can do the recursion in pci_bus_fire_intx_routing_notifier= >> directly instead of pushing this into the bridge. >> >>> >>>>> Also, you need to setup you cache after intx cache has been >>>>> initialized, and you provide no clean way to do that. >>>> >>>> Once a PCI device is registered, the INTx route can be queried. So t= he >>>> device user will call pci_device_route_intx_to_irq once (e.g. in the= >>>> device init function which is invoked afterward) to fill its cache a= nd >>>> receive a notification if an update is needed. I do not see why, and= >>>> specifically how you could query the route earlier or register a cal= lback. >>> >>> Before pci_bus_irqs is called. >>> Why is another question. >>> >>>>> >>>>> One way to fix all this is call the notifier for devices, if set, f= rom >>>>> pci_set_bus_intx_routing. >>>>> Then assume that intx to irq translations can be cached >>>>> even though they aren't now. So you will need to invoke >>>>> pci_set_bus_intx_routing on intx to irq mapping changes, >>>>> and that fires the notifier for free. >>>> >>>> pci_set_bus_intx_routing is really only for the initial setup of the= >>>> static INTx pin routes. And this happens on >>>> pci_bus_irqs/pci_register_bus, ie. triggered by the host bridge. By = that >>>> time, there can't be any notifier listeners - as there are no device= s yet. >>>> >>>> Jan >>>> >>> >>> What I am saying is we'll cache the final IRQ at some point. >>> Pretend it's already that way so callers are ready for this. >> >> This wouldn't change the picture very much: Before the host bridge is >> fully initialized, there is no valid route available. But before that,= >> there is also no device attached to it. So the invocation pattern >> wouldn't change. >> >> What would change is the semantic of the interface to the host bridge.= >> So what about this: provide a public pci_root_bus_intx_routing_updated= >> which so far just calls the internal-use-only >> pci_bus_fire_intx_routing_notifier? >> >> Jan >> >=20 > I think a better name is pci_bus_update_intx_irq_cache > or something like that. At least "update cache" is not a better description as it implies in the function name the required steps of the implementation. In fact, this function /may/ update a cache and will fire notifiers. But that's nothing the care should worry about. >=20 > And I really think it's better to recalculate the > intx routing there as well, so that if some bus > implements a dynamic route_intx it just needs to > call this after update. I thought dynamic routing is disallowed according to the spec? If not, I agree. Jan --------------enig4ACCE1A25AB42BFF2CD61BC2 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/Ul2wACgkQitSsb3rl5xQVSQCaAtnU25kwHfGiE87YO2Eia0f4 vWsAni+tO5Z/cxKOVUN95zOShlQEYUOD =o+nf -----END PGP SIGNATURE----- --------------enig4ACCE1A25AB42BFF2CD61BC2--