From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53912) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SdhSi-0007cJ-Bi for qemu-devel@nongnu.org; Sun, 10 Jun 2012 08:41:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SdhSf-0002wv-Cd for qemu-devel@nongnu.org; Sun, 10 Jun 2012 08:41:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26040) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SdhSf-0002we-4p for qemu-devel@nongnu.org; Sun, 10 Jun 2012 08:41:41 -0400 Date: Sun, 10 Jun 2012 15:42:08 +0300 From: "Michael S. Tsirkin" Message-ID: <20120610124207.GC7852@redhat.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FD493FF.5030907@web.de> 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: Jan Kiszka Cc: Alex Williamson , qemu-devel 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 their > >> 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 the > >> 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 and > >> 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 callback. > > > > Before pci_bus_irqs is called. > > Why is another question. > > > >>> > >>> One way to fix all this is call the notifier for devices, if set, from > >>> 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 devices 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 > I think a better name is pci_bus_update_intx_irq_cache or something like that. 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. -- MST