Re: [Qemu-devel] [RFC PATCH] qemu spapr-pci: added IRQ list to PCIBus

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2012-05-14 at 14:21 +1000, Alexey Kardashevskiy wrote:
> On 14/05/12 11:58, David Gibson wrote:
> > On Sat, May 12, 2012 at 05:29:53PM +1000, Alexey Kardashevskiy wrote:
> >> There is a need for a mechanism to obtain an IRQ line number to
> >> initialize End-Of-Interrupt handler.
> >>
> >> There is another proposed solution (commit
> >> b7790763828b732059ad24ba0e64ce327563fe1a "pci: Add callbacks
> >> to support retrieving and updating interrupts") which adds pci_get_irq
> >> callback to every PCI bus to allow an external caller to calculate
> >> IRQ number from IRQ line (ABDD).
> >>
> >> However it seems to be too complicated as it affects all PCI buses
> >> while the only user of it is VFIO-PCI so this could be done simpler
> >> by an array of 4 IRQs (lines A, B, C, D) in struct PCIBus which
> >> every platform would initialize in its own way.
> > 
> > I think you need to pin down the definition of what's going on here a
> > bit better.  Not all platforms have a concept of global IRQ number,
> > and the usual qemu_irq model supports that.  So for this function who
> > is it that is defining the number space in which pci_get_irq() is
> > returning values.
> 
> 
> The idea is that legacy interrupt (INTx) is caught by a host driver (for example, vfio-pci). A
> driver disables it and transfers to a guest. In order to enable it back, a host driver has to make
> sure that the interrupt has been handled by a guest. It is done by an End-Of-Interrupt (EOI) message
> sent by a guest. Then QEMU forwards the message to a host driver which enables INTx back.

Right, the gap is that we can signal the interrupt in qemu, but don't
know which EOI to look for to re-enable the physical device.  Later
we'll want to know the interrupt when we inject it so we can do so via
an irqfd directly into kvm.

> At the moment this mechanism - EOI callbacks - operates with global IRQ numbers, both on x86 (acpi)
> and power (xics). So pci_get_irq returns only global numbers which PCIBus receives from the calling
> code somehow (platform specific code knows how to initialize them, a bus cannot resolve it itself
> anyway). And this is not dynamically changing information as PCI _domain_ hotplug does not exist (am
> I right? :) ).

It actually can change dynamically on x86 due to acpi interrupt links
which allow the guest a generic way to select from a set of possible
interrupt routing schemes.  And of course a chipset driver could twiddle
bits if it wanted as well.  So, we really do need the update notifiers
from my tree that this patch drops.  pci_get_irq is one of the few
things a qemu chipset needs to implement to get device assignment with
vfio.  Doing it this way with a common array in PCIBus makes the patch
smaller, but I don't know that it really simplifies anything.  The
chipset function is trivial on x86, it's just knowing what to do that's
the problem.

> If we do not want pci_get_irq to return global IRQ numbers than it makes more sense to keep using
> pins (A,B,C,D) plus some bus ID and not introduce any new numbers at all.

How does that solve the initial problem of getting the EOI?

> How can PCIBus::pci_get_irq() _callbacks_ be useful?

It indicates support?  Thanks,

Alex