[Qemu-devel] [RFC][PATCH 23/45] qemu-kvm: Rework MSI-X mask notifier to generic MSI config notifiers

[Jan Kiszka]

MSI config notifiers are supposed to be triggered on every relevant
configuration change of MSI vectors or if MSI is enabled/disabled.

Two notifiers are established, one for vector changes and one for general
enabling. The former notifier additionally passes the currently active
MSI message. This will allow to update potential in-kernel IRQ routes on
changes. The latter notifier is optional and will only be used by a
subset of clients.

These notifiers are currently only available for MSI-X but will be
extended to legacy MSI as well.

Signed-off-by: Jan Kiszka <address@hidden>
---
 hw/msix.c       |  119 +++++++++++++++++++++++++++++++++++++-----------------
 hw/msix.h       |    6 ++-
 hw/pci.h        |    8 ++-
 hw/virtio-pci.c |   24 ++++++------
 4 files changed, 102 insertions(+), 55 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 247b255..176bc76 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -219,16 +219,24 @@ static bool msix_is_masked(PCIDevice *dev, int vector)
           dev->msix_table_page[offset] & PCI_MSIX_ENTRY_CTRL_MASKBIT;
 }
 
-static void msix_handle_mask_update(PCIDevice *dev, int vector)
+static void msix_fire_vector_config_notifier(PCIDevice *dev,
+                                             unsigned int vector, bool masked)
 {
-    bool masked = msix_is_masked(dev, vector);
+    MSIMessage msg;
     int ret;
 
-    if (dev->msix_mask_notifier) {
-        ret = dev->msix_mask_notifier(dev, vector,
-                                      msix_is_masked(dev, vector));
+    if (dev->msix_vector_config_notifier) {
+        msix_message_from_vector(dev, vector, &msg);
+        ret = dev->msix_vector_config_notifier(dev, vector, &msg, masked);
         assert(ret >= 0);
     }
+}
+
+static void msix_handle_mask_update(PCIDevice *dev, int vector)
+{
+    bool masked = msix_is_masked(dev, vector);
+
+    msix_fire_vector_config_notifier(dev, vector, masked);
     if (!masked && msix_is_pending(dev, vector)) {
         msix_clr_pending(dev, vector);
         msix_notify(dev, vector);
@@ -240,20 +248,27 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
                        uint32_t old_val, int len)
 {
     unsigned enable_pos = dev->msix_cap + MSIX_CONTROL_OFFSET;
-    bool was_masked;
+    bool was_masked, was_enabled, is_enabled;
     int vector;
 
     if (!msix_present(dev) || !range_covers_byte(addr, len, enable_pos)) {
         return;
     }
 
-    if (!msix_enabled(dev)) {
+    old_val >>= (enable_pos - addr) * 8;
+
+    was_enabled = old_val & MSIX_ENABLE_MASK;
+    is_enabled = msix_enabled(dev);
+    if (was_enabled != is_enabled && dev->msix_enable_notifier) {
+        dev->msix_enable_notifier(dev, is_enabled);
+    }
+
+    if (!is_enabled) {
         return;
     }
 
     pci_device_deassert_intx(dev);
 
-    old_val >>= (enable_pos - addr) * 8;
     was_masked =
         (old_val & (MSIX_MASKALL_MASK | MSIX_ENABLE_MASK)) != MSIX_ENABLE_MASK;
     if (was_masked != msix_function_masked(dev)) {
@@ -270,15 +285,20 @@ static void msix_mmio_write(void *opaque, 
target_phys_addr_t addr,
     unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
     unsigned int vector = offset / PCI_MSIX_ENTRY_SIZE;
     bool was_masked = msix_is_masked(dev, vector);
+    bool is_masked;
 
     pci_set_long(dev->msix_table_page + offset, val);
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector));
     }
 
-    if (vector < dev->msix_entries_nr &&
-        was_masked != msix_is_masked(dev, vector)) {
-        msix_handle_mask_update(dev, vector);
+    if (vector < dev->msix_entries_nr) {
+        is_masked = msix_is_masked(dev, vector);
+        if (was_masked != is_masked) {
+            msix_handle_mask_update(dev, vector);
+        } else {
+            msix_fire_vector_config_notifier(dev, vector, is_masked);
+        }
     }
 }
 
@@ -305,17 +325,17 @@ static void msix_mmio_setup(PCIDevice *d, MemoryRegion 
*bar)
 
 static void msix_mask_all(struct PCIDevice *dev, unsigned nentries)
 {
-    int vector, r;
+    int vector;
+
     for (vector = 0; vector < nentries; ++vector) {
         unsigned offset =
             vector * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL;
         bool was_masked = msix_is_masked(dev, vector);
+
         dev->msix_table_page[offset] |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
-        if (was_masked != msix_is_masked(dev, vector) &&
-            dev->msix_mask_notifier) {
-            r = dev->msix_mask_notifier(dev, vector,
-                                        msix_is_masked(dev, vector));
-            assert(r >= 0);
+
+        if (!was_masked) {
+            msix_handle_mask_update(dev, vector);
         }
     }
 }
@@ -337,7 +357,6 @@ int msix_init(struct PCIDevice *dev, unsigned short 
nentries,
     if (nentries > MSIX_MAX_ENTRIES)
         return -EINVAL;
 
-    dev->msix_mask_notifier = NULL;
     dev->msix_entry_used = g_malloc0(MSIX_MAX_ENTRIES *
                                         sizeof *dev->msix_entry_used);
 
@@ -529,36 +548,50 @@ void msix_unuse_all_vectors(PCIDevice *dev)
 }
 
 /* Invoke the notifier if vector entry is used and unmasked. */
-static int msix_notify_if_unmasked(PCIDevice *dev, unsigned vector, int masked)
+static int
+msix_notify_if_unmasked(PCIDevice *dev, unsigned int vector, bool masked)
 {
-    assert(dev->msix_mask_notifier);
+    MSIMessage msg;
+
+    assert(dev->msix_vector_config_notifier);
+
     if (!dev->msix_entry_used[vector] || msix_is_masked(dev, vector)) {
         return 0;
     }
-    return dev->msix_mask_notifier(dev, vector, masked);
+    msix_message_from_vector(dev, vector, &msg);
+    return dev->msix_vector_config_notifier(dev, vector, &msg, masked);
 }
 
-static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+static int
+msix_set_config_notifier_for_vector(PCIDevice *dev, unsigned int vector)
 {
-       /* Notifier has been set. Invoke it on unmasked vectors. */
-       return msix_notify_if_unmasked(dev, vector, 0);
+    /* Notifier has been set. Invoke it on unmasked vectors. */
+    return msix_notify_if_unmasked(dev, vector, false);
 }
 
-static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+static int
+msix_unset_config_notifier_for_vector(PCIDevice *dev, unsigned int vector)
 {
-       /* Notifier will be unset. Invoke it to mask unmasked entries. */
-       return msix_notify_if_unmasked(dev, vector, 1);
+    /* Notifier will be unset. Invoke it to mask unmasked entries. */
+    return msix_notify_if_unmasked(dev, vector, true);
 }
 
-int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f)
+int msix_set_config_notifiers(PCIDevice *dev,
+                              MSIEnableNotifier enable_notifier,
+                              MSIVectorConfigNotifier vector_config_notifier)
 {
     int r, n;
-    assert(!dev->msix_mask_notifier);
-    dev->msix_mask_notifier = f;
+
+    dev->msix_enable_notifier = enable_notifier;
+    dev->msix_vector_config_notifier = vector_config_notifier;
+
+    if (enable_notifier && msix_enabled(dev)) {
+        enable_notifier(dev, true);
+    }
     if ((dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
         (MSIX_ENABLE_MASK | MSIX_MASKALL_MASK)) == MSIX_ENABLE_MASK) {
         for (n = 0; n < dev->msix_entries_nr; ++n) {
-            r = msix_set_mask_notifier_for_vector(dev, n);
+            r = msix_set_config_notifier_for_vector(dev, n);
             if (r < 0) {
                 goto undo;
             }
@@ -568,31 +601,41 @@ int msix_set_mask_notifier(PCIDevice *dev, 
msix_mask_notifier_func f)
 
 undo:
     while (--n >= 0) {
-        msix_unset_mask_notifier_for_vector(dev, n);
+        msix_unset_config_notifier_for_vector(dev, n);
     }
-    dev->msix_mask_notifier = NULL;
+    if (enable_notifier && msix_enabled(dev)) {
+        enable_notifier(dev, false);
+    }
+    dev->msix_enable_notifier = NULL;
+    dev->msix_vector_config_notifier = NULL;
     return r;
 }
 
-int msix_unset_mask_notifier(PCIDevice *dev)
+int msix_unset_config_notifiers(PCIDevice *dev)
 {
     int r, n;
-    assert(dev->msix_mask_notifier);
+
+    assert(dev->msix_vector_config_notifier);
+
     if ((dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
         (MSIX_ENABLE_MASK | MSIX_MASKALL_MASK)) == MSIX_ENABLE_MASK) {
         for (n = 0; n < dev->msix_entries_nr; ++n) {
-            r = msix_unset_mask_notifier_for_vector(dev, n);
+            r = msix_unset_config_notifier_for_vector(dev, n);
             if (r < 0) {
                 goto undo;
             }
         }
     }
-    dev->msix_mask_notifier = NULL;
+    if (dev->msix_enable_notifier && msix_enabled(dev)) {
+        dev->msix_enable_notifier(dev, false);
+    }
+    dev->msix_enable_notifier = NULL;
+    dev->msix_vector_config_notifier = NULL;
     return 0;
 
 undo:
     while (--n >= 0) {
-        msix_set_mask_notifier_for_vector(dev, n);
+        msix_set_config_notifier_for_vector(dev, n);
     }
     return r;
 }
diff --git a/hw/msix.h b/hw/msix.h
index 685dbe2..978f417 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -29,6 +29,8 @@ void msix_notify(PCIDevice *dev, unsigned vector);
 
 void msix_reset(PCIDevice *dev);
 
-int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func);
-int msix_unset_mask_notifier(PCIDevice *dev);
+int msix_set_config_notifiers(PCIDevice *dev,
+                              MSIEnableNotifier enable_notifier,
+                              MSIVectorConfigNotifier vector_config_notifier);
+int msix_unset_config_notifiers(PCIDevice *dev);
 #endif
diff --git a/hw/pci.h b/hw/pci.h
index 0177df4..4249c6a 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -127,8 +127,9 @@ enum {
     QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
 };
 
-typedef int (*msix_mask_notifier_func)(PCIDevice *, unsigned vector,
-                                      int masked);
+typedef void (*MSIEnableNotifier)(PCIDevice *dev, bool enabled);
+typedef int (*MSIVectorConfigNotifier)(PCIDevice *dev, unsigned int vector,
+                                       MSIMessage *msg, bool masked);
 
 struct PCIDevice {
     DeviceState qdev;
@@ -210,7 +211,8 @@ struct PCIDevice {
      * on the rest of the region. */
     target_phys_addr_t msix_page_size;
 
-    msix_mask_notifier_func msix_mask_notifier;
+    MSIEnableNotifier msix_enable_notifier;
+    MSIVectorConfigNotifier msix_vector_config_notifier;
 };
 
 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index ad6a002..6718945 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -520,8 +520,8 @@ static void virtio_pci_guest_notifier_read(void *opaque)
     }
 }
 
-static int virtio_pci_mask_vq(PCIDevice *dev, unsigned vector,
-                              VirtQueue *vq, int masked)
+static int virtio_pci_mask_vq(PCIDevice *dev, unsigned int vector,
+                              VirtQueue *vq, bool masked)
 {
     EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
     int r = kvm_msi_irqfd_set(&dev->msix_cache[vector],
@@ -540,8 +540,8 @@ static int virtio_pci_mask_vq(PCIDevice *dev, unsigned 
vector,
     return 0;
 }
 
-static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector,
-                                    int masked)
+static int virtio_pci_msi_vector_config(PCIDevice *dev, unsigned int vector,
+                                        MSIMessage *msg, bool masked)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
     VirtIODevice *vdev = proxy->vdev;
@@ -608,11 +608,11 @@ static int virtio_pci_set_guest_notifiers(void *opaque, 
bool assign)
     VirtIODevice *vdev = proxy->vdev;
     int r, n;
 
-    /* Must unset mask notifier while guest notifier
+    /* Must unset vector config notifier while guest notifier
      * is still assigned */
     if (!assign) {
-           r = msix_unset_mask_notifier(&proxy->pci_dev);
-            assert(r >= 0);
+        r = msix_unset_config_notifiers(&proxy->pci_dev);
+        assert(r >= 0);
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
@@ -626,11 +626,11 @@ static int virtio_pci_set_guest_notifiers(void *opaque, 
bool assign)
         }
     }
 
-    /* Must set mask notifier after guest notifier
+    /* Must set vector config notifier after guest notifier
      * has been assigned */
     if (assign) {
-        r = msix_set_mask_notifier(&proxy->pci_dev,
-                                   virtio_pci_mask_notifier);
+        r = msix_set_config_notifiers(&proxy->pci_dev, NULL,
+                                      virtio_pci_msi_vector_config);
         if (r < 0) {
             goto assign_error;
         }
@@ -645,8 +645,8 @@ assign_error:
     }
 
     if (!assign) {
-        msix_set_mask_notifier(&proxy->pci_dev,
-                               virtio_pci_mask_notifier);
+        msix_set_config_notifiers(&proxy->pci_dev, NULL,
+                                  virtio_pci_msi_vector_config);
     }
     return r;
 }
-- 
1.7.3.4