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

[Alexander Graf]

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.


Alex