diff mbox

[2/2] ppc-e500: implement PCI INTx routing

Message ID 1385620533-17685-3-git-send-email-Bharat.Bhushan@freescale.com
State New
Headers show

Commit Message

Bharat Bhushan Nov. 28, 2013, 6:35 a.m. UTC
This patch adds pci pin to irq_num routing callback
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)
"

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
 hw/pci-host/ppce500.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

Comments

Alexander Graf Dec. 18, 2013, 9:53 p.m. UTC | #1
On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:

> This patch adds pci pin to irq_num routing callback
> 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)
> "
> 
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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 49bfcc6..3c4cf9e 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 *pic = opaque;
> +    PPCE500PCIState *s = opaque;
> +    qemu_irq *pic = s->irq;;

Double semicolon?

> 
>     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> 
>     qemu_set_irq(pic[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;

Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()? I don't understand the purpose of this whole exercise to be honest.

Michael, could you please shed some light on this?


Alex

>     }
> 
>     memory_region_init(&s->pio, OBJECT(s), "pci-pio", PCIE500_PCI_IOLEN);
> 
>     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> -                         mpc85xx_pci_map_irq, s->irq, address_space_mem,
> +                         mpc85xx_pci_map_irq, s, address_space_mem,
>                          &s->pio, PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
>     h->bus = b;
> 
> @@ -373,6 +388,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
>     sysbus_init_mmio(dev, &s->container);
>     sysbus_init_mmio(dev, &s->pio);
> +    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
> 
>     return 0;
> }
> -- 
> 1.7.0.4
> 
>
Michael S. Tsirkin Dec. 18, 2013, 10:24 p.m. UTC | #2
On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> 
> On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> 
> > This patch adds pci pin to irq_num routing callback
> > 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)
> > "
> > 
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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 49bfcc6..3c4cf9e 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 *pic = opaque;
> > +    PPCE500PCIState *s = opaque;
> > +    qemu_irq *pic = s->irq;;
> 
> Double semicolon?
> 
> > 
> >     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> > 
> >     qemu_set_irq(pic[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;
> 
> Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()? I don't understand the purpose of this whole exercise to be honest.
> 
> Michael, could you please shed some light on this?
> 
> 
> Alex

This is printed by pci_device_route_intx_to_irq - it's used by device
assignment and vfio to figure out which irq does a
given pci device drive.

> >     }
> > 
> >     memory_region_init(&s->pio, OBJECT(s), "pci-pio", PCIE500_PCI_IOLEN);
> > 
> >     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> > -                         mpc85xx_pci_map_irq, s->irq, address_space_mem,
> > +                         mpc85xx_pci_map_irq, s, address_space_mem,
> >                          &s->pio, PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
> >     h->bus = b;
> > 
> > @@ -373,6 +388,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
> >     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
> >     sysbus_init_mmio(dev, &s->container);
> >     sysbus_init_mmio(dev, &s->pio);
> > +    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
> > 
> >     return 0;
> > }
> > -- 
> > 1.7.0.4
> > 
> >
Bharat Bhushan Dec. 19, 2013, 3:39 p.m. UTC | #3
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Thursday, December 19, 2013 3:55 AM
> To: Alexander Graf
> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc; Bhushan
> Bharat-R65777
> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> 
> On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> >
> > On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> >
> > > This patch adds pci pin to irq_num routing callback 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) "
> > >
> > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
> > > 49bfcc6..3c4cf9e 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 *pic = opaque;
> > > +    PPCE500PCIState *s = opaque;
> > > +    qemu_irq *pic = s->irq;;
> >
> > Double semicolon?

Ok, will correct.

