virtio: don't poll masked vectors with irqfd

At the moment, when irqfd is in use but a vector is masked, qemu will poll it and handle vector masks in userspace. Since almost no one ever looks at the pending bits, it is better to defer this until pending bits
are actually read.
Implement this optimization using the new poll notifier.

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 hw/virtio-pci.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 12 deletions(-)

hw/virtio-pci.c +30 -5

index 1c03bb5..bc6b4e0 100644
@@ -509,8 +509,6 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
         }
         return ret;
     }
-
-    virtio_queue_set_guest_notifier_fd_handler(vq, true, true);
     return 0;
 }
 

@@ -529,8 +527,6 @@ static void kvm_virtio_pci_vq_vector_release(VirtIOPCIProxy *proxy,
     if (--irqfd->users == 0) {
         kvm_irqchip_release_virq(kvm_state, irqfd->virq);
     }
-
-    virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
 }
 
 static int kvm_virtio_pci_vector_use(PCIDevice *dev, unsigned vector,

@@ -581,7 +577,36 @@ static void kvm_virtio_pci_vector_release(PCIDevice *dev, unsigned vector)
     }
 }
 
-static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign)
+static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
+                                       unsigned int vector_start,
+                                       unsigned int vector_end)
+{
+    VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
+    VirtIODevice *vdev = proxy->vdev;
+    int queue_no;
+    unsigned int vector;
+    EventNotifier *notifier;
+    VirtQueue *vq;
+
+    for (queue_no = 0; queue_no < VIRTIO_PCI_QUEUE_MAX; queue_no++) {
+        if (!virtio_queue_get_num(vdev, queue_no)) {
+            break;
+        }
+        vector = virtio_queue_vector(vdev, queue_no);
+        if (vector < vector_start || vector >= vector_end ||
+            !msix_is_masked(dev, vector)) {
+            continue;
+        }
+        vq = virtio_get_queue(vdev, queue_no);
+        notifier = virtio_queue_get_guest_notifier(vq);
+        if (event_notifier_test_and_clear(notifier)) {
+            msix_set_pending(dev, vector);
+        }
+    }
+}
+
+static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
+                                         bool with_irqfd)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
     VirtQueue *vq = virtio_get_queue(proxy->vdev, n);

hw/virtio-pci.c +8 -5

         event_notifier_cleanup(notifier);
     }
 

@@ -612,9 +637,11 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, bool assign)
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
     VirtIODevice *vdev = proxy->vdev;
     int r, n;
+    bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
+        kvm_msi_via_irqfd_enabled();
 
     /* Must unset vector notifier while guest notifier is still assigned */
-    if (kvm_msi_via_irqfd_enabled() && !assign) {
+    if (with_irqfd && !assign) {
         msix_unset_vector_notifiers(&proxy->pci_dev);
         g_free(proxy->vector_irqfd);
         proxy->vector_irqfd = NULL;

@@ -625,21 +652,22 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, bool assign)
             break;
         }
 
-        r = virtio_pci_set_guest_notifier(d, n, assign);
+        r = virtio_pci_set_guest_notifier(d, n, assign,
+                                          kvm_msi_via_irqfd_enabled());
         if (r < 0) {
             goto assign_error;
         }
     }
 
     /* Must set vector notifier after guest notifier has been assigned */
-    if (kvm_msi_via_irqfd_enabled() && assign) {
+    if (with_irqfd && assign) {
         proxy->vector_irqfd =
             g_malloc0(sizeof(*proxy->vector_irqfd) *
                       msix_nr_vectors_allocated(&proxy->pci_dev));
         r = msix_set_vector_notifiers(&proxy->pci_dev,
                                       kvm_virtio_pci_vector_use,
                                       kvm_virtio_pci_vector_release,
-                                      NULL);
+                                      kvm_virtio_pci_vector_poll);
         if (r < 0) {
             goto assign_error;
         }

@@ -651,7 +679,7 @@ assign_error:
     /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
     assert(assign);
     while (--n >= 0) {
-        virtio_pci_set_guest_notifier(d, n, !assign);
+        virtio_pci_set_guest_notifier(d, n, !assign, with_irqfd);
     }
     return r;
 }

