Re: [Qemu-devel] [PATCH 06/13] pci: Add INTx routing notifier

["Michael S. Tsirkin" <mst@redhat.com>]

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