> >
> > >
> > >     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> > >
> > >     qemu_set_irq(pic[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;
> >
> > Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()? I don't
> understand the purpose of this whole exercise to be honest.
> >
> > Michael, could you please shed some light on this?
> >
> >
> > Alex
> 
> This is printed by pci_device_route_intx_to_irq - it's used by device assignment
> and vfio to figure out which irq does a given pci device drive.

Yes, exactly same reason.

Thanks
-Bharat

> 
> > >     }
> > >
> > >     memory_region_init(&s->pio, OBJECT(s), "pci-pio",
> > > PCIE500_PCI_IOLEN);
> > >
> > >     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> > > -                         mpc85xx_pci_map_irq, s->irq, address_space_mem,
> > > +                         mpc85xx_pci_map_irq, s, address_space_mem,
> > >                          &s->pio, PCI_DEVFN(s->first_slot, 0), 4,
> TYPE_PCI_BUS);
> > >     h->bus = b;
> > >
> > > @@ -373,6 +388,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
> > >     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
> > >     sysbus_init_mmio(dev, &s->container);
> > >     sysbus_init_mmio(dev, &s->pio);
> > > +    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
> > >
> > >     return 0;
> > > }
> > > --
> > > 1.7.0.4
> > >
> > >
>
Michael S. Tsirkin Dec. 19, 2013, 3:50 p.m. UTC | #4
On Thu, Dec 19, 2013 at 03:39:58PM +0000, Bharat.Bhushan@freescale.com wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Thursday, December 19, 2013 3:55 AM
> > To: Alexander Graf
> > Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc; Bhushan
> > Bharat-R65777
> > Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> > 
> > On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> > >
> > > On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> > >
> > > > This patch adds pci pin to irq_num routing callback 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) "
> > > >
> > > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
> > > > 49bfcc6..3c4cf9e 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 *pic = opaque;
> > > > +    PPCE500PCIState *s = opaque;
> > > > +    qemu_irq *pic = s->irq;;
> > >
> > > Double semicolon?
> 
> Ok, will correct.
> 
> > >
> > > >
> > > >     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> > > >
> > > >     qemu_set_irq(pic[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;
> > >
> > > Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()? I don't
> > understand the purpose of this whole exercise to be honest.
> > >
> > > Michael, could you please shed some light on this?
> > >
> > >
> > > Alex
> > 
> > This is printed by pci_device_route_intx_to_irq - it's used by device assignment
> > and vfio to figure out which irq does a given pci device drive.
> 
> Yes, exactly same reason.
> 
> Thanks
> -Bharat


you probably should say in commit log 
"this makes vfio work on ppc" - assumng it works for you of course.
as it is it makes it look like you are just trying to shut off
a warning.

> > 
> > > >     }
> > > >
> > > >     memory_region_init(&s->pio, OBJECT(s), "pci-pio",
> > > > PCIE500_PCI_IOLEN);
> > > >
> > > >     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> > > > -                         mpc85xx_pci_map_irq, s->irq, address_space_mem,
> > > > +                         mpc85xx_pci_map_irq, s, address_space_mem,
> > > >                          &s->pio, PCI_DEVFN(s->first_slot, 0), 4,
> > TYPE_PCI_BUS);
> > > >     h->bus = b;
> > > >
> > > > @@ -373,6 +388,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
> > > >     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
> > > >     sysbus_init_mmio(dev, &s->container);
> > > >     sysbus_init_mmio(dev, &s->pio);
> > > > +    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
> > > >
> > > >     return 0;
> > > > }
> > > > --
> > > > 1.7.0.4
> > > >
> > > >
> >
Bharat Bhushan Dec. 19, 2013, 3:50 p.m. UTC | #5
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Thursday, December 19, 2013 9:20 PM
> To: Bhushan Bharat-R65777
> Cc: Alexander Graf; Wood Scott-B07421; QEMU Developers; qemu-ppc
> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> 
> On Thu, Dec 19, 2013 at 03:39:58PM +0000, Bharat.Bhushan@freescale.com wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Thursday, December 19, 2013 3:55 AM
> > > To: Alexander Graf
> > > Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers;
> > > qemu-ppc; Bhushan
> > > Bharat-R65777
> > > Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> > >
> > > On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> > > >
> > > > On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> > > >
> > > > > This patch adds pci pin to irq_num routing callback 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) "
> > > > >
> > > > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
> > > > > 49bfcc6..3c4cf9e 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 *pic = opaque;
> > > > > +    PPCE500PCIState *s = opaque;
> > > > > +    qemu_irq *pic = s->irq;;
> > > >
> > > > Double semicolon?
> >
> > Ok, will correct.
> >
> > > >
> > > > >
> > > > >     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin ,
> > > > > level);
> > > > >
> > > > >     qemu_set_irq(pic[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;
> > > >
> > > > Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()?
> > > > I don't
> > > understand the purpose of this whole exercise to be honest.
> > > >
> > > > Michael, could you please shed some light on this?
> > > >
> > > >
> > > > Alex
> > >
> > > This is printed by pci_device_route_intx_to_irq - it's used by
> > > device assignment and vfio to figure out which irq does a given pci device
> drive.
> >
> > Yes, exactly same reason.
> >
> > Thanks
> > -Bharat
> 
> 
> you probably should say in commit log
> "this makes vfio work on ppc" - assumng it works for you of course.

Yes :)

> as it is it makes it look like you are just trying to shut off a warning.

Ok I will update the log.
vfio on powerpc is not yet upstream so I restrained myself from writing that.

Thanks
-Bharat

> 
> > >
> > > > >     }
> > > > >
> > > > >     memory_region_init(&s->pio, OBJECT(s), "pci-pio",
> > > > > PCIE500_PCI_IOLEN);
> > > > >
> > > > >     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> > > > > -                         mpc85xx_pci_map_irq, s->irq,
> address_space_mem,
> > > > > +                         mpc85xx_pci_map_irq, s,
> > > > > + address_space_mem,
> > > > >                          &s->pio, PCI_DEVFN(s->first_slot, 0),
> > > > > 4,
> > > TYPE_PCI_BUS);
> > > > >     h->bus = b;
> > > > >
> > > > > @@ -373,6 +388,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
> > > > >     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s-
> >iomem);
> > > > >     sysbus_init_mmio(dev, &s->container);
> > > > >     sysbus_init_mmio(dev, &s->pio);
> > > > > +    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
> > > > >
> > > > >     return 0;
> > > > > }
> > > > > --
> > > > > 1.7.0.4
> > > > >
> > > > >
> > >
>
Alexander Graf Dec. 19, 2013, 4:28 p.m. UTC | #6
On 19.12.2013, at 16:39, Bharat.Bhushan@freescale.com wrote:

