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

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-05-21 16:05, Michael S. Tsirkin wrote:
> On Mon, May 21, 2012 at 10:13:47AM -0300, Jan Kiszka wrote:
>> @@ -386,6 +387,14 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
>>      piix3_set_irq_level(piix3, pirq, level);
>>  }
>>  
>> +static int piix3_map_host_irq(void *opaque, int pci_intx)
>> +{
>> +    PIIX3State *piix3 = opaque;
>> +    int host_irq = piix3->dev.config[PIIX_PIRQC + pci_intx];
>> +
>> +    return host_irq < PIIX_NUM_PIC_IRQS ? host_irq : -1;
>> +}
>> +
>>  /* irq routing is changed. so rebuild bitmap */
>>  static void piix3_update_irq_levels(PIIX3State *piix3)
>>  {
> 
> 
> So, instead of special API just for assignment,
> I would like to see map_irq in piix being reworked
> to take dev config into account.
> I think piix is almost unique in this but need to check,

Maybe it is the only host bridge with routing that we have in QEMU right
now, but it is surely not unique (e.g. all later Intel chipsets support
this).

> if not fix other host buses that are programmable.
> PCI bridges are all fixed routing.
> 
> Then we can drop set_irq callback.

set_irq may do more than IRQ routing. It may also flip polarity (see
bonito.c). I guess we need to analyze the requirements of all in-tree
host bridges first.

> 
> Finally all devices can cache the irq#,
> and piix would scan devices behind it
> and update the irq.
> 
> Assignment then just needs a notifier, since
> it owns the device just a pointer in device is
> enough.

I cannot completely follow your ideas here yet. Do you want to pass the
mapping information along the notifier, or why "just ... a notifier"?

> 
> Could you look at doing this please?
> If no I can give it a stub.
> 

Unless we can converge over a final API quickly, I would suggest to base
these refactorings on top of what I propose here. We will have to touch
all host bridges more invasively for this anyway. If you can explain to
me how simple the refactoring can be (but I'm a bit skeptical ;) ), I
could do this, otherwise I would prefer to focus on the device
assignment topic which provide some more nuts.

Jan

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