Re: [Qemu-devel] [PATCH 1/2] pci: Add pci_device_get_host_irq

[Michael S. Tsirkin]

On Wed, May 30, 2012 at 06:46:03PM +0200, Jan Kiszka wrote:
> On 2012-05-30 18:17, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2012 at 05:47:45PM +0200, Jan Kiszka wrote:
> >> On 2012-05-30 16:59, Michael S. Tsirkin wrote:
> >>> On Wed, May 30, 2012 at 04:46:14PM +0200, Jan Kiszka wrote:
> >>>> On 2012-05-30 16:42, Michael S. Tsirkin wrote:
> >>>>> On Wed, May 30, 2012 at 04:38:25PM +0200, Jan Kiszka wrote:
> >>>>>> On 2012-05-30 15:34, Michael S. Tsirkin wrote:
> >>>>>>> Below's as far as I got - hopefully this
> >>>>>>> explains what I had in mind: for deep hierarchies
> >>>>>>> this will save a bus scan so I think this makes sense
> >>>>>>> even in the absense of kvm irqchip.
> >>>>>>>
> >>>>>>> Warning: completely untested and known to be incomplete.
> >>>>>>> Please judge whether this is going in a direction that
> >>>>>>> is helpful for your efforts. If you respond in the positive,
> >>>>>>> I hope to be able to get back to this next week.
> >>>>>>
> >>>>>> We need to
> >>>>>>  - account for polarity changes along the route
> >>>>>>  - get rid of irq_count as it is no longer updated in the fast path,
> >>>>>>    thus useless on routing updates
> >>>>>
> >>>>> I'll need to consider this more to understand what you mean here.
> >>>>
> >>>> If, e.g., the host bridge is configured to flip the polarity of some
> >>>> interrupt on delivery, the fast path must be able to explore this in
> >>>> order to do the same.
> >>>
> >>> This can be solved incrementally I think - handle polarity
> >>> change like mapping change, no?
> >>
> >> Both belong together if we want to do it generically, IMHO.
> > 
> > So irq_polarity callback like we have for map_irq ATM?  But at least pci
> > bridges never do this flip.  Maybe this is the property of the interrupt
> > controller after all?
> 
> It is a property of some host bridges apparently (e.g. bonito).

So I'm not sure it's worth it to abstract that but I don't mind either.

> In
> theory, someone could also add a PCI-PCI bridge that does this (or does
> the spec disallow it?).

It seems to disallow it.

> > 
> >>>
> >>>> Then you may want to have a look at how irq_count is maintained and when
> >>>> it is used.
> >>>
> >>> In my patch it simply there to OR irq levels of all devices
> >>> connected to a specific pin.
> >>
> >> I think what we want is to avoid any walks and intermediate state
> >> updates for all IRQ deliveries.
> > 
> > Well the bus is not an intermediate state ATM as piix
> > only has a bit per IRQ so it can't OR devices together.
> > So you want to move the counter over to piix?
> 
> irq_count can't be moved logically as it is part of the vmstate. But it
> should only be generated for saving the state by polling all devices
> (and bridges).
> 
> For that we need is an optional callback for devices via which we can
> ask them to update PCIDevice::irq_state in case they don't trigger
> pci_set_irq regularly.

Let's worry about migration compatibility separately.

> And pci_set_irq could simply change the input
> line of the IRQ controller according to the cached route and polarity
> mapping.

But the line is shared between multiple devices.
You need to perform a logical OR function between them all,
this is what the counter does.

> That's basically what I have in mind for any bus. But we could first try
> it out on PCI, then later on generalize the design to make it useable
> for all IRQ routings. I'm afraid there is no simpler way to introduce
> direct IRQ delivery for PCI (unless ignoring corner cases like in qemu-kvm).
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux