Re: [Qemu-devel] [PATCH 2/2] ppc-e500: implement PCI INTx routing

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

On Fri, Dec 20, 2013 at 11:31:17AM +0100, Alexander Graf wrote:
> 
> On 20.12.2013, at 10:42, Bharat Bhushan <r65777@freescale.com> wrote:
> 
> > This patch adds pci pin to irq_num routing callback.
> > This callback is called from pci_device_route_intx_to_irq to find which pci device
> > maps to which irq. This is used for pci-device passthrough using vfio.
> > 
> > Also without this patch we gets below warning
> > 
> > "
> >  PCI: Bug - unimplemented PCI INTx routing (e500-pcihost)
> >  qemu-system-ppc64: PCI: Bug - unimplemented PCI INTx routing (e500-pcihost)
> > "
> > and Legacy interrupt does not work with pci device passthrough.
> > 
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > hw/pci-host/ppce500.c |   20 ++++++++++++++++++--
> > 1 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> > index 71e5ca9..ffea782 100644
> > --- a/hw/pci-host/ppce500.c
> > +++ b/hw/pci-host/ppce500.c
> > @@ -88,6 +88,7 @@ struct PPCE500PCIState {
> >     struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> >     uint32_t gasket_time;
> >     qemu_irq irq[PCI_NUM_PINS];
> > +    uint32_t irq_num[PCI_NUM_PINS];
> >     uint32_t first_slot;
> >     /* mmio maps */
> >     MemoryRegion container;
> > @@ -267,13 +268,26 @@ static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int pin)
> > 
> > static void mpc85xx_pci_set_irq(void *opaque, int pin, int level)
> > {
> > -    qemu_irq *irq = opaque;
> > +    PPCE500PCIState *s = opaque;
> > +    qemu_irq *pic = s->irq;
> > 
> >     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> > 
> >     qemu_set_irq(irq[pin], level);
> > }
> > 
> > +static PCIINTxRoute e500_route_intx_pin_to_irq(void *opaque, int pin)
> > +{
> > +    PCIINTxRoute route;
> > +    PPCE500PCIState *s = opaque;
> > +
> > +    route.mode = PCI_INTX_ENABLED;
> > +    route.irq = s->irq_num[pin];
> > +
> > +    pci_debug("%s: PCI irq-pin = %d, irq_num= %d\n", __func__, pin, route.irq);
> > +    return route;
> > +}
> > +
> > static const VMStateDescription vmstate_pci_outbound = {
> >     .name = "pci_outbound",
> >     .version_id = 0,
> > @@ -350,12 +364,13 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
> > 
> >     for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
> >         sysbus_init_irq(dev, &s->irq[i]);
> > +        s->irq_num[i] = i + 1;
> 
> I still don't like how you duplicate the logic which MPIC irq line is associated with which pci host controller irq line. Please pass this information into the pcihost object somehow from ppce500_init() so that it's only at a single spot.
> 
> If you like, you can just use qom/qdev properties for that, but it needs to be configured from the outside.
> 
> 
> Alex


Maybe just call ppce500_pci_map_irq_slot instead of using irq_num?