Quoted 73 lines · Michael S. Tsirkin

At the moment, when irqfd is in use but a vector is masked, qemu will poll it and handle vector masks in userspace. Since almost no one ever looks at the pending bits, it is better to defer this until pending bits
are actually read.
Implement this optimization using the new poll notifier.

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 hw/virtio-pci.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 12 deletions(-)

hw/virtio-pci.c +30 -5

index 1c03bb5..bc6b4e0 100644
@@ -509,8 +509,6 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
         }
         return ret;
     }
-
-    virtio_queue_set_guest_notifier_fd_handler(vq, true, true);
     return 0;
 }
 

@@ -529,8 +527,6 @@ static void kvm_virtio_pci_vq_vector_release(VirtIOPCIProxy *proxy,
     if (--irqfd->users == 0) {
         kvm_irqchip_release_virq(kvm_state, irqfd->virq);
     }
-
-    virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
 }
 
 static int kvm_virtio_pci_vector_use(PCIDevice *dev, unsigned vector,

@@ -581,7 +577,36 @@ static void kvm_virtio_pci_vector_release(PCIDevice *dev, unsigned vector)
     }
 }
 
-static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign)
+static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
+                                       unsigned int vector_start,
+                                       unsigned int vector_end)
+{
+    VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
+    VirtIODevice *vdev = proxy->vdev;
+    int queue_no;
+    unsigned int vector;
+    EventNotifier *notifier;
+    VirtQueue *vq;
+
+    for (queue_no = 0; queue_no < VIRTIO_PCI_QUEUE_MAX; queue_no++) {
+        if (!virtio_queue_get_num(vdev, queue_no)) {
+            break;
+        }
+        vector = virtio_queue_vector(vdev, queue_no);
+        if (vector < vector_start || vector >= vector_end ||
+            !msix_is_masked(dev, vector)) {
+            continue;
+        }
+        vq = virtio_get_queue(vdev, queue_no);
+        notifier = virtio_queue_get_guest_notifier(vq);
+        if (event_notifier_test_and_clear(notifier)) {
+            msix_set_pending(dev, vector);
+        }
+    }
+}
+
+static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
+                                         bool with_irqfd)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
     VirtQueue *vq = virtio_get_queue(proxy->vdev, n);

hw/virtio-pci.c static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign) · line 620

Quoted 5 lines

hw/virtio-pci.c +1 -1

@@ -592,9 +617,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign)
         if (r < 0) {
             return r;
         }
-        virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
+        virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);

+ virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);

hw/virtio-pci.c static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign) · line 622

Quoted 3 lines

hw/virtio-pci.c +1 -1

     } else {
-        virtio_queue_set_guest_notifier_fd_handler(vq, false, false);
+        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);

+ virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);

Use the 'assign' variable instead of the constants?

Quoted 52 lines

hw/virtio-pci.c +8 -5

         event_notifier_cleanup(notifier);
     }
 

@@ -612,9 +637,11 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, bool assign)
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
     VirtIODevice *vdev = proxy->vdev;
     int r, n;
+    bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
+        kvm_msi_via_irqfd_enabled();
 
     /* Must unset vector notifier while guest notifier is still assigned */
-    if (kvm_msi_via_irqfd_enabled() && !assign) {
+    if (with_irqfd && !assign) {
         msix_unset_vector_notifiers(&proxy->pci_dev);
         g_free(proxy->vector_irqfd);
         proxy->vector_irqfd = NULL;

@@ -625,21 +652,22 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, bool assign)
             break;
         }
 
-        r = virtio_pci_set_guest_notifier(d, n, assign);
+        r = virtio_pci_set_guest_notifier(d, n, assign,
+                                          kvm_msi_via_irqfd_enabled());
         if (r < 0) {
             goto assign_error;
         }
     }
 
     /* Must set vector notifier after guest notifier has been assigned */
