[Qemu-devel] [RFC][PATCH 16/45] qemu-kvm: Use MSIMessage and MSIRoutingCache

[Jan Kiszka]

Start benefiting from the new abstractions and drop the KVM-specific
vector tracking to generic MSIMessage and MSIRoutingCache data
structures and helpers, also reducing the diff to upstream.

Signed-off-by: Jan Kiszka <address@hidden>
---
 hw/msi.c        |   49 +++++++++++--------------------------------------
 hw/msix.c       |   37 +++++++++----------------------------
 hw/pci.h        |    4 ----
 hw/virtio-pci.c |    3 ++-
 kvm-stub.c      |    6 +++---
 kvm.h           |   13 +++----------
 qemu-kvm.c      |   46 +++++++++++++++++++++++++++++-----------------
 7 files changed, 57 insertions(+), 101 deletions(-)

diff --git a/hw/msi.c b/hw/msi.c
index c8ccb17..b947104 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -140,49 +140,29 @@ static void msi_message_from_vector(PCIDevice *dev, 
uint16_t msi_flags,
     }
 }
 
-static void kvm_msi_message_from_vector(PCIDevice *dev, unsigned vector,
-                                        KVMMsiMessage *kmm)
-{
-    uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
-    bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
-    unsigned int nr_vectors = msi_nr_vectors(flags);
-
-    kmm->addr_lo = pci_get_long(dev->config + msi_address_lo_off(dev));
-    if (msi64bit) {
-        kmm->addr_hi = pci_get_long(dev->config + msi_address_hi_off(dev));
-    } else {
-        kmm->addr_hi = 0;
-    }
-
-    kmm->data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
-    if (nr_vectors > 1) {
-        kmm->data &= ~(nr_vectors - 1);
-        kmm->data |= vector;
-    }
-}
-
 static void kvm_msi_update(PCIDevice *dev)
 {
     uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
     unsigned int max_vectors = 1 <<
         ((flags & PCI_MSI_FLAGS_QMASK) >> (ffs(PCI_MSI_FLAGS_QMASK) - 1));
     unsigned int nr_vectors = msi_nr_vectors(flags);
-    KVMMsiMessage new_entry, *entry;
+    MSIRoutingCache *cache;
     bool changed = false;
     unsigned int vector;
+    MSIMessage msg;
     int r;
 
     for (vector = 0; vector < max_vectors; vector++) {
-        entry = dev->msi_irq_entries + vector;
+        cache = &dev->msi_cache[vector];
 
         if (vector >= nr_vectors) {
             if (vector < dev->msi_entries_nr) {
-                kvm_msi_message_del(entry);
+                kvm_msi_message_del(cache);
                 changed = true;
             }
         } else if (vector >= dev->msi_entries_nr) {
-            kvm_msi_message_from_vector(dev, vector, entry);
-            r = kvm_msi_message_add(entry);
+            msi_message_from_vector(dev, flags, vector, &msg);
+            r = kvm_msi_message_add(&msg, cache);
             if (r) {
                 fprintf(stderr, "%s: kvm_msi_add failed: %s\n", __func__,
                         strerror(-r));
@@ -190,15 +170,14 @@ static void kvm_msi_update(PCIDevice *dev)
             }
             changed = true;
         } else {
-            kvm_msi_message_from_vector(dev, vector, &new_entry);
-            r = kvm_msi_message_update(entry, &new_entry);
+            msi_message_from_vector(dev, flags, vector, &msg);
+            r = kvm_msi_message_update(&msg, cache);
             if (r < 0) {
                 fprintf(stderr, "%s: kvm_update_msi failed: %s\n",
                         __func__, strerror(-r));
                 exit(1);
             }
             if (r > 0) {
-                *entry = new_entry;
                 changed = true;
             }
         }
@@ -220,7 +199,7 @@ static void kvm_msi_free(PCIDevice *dev)
     unsigned int vector;
 
     for (vector = 0; vector < dev->msi_entries_nr; ++vector) {
-        kvm_msi_message_del(&dev->msi_irq_entries[vector]);
+        kvm_msi_message_del(&dev->msi_cache[vector]);
     }
     if (dev->msi_entries_nr > 0) {
         kvm_commit_irq_routes();
@@ -290,11 +269,6 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
 
     dev->msi_cache = g_malloc0(nr_vectors * sizeof(*dev->msi_cache));
 
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        dev->msi_irq_entries = g_malloc(nr_vectors *
-                                        sizeof(*dev->msix_irq_entries));
-    }
-
     return config_offset;
 }
 
@@ -311,7 +285,6 @@ void msi_uninit(struct PCIDevice *dev)
 
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_msi_free(dev);
-        g_free(dev->msi_irq_entries);
     }
 
     g_free(dev->msi_cache);
