[Qemu-devel] [RFC][PATCH 36/45] qemu-kvm: Factor out kvm_device_msix_* services

[Jan Kiszka]

Create kvm_device_msix_{supported,init_vectors,set_vector,assign},
replacing the old kvm_assign_set_msix_{nr,entry} services. The new API
no longer requires direct fiddling with the KVM API data structures and
just takes the required parameters. kvm_device_msix_set_vector also
combines MSI route creation/update with registering the vector with the
device assignment kernel part. The routing information is now stored in
the msix_cache of the backing QEMU PCI device, maintained by the device
assigment code until we switch to generic MSI-X support.

Signed-off-by: Jan Kiszka <address@hidden>
---
 hw/device-assignment.c |  103 +++++++++++++++--------------------------------
 hw/device-assignment.h |    1 -
 qemu-kvm.c             |   42 +++++++++++++++++--
 qemu-kvm.h             |   11 +++--
 4 files changed, 76 insertions(+), 81 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 83951a3..2484afd 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -648,15 +648,13 @@ again:
 
 static QLIST_HEAD(, AssignedDevice) devs = QLIST_HEAD_INITIALIZER(devs);
 
-static void free_dev_irq_entries(AssignedDevice *dev)
+static void invalidate_msix_vectors(AssignedDevice *dev)
 {
     int i;
 
-    for (i = 0; i < dev->irq_entries_nr; i++)
-        kvm_del_routing_entry(&dev->entry[i]);
-    g_free(dev->entry);
-    dev->entry = NULL;
-    dev->irq_entries_nr = 0;
+    for (i = 0; i < dev->irq_entries_nr; i++) {
+        kvm_msi_cache_invalidate(&dev->dev.msix_cache[i]);
+    }
 }
 
 static void free_assigned_device(AssignedDevice *dev)
@@ -701,12 +699,12 @@ static void free_assigned_device(AssignedDevice *dev)
         close(dev->real_device.config_fd);
     }
 
-    free_dev_irq_entries(dev);
-
     if (dev->dev.msi_cache) {
         kvm_msi_cache_invalidate(&dev->dev.msi_cache[0]);
         g_free(dev->dev.msi_cache);
     }
+    invalidate_msix_vectors(dev);
+    g_free(dev->dev.msix_cache);
 }
 
 static uint32_t calc_assigned_dev_id(AssignedDevice *dev)
@@ -953,11 +951,12 @@ static int assigned_dev_set_msix_vectors(PCIDevice 
*pci_dev)
 {
     AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint16_t entries_nr = 0, entries_max_nr;
-    int pos = 0, i, r = 0;
-    uint32_t msg_addr, msg_upper_addr, msg_data;
-    struct kvm_assigned_msix_nr msix_nr;
-    struct kvm_assigned_msix_entry msix_entry;
     void *msix_page = adev->msix_table_page;
+    uint32_t dev_id;
+    MSIMessage msg;
+    int pos, i, r;
+
+    assert(adev->irq_entries_nr == 0);
 
     pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSIX);
 
@@ -980,72 +979,40 @@ static int assigned_dev_set_msix_vectors(PCIDevice 
*pci_dev)
         return -EINVAL;
     }
 
