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

Peter Crosthwaite
2015-04-27T04:01:16Z

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?

> +
> + 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.

Regards,
Peter

> } SysBusDeviceClass;
>
> struct SysBusDevice {
> --
> 1.8.3.2
>
>