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

[address@hidden]

> -----Original Message-----
> From: Alexander Graf [mailto:address@hidden
> Sent: Friday, December 20, 2013 4:01 PM
> To: Bhushan Bharat-R65777
> Cc: Michael S. Tsirkin; Wood Scott-B07421; qemu-ppc; QEMU Developers; Bhushan
> Bharat-R65777
> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> 
> 
> On 20.12.2013, at 10:42, Bharat Bhushan <address@hidden> 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 <address@hidden>
> > Acked-by: Michael S. Tsirkin <address@hidden>
> > ---
> > 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.

Ok :)

I thought we guys agreed that let this patch come as is and Anthony's work will 
help globally.

Thanks
-Bharat

> 
> 
> Alex
> 
>