Re: [RFC PATCH 3/3] util/vfio-helpers: Let qemu_vfio_pci_init_irq take IRQ index argument

[Alex Williamson]

On Tue, 11 Aug 2020 19:28:45 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Add a new 'index' argument to qemu_vfio_pci_init_irq() to be able
> to initialize other IRQs than IRQ #0. Adapt the single user of this
> API in nvme_init().

This is actually addressing the what the vfio uAPI refers to as the
subindex, the index is the interrupt type, ex. MSI-X.

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/qemu/vfio-helpers.h |  2 +-
>  block/nvme.c                |  2 +-
>  util/vfio-helpers.c         | 12 +++++++++---
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h
> index 1f057c2b9e..ff63e75096 100644
> --- a/include/qemu/vfio-helpers.h
> +++ b/include/qemu/vfio-helpers.h
> @@ -27,6 +27,6 @@ void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
>  void qemu_vfio_pci_unmap_bar(QEMUVFIOState *s, int index, void *bar,
>                               uint64_t offset, uint64_t size);
>  int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
> -                           int irq_type, Error **errp);
> +                           int irq_type, unsigned index, Error **errp);
>  
>  #endif
> diff --git a/block/nvme.c b/block/nvme.c
> index 374e268915..2b3986b66d 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -757,7 +757,7 @@ static int nvme_init(BlockDriverState *bs, const char 
> *device, int namespace,
>      }
>  
>      ret = qemu_vfio_pci_init_irq(s->vfio, &s->irq_notifier,
> -                                 VFIO_PCI_MSIX_IRQ_INDEX, errp);
> +                                 VFIO_PCI_MSIX_IRQ_INDEX, 0, errp);
>      if (ret) {
>          goto out;
>      }
> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> index 3ad7e6be52..ba9a869364 100644
> --- a/util/vfio-helpers.c
> +++ b/util/vfio-helpers.c
> @@ -173,10 +173,11 @@ void qemu_vfio_pci_unmap_bar(QEMUVFIOState *s, int 
> index, void *bar,
>  }
>  
>  /**
> - * Initialize device IRQ with @irq_type and register an event notifier.
> + * Initialize device IRQ @index with @irq_type
> + * and register an event notifier.
>   */
>  int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
> -                           int irq_type, Error **errp)
> +                           int irq_type, unsigned index, Error **errp)
>  {


But MSI-X will expose the VFIO_IRQ_INFO_NORESIZE flag, which means that
we cannot incrementally enable additional subindexes, aka vectors,
without first disabling and re-enabling the entire index/irq_type.
Therefore this is exposing an API that isn't actually supported.
Thanks,

Alex

>      int r;
>      struct vfio_irq_set *irq_set;
> @@ -193,6 +194,11 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, 
> EventNotifier *e,
>          return -EINVAL;
>      }
>      trace_qemu_vfio_pci_init_irq(irq_info.count);
> +    if (index >= irq_info.count) {
> +        error_setg(errp, "Device has %"PRIu32" interrupts (requested index 
> %u)",
> +                   irq_info.count, index);
> +        return -EINVAL;
> +    }
>  
>      irq_set_size = sizeof(*irq_set) + sizeof(int32_t);
>      irq_set = g_malloc0(irq_set_size);
> @@ -202,7 +208,7 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, 
> EventNotifier *e,
>          .argsz = irq_set_size,
>          .flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_TRIGGER,
>          .index = irq_info.index,
> -        .start = 0,
> +        .start = index,
>          .count = 1,
>      };
>