Guests typically enable MSI-X with all of the vectors masked in the MSI-X
vector table. To match the guest state of device, Qemu enables MSI-X by
enabling vector 0 with userspace triggering and immediately release.
However the release function actually does not release it due to already
using userspace mode.

It is no need to enable triggering on host and rely on the mask bit to
avoid spurious interrupts. Use an invalid fd (i.e. fd = -1) is enough
to get MSI-X enabled.

After dynamic MSI-X allocation is supported, the interrupt restoring
also need use such way to enable MSI-X, therefore, create a function
for that.

Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Jing Liu <jing2.liu@intel.com>
---
Changes since RFC v1:
- A new patch. Use an invalid fd to get MSI-X enabled instead of using
  userspace triggering. (Alex)
---
 hw/vfio/pci.c | 50 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 31f36d68bb19..e24c21241a0c 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -369,6 +369,39 @@ static void vfio_msi_interrupt(void *opaque)
     notify(&vdev->pdev, nr);
 }
 
+/*
+ * Get MSI-X enabled, but no vector enabled, by setting vector 0 with an invalid
+ * fd to kernel.
+ */
+static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)
+{
+    struct vfio_irq_set *irq_set;
+    int ret = 0, argsz;
+    int32_t *fd;
+
+    argsz = sizeof(*irq_set) + sizeof(*fd);
+
+    irq_set = g_malloc0(argsz);
+    irq_set->argsz = argsz;
+    irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
+                     VFIO_IRQ_SET_ACTION_TRIGGER;
+    irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
+    irq_set->start = 0;
+    irq_set->count = 1;
+    fd = (int32_t *)&irq_set->data;
+    *fd = -1;
+
+    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
+    if (ret) {
+        error_report("vfio: failed to enable MSI-X with vector 0 trick, %d",
+                     ret);
+    }
+
+    g_free(irq_set);
+
+    return ret;
+}
+
 static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
 {
     struct vfio_irq_set *irq_set;
@@ -618,6 +651,8 @@ static void vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
 
 static void vfio_msix_enable(VFIOPCIDevice *vdev)
 {
+    int ret;
+
     vfio_disable_interrupts(vdev);
 
     vdev->msi_vectors = g_new0(VFIOMSIVector, vdev->msix->entries);
@@ -640,8 +675,6 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
     vfio_commit_kvm_msi_virq_batch(vdev);
 
     if (vdev->nr_vectors) {
-        int ret;
-
         ret = vfio_enable_vectors(vdev, true);
         if (ret) {
             error_report("vfio: failed to enable vectors, %d", ret);
@@ -655,13 +688,14 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
          * MSI-X capability, but leaves the vector table masked.  We therefore
          * can't rely on a vector_use callback (from request_irq() in the guest)
          * to switch the physical device into MSI-X mode because that may come a
-         * long time after pci_enable_msix().  This code enables vector 0 with
-         * triggering to userspace, then immediately release the vector, leaving
-         * the physical device with no vectors enabled, but MSI-X enabled, just
-         * like the guest view.
+         * long time after pci_enable_msix().  This code sets vector 0 with an
+         * invalid fd to make the physical device MSI-X enabled, but with no
+         * vectors enabled, just like the guest view.
          */
-        vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
-        vfio_msix_vector_release(&vdev->pdev, 0);
+        ret = vfio_enable_msix_no_vec(vdev);
+        if (ret) {
+            error_report("vfio: failed to enable MSI-X, %d", ret);
+        }
     }
 
     trace_vfio_msix_enable(vdev->vbasedev.name);
-- 
2.27.0

On 8/22/23 09:29, Jing Liu wrote:
> Guests typically enable MSI-X with all of the vectors masked in the MSI-X
> vector table. To match the guest state of device, Qemu enables MSI-X by

QEMU is preferred to Qemu.

> enabling vector 0 with userspace triggering and immediately release.
> However the release function actually does not release it due to already
> using userspace mode.
> 
> It is no need to enable triggering on host and rely on the mask bit to
> avoid spurious interrupts. Use an invalid fd (i.e. fd = -1) is enough
> to get MSI-X enabled.
> 
> After dynamic MSI-X allocation is supported, the interrupt restoring
> also need use such way to enable MSI-X, therefore, create a function
> for that.
> 
> Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Jing Liu <jing2.liu@intel.com>
> ---
> Changes since RFC v1:
> - A new patch. Use an invalid fd to get MSI-X enabled instead of using
>    userspace triggering. (Alex)
> ---
>   hw/vfio/pci.c | 50 ++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 31f36d68bb19..e24c21241a0c 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -369,6 +369,39 @@ static void vfio_msi_interrupt(void *opaque)
>       notify(&vdev->pdev, nr);
>   }
>   
> +/*
> + * Get MSI-X enabled, but no vector enabled, by setting vector 0 with an invalid
> + * fd to kernel.
> + */
> +static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)> +{
> +    struct vfio_irq_set *irq_set;

This could be a 'g_autofree' variable.

> +    int ret = 0, argsz;
> +    int32_t *fd;
> +
> +    argsz = sizeof(*irq_set) + sizeof(*fd);
> +
> +    irq_set = g_malloc0(argsz);
> +    irq_set->argsz = argsz;
> +    irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> +                     VFIO_IRQ_SET_ACTION_TRIGGER;
> +    irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
> +    irq_set->start = 0;
> +    irq_set->count = 1;
> +    fd = (int32_t *)&irq_set->data;
> +    *fd = -1;
> +
> +    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +    if (ret) {
> +        error_report("vfio: failed to enable MSI-X with vector 0 trick, %d",
> +                     ret);

The above message seems redundant. I would simply return 'ret' and let
the caller report the error. Same as vfio_enable_vectors().

Thanks,

C.


> +    }
> +
> +    g_free(irq_set);
> +
> +    return ret;
> +}
> +
>   static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
>   {
>       struct vfio_irq_set *irq_set;
> @@ -618,6 +651,8 @@ static void vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
>   
>   static void vfio_msix_enable(VFIOPCIDevice *vdev)
>   {
> +    int ret;
> +
>       vfio_disable_interrupts(vdev);
>   
>       vdev->msi_vectors = g_new0(VFIOMSIVector, vdev->msix->entries);
> @@ -640,8 +675,6 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
>       vfio_commit_kvm_msi_virq_batch(vdev);
>   
>       if (vdev->nr_vectors) {
> -        int ret;
> -
>           ret = vfio_enable_vectors(vdev, true);
>           if (ret) {
>               error_report("vfio: failed to enable vectors, %d", ret);
> @@ -655,13 +688,14 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
>            * MSI-X capability, but leaves the vector table masked.  We therefore
>            * can't rely on a vector_use callback (from request_irq() in the guest)
>            * to switch the physical device into MSI-X mode because that may come a
> -         * long time after pci_enable_msix().  This code enables vector 0 with
> -         * triggering to userspace, then immediately release the vector, leaving
> -         * the physical device with no vectors enabled, but MSI-X enabled, just
> -         * like the guest view.
> +         * long time after pci_enable_msix().  This code sets vector 0 with an
> +         * invalid fd to make the physical device MSI-X enabled, but with no
> +         * vectors enabled, just like the guest view.
>            */
> -        vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
> -        vfio_msix_vector_release(&vdev->pdev, 0);
> +        ret = vfio_enable_msix_no_vec(vdev);
> +        if (ret) {
> +            error_report("vfio: failed to enable MSI-X, %d", ret);
> +        }
>       }
>   
>       trace_vfio_msix_enable(vdev->vbasedev.name);

Hi Cédric，

On 8/29/2023 10:04 PM, Cédric Le Goater wrote:
> On 8/22/23 09:29, Jing Liu wrote:
> > Guests typically enable MSI-X with all of the vectors masked in the
> > MSI-X vector table. To match the guest state of device, Qemu enables
> > MSI-X by
> 
> QEMU is preferred to Qemu.
Got it. 

> 
> > enabling vector 0 with userspace triggering and immediately release.
> > However the release function actually does not release it due to
> > already using userspace mode.
> >
> > It is no need to enable triggering on host and rely on the mask bit to
> > avoid spurious interrupts. Use an invalid fd (i.e. fd = -1) is enough
> > to get MSI-X enabled.
> >
> > After dynamic MSI-X allocation is supported, the interrupt restoring
> > also need use such way to enable MSI-X, therefore, create a function
> > for that.
> >
> > Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Jing Liu <jing2.liu@intel.com>
> > ---
> > Changes since RFC v1:
> > - A new patch. Use an invalid fd to get MSI-X enabled instead of using
> >    userspace triggering. (Alex)
> > ---
> >   hw/vfio/pci.c | 50 ++++++++++++++++++++++++++++++++++++++++++--------
> >   1 file changed, 42 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> > 31f36d68bb19..e24c21241a0c 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -369,6 +369,39 @@ static void vfio_msi_interrupt(void *opaque)
> >       notify(&vdev->pdev, nr);
> >   }
> >
> > +/*
> > + * Get MSI-X enabled, but no vector enabled, by setting vector 0 with
> > +an invalid
> > + * fd to kernel.
> > + */
> > +static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)> +{
> > +    struct vfio_irq_set *irq_set;
> 
> This could be a 'g_autofree' variable.

Thanks for pointing this to me. I just realized QEMU doc recommends 
g_autofree/g_autoptr to do automatic memory deallocation.

I will replace to g_autofree style in next version.

I have a question for a specific example (not related to this patch), and
I failed to find an example in current QEMU code to understand it.
If one g_autofree structure includes a pointer that we also allocate
space for the inside pointer, could g_autofree release the inside space?

struct dummy1 {
    int data;
    struct *p;
}
struct p {
    char member[];
}
void func() {
    g_autofree struct *dummy1 = NULL;

    dummy1 = g_malloc();
    dummy1->p = g_malloc();
    ...
    return;	// is this correct or need g_free(p)?
}

> 
> > +    int ret = 0, argsz;
> > +    int32_t *fd;
> > +
> > +    argsz = sizeof(*irq_set) + sizeof(*fd);
> > +
> > +    irq_set = g_malloc0(argsz);
> > +    irq_set->argsz = argsz;
> > +    irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> > +                     VFIO_IRQ_SET_ACTION_TRIGGER;
> > +    irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
> > +    irq_set->start = 0;
> > +    irq_set->count = 1;
> > +    fd = (int32_t *)&irq_set->data;
> > +    *fd = -1;
> > +
> > +    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> > +    if (ret) {
> > +        error_report("vfio: failed to enable MSI-X with vector 0 trick, %d",
> > +                     ret);
> 
> The above message seems redundant. I would simply return 'ret' and let the
> caller report the error. Same as vfio_enable_vectors().

Understood. Will change.

Thanks,
Jing

> Thanks,
> 
> C.
> 
> 
> > +    }
> > +
> > +    g_free(irq_set);
> > +
> > +    return ret;
> > +}
> > +
> >   static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
> >   {
> >       struct vfio_irq_set *irq_set;
> > @@ -618,6 +651,8 @@ static void
> > vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
> >
> >   static void vfio_msix_enable(VFIOPCIDevice *vdev)
> >   {
> > +    int ret;
> > +
> >       vfio_disable_interrupts(vdev);
> >
> >       vdev->msi_vectors = g_new0(VFIOMSIVector, vdev->msix->entries);
> > @@ -640,8 +675,6 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
> >       vfio_commit_kvm_msi_virq_batch(vdev);
> >
> >       if (vdev->nr_vectors) {
> > -        int ret;
> > -
> >           ret = vfio_enable_vectors(vdev, true);
> >           if (ret) {
> >               error_report("vfio: failed to enable vectors, %d", ret);
> > @@ -655,13 +688,14 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
> >            * MSI-X capability, but leaves the vector table masked.  We therefore
> >            * can't rely on a vector_use callback (from request_irq() in the guest)
> >            * to switch the physical device into MSI-X mode because that may come
> a
> > -         * long time after pci_enable_msix().  This code enables vector 0 with
> > -         * triggering to userspace, then immediately release the vector, leaving
> > -         * the physical device with no vectors enabled, but MSI-X enabled, just
> > -         * like the guest view.
> > +         * long time after pci_enable_msix().  This code sets vector 0 with an
> > +         * invalid fd to make the physical device MSI-X enabled, but with no
> > +         * vectors enabled, just like the guest view.
> >            */
> > -        vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
> > -        vfio_msix_vector_release(&vdev->pdev, 0);
> > +        ret = vfio_enable_msix_no_vec(vdev);
> > +        if (ret) {
> > +            error_report("vfio: failed to enable MSI-X, %d", ret);
> > +        }
> >       }
> >
> >       trace_vfio_msix_enable(vdev->vbasedev.name);

On Wed, 30 Aug 2023 10:03:33 +0000
"Liu, Jing2" <jing2.liu@intel.com> wrote:

> Hi Cédric，
> 
> On 8/29/2023 10:04 PM, Cédric Le Goater wrote:
> > On 8/22/23 09:29, Jing Liu wrote:  
> > > Guests typically enable MSI-X with all of the vectors masked in the
> > > MSI-X vector table. To match the guest state of device, Qemu enables
> > > MSI-X by  
> > 
> > QEMU is preferred to Qemu.  
> Got it. 
> 
> >   
> > > enabling vector 0 with userspace triggering and immediately release.
> > > However the release function actually does not release it due to
> > > already using userspace mode.
> > >
> > > It is no need to enable triggering on host and rely on the mask bit to
> > > avoid spurious interrupts. Use an invalid fd (i.e. fd = -1) is enough
> > > to get MSI-X enabled.
> > >
> > > After dynamic MSI-X allocation is supported, the interrupt restoring
> > > also need use such way to enable MSI-X, therefore, create a function
> > > for that.
> > >
> > > Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> > > Signed-off-by: Jing Liu <jing2.liu@intel.com>
> > > ---
> > > Changes since RFC v1:
> > > - A new patch. Use an invalid fd to get MSI-X enabled instead of using
> > >    userspace triggering. (Alex)
> > > ---
> > >   hw/vfio/pci.c | 50 ++++++++++++++++++++++++++++++++++++++++++--------
> > >   1 file changed, 42 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> > > 31f36d68bb19..e24c21241a0c 100644
> > > --- a/hw/vfio/pci.c
> > > +++ b/hw/vfio/pci.c
> > > @@ -369,6 +369,39 @@ static void vfio_msi_interrupt(void *opaque)
> > >       notify(&vdev->pdev, nr);
> > >   }
> > >
> > > +/*
> > > + * Get MSI-X enabled, but no vector enabled, by setting vector 0 with
> > > +an invalid
> > > + * fd to kernel.
> > > + */
> > > +static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)> +{
> > > +    struct vfio_irq_set *irq_set;  
> > 
> > This could be a 'g_autofree' variable.  
> 
> Thanks for pointing this to me. I just realized QEMU doc recommends 
> g_autofree/g_autoptr to do automatic memory deallocation.
> 
> I will replace to g_autofree style in next version.
> 
> I have a question for a specific example (not related to this patch), and
> I failed to find an example in current QEMU code to understand it.
> If one g_autofree structure includes a pointer that we also allocate
> space for the inside pointer, could g_autofree release the inside space?

it might be too cumbersome for 1-off use
see following for how 'auto' works https://docs.gtk.org/glib/auto-cleanup.html
 
> struct dummy1 {
>     int data;
>     struct *p;
> }
> struct p {
>     char member[];
> }
> void func() {
>     g_autofree struct *dummy1 = NULL;
> 
>     dummy1 = g_malloc();
>     dummy1->p = g_malloc();
>     ...
>     return;	// is this correct or need g_free(p)?
> }
> 
> >   
> > > +    int ret = 0, argsz;
> > > +    int32_t *fd;
> > > +
> > > +    argsz = sizeof(*irq_set) + sizeof(*fd);
> > > +
> > > +    irq_set = g_malloc0(argsz);
> > > +    irq_set->argsz = argsz;
> > > +    irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> > > +                     VFIO_IRQ_SET_ACTION_TRIGGER;
> > > +    irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
> > > +    irq_set->start = 0;
> > > +    irq_set->count = 1;
> > > +    fd = (int32_t *)&irq_set->data;
> > > +    *fd = -1;
> > > +
> > > +    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> > > +    if (ret) {
> > > +        error_report("vfio: failed to enable MSI-X with vector 0 trick, %d",
> > > +                     ret);  
> > 
> > The above message seems redundant. I would simply return 'ret' and let the
> > caller report the error. Same as vfio_enable_vectors().  
> 
> Understood. Will change.
> 
> Thanks,
> Jing
> 
> > Thanks,
> > 
> > C.
> > 
> >   
> > > +    }
> > > +
> > > +    g_free(irq_set);
> > > +
> > > +    return ret;
> > > +}
> > > +
> > >   static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool msix)
> > >   {
> > >       struct vfio_irq_set *irq_set;
> > > @@ -618,6 +651,8 @@ static void
> > > vfio_commit_kvm_msi_virq_batch(VFIOPCIDevice *vdev)
> > >
> > >   static void vfio_msix_enable(VFIOPCIDevice *vdev)
> > >   {
> > > +    int ret;
> > > +
> > >       vfio_disable_interrupts(vdev);
> > >
> > >       vdev->msi_vectors = g_new0(VFIOMSIVector, vdev->msix->entries);
> > > @@ -640,8 +675,6 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
> > >       vfio_commit_kvm_msi_virq_batch(vdev);
> > >
> > >       if (vdev->nr_vectors) {
> > > -        int ret;
> > > -
> > >           ret = vfio_enable_vectors(vdev, true);
> > >           if (ret) {
> > >               error_report("vfio: failed to enable vectors, %d", ret);
> > > @@ -655,13 +688,14 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
> > >            * MSI-X capability, but leaves the vector table masked.  We therefore
> > >            * can't rely on a vector_use callback (from request_irq() in the guest)
> > >            * to switch the physical device into MSI-X mode because that may come  
> > a  
> > > -         * long time after pci_enable_msix().  This code enables vector 0 with
> > > -         * triggering to userspace, then immediately release the vector, leaving
> > > -         * the physical device with no vectors enabled, but MSI-X enabled, just
> > > -         * like the guest view.
> > > +         * long time after pci_enable_msix().  This code sets vector 0 with an
> > > +         * invalid fd to make the physical device MSI-X enabled, but with no
> > > +         * vectors enabled, just like the guest view.
> > >            */
> > > -        vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
> > > -        vfio_msix_vector_release(&vdev->pdev, 0);
> > > +        ret = vfio_enable_msix_no_vec(vdev);
> > > +        if (ret) {
> > > +            error_report("vfio: failed to enable MSI-X, %d", ret);
> > > +        }
> > >       }
> > >
> > >       trace_vfio_msix_enable(vdev->vbasedev.name);  
> 

Hi Igor,

On Wed, August 30, 2023 6:49 PM, Igor Mammedov  wrote:
> 
> On Wed, 30 Aug 2023 10:03:33 +0000
> "Liu, Jing2" <jing2.liu@intel.com> wrote:
> 
...
> > > > +/*
> > > > + * Get MSI-X enabled, but no vector enabled, by setting vector 0
> > > > +with an invalid
> > > > + * fd to kernel.
> > > > + */
> > > > +static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)> +{
> > > > +    struct vfio_irq_set *irq_set;
> > >
> > > This could be a 'g_autofree' variable.
> >
> > Thanks for pointing this to me. I just realized QEMU doc recommends
> > g_autofree/g_autoptr to do automatic memory deallocation.
> >
> > I will replace to g_autofree style in next version.
> >
> > I have a question for a specific example (not related to this patch),
> > and I failed to find an example in current QEMU code to understand it.
> > If one g_autofree structure includes a pointer that we also allocate
> > space for the inside pointer, could g_autofree release the inside space?
> 
> it might be too cumbersome for 1-off use see following for how 'auto' works
> https://docs.gtk.org/glib/auto-cleanup.html

Thank you for the guide. It's now clear to me that for such type, there need define 
specific free function by macros and g_auto can help call it automatically. 

Jing

> 
> > struct dummy1 {
> >     int data;
> >     struct *p;
> > }
> > struct p {
> >     char member[];
> > }
> > void func() {
> >     g_autofree struct *dummy1 = NULL;
> >
> >     dummy1 = g_malloc();
> >     dummy1->p = g_malloc();
> >     ...
> >     return;	// is this correct or need g_free(p)?
> > }
> >
> > >
> > > > +    int ret = 0, argsz;
> > > > +    int32_t *fd;
> > > > +
> > > > +    argsz = sizeof(*irq_set) + sizeof(*fd);
> > > > +
> > > > +    irq_set = g_malloc0(argsz);
> > > > +    irq_set->argsz = argsz;
> > > > +    irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> > > > +                     VFIO_IRQ_SET_ACTION_TRIGGER;
> > > > +    irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
> > > > +    irq_set->start = 0;
> > > > +    irq_set->count = 1;
> > > > +    fd = (int32_t *)&irq_set->data;
> > > > +    *fd = -1;
> > > > +
> > > > +    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> > > > +    if (ret) {
> > > > +        error_report("vfio: failed to enable MSI-X with vector 0 trick, %d",
> > > > +                     ret);
> > >
> > > The above message seems redundant. I would simply return 'ret' and
> > > let the caller report the error. Same as vfio_enable_vectors().
> >
> > Understood. Will change.
> >
> > Thanks,
> > Jing
> >
> > > Thanks,
> > >
> > > C.