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

[Alexander Graf]

> Am 22.12.2013 um 12:31 schrieb "Michael S. Tsirkin" <address@hidden>:
> 
>> On Fri, Dec 20, 2013 at 11:31:17AM +0100, Alexander Graf wrote:
>> 
>>> 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
> 
> 
> 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

> 
>