-    msix_nr.assigned_dev_id = calc_assigned_dev_id(adev);
-    msix_nr.entry_nr = entries_nr;
-    r = kvm_assign_set_msix_nr(kvm_state, &msix_nr);
-    if (r != 0) {
-        fprintf(stderr, "fail to set MSI-X entry number for MSIX! %s\n",
-                strerror(-r));
+    dev_id = calc_assigned_dev_id(adev);
+
+    r = kvm_device_msix_init_vectors(kvm_state, dev_id, entries_nr);
+    if (r < 0) {
         return r;
     }
-
-    free_dev_irq_entries(adev);
+    pci_dev->msix_cache = g_malloc0(entries_nr * sizeof(MSIRoutingCache));
     adev->irq_entries_nr = entries_nr;
-    adev->entry = g_malloc0(entries_nr * sizeof(*(adev->entry)));
 
-    msix_entry.assigned_dev_id = msix_nr.assigned_dev_id;
-    entries_nr = 0;
     for (i = 0; i < entries_max_nr; i++) {
-        if (entries_nr >= msix_nr.entry_nr) {
+        if (entries_nr == 0) {
             break;
         }
-        msg_data = pci_get_long(msix_page + i * PCI_MSIX_ENTRY_SIZE +
+        msg.data = pci_get_long(msix_page + i * PCI_MSIX_ENTRY_SIZE +
                                 PCI_MSIX_ENTRY_DATA);
-        if (msg_data == 0) {
+        if (msg.data == 0) {
             continue;
         }
-        msg_addr = pci_get_long(msix_page + i * PCI_MSIX_ENTRY_SIZE +
-                                PCI_MSIX_ENTRY_LOWER_ADDR);
-        msg_upper_addr = pci_get_long(msix_page + i * PCI_MSIX_ENTRY_SIZE +
-                                      PCI_MSIX_ENTRY_UPPER_ADDR);
+        msg.address = pci_get_quad(msix_page + i * PCI_MSIX_ENTRY_SIZE +
+                                   PCI_MSIX_ENTRY_LOWER_ADDR);
 
-        r = kvm_get_irq_route_gsi();
+        r = kvm_device_msix_set_vector(kvm_state, dev_id, i, &msg,
+                                       &pci_dev->msix_cache[i]);
         if (r < 0) {
             return r;
         }
-
-        adev->entry[entries_nr].gsi = r;
-        adev->entry[entries_nr].type = KVM_IRQ_ROUTING_MSI;
-        adev->entry[entries_nr].flags = 0;
-        adev->entry[entries_nr].u.msi.address_lo = msg_addr;
-        adev->entry[entries_nr].u.msi.address_hi = msg_upper_addr;
-        adev->entry[entries_nr].u.msi.data = msg_data;
-        DEBUG("MSI-X data 0x%x, MSI-X addr_lo 0x%x\n!", msg_data, msg_addr);
-        kvm_add_routing_entry(&adev->entry[entries_nr], NULL);
-
-        msix_entry.gsi = adev->entry[entries_nr].gsi;
-        msix_entry.entry = i;
-        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
-        if (r) {
-            fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
-            break;
-        }
-        DEBUG("MSI-X entry gsi 0x%x, entry %d\n!",
-              msix_entry.gsi, msix_entry.entry);
-        entries_nr++;
-    }
-
-    if (r == 0 && kvm_commit_irq_routes() < 0) {
-        perror("assigned_dev_update_msix_mmio: kvm_commit_irq_routes");
-        return -EINVAL;
+        entries_nr--;
     }
 
-    return r;
+    return 0;
 }
 
 static void assigned_dev_update_msix(PCIDevice *pci_dev)
 {
-    struct kvm_assigned_irq assigned_irq_data;
     AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint16_t ctrl_word = pci_get_word(pci_dev->config + pci_dev->msix_cap +
                                       PCI_MSIX_FLAGS);
@@ -1059,7 +1026,10 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
      * MSIX or intends to start. */
     if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) ||
         (ctrl_word & PCI_MSIX_FLAGS_ENABLE)) {
-        free_dev_irq_entries(assigned_dev);
+        invalidate_msix_vectors(assigned_dev);
+        g_free(pci_dev->msix_cache);
+        assigned_dev->irq_entries_nr = 0;
+
         r = kvm_device_irq_deassign(kvm_state, dev_id,
                                     assigned_dev->irq_requested_type);
         /* -ENXIO means no assigned irq */
@@ -1070,21 +1040,17 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
     }
 
     if (ctrl_word & PCI_MSIX_FLAGS_ENABLE) {
-        memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
-        assigned_irq_data.assigned_dev_id = dev_id;
-        assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX |
-                                  KVM_DEV_IRQ_GUEST_MSIX;
-
         if (assigned_dev_set_msix_vectors(pci_dev) < 0) {
             perror("assigned_dev_update_msix_mmio");
             return;
         }
-        if (kvm_assign_irq(kvm_state, &assigned_irq_data) < 0) {
+        if (kvm_device_msix_assign(kvm_state, dev_id) < 0) {
             perror("assigned_dev_enable_msix: assign irq");
             return;
         }
         assigned_dev->girq = -1;
-        assigned_dev->irq_requested_type = assigned_irq_data.flags;
+        assigned_dev->irq_requested_type = KVM_DEV_IRQ_HOST_MSIX |
+                                           KVM_DEV_IRQ_GUEST_MSIX;
     } else {
         assign_intx(assigned_dev);
     }
@@ -1193,10 +1159,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
     }
     /* Expose MSI-X capability */
     pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0);
-    /* Would really like to test kvm_check_extension(, KVM_CAP_DEVICE_MSIX),
-     * but the kernel doesn't expose it.  Instead do a dummy call to
-     * KVM_ASSIGN_SET_MSIX_NR to see if it exists. */
-    if (pos != 0 && kvm_assign_set_msix_nr(kvm_state, NULL) == -EFAULT) {
+    if (pos != 0 && kvm_device_msix_supported(kvm_state)) {
         int bar_nr;
         uint32_t msix_table_entry;
 
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index 1b4aecc..4b67f14 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -107,7 +107,6 @@ typedef struct AssignedDevice {
     uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
     uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
     int irq_entries_nr;
-    struct kvm_irq_routing_entry *entry;
     void *msix_table_page;
     target_phys_addr_t msix_table_addr;
     MemoryRegion mmio;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 27723a6..c9b348c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -617,15 +617,47 @@ int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, 
MSIMessage *msg,
 }
 
 #ifdef KVM_CAP_DEVICE_MSIX
-int kvm_assign_set_msix_nr(KVMState *s, struct kvm_assigned_msix_nr *msix_nr)
+bool kvm_device_msix_supported(KVMState *s)
 {
-    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
+    /* Would really like to test kvm_check_extension(, KVM_CAP_DEVICE_MSIX),
+     * but the kernel doesn't expose it.  Instead do a dummy call to
+     * KVM_ASSIGN_SET_MSIX_NR to see if it exists. */
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, NULL) == -EFAULT;
 }
 
-int kvm_assign_set_msix_entry(KVMState *s,
-                              struct kvm_assigned_msix_entry *entry)
+int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
+                                 uint32_t nr_vectors)
 {
-    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
+    struct kvm_assigned_msix_nr msix_nr;
+
+    msix_nr.assigned_dev_id = dev_id;
+    msix_nr.entry_nr = nr_vectors;
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
+}
+
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+                               MSIMessage *msg, MSIRoutingCache *cache)
+{
+    struct kvm_assigned_msix_entry msix_entry;
+    int ret;
+
+    ret = kvm_msi_message_update(msg, cache, MSI_ROUTE_STATIC);
+    if (ret < 0) {
+        return ret;
+    }
+    msix_entry.assigned_dev_id = dev_id;
+    msix_entry.gsi = cache->kvm_gsi;
+    msix_entry.entry = vector;
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
+}
+
+int kvm_device_msix_assign(KVMState *s, uint32_t dev_id)
+{
+    struct kvm_assigned_irq assigned_irq;
+
+    assigned_irq.assigned_dev_id = dev_id;
+    assigned_irq.flags = KVM_DEV_IRQ_HOST_MSIX | KVM_DEV_IRQ_GUEST_MSIX;
+    return kvm_vm_ioctl(s, KVM_ASSIGN_DEV_IRQ, &assigned_irq);
 }
 #endif
 
diff --git a/qemu-kvm.h b/qemu-kvm.h
index d987d41..552b668 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -154,6 +154,12 @@ int kvm_device_intx_assign(KVMState *s, uint32_t dev_id,
                            uint32_t host_irq_type, uint32_t guest_irq);
 int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, MSIMessage *msg,
                           MSIRoutingCache *cache);
+bool kvm_device_msix_supported(KVMState *s);
+int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
+                                 uint32_t nr_vectors);
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+                               MSIMessage *msg, MSIRoutingCache *cache);
+int kvm_device_msix_assign(KVMState *s, uint32_t dev_id);
 int kvm_device_irq_deassign(KVMState *s, uint32_t dev_id, uint32_t type);
 
 /*!
@@ -204,11 +210,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry 
*entry);
 int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
                              struct kvm_irq_routing_entry *newentry);
 
-
-int kvm_assign_set_msix_nr(KVMState *s, struct kvm_assigned_msix_nr *msix_nr);
-int kvm_assign_set_msix_entry(KVMState *s,
-                              struct kvm_assigned_msix_entry *entry);
-
 #else                           /* !CONFIG_KVM */
 
 struct kvm_pit_state {
-- 
1.7.3.4