Re: [Qemu-devel] [PATCH v12 4/9] hw/vfio/platform: add capability to start IRQ propagation

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2015-03-19 at 17:16 +0000, Eric Auger wrote:
> Add a reset notify function that enables to start the propagation of
> interrupts to the guest.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> v10 -> v11:
> - comment reword
> 
> v8 -> v9:
> - handle failure in vfio_irq_starter
> ---
>  hw/vfio/platform.c              | 64 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/vfio/vfio-platform.h |  8 ++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 31c2701..361e01b 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -572,6 +572,70 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
>      }
>  }
>  
> +/**
> + * vfio_irq_starter: Start IRQ forwarding for a specific VFIO device
> + * @sbdev: Sysbus device handle
> + * @opaque: DeviceState handle of the interrupt controller the device
> + *          is attached to
> + *
> + * The function retrieves the absolute GSI number each VFIO IRQ
> + * corresponds to and start forwarding.
> + */

Using "forwarding" here is really making me look for your platform's EOI
trick (IRQ Forwarding), which isn't here.

> +static int vfio_irq_starter(SysBusDevice *sbdev, void *opaque)
> +{
> +    DeviceState *intc = (DeviceState *)opaque;
> +    VFIOPlatformDevice *vdev;
> +    VFIOINTp *intp;
> +    qemu_irq irq;
> +    int gsi, ret;
> +
> +    if (object_dynamic_cast(OBJECT(sbdev), TYPE_VFIO_PLATFORM)) {
> +        vdev = VFIO_PLATFORM_DEVICE(sbdev);
> +
> +        QLIST_FOREACH(intp, &vdev->intp_list, next) {
> +            gsi = 0;
> +            while (1) {
> +                irq = qdev_get_gpio_in(intc, gsi);
> +                if (irq == intp->qemuirq) {
> +                    break;
> +                }
> +                gsi++;
> +            }

A for() loop would be more compact here, but is there some other exit
condition we can add?  gsi < INT_MAX?

> +            intp->virtualID = gsi;
> +            ret = vdev->start_irq_fn(intp);
> +            if (ret) {
> +                error_report("%s unable to start propagation of IRQ index %d",
> +                             vdev->vbasedev.name, intp->pin);
> +                exit(1);
> +            }
> +        }
> +    }
> +    return 0;
> +}
> +
> +/**
> + * vfio_kick_irqs: start IRQ forwarding for all the VFIO devices
> + * attached to the platform bus
> + * @data: Device state handle of the interrupt controller the VFIO IRQs
> + *        must be bound to
> + *
> + * Binding to the platform bus IRQs happens on a machine init done
> + * notifier registered by the platform bus. Only at that time the
> + * absolute virtual IRQ = GSI is known and allows to setup IRQFD.
> + * This function typically can be called in a reset notifier registered
> + * by the machine file.
> + */

So there's not actually an irqfd setup done here yet and what is
currently done by the above starter function doesn't depend at all on
the GSI.  Do you perhaps want to setup the vfio->eventfd signaling
during your initfn and then only connect the eventfd->irqfd to KVM in
the reset function?  Sort of how vfio-pci does the routing update.

> +void vfio_kick_irqs(void *data)
> +{
> +    DeviceState *intc = (DeviceState *)data;
> +    DeviceState *dev =
> +        qdev_find_recursive(sysbus_get_default(), TYPE_PLATFORM_BUS_DEVICE);
> +    PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(dev);
> +
> +    assert(pbus->done_gathering);
> +    foreach_dynamic_sysbus_device(vfio_irq_starter, intc);
> +}
> +
>  static const VMStateDescription vfio_platform_vmstate = {
>      .name = TYPE_VFIO_PLATFORM,
>      .unmigratable = 1,
> diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
> index 31893a3..c9ee898 100644
> --- a/include/hw/vfio/vfio-platform.h
> +++ b/include/hw/vfio/vfio-platform.h
> @@ -77,4 +77,12 @@ typedef struct VFIOPlatformDeviceClass {
>  #define VFIO_PLATFORM_DEVICE_GET_CLASS(obj) \
>       OBJECT_GET_CLASS(VFIOPlatformDeviceClass, (obj), TYPE_VFIO_PLATFORM)
>  
> +/**
> + * vfio_kick_irqs - reset notifier that starts IRQ injection
> + * for all VFIO dynamic sysbus devices attached to the platform bus.
> + *
> + * @opaque: handle to the interrupt controller DeviceState
> + */
> +void vfio_kick_irqs(void *opaque);
> +
>  #endif /*HW_VFIO_VFIO_PLATFORM_H*/