[v3,7/9] vfio: add infrastructure to commit the deferred kvm routing

'defer_kvm_irq_routing' indicates whether we should defer to commit
the kvm routing.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
 hw/vfio/pci.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 hw/vfio/pci.h |  1 +
 2 files changed, 43 insertions(+), 1 deletion(-)

On Tue, 21 Sep 2021 07:02:00 +0800
"Longpeng(Mike)" <longpeng2@huawei.com> wrote:

> 'defer_kvm_irq_routing' indicates whether we should defer to commit
> the kvm routing.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  hw/vfio/pci.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  hw/vfio/pci.h |  1 +
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 8e97ca93cf..8fe238b11d 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -423,12 +423,24 @@ static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
>          return;
>      }
>  
> -    virq = kvm_irqchip_add_msi_route(kvm_state, vector_n, &vdev->pdev);
> +    virq = kvm_irqchip_add_deferred_msi_route(kvm_state, vector_n, &vdev->pdev);
>      if (virq < 0) {
>          event_notifier_cleanup(&vector->kvm_interrupt);
>          return;
>      }
>  
> +    if (vdev->defer_kvm_irq_routing) {
> +        /*
> +         * Hold the allocated virq in vector->virq temporarily, will
> +         * reset it to -1 when we fail to add the corresponding irqfd
> +         * in vfio_commit_kvm_msi_virq().

s/when/if/

> +         */
> +        vector->virq = virq;

Do we need to make this unique to the deferred case or could we use
vector->virq directly and fill it with -1 on all error paths like we do
on a failure in vfio_commit_kvm_msi_virq()?


> +        return;
> +    }
> +
> +    kvm_irqchip_commit_routes(kvm_state);
> +
>      if (kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, &vector->kvm_interrupt,
>                                         NULL, virq) < 0) {
>          kvm_irqchip_release_virq(kvm_state, virq);
> @@ -567,6 +579,35 @@ static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
>      }
>  }
>  
> +/* TODO: invoked when enclabe msi/msix vectors */

"enclabe"?  Is this meant to be "enable"?

> +static __attribute__((unused)) void vfio_commit_kvm_msi_virq(VFIOPCIDevice *vdev)

I'd move this function, if not this entire change, to patch 9 rather
than adding these attributes for an unused function.  Thanks,

Alex

