[PATCH 1/5] vfio/pci: Disable INTx fast path if using split irqchip

Peter Xu posted 5 patches 5 years, 8 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Alex Williamson <alex.williamson@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[PATCH 1/5] vfio/pci: Disable INTx fast path if using split irqchip
Posted by Peter Xu 5 years, 8 months ago
It's currently broken.  Let's use the slow path to at least make it
functional.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/vfio/pci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5e75a95129..98e0e0c994 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -128,6 +128,18 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
         return;
     }
 
+    if (kvm_irqchip_is_split()) {
+        /*
+         * VFIO INTx is currently not working with split kernel
+         * irqchip for level triggered interrupts.  Go the slow path
+         * as long as split is enabled so we can be at least
+         * functional (even with poor performance).
+         *
+         * TODO: Remove this after all things fixed up.
+         */
+        return;
+    }
+
     /* Get to a known interrupt state */
     qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
     vfio_mask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
-- 
2.24.1


Re: [PATCH 1/5] vfio/pci: Disable INTx fast path if using split irqchip
Posted by Auger Eric 5 years, 8 months ago
Hi Peter,

On 2/26/20 11:50 PM, Peter Xu wrote:
> It's currently broken.  Let's use the slow path to at least make it
> functional.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

> ---
>  hw/vfio/pci.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 5e75a95129..98e0e0c994 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -128,6 +128,18 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>          return;
>      }
>  
> +    if (kvm_irqchip_is_split()) {
> +        /*
> +         * VFIO INTx is currently not working with split kernel
> +         * irqchip for level triggered interrupts.  Go the slow path
> +         * as long as split is enabled so we can be at least
> +         * functional (even with poor performance).
> +         *
> +         * TODO: Remove this after all things fixed up.
If this patch were to be applied sooner than the other patches as
suggested in the cover letter, We may emit a warning message saying that
slow path is selected due to split irqchip and this will induce perf
downgrade
> +         */
> +        return;
> +    }
> +
>      /* Get to a known interrupt state */
>      qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
>      vfio_mask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
> 


Tested with a 5.2-rc1 kernel with reverted "654f1f13ea56  kvm: Check
irqchip mode before assign irqfd" and guest booting with nomsi.

Without this patch the assigned NIC does not work. This patch fixes the
issue.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


Re: [PATCH 1/5] vfio/pci: Disable INTx fast path if using split irqchip
Posted by Peter Xu 5 years, 8 months ago
On Thu, Feb 27, 2020 at 05:53:32PM +0100, Auger Eric wrote:
> Hi Peter,
> 
> On 2/26/20 11:50 PM, Peter Xu wrote:
> > It's currently broken.  Let's use the slow path to at least make it
> > functional.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> > ---
> >  hw/vfio/pci.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 5e75a95129..98e0e0c994 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -128,6 +128,18 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
> >          return;
> >      }
> >  
> > +    if (kvm_irqchip_is_split()) {
> > +        /*
> > +         * VFIO INTx is currently not working with split kernel
> > +         * irqchip for level triggered interrupts.  Go the slow path
> > +         * as long as split is enabled so we can be at least
> > +         * functional (even with poor performance).
> > +         *
> > +         * TODO: Remove this after all things fixed up.
> If this patch were to be applied sooner than the other patches as
> suggested in the cover letter, We may emit a warning message saying that
> slow path is selected due to split irqchip and this will induce perf
> downgrade

Yes we can.  Here I followed the rest of the cases where we'll
silently fallback to slow path if e.g. we used an even older kernel
that does not support resamplefd at all.  IMHO it's the same as that
(feature not supported yet, silent fallback, just in case it scares
the user a bit, which makes some sense).

> > +         */
> > +        return;
> > +    }
> > +
> >      /* Get to a known interrupt state */
> >      qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
> >      vfio_mask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
> > 
> 
> 
> Tested with a 5.2-rc1 kernel with reverted "654f1f13ea56  kvm: Check
> irqchip mode before assign irqfd" and guest booting with nomsi.
> 
> Without this patch the assigned NIC does not work. This patch fixes the
> issue.
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks Eric!

-- 
Peter Xu