> 
> 
>> -----Original Message-----
>> From: Michael S. Tsirkin [mailto:mst@redhat.com]
>> Sent: Thursday, December 19, 2013 3:55 AM
>> To: Alexander Graf
>> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc; Bhushan
>> Bharat-R65777
>> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
>> 
>> On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
>>> 
>>> On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
>>> 
>>>> This patch adds pci pin to irq_num routing callback 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) "
>>>> 
>>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
>>>> 49bfcc6..3c4cf9e 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 *pic = opaque;
>>>> +    PPCE500PCIState *s = opaque;
>>>> +    qemu_irq *pic = s->irq;;
>>> 
>>> Double semicolon?
> 
> Ok, will correct.
> 
>>> 
>>>> 
>>>>    pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
>>>> 
>>>>    qemu_set_irq(pic[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;
>>> 
>>> Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()? I don't
>> understand the purpose of this whole exercise to be honest.
>>> 
>>> Michael, could you please shed some light on this?
>>> 
>>> 
>>> Alex
>> 
>> This is printed by pci_device_route_intx_to_irq - it's used by device assignment
>> and vfio to figure out which irq does a given pci device drive.
> 
> Yes, exactly same reason.

Is there any way we could get rid of the information duplication? The fact that INTA/B/C/D are mapped to 1,2,3,4 is really a configuration parameter that should only live at a single spot.


Alex
Michael S. Tsirkin Dec. 19, 2013, 6:32 p.m. UTC | #7
On Thu, Dec 19, 2013 at 05:28:17PM +0100, Alexander Graf wrote:
> 
> On 19.12.2013, at 16:39, Bharat.Bhushan@freescale.com wrote:
> 
> > 
> > 
> >> -----Original Message-----
> >> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> >> Sent: Thursday, December 19, 2013 3:55 AM
> >> To: Alexander Graf
> >> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc; Bhushan
> >> Bharat-R65777
> >> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> >> 
> >> On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> >>> 
> >>> On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> >>> 
> >>>> This patch adds pci pin to irq_num routing callback 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) "
> >>>> 
> >>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
> >>>> 49bfcc6..3c4cf9e 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 *pic = opaque;
> >>>> +    PPCE500PCIState *s = opaque;
> >>>> +    qemu_irq *pic = s->irq;;
> >>> 
> >>> Double semicolon?
> > 
> > Ok, will correct.
> > 
> >>> 
> >>>> 
> >>>>    pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> >>>> 
> >>>>    qemu_set_irq(pic[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;
> >>> 
> >>> Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()? I don't
> >> understand the purpose of this whole exercise to be honest.
> >>> 
> >>> Michael, could you please shed some light on this?
> >>> 
> >>> 
> >>> Alex
> >> 
> >> This is printed by pci_device_route_intx_to_irq - it's used by device assignment
> >> and vfio to figure out which irq does a given pci device drive.
> > 
> > Yes, exactly same reason.
> 
> Is there any way we could get rid of the information duplication? The fact that INTA/B/C/D are mapped to 1,2,3,4 is really a configuration parameter that should only live at a single spot.
> 
> 
> Alex

Yes. In fact I had the idea to only have something like
pci_device_route_intx_to_irq and call it once for all interrupts
and cache that, then use this to send interrupts directly to apic.
Redo this each time routing changes.
I had a patch like this (and I think Jan had one too), but Anthony said
he'll rewrite all interrupt routing using QOM so I dropped it. I'll try
to resurrect it.
Bharat Bhushan Dec. 20, 2013, 4:15 a.m. UTC | #8
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Friday, December 20, 2013 12:02 AM
> To: Alexander Graf
> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc
> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> 
> On Thu, Dec 19, 2013 at 05:28:17PM +0100, Alexander Graf wrote:
> >
> > On 19.12.2013, at 16:39, Bharat.Bhushan@freescale.com wrote:
> >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > >> Sent: Thursday, December 19, 2013 3:55 AM
> > >> To: Alexander Graf
> > >> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers;
> > >> qemu-ppc; Bhushan
> > >> Bharat-R65777
> > >> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> > >>
> > >> On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> > >>>
> > >>> On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> > >>>
> > >>>> This patch adds pci pin to irq_num routing callback 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) "
> > >>>>
> > >>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
> > >>>> 49bfcc6..3c4cf9e 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 *pic = opaque;
> > >>>> +    PPCE500PCIState *s = opaque;
> > >>>> +    qemu_irq *pic = s->irq;;
> > >>>
> > >>> Double semicolon?
> > >
> > > Ok, will correct.
> > >
> > >>>
> > >>>>
> > >>>>    pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin ,
> > >>>> level);
> > >>>>
> > >>>>    qemu_set_irq(pic[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;
> > >>>
> > >>> Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()?
> > >>> I don't
> > >> understand the purpose of this whole exercise to be honest.
> > >>>
> > >>> Michael, could you please shed some light on this?
> > >>>
> > >>>
> > >>> Alex
> > >>
> > >> This is printed by pci_device_route_intx_to_irq - it's used by
> > >> device assignment and vfio to figure out which irq does a given pci device
> drive.
> > >
> > > Yes, exactly same reason.
> >
> > Is there any way we could get rid of the information duplication? The fact
> that INTA/B/C/D are mapped to 1,2,3,4 is really a configuration parameter that
> should only live at a single spot.
> >
> >
> > Alex
> 
> Yes. In fact I had the idea to only have something like
> pci_device_route_intx_to_irq and call it once for all interrupts and cache that,
> then use this to send interrupts directly to apic.
> Redo this each time routing changes.
> I had a patch like this (and I think Jan had one too), but Anthony said he'll
> rewrite all interrupt routing using QOM so I dropped it. I'll try to resurrect
> it.

So do we want to have this patch almost in this shape and hope Anthony's changes will handle this well or wait for Anthony patches first ?

Thanks
-Bharat

>
Michael S. Tsirkin Dec. 20, 2013, 5:01 a.m. UTC | #9
On Fri, Dec 20, 2013 at 04:15:09AM +0000, Bharat.Bhushan@freescale.com wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Friday, December 20, 2013 12:02 AM
> > To: Alexander Graf
> > Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc
> > Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> > 
> > On Thu, Dec 19, 2013 at 05:28:17PM +0100, Alexander Graf wrote:
> > >
> > > On 19.12.2013, at 16:39, Bharat.Bhushan@freescale.com wrote:
> > >
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > >> Sent: Thursday, December 19, 2013 3:55 AM
> > > >> To: Alexander Graf
> > > >> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers;
> > > >> qemu-ppc; Bhushan
> > > >> Bharat-R65777
> > > >> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> > > >>
> > > >> On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> > > >>>
> > > >>> On 28.11.2013, at 07:35, Bharat Bhushan <r65777@freescale.com> wrote:
> > > >>>
> > > >>>> This patch adds pci pin to irq_num routing callback 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) "
> > > >>>>
> > > >>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.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
> > > >>>> 49bfcc6..3c4cf9e 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 *pic = opaque;
> > > >>>> +    PPCE500PCIState *s = opaque;
> > > >>>> +    qemu_irq *pic = s->irq;;
> > > >>>
> > > >>> Double semicolon?
> > > >
> > > > Ok, will correct.
> > > >
> > > >>>
> > > >>>>
> > > >>>>    pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin ,
> > > >>>> level);
> > > >>>>
> > > >>>>    qemu_set_irq(pic[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;
> > > >>>
> > > >>> Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()?
> > > >>> I don't
> > > >> understand the purpose of this whole exercise to be honest.
> > > >>>
> > > >>> Michael, could you please shed some light on this?
> > > >>>
> > > >>>
> > > >>> Alex
> > > >>
> > > >> This is printed by pci_device_route_intx_to_irq - it's used by
> > > >> device assignment and vfio to figure out which irq does a given pci device
> > drive.
> > > >
> > > > Yes, exactly same reason.
> > >
> > > Is there any way we could get rid of the information duplication? The fact
> > that INTA/B/C/D are mapped to 1,2,3,4 is really a configuration parameter that
> > should only live at a single spot.
> > >
> > >
> > > Alex
> > 
> > Yes. In fact I had the idea to only have something like
> > pci_device_route_intx_to_irq and call it once for all interrupts and cache that,
> > then use this to send interrupts directly to apic.
> > Redo this each time routing changes.
> > I had a patch like this (and I think Jan had one too), but Anthony said he'll
> > rewrite all interrupt routing using QOM so I dropped it. I'll try to resurrect
> > it.
> 
> So do we want to have this patch almost in this shape and hope Anthony's changes will handle this well or wait for Anthony patches first ?
> 
> Thanks
> -Bharat

I think your patch is the right thing to do ATM.


> >
Michael S. Tsirkin Dec. 20, 2013, 5:03 a.m. UTC | #10
On Thu, Nov 28, 2013 at 12:05:33PM +0530, Bharat Bhushan wrote:
> This patch adds pci pin to irq_num routing callback
> 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)
> "
> 
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>


Please tweak commit log to explain thereal motivation for the change
which is to use vfio on ppc.
Besides that

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 49bfcc6..3c4cf9e 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 *pic = opaque;
> +    PPCE500PCIState *s = opaque;
> +    qemu_irq *pic = s->irq;;
>  
>      pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
>  
>      qemu_set_irq(pic[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;
>      }
>  
>      memory_region_init(&s->pio, OBJECT(s), "pci-pio", PCIE500_PCI_IOLEN);
>  
>      b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
> -                         mpc85xx_pci_map_irq, s->irq, address_space_mem,
> +                         mpc85xx_pci_map_irq, s, address_space_mem,
>                           &s->pio, PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
>      h->bus = b;
>  
> @@ -373,6 +388,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
>      memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
>      sysbus_init_mmio(dev, &s->container);
>      sysbus_init_mmio(dev, &s->pio);
> +    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
>  
>      return 0;
>  }
> -- 
> 1.7.0.4
> 
>
diff mbox

Patch

diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 49bfcc6..3c4cf9e 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 *pic = opaque;
+    PPCE500PCIState *s = opaque;
+    qemu_irq *pic = s->irq;;
 
     pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
 
     qemu_set_irq(pic[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;
     }
 
     memory_region_init(&s->pio, OBJECT(s), "pci-pio", PCIE500_PCI_IOLEN);
 
     b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
-                         mpc85xx_pci_map_irq, s->irq, address_space_mem,
+                         mpc85xx_pci_map_irq, s, address_space_mem,
                          &s->pio, PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
     h->bus = b;
 
@@ -373,6 +388,7 @@  static int e500_pcihost_initfn(SysBusDevice *dev)
     memory_region_add_subregion(&s->container, PCIE500_REG_BASE, &s->iomem);
     sysbus_init_mmio(dev, &s->container);
     sysbus_init_mmio(dev, &s->pio);
+    pci_bus_set_route_irq_fn(b, e500_route_intx_pin_to_irq);
 
     return 0;
 }