[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v3 7/9] vfio: add infrastructure to commit the deferred kvm r
From: |
Longpeng (Mike, Cloud Infrastructure Service Product Dept.) |
Subject: |
RE: [PATCH v3 7/9] vfio: add infrastructure to commit the deferred kvm routing |
Date: |
Fri, 8 Oct 2021 01:26:45 +0000 |
> -----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;
> > };