@@ -383,7 +356,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
     }
 
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_set_irq(dev->msi_irq_entries[vector].gsi, 1, NULL);
+        kvm_set_irq(dev->msi_cache[vector].kvm_gsi, 1, NULL);
         return;
     }
 
@@ -504,7 +477,7 @@ void msi_post_load(PCIDevice *dev)
 {
     uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
 
-    if (kvm_enabled() && dev->msi_irq_entries) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_msi_free(dev);
 
         if (flags & PCI_MSI_FLAGS_ENABLE) {
diff --git a/hw/msix.c b/hw/msix.c
index e824aef..0be022e 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -49,7 +49,7 @@ static void kvm_msix_free(PCIDevice *dev)
 
     for (vector = 0; vector < dev->msix_entries_nr; ++vector) {
         if (dev->msix_entry_used[vector]) {
-            kvm_msi_message_del(&dev->msix_irq_entries[vector]);
+            kvm_msi_message_del(&dev->msix_cache[vector]);
             changed = 1;
         }
     }
@@ -58,21 +58,11 @@ static void kvm_msix_free(PCIDevice *dev)
     }
 }
 
-static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,
-                                         KVMMsiMessage *kmm)
-{
-    uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
-
-    kmm->addr_lo = pci_get_long(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR);
-    kmm->addr_hi = pci_get_long(table_entry + PCI_MSIX_ENTRY_UPPER_ADDR);
-    kmm->data = pci_get_long(table_entry + PCI_MSIX_ENTRY_DATA);
-}
-
 static void kvm_msix_update(PCIDevice *dev, int vector,
                             int was_masked, int is_masked)
 {
-    KVMMsiMessage new_entry, *entry;
     int mask_cleared = was_masked && !is_masked;
+    MSIMessage msg;
     int r;
 
     /* It is only legal to change an entry when it is masked. Therefore, it is
@@ -84,16 +74,14 @@ static void kvm_msix_update(PCIDevice *dev, int vector,
         return;
     }
 
-    entry = dev->msix_irq_entries + vector;
-    kvm_msix_message_from_vector(dev, vector, &new_entry);
-    r = kvm_msi_message_update(entry, &new_entry);
+    msix_message_from_vector(dev, vector, &msg);
+    r = kvm_msi_message_update(&msg, &dev->msix_cache[vector]);
     if (r < 0) {
         fprintf(stderr, "%s: kvm_update_msix failed: %s\n", __func__,
                 strerror(-r));
         exit(1);
     }
     if (r > 0) {
-        *entry = new_entry;
         r = kvm_commit_irq_routes();
         if (r) {
             fprintf(stderr, "%s: kvm_commit_irq_routes failed: %s\n", __func__,
@@ -105,11 +93,11 @@ static void kvm_msix_update(PCIDevice *dev, int vector,
 
 static int kvm_msix_vector_add(PCIDevice *dev, unsigned vector)
 {
-    KVMMsiMessage *kmm = dev->msix_irq_entries + vector;
+    MSIMessage msg;
     int r;
 
-    kvm_msix_message_from_vector(dev, vector, kmm);
-    r = kvm_msi_message_add(kmm);
+    msix_message_from_vector(dev, vector, &msg);
+    r = kvm_msi_message_add(&msg, &dev->msix_cache[vector]);
     if (r < 0) {
         fprintf(stderr, "%s: kvm_add_msix failed: %s\n", __func__, 
strerror(-r));
         return r;
@@ -125,7 +113,7 @@ static int kvm_msix_vector_add(PCIDevice *dev, unsigned 
vector)
 
 static void kvm_msix_vector_del(PCIDevice *dev, unsigned vector)
 {
-    kvm_msi_message_del(&dev->msix_irq_entries[vector]);
+    kvm_msi_message_del(&dev->msix_cache[vector]);
     kvm_commit_irq_routes();
 }
 
@@ -360,11 +348,6 @@ int msix_init(struct PCIDevice *dev, unsigned short 
nentries,
 
     dev->msix_cache = g_malloc0(nentries * sizeof *dev->msix_cache);
 
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        dev->msix_irq_entries = g_malloc(nentries *
-                                         sizeof *dev->msix_irq_entries);
-    }
-
     dev->cap_present |= QEMU_PCI_CAP_MSIX;
     msix_mmio_setup(dev, bar);
     return 0;
@@ -409,8 +392,6 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
     dev->msix_table_page = NULL;
     g_free(dev->msix_entry_used);
     dev->msix_entry_used = NULL;
-    g_free(dev->msix_irq_entries);
-    dev->msix_irq_entries = NULL;
 
     g_free(dev->msix_cache);
 
@@ -477,7 +458,7 @@ void msix_notify(PCIDevice *dev, unsigned vector)
     }
 
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
+        kvm_set_irq(dev->msix_cache[vector].kvm_gsi, 1, NULL);
         return;
     }
 
diff --git a/hw/pci.h b/hw/pci.h
index 5b5d2fd..0177df4 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -6,7 +6,6 @@
 
 #include "qdev.h"
 #include "memory.h"
-#include "kvm.h"
 
 /* PCI includes legacy ISA access.  */
 #include "isa.h"
@@ -203,7 +202,6 @@ struct PCIDevice {
 
     /* MSI entries */
     int msi_entries_nr;
-    struct KVMMsiMessage *msi_irq_entries;
 
     /* How much space does an MSIX table need. */
     /* The spec requires giving the table structure
@@ -212,8 +210,6 @@ struct PCIDevice {
      * on the rest of the region. */
     target_phys_addr_t msix_page_size;
 
-    KVMMsiMessage *msix_irq_entries;
-
     msix_mask_notifier_func msix_mask_notifier;
 };
 
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 615295e..23880e0 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -21,6 +21,7 @@
 #include "virtio-serial.h"
 #include "pci.h"
 #include "qemu-error.h"
+#include "msi.h"
 #include "msix.h"
 #include "net.h"
 #include "loader.h"
@@ -523,7 +524,7 @@ static int virtio_pci_mask_vq(PCIDevice *dev, unsigned 
vector,
                               VirtQueue *vq, int masked)
 {
     EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
-    int r = kvm_set_irqfd(dev->msix_irq_entries[vector].gsi,
+    int r = kvm_set_irqfd(dev->msix_cache[vector].kvm_gsi,
                           event_notifier_get_fd(notifier),
                           !masked);
     if (r < 0) {
diff --git a/kvm-stub.c b/kvm-stub.c
index c98170e..ca4382a 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -140,17 +140,17 @@ int kvm_get_irq_route_gsi(void)
     return -ENOSYS;
 }
 
-int kvm_msi_message_add(KVMMsiMessage *msg)
+int kvm_msi_message_add(MSIMessage *msg, MSIRoutingCache *cache)
 {
     return -ENOSYS;
 }
 
-int kvm_msi_message_del(KVMMsiMessage *msg)
+int kvm_msi_message_del(MSIRoutingCache *cache)
 {
     return -ENOSYS;
 }
 
-int kvm_msi_message_update(KVMMsiMessage *old, KVMMsiMessage *new)
+int kvm_msi_message_update(MSIMessage *msg, MSIRoutingCache *cache)
 {
     return -ENOSYS;
 }
diff --git a/kvm.h b/kvm.h
index b15e1dd..3706fc6 100644
--- a/kvm.h
+++ b/kvm.h
@@ -200,20 +200,13 @@ int kvm_set_irqfd(int gsi, int fd, bool assigned);
 
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool 
assign);
 
-typedef struct KVMMsiMessage {
-    uint32_t gsi;
-    uint32_t addr_lo;
-    uint32_t addr_hi;
-    uint32_t data;
-} KVMMsiMessage;
-
 int kvm_has_gsi_routing(void);
 int kvm_allows_irq0_override(void);
 int kvm_get_irq_route_gsi(void);
 
-int kvm_msi_message_add(KVMMsiMessage *msg);
-int kvm_msi_message_del(KVMMsiMessage *msg);
-int kvm_msi_message_update(KVMMsiMessage *old, KVMMsiMessage *new);
+int kvm_msi_message_add(MSIMessage *msg, MSIRoutingCache *cache);
+int kvm_msi_message_del(MSIRoutingCache *cache);
+int kvm_msi_message_update(MSIMessage *msg, MSIRoutingCache *cache);
 
 int kvm_commit_irq_routes(void);
 
diff --git a/qemu-kvm.c b/qemu-kvm.c
index e8dc537..253cf75 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -19,6 +19,7 @@
 #include "gdbstub.h"
 #include "monitor.h"
 #include "cpus.h"
+#include "hw/msi.h"
 
 #include "qemu-kvm.h"
 
@@ -442,18 +443,18 @@ int kvm_get_irq_route_gsi(void)
 }
 
 static void kvm_msi_routing_entry(struct kvm_irq_routing_entry *e,
-                                  KVMMsiMessage *msg)
+                                  MSIRoutingCache *cache)
 
 {
-    e->gsi = msg->gsi;
+    e->gsi = cache->kvm_gsi;
     e->type = KVM_IRQ_ROUTING_MSI;
     e->flags = 0;
-    e->u.msi.address_lo = msg->addr_lo;
-    e->u.msi.address_hi = msg->addr_hi;
-    e->u.msi.data = msg->data;
+    e->u.msi.address_lo = (uint32_t)cache->msg.address;
+    e->u.msi.address_hi = cache->msg.address >> 32;
+    e->u.msi.data = cache->msg.data;
 }
 