-    if (kvm_msi_via_irqfd_enabled() && assign) {
+    if (with_irqfd && assign) {
         proxy->vector_irqfd =
             g_malloc0(sizeof(*proxy->vector_irqfd) *
                       msix_nr_vectors_allocated(&proxy->pci_dev));
         r = msix_set_vector_notifiers(&proxy->pci_dev,
                                       kvm_virtio_pci_vector_use,
                                       kvm_virtio_pci_vector_release,
-                                      NULL);
+                                      kvm_virtio_pci_vector_poll);
         if (r < 0) {
             goto assign_error;
         }

@@ -651,7 +679,7 @@ assign_error:
     /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
     assert(assign);
     while (--n >= 0) {
-        virtio_pci_set_guest_notifier(d, n, !assign);
+        virtio_pci_set_guest_notifier(d, n, !assign, with_irqfd);
     }
     return r;
 }

Signature · 1 line

Asias

hw/virtio-pci.c +15 -1

-static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign)
+static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
+                                       unsigned int vector_start,
+                                       unsigned int vector_end)
+{
+    VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
+    VirtIODevice *vdev = proxy->vdev;
+    int queue_no;
+    unsigned int vector;
+    EventNotifier *notifier;
+    VirtQueue *vq;
+
+    for (queue_no = 0; queue_no < VIRTIO_PCI_QUEUE_MAX; queue_no++) {
+        if (!virtio_queue_get_num(vdev, queue_no)) {
+            break;
+        }

At the moment when vector is masked virtio will poll it in userspace, even if it is handled by irqfd.
This is done in order to update pending bits, but it's not really required until someone reads the pending bits. On the other hand this read results in extra io thread wakeups.

As we only implement the pending bits as a compatibility feature (read - real drivers don't use it), we can defer the irqfd poll until the read actually happens.

This does not seem to affect vhost-net speed
in simple benchmarks but could help block: both vhost-blk and dataplane when using irqfd,
and I also think this is cleaner than enabling/disabling notifiers all the time.

This will also be the basis for future optimizations.

At the moment when vector is masked virtio will poll it in userspace, even if it is handled by irqfd.
This is done in order to update pending bits, but it's not really required until someone reads the pending bits. On the other hand this read results in extra io thread wakeups.

As we only implement the pending bits as a compatibility feature (read - real drivers don't use it), we can defer the irqfd poll until the read actually happens.

This does not seem to affect vhost-net speed
in simple benchmarks but could help block: both vhost-blk and dataplane when using irqfd,
and I also think this is cleaner than enabling/disabling notifiers all the time.

This will also be the basis for future optimizations.

Michael S. Tsirkin (3):
msi: add API to get notified about pending bit poll
msix: expose access to masked/pending state
virtio-pci: don't poll masked vectors

 hw/pci/msix.c   | 19 +++++++++++++++----
 hw/pci/msix.h   |  6 +++++-
 hw/pci/pci.h    |  4 ++++
 hw/vfio_pci.c   |  2 +-
 hw/virtio-pci.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
 5 files changed, 66 insertions(+), 18 deletions(-)

Quoted 23 lines · Michael S. Tsirkin

At the moment when vector is masked virtio will poll it in userspace, even if it is handled by irqfd.
This is done in order to update pending bits, but it's not really required until someone reads the pending bits. On the other hand this read results in extra io thread wakeups.

As we only implement the pending bits as a compatibility feature (read - real drivers don't use it), we can defer the irqfd poll until the read actually happens.

This does not seem to affect vhost-net speed
in simple benchmarks but could help block: both vhost-blk and dataplane when using irqfd,
and I also think this is cleaner than enabling/disabling notifiers all the time.

This will also be the basis for future optimizations.

Michael S. Tsirkin (3):
msi: add API to get notified about pending bit poll
msix: expose access to masked/pending state
virtio-pci: don't poll masked vectors

Quoted 6 lines

 hw/pci/msix.c   | 19 +++++++++++++++----
 hw/pci/msix.h   |  6 +++++-
 hw/pci/pci.h    |  4 ++++
 hw/vfio_pci.c   |  2 +-
 hw/virtio-pci.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
 5 files changed, 66 insertions(+), 18 deletions(-)

The performance boost is significant here. It is close to the result of dropping msix_fire_vector_notifier() hack.

Signature · 1 line

Asias