> +{
> +    int i;
> +    VFIOMSIVector *vector;
> +
> +    if (!vdev->defer_kvm_irq_routing || !vdev->nr_vectors) {
> +        return;
> +    }
> +
> +    kvm_irqchip_commit_routes(kvm_state);
> +
> +    for (i = 0; i < vdev->nr_vectors; i++) {
> +        vector = &vdev->msi_vectors[i];
> +
> +        if (!vector->use || vector->virq < 0) {
> +            continue;
> +        }
> +
> +        if (kvm_irqchip_add_irqfd_notifier_gsi(kvm_state,
> +                                               &vector->kvm_interrupt,
> +                                               NULL, vector->virq) < 0) {
> +            kvm_irqchip_release_virq(kvm_state, vector->virq);
> +            event_notifier_cleanup(&vector->kvm_interrupt);
> +            vector->virq = -1;
> +        }
> +    }
> +}
> +
>  static void vfio_msix_enable(VFIOPCIDevice *vdev)
>  {
>      PCIDevice *pdev = &vdev->pdev;
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 64777516d1..d3c5177d37 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -171,6 +171,7 @@ struct VFIOPCIDevice {
>      bool no_kvm_ioeventfd;
>      bool no_vfio_ioeventfd;
>      bool enable_ramfb;
> +    bool defer_kvm_irq_routing;
>      VFIODisplay *dpy;
>      Notifier irqchip_change_notifier;
>  };

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Saturday, October 2, 2021 7:05 AM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpeng2@huawei.com>
> Cc: philmd@redhat.com; pbonzini@redhat.com; marcel.apfelbaum@gmail.com;
> mst@redhat.com; qemu-devel@nongnu.org; Gonglei (Arei)
> <arei.gonglei@huawei.com>; chenjiashang <chenjiashang@huawei.com>
> Subject: Re: [PATCH v3 7/9] vfio: add infrastructure to commit the deferred kvm
> routing
> 
> On Tue, 21 Sep 2021 07:02:00 +0800
> "Longpeng(Mike)" <longpeng2@huawei.com> wrote:
> 
> > 'defer_kvm_irq_routing' indicates whether we should defer to commit
> > the kvm routing.
> >
> > Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> > ---
> >  hw/vfio/pci.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> >  hw/vfio/pci.h |  1 +
> >  2 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 8e97ca93cf..8fe238b11d 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -423,12 +423,24 @@ static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev,
> VFIOMSIVector *vector,
> >          return;
> >      }
> >
> > -    virq = kvm_irqchip_add_msi_route(kvm_state, vector_n, &vdev->pdev);
> > +    virq = kvm_irqchip_add_deferred_msi_route(kvm_state, vector_n,
> &vdev->pdev);
> >      if (virq < 0) {
> >          event_notifier_cleanup(&vector->kvm_interrupt);
> >          return;
> >      }
> >
> > +    if (vdev->defer_kvm_irq_routing) {
> > +        /*
> > +         * Hold the allocated virq in vector->virq temporarily, will
> > +         * reset it to -1 when we fail to add the corresponding irqfd
> > +         * in vfio_commit_kvm_msi_virq().
> 
> s/when/if/
> 

OK, thanks.

> > +         */
> > +        vector->virq = virq;
> 
> Do we need to make this unique to the deferred case or could we use
> vector->virq directly and fill it with -1 on all error paths like we do
> on a failure in vfio_commit_kvm_msi_virq()?
> 

OK, I will use vector->irq directly, it looks neater.

> 
> > +        return;
> > +    }
> > +
> > +    kvm_irqchip_commit_routes(kvm_state);
> > +
> >      if (kvm_irqchip_add_irqfd_notifier_gsi(kvm_state,
> &vector->kvm_interrupt,
> >                                         NULL, virq) < 0) {
> >          kvm_irqchip_release_virq(kvm_state, virq);
> > @@ -567,6 +579,35 @@ static void vfio_msix_vector_release(PCIDevice *pdev,
> unsigned int nr)
> >      }
> >  }
> >
> > +/* TODO: invoked when enclabe msi/msix vectors */
> 
> "enclabe"?  Is this meant to be "enable"?
> 

Yes, it's a typo.

> > +static __attribute__((unused)) void vfio_commit_kvm_msi_virq(VFIOPCIDevice
> *vdev)
> 
> I'd move this function, if not this entire change, to patch 9 rather
> than adding these attributes for an unused function.  Thanks,
> 

OK. I think I should merge this patch into patch 9 entirely if we decide to
move this function.

> Alex
> 
> > +{
> > +    int i;
> > +    VFIOMSIVector *vector;
> > +
> > +    if (!vdev->defer_kvm_irq_routing || !vdev->nr_vectors) {
> > +        return;
> > +    }
> > +
> > +    kvm_irqchip_commit_routes(kvm_state);
> > +
> > +    for (i = 0; i < vdev->nr_vectors; i++) {
> > +        vector = &vdev->msi_vectors[i];
> > +
> > +        if (!vector->use || vector->virq < 0) {
> > +            continue;
> > +        }
> > +
> > +        if (kvm_irqchip_add_irqfd_notifier_gsi(kvm_state,
> > +                                               &vector->kvm_interrupt,
> > +                                               NULL, vector->virq) < 0) {
> > +            kvm_irqchip_release_virq(kvm_state, vector->virq);
> > +            event_notifier_cleanup(&vector->kvm_interrupt);
> > +            vector->virq = -1;
> > +        }
> > +    }
> > +}
> > +
> >  static void vfio_msix_enable(VFIOPCIDevice *vdev)
> >  {
> >      PCIDevice *pdev = &vdev->pdev;
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> > index 64777516d1..d3c5177d37 100644
> > --- a/hw/vfio/pci.h
> > +++ b/hw/vfio/pci.h
> > @@ -171,6 +171,7 @@ struct VFIOPCIDevice {
> >      bool no_kvm_ioeventfd;
> >      bool no_vfio_ioeventfd;
> >      bool enable_ramfb;
> > +    bool defer_kvm_irq_routing;
> >      VFIODisplay *dpy;
> >      Notifier irqchip_change_notifier;
> >  };