diff mbox

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

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

Commit Message

Bharat Bhushan Dec. 20, 2013, 9:42 a.m. UTC 
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(-)

Comments

Alexander Graf Dec. 20, 2013, 10:31 a.m. UTC | #1 
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
Bharat Bhushan Dec. 20, 2013, 11:23 a.m. UTC | #2 
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> 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 <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.

Ok :)

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

Thanks
-Bharat

> 
> 
> Alex
> 
>
Michael S. Tsirkin Dec. 22, 2013, 11:31 a.m. UTC | #3 
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?
Alexander Graf Dec. 22, 2013, 2:04 p.m. UTC | #4 
> Am 22.12.2013 um 12:31 schrieb "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?

That'd be a step into the wrong direction. Wiring is a configuration thing.

Alex

> 
>
diff mbox

Patch

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;
     }
 
     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;
 }