RE: [PATCH v3 8/9] Revert "vfio: Avoid disabling and enabling vectors repeatedly in VFIO migration"

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

> -----Original Message-----
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Saturday, October 2, 2021 7:04 AM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpe...@huawei.com>
> Cc: phi...@redhat.com; pbonz...@redhat.com; marcel.apfelb...@gmail.com;
> m...@redhat.com; qemu-devel@nongnu.org; Gonglei (Arei)
> <arei.gong...@huawei.com>; chenjiashang <chenjiash...@huawei.com>
> Subject: Re: [PATCH v3 8/9] Revert "vfio: Avoid disabling and enabling vectors
> repeatedly in VFIO migration"
> 
> On Tue, 21 Sep 2021 07:02:01 +0800
> "Longpeng(Mike)" <longpe...@huawei.com> wrote:
> 
> > Commit ecebe53fe993 ("vfio: Avoid disabling and enabling vectors
> > repeatedly in VFIO migration") avoid inefficiently disabling and
> 
> s/avoid/avoids/
> 
> > enabling vectors repeatedly and let the unmasked vectors to be
> 
> s/let/lets/  s/to//
> 
> > enabled one by one.
> >
> > But we want to batch multiple routes and defer the commit, and only
> > commit once out side the loop of setting vector notifiers, so we
> 
> s/out side/outside/
> 
> > cannot to enable the vectors one by one in the loop now.
> 
> s/to//
> 

Thanks. All the typos and grammatical errors you pointed out will
be fixed in v4.

> Thanks,
> Alex
> 
> >
> > Revert that commit and we will take another way in the next patch,
> > it can not only avoid disabling/enabling vectors repeatedly, but
> > also satisfy our requirement of defer to commit.
> >
> > Signed-off-by: Longpeng(Mike) <longpe...@huawei.com>
> > ---
> >  hw/vfio/pci.c | 20 +++-----------------
> >  1 file changed, 3 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 8fe238b11d..2de1cc5425 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -610,9 +610,6 @@ static __attribute__((unused)) void
> vfio_commit_kvm_msi_virq(VFIOPCIDevice *vdev
> >
> >  static void vfio_msix_enable(VFIOPCIDevice *vdev)
> >  {
> > -    PCIDevice *pdev = &vdev->pdev;
> > -    unsigned int nr, max_vec = 0;
> > -
> >      vfio_disable_interrupts(vdev);
> >
> >      vdev->msi_vectors = g_new0(VFIOMSIVector, vdev->msix->entries);
> > @@ -631,22 +628,11 @@ static void vfio_msix_enable(VFIOPCIDevice *vdev)
> >       * 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.
> > -     * If there are already unmasked vectors (in migration resume phase and
> > -     * some guest startups) which will be enabled soon, we can allocate all
> > -     * of them here to avoid inefficiently disabling and enabling vectors
> > -     * repeatedly later.
> >       */
> > -    if (!pdev->msix_function_masked) {
> > -        for (nr = 0; nr < msix_nr_vectors_allocated(pdev); nr++) {
> > -            if (!msix_is_masked(pdev, nr)) {
> > -                max_vec = nr;
> > -            }
> > -        }
> > -    }
> > -    vfio_msix_vector_do_use(pdev, max_vec, NULL, NULL);
> > -    vfio_msix_vector_release(pdev, max_vec);
> > +    vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
> > +    vfio_msix_vector_release(&vdev->pdev, 0);
> >
> > -    if (msix_set_vector_notifiers(pdev, vfio_msix_vector_use,
> > +    if (msix_set_vector_notifiers(&vdev->pdev, vfio_msix_vector_use,
> >                                    vfio_msix_vector_release, NULL)) {
> >          error_report("vfio: msix_set_vector_notifiers failed");
> >      }