Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

[Eric Auger <eric.auger@linaro.org>]

Hi Peter,
On 04/27/2015 06:09 AM, Peter Crosthwaite wrote:
> On Fri, Apr 24, 2015 at 5:39 AM, Eric Auger <eric.auger@linaro.org> wrote:
>> Add a new irq_routing_notifier notifier in the SysBusDeviceClass. This
>> notifier, if populated, is called after sysbus_connect_irq.
>>
>> This mechanism is used to setup VFIO signaling once VFIO platform
>> devices get attached to their platform bus, on a machine init done
>> notifier.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> v1 -> v2:
>> - duly put the notifier in the class and not in the device
>> ---
>>  hw/core/sysbus.c    | 6 ++++++
>>  include/hw/sysbus.h | 1 +
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index b53c351..8553a6f 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -109,7 +109,13 @@ qemu_irq sysbus_get_connected_irq(SysBusDevice *dev, int n)
>>
>>  void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
>>  {
>> +    SysBusDeviceClass *sbd = SYS_BUS_DEVICE_GET_CLASS(dev);
>> +
>>      qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
> 
> One of my long term goals is to try and get rid of sysbus IRQ
> abstraction completely in favor of just qdev gpios. This means
> features that apply to GPIOs automatically apply to IRQs and vice
> versa. Can your notifier hook be pushed up to the qdev GPIO level to
> make it more globally usable and avoid a new feature to sysbus IRQs?
Yes sure, I am going to put the notifier in DeviceClass then.
> 
>> +
>> +    if (sbd->irq_routing_notifier) {
>> +        sbd->irq_routing_notifier(dev, irq);
>> +    }
>>  }
>>
>>  /* Check whether an MMIO region exists */
>> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
>> index d1f3f00..dbf3f0f 100644
>> --- a/include/hw/sysbus.h
>> +++ b/include/hw/sysbus.h
>> @@ -41,6 +41,7 @@ typedef struct SysBusDeviceClass {
>>      /*< public >*/
>>
>>      int (*init)(SysBusDevice *dev);
>> +    void (*irq_routing_notifier)(SysBusDevice *dev, qemu_irq irq);
> 
> Is it better to make the name more matched to sysbus_connect_irq?
> Perhaps connect_irq_notifier. But with the qdev approach this would be
> connect_gpio_out_notifier or something along those lines.
OK for that naming

Thanks

Eric
> 
> Regards,
> Peter
> 
>>  } SysBusDeviceClass;
>>
>>  struct SysBusDevice {
>> --
>> 1.8.3.2
>>
>>