Re: [Qemu-devel] [PATCH 1/2] pci: Add pci_device_get_host_irq

[Jan Kiszka]

On 2012-05-21 11:36, Avi Kivity wrote:
> On 05/21/2012 04:13 PM, Jan Kiszka wrote:
>> Add a PCI IRQ path discovery function that walks from a given device to
>> the host bridge, returning the IRQ number that is reported to the
>> attached interrupt controller. For this purpose, another PCI bridge
>> callback function is introduced: map_host_irq. It is so far only
>> implemented by the PIIX3, other host bridges can be added later on as
>> required.
>>
>> Will be used for KVM PCI device assignment.
> 
> This is similar to the memory API, which converts a memory region
> hierarchy to a flat list and fires notifiers whenever it changes.

In fact, this should become a generic thing one day, independent of PCI.
But that's an exercise to be done while reworking the IRQ layer of QEMU
(e.g. to support delivery feedback...).

> 
>> +int pci_device_get_host_irq(PCIDevice *pci_dev, int irq_num)
>> +{
>> +    PCIBus *bus;
>> +
>> +    for (;;) {
>> +        bus = pci_dev->bus;
>> +        irq_num = bus->map_irq(pci_dev, irq_num);
>> +        if (bus->map_host_irq) {
>> +            break;
>> +        }
>> +        pci_dev = bus->parent_dev;
>> +        assert(pci_dev);
>> +    }
>> +    return bus->map_host_irq(bus->irq_opaque, irq_num);
>> +}
>> +
> 
> My personal preference is to avoid infinite loops with breaks, I'd write
> this as a do/while (without the assert).  Or maybe supply all buses with
> a default map_host_irq that recurses back into
> pci_device_get_host_irq().  But this is not an objection to the patch.

I've modeled it after pci_change_irq_level. I would suggest to change
both later on.

BTW, the assert catches host bridges that do not yet implement the
required mapping service.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux