[Qemu-devel] [RFC][PATCH 42/45] msix: Introduce msix_init_simple

[Jan Kiszka]

Devices models are usually not interested in specifying MSI-X
configuration details beyond the number of vectors to provide and the
BAR number to use. Layout of an exclusively used BAR and its
registration can also be handled centrally.

This is the purpose of msix_init_simple. It provides handy services to
the existing users. Future users like device assignment may require more
detailed setup specification. For them we will (re-)introduce msix_init
with the full list of configuration option (in contrast to the current
code).

Signed-off-by: Jan Kiszka <address@hidden>
---
 hw/ivshmem.c    |    6 +-----
 hw/msix.c       |   35 ++++++++++++++---------------------
 hw/msix.h       |    7 +++----
 hw/virtio-pci.c |   15 +++++----------
 hw/virtio-pci.h |    1 -
 5 files changed, 23 insertions(+), 41 deletions(-)

diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index a402c98..d9dbd18 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -65,7 +65,6 @@ typedef struct IVShmemState {
      */
     MemoryRegion bar;
     MemoryRegion ivshmem;
-    MemoryRegion msix_bar;
     uint64_t ivshmem_size; /* size of shared memory region */
     int shm_fd; /* shared memory file descriptor */
 
@@ -539,10 +538,7 @@ static void ivshmem_setup_msi(IVShmemState *s)
 {
     /* allocate the MSI-X vectors */
 
-    memory_region_init(&s->msix_bar, "ivshmem-msix", 4096);
-    if (!msix_init(&s->dev, s->vectors, &s->msix_bar, 1, 0)) {
-        pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
-                         &s->msix_bar);
+    if (!msix_init_simple(&s->dev, s->vectors, 1)) {
         IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
     } else {
         IVSHMEM_DPRINTF("msix initialization failed\n");
diff --git a/hw/msix.c b/hw/msix.c
index bccd8b1..258b9c1 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -244,17 +244,6 @@ static const MemoryRegionOps msix_mmio_ops = {
     },
 };
 
-static void msix_mmio_setup(PCIDevice *d, MemoryRegion *bar)
-{
-    uint8_t *config = d->config + d->msix_cap;
-    uint32_t table = pci_get_long(config + PCI_MSIX_TABLE);
-    uint32_t offset = table & ~(MSIX_PAGE_SIZE - 1);
-    /* TODO: for assigned devices, we'll want to make it possible to map
-     * pending bits separately in case they are in a separate bar. */
-
-    memory_region_add_subregion(bar, offset, &d->msix_mmio);
-}
-
 static void msix_mask_all(struct PCIDevice *dev, unsigned nentries)
 {
     int vector;
@@ -272,11 +261,9 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned 
nentries)
     }
 }
 
-/* Initialize the MSI-X structures. Note: if MSI-X is supported, BAR size is
- * modified, it should be retrieved with msix_bar_size. */
-int msix_init(struct PCIDevice *dev, unsigned short nentries,
-              MemoryRegion *bar,
-              unsigned bar_nr, unsigned bar_size)
+/* Initialize the MSI-X structures in a single dedicated BAR
+ * and register it. */
+int msix_init_simple(PCIDevice *dev, unsigned short nentries, unsigned bar_nr)
 {
     int ret;
 
@@ -296,14 +283,16 @@ int msix_init(struct PCIDevice *dev, unsigned short 
nentries,
                           "msix", MSIX_PAGE_SIZE);
 
     dev->msix_entries_nr = nentries;
-    ret = msix_add_config(dev, nentries, bar_nr, bar_size);
+    ret = msix_add_config(dev, nentries, bar_nr, 0);
     if (ret)
         goto err_config;
 
     dev->msix_cache = g_malloc0(nentries * sizeof *dev->msix_cache);
 
     dev->cap_present |= QEMU_PCI_CAP_MSIX;
-    msix_mmio_setup(dev, bar);
+
+    pci_register_bar(dev, bar_nr, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                     &dev->msix_mmio);
     return 0;
 
 err_config:
@@ -315,10 +304,10 @@ err_config:
 }
 
 /* Clean up resources for the device. */
-int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
+void msix_uninit(PCIDevice *dev, MemoryRegion *bar)
 {
     if (!msix_present(dev)) {
-        return 0;
+        return;
     }
     pci_del_capability(dev, PCI_CAP_ID_MSIX, MSIX_CAP_LENGTH);
     dev->msix_cap = 0;
@@ -332,7 +321,11 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
     g_free(dev->msix_cache);
 
     dev->cap_present &= ~QEMU_PCI_CAP_MSIX;
-    return 0;
+}
+
+void msix_uninit_simple(PCIDevice *dev)
+{
+    msix_uninit(dev, &dev->msix_mmio);
 }
 
 void msix_save(PCIDevice *dev, QEMUFile *f)
diff --git a/hw/msix.h b/hw/msix.h
index dfc6087..56e7ba5 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,14 +4,13 @@
 #include "qemu-common.h"
 #include "pci.h"
 
-int msix_init(PCIDevice *pdev, unsigned short nentries,
-              MemoryRegion *bar,
-              unsigned bar_nr, unsigned bar_size);
+int msix_init_simple(PCIDevice *dev, unsigned short nentries, unsigned bar_nr);
 
 void msix_write_config(PCIDevice *pci_dev, uint32_t address,
                        uint32_t old_val, int len);
 
-int msix_uninit(PCIDevice *d, MemoryRegion *bar);
+void msix_uninit(PCIDevice *d, MemoryRegion *bar);
+void msix_uninit_simple(PCIDevice *d);
 
 void msix_save(PCIDevice *dev, QEMUFile *f);
 void msix_load(PCIDevice *dev, QEMUFile *f);
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 5004d7d..6fe2b5e 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -713,13 +713,10 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice 
*vdev)
     pci_set_word(config + 0x2e, vdev->device_id);
     config[0x3d] = 1;
 
-    memory_region_init(&proxy->msix_bar, "virtio-msix", 4096);
-    if (vdev->nvectors && !msix_init(&proxy->pci_dev, vdev->nvectors,
-                                     &proxy->msix_bar, 1, 0)) {
-        pci_register_bar(&proxy->pci_dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
-                         &proxy->msix_bar);
-    } else
+    if (vdev->nvectors &&
+        msix_init_simple(&proxy->pci_dev, vdev->nvectors, 1)) {
         vdev->nvectors = 0;
+    }
 
     proxy->pci_dev.config_write = virtio_write_config;
 
@@ -766,12 +763,10 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
 static int virtio_exit_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-    int r;
 
     memory_region_destroy(&proxy->bar);
-    r = msix_uninit(pci_dev, &proxy->msix_bar);
-    memory_region_destroy(&proxy->msix_bar);
-    return r;
+    msix_uninit_simple(pci_dev);
+    return 0;
 }
 
 static int virtio_blk_exit_pci(PCIDevice *pci_dev)
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index 14c10f7..5af1c8c 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -22,7 +22,6 @@ typedef struct {
     PCIDevice pci_dev;
     VirtIODevice *vdev;
     MemoryRegion bar;
-    MemoryRegion msix_bar;
     uint32_t flags;
     uint32_t class_code;
     uint32_t nvectors;
-- 
1.7.3.4