Re: [Qemu-devel] [PATCH 05/13] pci: Add pci_device_route_intx_to_irq

[Jan Kiszka]

On 2012-06-07 16:32, Michael S. Tsirkin wrote:
> On Mon, Jun 04, 2012 at 10:52:13AM +0200, Jan Kiszka wrote:
>> @@ -1089,6 +1093,14 @@ static void pci_set_irq(void *opaque, int irq_num, 
>> int level)
>>      pci_change_irq_level(pci_dev, irq_num, change);
>>  }
>>  
>> +PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin)
>> +{
>> +    PCIBus *bus = dev->host_bus;
>> +
>> +    assert(bus->route_intx_to_irq);
>> +    return bus->route_intx_to_irq(bus->irq_opaque, dev->host_intx_pin[pin]);
>> +}
>> +
>>  /***********************************************************/
>>  /* monitor info on PCI */
>>  
> 
> Just an idea: can devices cache this result, bypassing the
> intx to irq lookup on data path?

That lookup is part of set_irq which we don't bypass so far and where
this is generally trivial. If we want to cache the effects of set_irq as
well, I guess things would become pretty complex (e.g. due to vmstate
compatibility), and I'm unsure if it would buy us much.

> 
>> diff --git a/hw/pci.h b/hw/pci.h
>> index 5b54e2d..bbba01e 100644
>> --- a/hw/pci.h
>> +++ b/hw/pci.h
>> @@ -141,6 +141,15 @@ enum {
>>  #define PCI_DEVICE_GET_CLASS(obj) \
>>       OBJECT_GET_CLASS(PCIDeviceClass, (obj), TYPE_PCI_DEVICE)
>>  
>> +typedef struct PCIINTxRoute {
>> +    enum {
>> +        PCI_INTX_ENABLED,
>> +        PCI_INTX_INVERTED,
>> +        PCI_INTX_DISABLED,
>> +    } mode;
>> +    int irq;
>> +} PCIINTxRoute;
> 
> Is this INTX route or IRQ route?
> Is the INTX enabled/disabled/inverted or the IRQ?
> 
> I have the impression it's the IRQ, in the apic.
> PCI INTX are never inverted they are always active low.

This should be considered as "the route *of* an INTx", not "to some
IRQ". I could call it PCIINTxToIRQRoute if you prefer, but it's a bit
lengthy.

Jan

signature.asc
Description: OpenPGP digital signature