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

[Paolo Bonzini]

On 23/04/2015 10:55, Eric Auger wrote:
> Add a new irq_routing_notifier notifier and its setter. This
> notifier is called on sysbus_connect_irq. The same notifier
> is used for all interrupts.
> 
> Signed-off-by: Eric Auger <address@hidden>

I'm sorry, same problem here.  The question is: why? :)

I can sort of understand why you want this, but not completely.  I also
have all sorts of questions on the code that aren't answered.  For
example, why does the notifier not get the N argument?

I also cannot understand why this is not a layering violation.  Maybe
it's not, but it's hard to say without seeing the user.

Paolo

> ---
>  hw/core/sysbus.c    | 9 +++++++++
>  include/hw/sysbus.h | 6 ++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index b53c351..26579f1 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -110,6 +110,9 @@ qemu_irq sysbus_get_connected_irq(SysBusDevice *dev, int 
> n)
>  void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
>  {
>      qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
> +    if (dev->irq_routing_notifier) {
> +        dev->irq_routing_notifier(dev, irq);
> +    }
>  }
>  
>  /* Check whether an MMIO region exists */
> @@ -307,6 +310,12 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
>      return get_system_memory();
>  }
>  
> +void sysbus_set_irq_routing_notifier(SysBusDevice *dev,
> +                                     IRQRoutingNotifier notifier)
> +{
> +    dev->irq_routing_notifier = notifier;
> +}
> +
>  static void sysbus_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *k = DEVICE_CLASS(klass);
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index d1f3f00..84166d9 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -13,6 +13,7 @@
>  #define SYSTEM_BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_SYSTEM_BUS)
>  
>  typedef struct SysBusDevice SysBusDevice;
> +typedef void (*IRQRoutingNotifier)(SysBusDevice *dev, qemu_irq irq);
>  
>  #define TYPE_SYS_BUS_DEVICE "sys-bus-device"
>  #define SYS_BUS_DEVICE(obj) \
> @@ -55,6 +56,8 @@ struct SysBusDevice {
>      } mmio[QDEV_MAX_MMIO];
>      int num_pio;
>      pio_addr_t pio[QDEV_MAX_PIO];
> +
> +    IRQRoutingNotifier irq_routing_notifier;
>  };
>  
>  typedef int FindSysbusDeviceFunc(SysBusDevice *sbdev, void *opaque);
> @@ -100,4 +103,7 @@ static inline DeviceState *sysbus_try_create_simple(const 
> char *name,
>      return sysbus_try_create_varargs(name, addr, irq, NULL);
>  }
>  
> +void sysbus_set_irq_routing_notifier(SysBusDevice *dev,
> +                                     IRQRoutingNotifier notifier);
> +
>  #endif /* !HW_SYSBUS_H */
>