[RFC PATCH 0/3] util/vfio-helpers: Fixes to allow using multiple IRQs

[RFC PATCH 0/3] util/vfio-helpers: Fixes to allow using multiple IRQs

[Philippe Mathieu-Daudé]

A pair of fixes to allow binding notifiers on different IRQs
(NVMe block driver series will follow).

Philippe Mathieu-Daudé (3):
  util/vfio-helpers: Store eventfd using int32_t type
  util/vfio-helpers: Add trace event to display device IRQs available
  util/vfio-helpers: Let qemu_vfio_pci_init_irq take IRQ index argument

 include/qemu/vfio-helpers.h |  2 +-
 block/nvme.c                |  2 +-
 util/vfio-helpers.c         | 17 ++++++++++++-----
 util/trace-events           |  1 +
 4 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.21.3

[RFC PATCH 1/3] util/vfio-helpers: Store eventfd using int32_t type

[Philippe Mathieu-Daudé]

Per the documentation in linux-headers/linux/vfio.h:

 VFIO_DEVICE_SET_IRQS

 * DATA_EVENTFD binds the specified ACTION to the provided __s32 eventfd.

Replace the 'int' by an 'int32_t' to match the documentation.

Fixes: 418026ca43 ("util: Introduce vfio helpers")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/vfio-helpers.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 583bdfb36f..6defefcc01 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -193,7 +193,7 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
         return -EINVAL;
     }
 
-    irq_set_size = sizeof(*irq_set) + sizeof(int);
+    irq_set_size = sizeof(*irq_set) + sizeof(int32_t);
     irq_set = g_malloc0(irq_set_size);
 
     /* Get to a known IRQ state */
@@ -205,7 +205,7 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
         .count = 1,
     };
 
-    *(int *)&irq_set->data = event_notifier_get_fd(e);
+    *(int32_t *)&irq_set->data = event_notifier_get_fd(e);
     r = ioctl(s->device, VFIO_DEVICE_SET_IRQS, irq_set);
     g_free(irq_set);
     if (r) {
-- 
2.21.3

[RFC PATCH 2/3] util/vfio-helpers: Add trace event to display device IRQs available

[Philippe Mathieu-Daudé]

Add a trace event to display the amount of IRQs available
on the device.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/vfio-helpers.c | 1 +
 util/trace-events   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 6defefcc01..3ad7e6be52 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -192,6 +192,7 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
         error_setg(errp, "Device interrupt doesn't support eventfd");
         return -EINVAL;
     }
+    trace_qemu_vfio_pci_init_irq(irq_info.count);
 
     irq_set_size = sizeof(*irq_set) + sizeof(int32_t);
     irq_set = g_malloc0(irq_set_size);
diff --git a/util/trace-events b/util/trace-events
index 0ce42822eb..351dbdbe3c 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -83,3 +83,4 @@ qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t iova
 qemu_vfio_do_mapping(void *s, void *host, size_t size, uint64_t iova) "s %p host %p size %zu iova 0x%"PRIx64
 qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t *iova) "s %p host %p size %zu temporary %d iova %p"
 qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
+qemu_vfio_pci_init_irq(uint32_t count) "device interrupt count: %"PRIu32
-- 
2.21.3

Re: [RFC PATCH 2/3] util/vfio-helpers: Add trace event to display device IRQs available

[Alex Williamson]

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

> Add a trace event to display the amount of IRQs available
> on the device.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  util/vfio-helpers.c | 1 +
>  util/trace-events   | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> index 6defefcc01..3ad7e6be52 100644
> --- a/util/vfio-helpers.c
> +++ b/util/vfio-helpers.c
> @@ -192,6 +192,7 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
>          error_setg(errp, "Device interrupt doesn't support eventfd");
>          return -EINVAL;
>      }
> +    trace_qemu_vfio_pci_init_irq(irq_info.count);
>  
>      irq_set_size = sizeof(*irq_set) + sizeof(int32_t);
>      irq_set = g_malloc0(irq_set_size);
> diff --git a/util/trace-events b/util/trace-events
> index 0ce42822eb..351dbdbe3c 100644
> --- a/util/trace-events
> +++ b/util/trace-events
> @@ -83,3 +83,4 @@ qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t iova
>  qemu_vfio_do_mapping(void *s, void *host, size_t size, uint64_t iova) "s %p host %p size %zu iova 0x%"PRIx64
>  qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t *iova) "s %p host %p size %zu temporary %d iova %p"
>  qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
> +qemu_vfio_pci_init_irq(uint32_t count) "device interrupt count: %"PRIu32

Knowing the count independent of which index we're looking at doesn't
seem very useful.  Thanks,

Alex

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

[Philippe Mathieu-Daudé]

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().

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)
 {
     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,
     };
 
-- 
2.21.3

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,
>      };
>  

Re: [RFC PATCH 0/3] util/vfio-helpers: Fixes to allow using multiple IRQs

[Philippe Mathieu-Daudé]

On 8/11/20 7:28 PM, Philippe Mathieu-Daudé wrote:
> A pair of fixes to allow binding notifiers on different IRQs
> (NVMe block driver series will follow).
> 
> Philippe Mathieu-Daudé (3):
>   util/vfio-helpers: Store eventfd using int32_t type
>   util/vfio-helpers: Add trace event to display device IRQs available
>   util/vfio-helpers: Let qemu_vfio_pci_init_irq take IRQ index argument
> 
>  include/qemu/vfio-helpers.h |  2 +-
>  block/nvme.c                |  2 +-
>  util/vfio-helpers.c         | 17 ++++++++++++-----
>  util/trace-events           |  1 +
>  4 files changed, 15 insertions(+), 7 deletions(-)
> 

Oops I forgot, it is based on "util/vfio-helpers: Fix typo in description":
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg02073.html
Based-on: <20200811151643.21293-4-philmd@redhat.com>