-int kvm_msi_message_add(KVMMsiMessage *msg)
+int kvm_msi_message_add(MSIMessage *msg, MSIRoutingCache *cache)
 {
     struct kvm_irq_routing_entry e;
     int ret;
@@ -462,37 +463,48 @@ int kvm_msi_message_add(KVMMsiMessage *msg)
     if (ret < 0) {
         return ret;
     }
-    msg->gsi = ret;
+    cache->msg = *msg;
+    cache->type = MSI_ROUTE_STATIC;
+    cache->kvm_gsi = ret;
+    cache->kvm_irqfd = -1;
 
-    kvm_msi_routing_entry(&e, msg);
+    kvm_msi_routing_entry(&e, cache);
     return kvm_add_routing_entry(&e);
 }
 
-int kvm_msi_message_del(KVMMsiMessage *msg)
+int kvm_msi_message_del(MSIRoutingCache *cache)
 {
     struct kvm_irq_routing_entry e;
 
-    kvm_msi_routing_entry(&e, msg);
+    kvm_msi_routing_entry(&e, cache);
     return kvm_del_routing_entry(&e);
 }
 
-int kvm_msi_message_update(KVMMsiMessage *old, KVMMsiMessage *new)
+int kvm_msi_message_update(MSIMessage *msg, MSIRoutingCache *cache)
 {
-    struct kvm_irq_routing_entry e1, e2;
+    struct kvm_irq_routing_entry old, new;
+    MSIRoutingCache new_cache;
     int ret;
 
-    new->gsi = old->gsi;
-    if (memcmp(old, new, sizeof(KVMMsiMessage)) == 0) {
+    assert(cache->type != MSI_ROUTE_NONE);
+
+    if (msg->address == cache->msg.address && msg->data == cache->msg.data) {
         return 0;
     }
 
-    kvm_msi_routing_entry(&e1, old);
-    kvm_msi_routing_entry(&e2, new);
+    kvm_msi_routing_entry(&old, cache);
+
+    new_cache.msg = *msg;
+    new_cache.type = cache->type;
+    new_cache.kvm_gsi = cache->kvm_gsi;
+    new_cache.kvm_irqfd = cache->kvm_irqfd;
+    kvm_msi_routing_entry(&new, &new_cache);
 
-    ret = kvm_update_routing_entry(&e1, &e2);
+    ret = kvm_update_routing_entry(&old, &new);
     if (ret < 0) {
         return ret;
     }
+    *cache = new_cache;
 
     return 1;
 }
-- 
1.7.3.4