[Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Alex Williamson]

v3:
 - more patches, smaller diff, must be headed in the right direction
 - macros for all hardcoded values in msix_init_exclusive_bar
 - fold msix_add_config into msix_init allowing less churn to moving
   around msix_uninit
 - note native endian bug
 - split msix_mmio_read move to separate patch
 - split changing return value of msix_uninit to separate patch

Thanks,

Alex

v2:
 - split patch
 - rename msix_[un]init_bar() to msix_[un]init_exclusive_bar()
 - add the cherry on top to cleaning up PCIDevice naming

v1:

msix_init has very little configurability as to how it lays out MSIX
for a device.  It claims to resize BARs, but doesn't actually do this
anymore.  This patch allows MSIX to be fully specified, which is
necessary both for emulated devices trying to match the physical
layout of a hardware device as well as for any kind of device
assignment.

New functions msix_init_bar & msix_uninit_bar provide wrappers around
the more detailed functions for drivers that just want a simple MSIX
setup.

---

Alex Williamson (8):
      msix: Switch msix_uninit to return void
      msix: Allow full specification of MSIX layout
      msix: Split PBA into it's own MemoryRegion
      msix: Note endian TODO item
      msix: Move msix_mmio_read
      virtio: Convert to msix_init_exclusive_bar() interface
      ivshmem: Convert to msix_init_exclusive_bar() interface
      msix: Add simple BAR allocation MSIX setup functions


 hw/ivshmem.c    |   10 +-
 hw/msix.c       |  271 +++++++++++++++++++++++++++++++------------------------
 hw/msix.h       |   13 ++-
 hw/pci.h        |   12 ++
 hw/virtio-pci.c |   15 +--
 hw/virtio-pci.h |    1 
 6 files changed, 176 insertions(+), 146 deletions(-)

[Qemu-devel] [PATCH v3 1/8] msix: Add simple BAR allocation MSIX setup functions

[Alex Williamson]

msi_init() takes over a BAR without really specifying or allowing
specification of how it does so.  Instead, let's split it into
two interfaces, one fully specified, and one trivially easy.  This
implements the latter.  msix_init_exclusive_bar() takes over
allocating and filling a PCI BAR _exclusively_ for the use of MSIX.
When used, the matching msi_uninit_exclusive_bar() should be used
to tear it down.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/msix.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/msix.h |    3 +++
 hw/pci.h  |    2 ++
 3 files changed, 52 insertions(+)

diff --git a/hw/msix.c b/hw/msix.c
index b64f109..bafea94 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -299,6 +299,45 @@ err_config:
     return ret;
 }
 
+int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
+                            uint8_t bar_nr)
+{
+    int ret;
+    char *name;
+
+    /*
+     * Migration compatibility dictates that this remains a 4k
+     * BAR with the vector table in the lower half and PBA in
+     * the upper half.  Do not use these elsewhere!
+     */
+#define MSIX_EXCLUSIVE_BAR_SIZE 4096
+#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2)
+
+    if (nentries * PCI_MSIX_ENTRY_SIZE > MSIX_EXCLUSIVE_BAR_PBA_OFFSET) {
+        return -EINVAL;
+    }
+
+    if (asprintf(&name, "%s-msix", dev->name) == -1) {
+        return -ENOMEM;
+    }
+
+    memory_region_init(&dev->msix_exclusive_bar, name, MSIX_EXCLUSIVE_BAR_SIZE);
+
+    free(name);
+
+    ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr,
+                    MSIX_EXCLUSIVE_BAR_SIZE);
+    if (ret) {
+        memory_region_destroy(&dev->msix_exclusive_bar);
+        return ret;
+    }
+
+    pci_register_bar(dev, bar_nr, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                     &dev->msix_exclusive_bar);
+
+    return 0;
+}
+
 static void msix_free_irq_entries(PCIDevice *dev)
 {
     int vector;
@@ -329,6 +368,14 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
     return 0;
 }
 
+void msix_uninit_exclusive_bar(PCIDevice *dev)
+{
+    if (msix_present(dev)) {
+        msix_uninit(dev, &dev->msix_exclusive_bar);
+        memory_region_destroy(&dev->msix_exclusive_bar);
+    }
+}
+
 void msix_save(PCIDevice *dev, QEMUFile *f)
 {
     unsigned n = dev->msix_entries_nr;
diff --git a/hw/msix.h b/hw/msix.h
index 4a17f94..f681bb0 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -7,10 +7,13 @@
 int msix_init(PCIDevice *pdev, unsigned short nentries,
               MemoryRegion *bar,
               unsigned bar_nr, unsigned bar_size);
+int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
+                            uint8_t bar_nr);
 
 void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len);
 
 int msix_uninit(PCIDevice *d, MemoryRegion *bar);
+void msix_uninit_exclusive_bar(PCIDevice *dev);
 
 unsigned int msix_nr_vectors_allocated(const PCIDevice *dev);
 
diff --git a/hw/pci.h b/hw/pci.h
index 4c96268..d517a54 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -226,6 +226,8 @@ struct PCIDevice {
 
     /* Space to store MSIX table */
     uint8_t *msix_table_page;
+    /* MemoryRegion container for msix exclusive BAR setup */
+    MemoryRegion msix_exclusive_bar;
     /* MMIO index used to map MSIX table and pending bit entries. */
     MemoryRegion msix_mmio;
     /* Reference-count for entries actually in use by driver. */

[Qemu-devel] [PATCH v3 2/8] ivshmem: Convert to msix_init_exclusive_bar() interface

[Alex Williamson]

Trivial conversion, failed to have an uninit before and after.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/ivshmem.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 05559b6..8b49eee 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -70,7 +70,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 */
 
@@ -563,16 +562,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) {
 
 static void ivshmem_setup_msi(IVShmemState * s)
 {
-    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);
-        IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
-    } else {
+    if (msix_init_exclusive_bar(&s->dev, s->vectors, 1)) {
         IVSHMEM_DPRINTF("msix initialization failed\n");
         exit(1);
     }
 
+    IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
+
     /* allocate QEMU char devices for receiving interrupts */
     s->eventfd_table = g_malloc0(s->vectors * sizeof(EventfdEntry));
 

[Qemu-devel] [PATCH v3 3/8] virtio: Convert to msix_init_exclusive_bar() interface

[Alex Williamson]

Simple conversion.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/virtio-pci.c |   15 +++++----------
 hw/virtio-pci.h |    1 -
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9342eed..3dca37f 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -782,13 +782,10 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
     pci_set_word(config + PCI_SUBSYSTEM_ID, vdev->device_id);
     config[PCI_INTERRUPT_PIN] = 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_exclusive_bar(&proxy->pci_dev, vdev->nvectors, 1)) {
         vdev->nvectors = 0;
+    }
 
     proxy->pci_dev.config_write = virtio_write_config;
 
@@ -834,12 +831,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_exclusive_bar(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 91b791b..ac9d522 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -34,7 +34,6 @@ typedef struct {
     PCIDevice pci_dev;
     VirtIODevice *vdev;
     MemoryRegion bar;
-    MemoryRegion msix_bar;
     uint32_t flags;
     uint32_t class_code;
     uint32_t nvectors;

[Qemu-devel] [PATCH v3 4/8] msix: Move msix_mmio_read

[Alex Williamson]

What's this doing so far from msix_mmio_ops?

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/msix.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index bafea94..50885ac 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -86,16 +86,6 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
     return 0;
 }
 
-static uint64_t msix_mmio_read(void *opaque, target_phys_addr_t addr,
-                               unsigned size)
-{
-    PCIDevice *dev = opaque;
-    unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
-    void *page = dev->msix_table_page;
-
-    return pci_get_long(page + offset);
-}
-
 static uint8_t msix_pending_mask(int vector)
 {
     return 1 << (vector % 8);
@@ -203,6 +193,16 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
     }
 }
 
+static uint64_t msix_mmio_read(void *opaque, target_phys_addr_t addr,
+                               unsigned size)
+{
+    PCIDevice *dev = opaque;
+    unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
+    void *page = dev->msix_table_page;
+
+    return pci_get_long(page + offset);
+}
+
 static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
                             uint64_t val, unsigned size)
 {

[Qemu-devel] [PATCH v3 5/8] msix: Note endian TODO item

[Alex Williamson]

MSIX, like PCI, is little endian.  Specifying native is wrong here,
but we need to check the rest of the file to determine if it's
as simple as flipping this macro.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/msix.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/msix.c b/hw/msix.c
index 50885ac..87d316a 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -224,6 +224,7 @@ static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
 static const MemoryRegionOps msix_mmio_ops = {
     .read = msix_mmio_read,
     .write = msix_mmio_write,
+    /* TODO: MSIX should be LITTLE_ENDIAN. */
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 4,

[Qemu-devel] [PATCH v3 6/8] msix: Split PBA into it's own MemoryRegion

[Alex Williamson]

These don't have to be contiguous.  Size them to only what
they need and use separate MemoryRegions for the vector
table and PBA.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/msix.c |  106 +++++++++++++++++++++++++++++++++++++++----------------------
 hw/pci.h  |   10 +++---
 2 files changed, 73 insertions(+), 43 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 87d316a..3312139 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -37,7 +37,7 @@
 
 static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
 {
-    uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
+    uint8_t *table_entry = dev->msix_table + vector * PCI_MSIX_ENTRY_SIZE;
     MSIMessage msg;
 
     msg.address = pci_get_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR);
@@ -93,7 +93,7 @@ static uint8_t msix_pending_mask(int vector)
 
 static uint8_t *msix_pending_byte(PCIDevice *dev, int vector)
 {
-    return dev->msix_table_page + MSIX_PAGE_PENDING + vector / 8;
+    return dev->msix_pba + vector / 8;
 }
 
 static int msix_is_pending(PCIDevice *dev, int vector)
@@ -114,7 +114,7 @@ static void msix_clr_pending(PCIDevice *dev, int vector)
 static bool msix_vector_masked(PCIDevice *dev, int vector, bool fmask)
 {
     unsigned offset = vector * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL;
-    return fmask || dev->msix_table_page[offset] & PCI_MSIX_ENTRY_CTRL_MASKBIT;
+    return fmask || dev->msix_table[offset] & PCI_MSIX_ENTRY_CTRL_MASKBIT;
 }
 
 static bool msix_is_masked(PCIDevice *dev, int vector)
@@ -193,37 +193,47 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
     }
 }
 
-static uint64_t msix_mmio_read(void *opaque, target_phys_addr_t addr,
-                               unsigned size)
+static uint64_t msix_table_mmio_read(void *opaque, target_phys_addr_t addr,
+                                     unsigned size)
 {
     PCIDevice *dev = opaque;
-    unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
-    void *page = dev->msix_table_page;
 
-    return pci_get_long(page + offset);
+    return pci_get_long(dev->msix_table + addr);
 }
 
-static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
-                            uint64_t val, unsigned size)
+static void msix_table_mmio_write(void *opaque, target_phys_addr_t addr,
+                                  uint64_t val, unsigned size)
 {
     PCIDevice *dev = opaque;
-    unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
-    int vector = offset / PCI_MSIX_ENTRY_SIZE;
+    int vector = addr / PCI_MSIX_ENTRY_SIZE;
     bool was_masked;
 
-    /* MSI-X page includes a read-only PBA and a writeable Vector Control. */
-    if (vector >= dev->msix_entries_nr) {
-        return;
-    }
-
     was_masked = msix_is_masked(dev, vector);
-    pci_set_long(dev->msix_table_page + offset, val);
+    pci_set_long(dev->msix_table + addr, val);
     msix_handle_mask_update(dev, vector, was_masked);
 }
 
-static const MemoryRegionOps msix_mmio_ops = {
-    .read = msix_mmio_read,
-    .write = msix_mmio_write,
+static const MemoryRegionOps msix_table_mmio_ops = {
+    .read = msix_table_mmio_read,
+    .write = msix_table_mmio_write,
+    /* TODO: MSIX should be LITTLE_ENDIAN. */
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static uint64_t msix_pba_mmio_read(void *opaque, target_phys_addr_t addr,
+                                   unsigned size)
+{
+    PCIDevice *dev = opaque;
+
+    return pci_get_long(dev->msix_pba + addr);
+}
+
+static const MemoryRegionOps msix_pba_mmio_ops = {
+    .read = msix_pba_mmio_read,
     /* TODO: MSIX should be LITTLE_ENDIAN. */
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
@@ -236,11 +246,14 @@ 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);
+    uint32_t table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
+    uint32_t pba = pci_get_long(config + PCI_MSIX_PBA);
+    uint32_t pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
     /* 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);
+    memory_region_add_subregion(bar, table_offset, &d->msix_table_mmio);
+    memory_region_add_subregion(bar, pba_offset, &d->msix_pba_mmio);
 }
 
 static void msix_mask_all(struct PCIDevice *dev, unsigned nentries)
@@ -252,7 +265,7 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned nentries)
             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;
+        dev->msix_table[offset] |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
         msix_handle_mask_update(dev, vector, was_masked);
     }
 }
@@ -264,6 +277,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
               unsigned bar_nr, unsigned bar_size)
 {
     int ret;
+    unsigned table_size, pba_size;
 
     /* Nothing to do if MSI is not supported by interrupt controller */
     if (!msi_supported) {
@@ -272,14 +286,20 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
     if (nentries > MSIX_MAX_ENTRIES)
         return -EINVAL;
 
+    table_size = nentries * PCI_MSIX_ENTRY_SIZE;
+    pba_size = QEMU_ALIGN_UP(nentries, 64) / 8;
+
     dev->msix_entry_used = g_malloc0(MSIX_MAX_ENTRIES *
                                         sizeof *dev->msix_entry_used);
 
-    dev->msix_table_page = g_malloc0(MSIX_PAGE_SIZE);
+    dev->msix_table = g_malloc0(table_size);
+    dev->msix_pba = g_malloc0(pba_size);
     msix_mask_all(dev, nentries);
 
-    memory_region_init_io(&dev->msix_mmio, &msix_mmio_ops, dev,
-                          "msix", MSIX_PAGE_SIZE);
+    memory_region_init_io(&dev->msix_table_mmio, &msix_table_mmio_ops, dev,
+                          "msix-table", table_size);
+    memory_region_init_io(&dev->msix_pba_mmio, &msix_pba_mmio_ops, dev,
+                          "msix-pba", pba_size);
 
     dev->msix_entries_nr = nentries;
     ret = msix_add_config(dev, nentries, bar_nr, bar_size);
@@ -292,9 +312,12 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
 
 err_config:
     dev->msix_entries_nr = 0;
-    memory_region_destroy(&dev->msix_mmio);
-    g_free(dev->msix_table_page);
-    dev->msix_table_page = NULL;
+    memory_region_destroy(&dev->msix_pba_mmio);
+    g_free(dev->msix_pba);
+    dev->msix_pba = NULL;
+    memory_region_destroy(&dev->msix_table_mmio);
+    g_free(dev->msix_table);
+    dev->msix_table = NULL;
     g_free(dev->msix_entry_used);
     dev->msix_entry_used = NULL;
     return ret;
@@ -359,10 +382,14 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
     dev->msix_cap = 0;
     msix_free_irq_entries(dev);
     dev->msix_entries_nr = 0;
-    memory_region_del_subregion(bar, &dev->msix_mmio);
-    memory_region_destroy(&dev->msix_mmio);
-    g_free(dev->msix_table_page);
-    dev->msix_table_page = NULL;
+    memory_region_del_subregion(bar, &dev->msix_pba_mmio);
+    memory_region_destroy(&dev->msix_pba_mmio);
+    g_free(dev->msix_pba);
+    dev->msix_pba = NULL;
+    memory_region_del_subregion(bar, &dev->msix_table_mmio);
+    memory_region_destroy(&dev->msix_table_mmio);
+    g_free(dev->msix_table);
+    dev->msix_table = NULL;
     g_free(dev->msix_entry_used);
     dev->msix_entry_used = NULL;
     dev->cap_present &= ~QEMU_PCI_CAP_MSIX;
@@ -385,8 +412,8 @@ void msix_save(PCIDevice *dev, QEMUFile *f)
         return;
     }
 
-    qemu_put_buffer(f, dev->msix_table_page, n * PCI_MSIX_ENTRY_SIZE);
-    qemu_put_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
+    qemu_put_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE);
+    qemu_put_buffer(f, dev->msix_pba, (n + 7) / 8);
 }
 
 /* Should be called after restoring the config space. */
@@ -400,8 +427,8 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
     }
 
     msix_free_irq_entries(dev);
-    qemu_get_buffer(f, dev->msix_table_page, n * PCI_MSIX_ENTRY_SIZE);
-    qemu_get_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
+    qemu_get_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE);
+    qemu_get_buffer(f, dev->msix_pba, (n + 7) / 8);
     msix_update_function_masked(dev);
 
     for (vector = 0; vector < n; vector++) {
@@ -448,7 +475,8 @@ void msix_reset(PCIDevice *dev)
     msix_free_irq_entries(dev);
     dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &=
 	    ~dev->wmask[dev->msix_cap + MSIX_CONTROL_OFFSET];
-    memset(dev->msix_table_page, 0, MSIX_PAGE_SIZE);
+    memset(dev->msix_table, 0, dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE);
+    memset(dev->msix_pba, 0, QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8);
     msix_mask_all(dev, dev->msix_entries_nr);
 }
 
diff --git a/hw/pci.h b/hw/pci.h
index d517a54..ac9d906 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -224,12 +224,14 @@ struct PCIDevice {
     /* MSI-X entries */
     int msix_entries_nr;
 
-    /* Space to store MSIX table */
-    uint8_t *msix_table_page;
+    /* Space to store MSIX table & pending bit array */
+    uint8_t *msix_table;
+    uint8_t *msix_pba;
     /* MemoryRegion container for msix exclusive BAR setup */
     MemoryRegion msix_exclusive_bar;
-    /* MMIO index used to map MSIX table and pending bit entries. */
-    MemoryRegion msix_mmio;
+    /* Memory Regions for MSIX table and pending bit entries. */
+    MemoryRegion msix_table_mmio;
+    MemoryRegion msix_pba_mmio;
     /* Reference-count for entries actually in use by driver. */
     unsigned *msix_entry_used;
     /* MSIX function mask set or MSIX disabled */

[Qemu-devel] [PATCH v3 7/8] msix: Allow full specification of MSIX layout

[Alex Williamson]

Finally, complete the fully specified interface.  msix_add_config()
gets folded into msix_init() because we now have quite a few parameters
to pass and rolling it in let's us error earlier, avoiding the ugly
unwind exit path.  msix_mmio_setup() also gets rolled in, just because
it's redundant to rediscover offsets when we already have them for
such a tiny function.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/msix.c |  145 +++++++++++++++++++++----------------------------------------
 hw/msix.h |   10 +++-
 2 files changed, 56 insertions(+), 99 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 3312139..15f8d7d 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -27,14 +27,6 @@
 #define MSIX_ENABLE_MASK (PCI_MSIX_FLAGS_ENABLE >> 8)
 #define MSIX_MASKALL_MASK (PCI_MSIX_FLAGS_MASKALL >> 8)
 
-/* How much space does an MSIX table need. */
-/* The spec requires giving the table structure
- * a 4K aligned region all by itself. */
-#define MSIX_PAGE_SIZE 0x1000
-/* Reserve second half of the page for pending bits */
-#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
-#define MSIX_MAX_ENTRIES 32
-
 static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
 {
     uint8_t *table_entry = dev->msix_table + vector * PCI_MSIX_ENTRY_SIZE;
@@ -45,47 +37,6 @@ static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
     return msg;
 }
 
-/* Add MSI-X capability to the config space for the device. */
-/* Given a bar and its size, add MSI-X table on top of it
- * and fill MSI-X capability in the config space.
- * Original bar size must be a power of 2 or 0.
- * New bar size is returned. */
-static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
-                           unsigned bar_nr, unsigned bar_size)
-{
-    int config_offset;
-    uint8_t *config;
-
-    if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1)
-        return -EINVAL;
-    if (bar_size > 0x80000000)
-        return -ENOSPC;
-
-    /* Require aligned offset for MSI-X structures */
-    if (bar_size & ~(MSIX_PAGE_SIZE - 1)) {
-        return -EINVAL;
-    }
-
-    config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
-                                       0, MSIX_CAP_LENGTH);
-    if (config_offset < 0)
-        return config_offset;
-    config = pdev->config + config_offset;
-
-    pci_set_word(config + PCI_MSIX_FLAGS, nentries - 1);
-    /* Table on top of BAR */
-    pci_set_long(config + PCI_MSIX_TABLE, bar_size | bar_nr);
-    /* Pending bits on top of that */
-    pci_set_long(config + PCI_MSIX_PBA, (bar_size + MSIX_PAGE_PENDING) |
-                 bar_nr);
-    pdev->msix_cap = config_offset;
-    /* Make flags bit writable. */
-    pdev->wmask[config_offset + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK |
-	    MSIX_MASKALL_MASK;
-    pdev->msix_function_masked = true;
-    return 0;
-}
-
 static uint8_t msix_pending_mask(int vector)
 {
     return 1 << (vector % 8);
@@ -242,20 +193,6 @@ static const MemoryRegionOps msix_pba_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 table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
-    uint32_t pba = pci_get_long(config + PCI_MSIX_PBA);
-    uint32_t pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
-    /* 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, table_offset, &d->msix_table_mmio);
-    memory_region_add_subregion(bar, pba_offset, &d->msix_pba_mmio);
-}
-
 static void msix_mask_all(struct PCIDevice *dev, unsigned nentries)
 {
     int vector;
@@ -270,57 +207,71 @@ 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. */
+/* Initialize the MSI-X structures */
 int msix_init(struct PCIDevice *dev, unsigned short nentries,
-              MemoryRegion *bar,
-              unsigned bar_nr, unsigned bar_size)
+              MemoryRegion *table_bar, uint8_t table_bar_nr,
+              unsigned table_offset, MemoryRegion *pba_bar,
+              uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos)
 {
-    int ret;
+    int cap;
     unsigned table_size, pba_size;
+    uint8_t *config;
 
     /* Nothing to do if MSI is not supported by interrupt controller */
     if (!msi_supported) {
         return -ENOTSUP;
     }
-    if (nentries > MSIX_MAX_ENTRIES)
+
+    if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) {
         return -EINVAL;
+    }
 
     table_size = nentries * PCI_MSIX_ENTRY_SIZE;
     pba_size = QEMU_ALIGN_UP(nentries, 64) / 8;
 
-    dev->msix_entry_used = g_malloc0(MSIX_MAX_ENTRIES *
-                                        sizeof *dev->msix_entry_used);
+    /* Sanity test: table & pba don't overlap, fit within BARs, min aligned */
+    if ((table_bar_nr == pba_bar_nr &&
+         ranges_overlap(table_offset, table_size, pba_offset, pba_size)) ||
+        table_offset + table_size > memory_region_size(table_bar) ||
+        pba_offset + pba_size > memory_region_size(pba_bar) ||
+        (table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) {
+        return -EINVAL;
+    }
+
+    cap = pci_add_capability(dev, PCI_CAP_ID_MSIX, cap_pos, MSIX_CAP_LENGTH);
+    if (cap < 0) {
+        return cap;
+    }
+
+    dev->msix_cap = cap;
+    dev->cap_present |= QEMU_PCI_CAP_MSIX;
+    config = dev->config + cap;
+
+    pci_set_word(config + PCI_MSIX_FLAGS, nentries - 1);
+    dev->msix_entries_nr = nentries;
+    dev->msix_function_masked = true;
+
+    pci_set_long(config + PCI_MSIX_TABLE, table_offset | table_bar_nr);
+    pci_set_long(config + PCI_MSIX_PBA, pba_offset | pba_bar_nr);
+
+    /* Make flags bit writable. */
+    dev->wmask[cap + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK |
+                                             MSIX_MASKALL_MASK;
 
     dev->msix_table = g_malloc0(table_size);
     dev->msix_pba = g_malloc0(pba_size);
+    dev->msix_entry_used = g_malloc0(nentries * sizeof *dev->msix_entry_used);
+
     msix_mask_all(dev, nentries);
 
     memory_region_init_io(&dev->msix_table_mmio, &msix_table_mmio_ops, dev,
                           "msix-table", table_size);
+    memory_region_add_subregion(table_bar, table_offset, &dev->msix_table_mmio);
     memory_region_init_io(&dev->msix_pba_mmio, &msix_pba_mmio_ops, dev,
                           "msix-pba", pba_size);
+    memory_region_add_subregion(pba_bar, pba_offset, &dev->msix_pba_mmio);
 
-    dev->msix_entries_nr = nentries;
-    ret = msix_add_config(dev, nentries, bar_nr, bar_size);
-    if (ret)
-        goto err_config;
-
-    dev->cap_present |= QEMU_PCI_CAP_MSIX;
-    msix_mmio_setup(dev, bar);
     return 0;
-
-err_config:
-    dev->msix_entries_nr = 0;
-    memory_region_destroy(&dev->msix_pba_mmio);
-    g_free(dev->msix_pba);
-    dev->msix_pba = NULL;
-    memory_region_destroy(&dev->msix_table_mmio);
-    g_free(dev->msix_table);
-    dev->msix_table = NULL;
-    g_free(dev->msix_entry_used);
-    dev->msix_entry_used = NULL;
-    return ret;
 }
 
 int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
@@ -335,7 +286,9 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
      * the upper half.  Do not use these elsewhere!
      */
 #define MSIX_EXCLUSIVE_BAR_SIZE 4096
+#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0
 #define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2)
+#define MSIX_EXCLUSIVE_CAP_OFFSET 0
 
     if (nentries * PCI_MSIX_ENTRY_SIZE > MSIX_EXCLUSIVE_BAR_PBA_OFFSET) {
         return -EINVAL;
@@ -350,7 +303,9 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
     free(name);
 
     ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr,
-                    MSIX_EXCLUSIVE_BAR_SIZE);
+                    MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, &dev->msix_exclusive_bar,
+                    bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET,
+                    MSIX_EXCLUSIVE_CAP_OFFSET);
     if (ret) {
         memory_region_destroy(&dev->msix_exclusive_bar);
         return ret;
@@ -373,7 +328,7 @@ static void msix_free_irq_entries(PCIDevice *dev)
 }
 
 /* Clean up resources for the device. */
-int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
+int msix_uninit(PCIDevice *dev, MemoryRegion *table_bar, MemoryRegion *pba_bar)
 {
     if (!msix_present(dev)) {
         return 0;
@@ -382,11 +337,11 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
     dev->msix_cap = 0;
     msix_free_irq_entries(dev);
     dev->msix_entries_nr = 0;
-    memory_region_del_subregion(bar, &dev->msix_pba_mmio);
+    memory_region_del_subregion(pba_bar, &dev->msix_pba_mmio);
     memory_region_destroy(&dev->msix_pba_mmio);
     g_free(dev->msix_pba);
     dev->msix_pba = NULL;
-    memory_region_del_subregion(bar, &dev->msix_table_mmio);
+    memory_region_del_subregion(table_bar, &dev->msix_table_mmio);
     memory_region_destroy(&dev->msix_table_mmio);
     g_free(dev->msix_table);
     dev->msix_table = NULL;
@@ -399,7 +354,7 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
 void msix_uninit_exclusive_bar(PCIDevice *dev)
 {
     if (msix_present(dev)) {
-        msix_uninit(dev, &dev->msix_exclusive_bar);
+        msix_uninit(dev, &dev->msix_exclusive_bar, &dev->msix_exclusive_bar);
         memory_region_destroy(&dev->msix_exclusive_bar);
     }
 }
diff --git a/hw/msix.h b/hw/msix.h
index f681bb0..f637797 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,15 +4,17 @@
 #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(PCIDevice *dev, unsigned short nentries,
+              MemoryRegion *table_bar, uint8_t table_bar_nr,
+              unsigned table_offset, MemoryRegion *pba_bar,
+              uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos);
 int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
                             uint8_t bar_nr);
 
 void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len);
 
-int msix_uninit(PCIDevice *d, MemoryRegion *bar);
+int msix_uninit(PCIDevice *dev, MemoryRegion *table_bar,
+                MemoryRegion *pba_bar);
 void msix_uninit_exclusive_bar(PCIDevice *dev);
 
 unsigned int msix_nr_vectors_allocated(const PCIDevice *dev);

[Qemu-devel] [PATCH v3 8/8] msix: Switch msix_uninit to return void

[Alex Williamson]

It can't fail.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/msix.c |    6 +++---
 hw/msix.h |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 15f8d7d..fd9ea95 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -328,10 +328,10 @@ static void msix_free_irq_entries(PCIDevice *dev)
 }
 
 /* Clean up resources for the device. */
-int msix_uninit(PCIDevice *dev, MemoryRegion *table_bar, MemoryRegion *pba_bar)
+void msix_uninit(PCIDevice *dev, MemoryRegion *table_bar, MemoryRegion *pba_bar)
 {
     if (!msix_present(dev)) {
-        return 0;
+        return;
     }
     pci_del_capability(dev, PCI_CAP_ID_MSIX, MSIX_CAP_LENGTH);
     dev->msix_cap = 0;
@@ -348,7 +348,7 @@ int msix_uninit(PCIDevice *dev, MemoryRegion *table_bar, MemoryRegion *pba_bar)
     g_free(dev->msix_entry_used);
     dev->msix_entry_used = NULL;
     dev->cap_present &= ~QEMU_PCI_CAP_MSIX;
-    return 0;
+    return;
 }
 
 void msix_uninit_exclusive_bar(PCIDevice *dev)
diff --git a/hw/msix.h b/hw/msix.h
index f637797..1786e27 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -13,8 +13,8 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
 
 void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len);
 
-int msix_uninit(PCIDevice *dev, MemoryRegion *table_bar,
-                MemoryRegion *pba_bar);
+void msix_uninit(PCIDevice *dev, MemoryRegion *table_bar,
+                 MemoryRegion *pba_bar);
 void msix_uninit_exclusive_bar(PCIDevice *dev);
 
 unsigned int msix_nr_vectors_allocated(const PCIDevice *dev);

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Michael S. Tsirkin]

On Thu, Jun 14, 2012 at 12:15:42PM -0600, Alex Williamson wrote:
> v3:
>  - more patches, smaller diff, must be headed in the right direction
>  - macros for all hardcoded values in msix_init_exclusive_bar
>  - fold msix_add_config into msix_init allowing less churn to moving
>    around msix_uninit
>  - note native endian bug
>  - split msix_mmio_read move to separate patch
>  - split changing return value of msix_uninit to separate patch
> 
> Thanks,
> 
> Alex

Thanks, applied all.
Will test/push next week.

> v2:
>  - split patch
>  - rename msix_[un]init_bar() to msix_[un]init_exclusive_bar()
>  - add the cherry on top to cleaning up PCIDevice naming
> 
> v1:
> 
> msix_init has very little configurability as to how it lays out MSIX
> for a device.  It claims to resize BARs, but doesn't actually do this
> anymore.  This patch allows MSIX to be fully specified, which is
> necessary both for emulated devices trying to match the physical
> layout of a hardware device as well as for any kind of device
> assignment.
> 
> New functions msix_init_bar & msix_uninit_bar provide wrappers around
> the more detailed functions for drivers that just want a simple MSIX
> setup.
> 
> ---
> 
> Alex Williamson (8):
>       msix: Switch msix_uninit to return void
>       msix: Allow full specification of MSIX layout
>       msix: Split PBA into it's own MemoryRegion
>       msix: Note endian TODO item
>       msix: Move msix_mmio_read
>       virtio: Convert to msix_init_exclusive_bar() interface
>       ivshmem: Convert to msix_init_exclusive_bar() interface
>       msix: Add simple BAR allocation MSIX setup functions
> 

One minor suggestion for the future: I think it's prettier
not to start with an upper case letter after ":".
So the commit log would become:
        msix: add simple BAR allocation MSIX setup functions

> 
>  hw/ivshmem.c    |   10 +-
>  hw/msix.c       |  271 +++++++++++++++++++++++++++++++------------------------
>  hw/msix.h       |   13 ++-
>  hw/pci.h        |   12 ++
>  hw/virtio-pci.c |   15 +--
>  hw/virtio-pci.h |    1 
>  6 files changed, 176 insertions(+), 146 deletions(-)

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On 2012-06-14 23:31, Michael S. Tsirkin wrote:
> On Thu, Jun 14, 2012 at 12:15:42PM -0600, Alex Williamson wrote:
>> v3:
>>  - more patches, smaller diff, must be headed in the right direction
>>  - macros for all hardcoded values in msix_init_exclusive_bar
>>  - fold msix_add_config into msix_init allowing less churn to moving
>>    around msix_uninit
>>  - note native endian bug
>>  - split msix_mmio_read move to separate patch
>>  - split changing return value of msix_uninit to separate patch
>>
>> Thanks,
>>
>> Alex
> 
> Thanks, applied all.
> Will test/push next week.

Could you publish your queue? I'd like to rebase my missing bits.

Thanks,
Ja


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Michael S. Tsirkin]

On Mon, Jun 18, 2012 at 09:06:01AM +0200, Jan Kiszka wrote:
> On 2012-06-14 23:31, Michael S. Tsirkin wrote:
> > On Thu, Jun 14, 2012 at 12:15:42PM -0600, Alex Williamson wrote:
> >> v3:
> >>  - more patches, smaller diff, must be headed in the right direction
> >>  - macros for all hardcoded values in msix_init_exclusive_bar
> >>  - fold msix_add_config into msix_init allowing less churn to moving
> >>    around msix_uninit
> >>  - note native endian bug
> >>  - split msix_mmio_read move to separate patch
> >>  - split changing return value of msix_uninit to separate patch
> >>
> >> Thanks,
> >>
> >> Alex
> > 
> > Thanks, applied all.
> > Will test/push next week.
> 
> Could you publish your queue? I'd like to rebase my missing bits.
> 
> Thanks,
> Ja
> 

Will do. FYI Anthony said on irc he objects to the caching approach,
asked for more time to review it all. Maybe we'll have to
go back to your original idea of a special API just for
assigned devices.

-- 
MST

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Jan Kiszka]

On 2012-06-18 09:19, Michael S. Tsirkin wrote:
> On Mon, Jun 18, 2012 at 09:06:01AM +0200, Jan Kiszka wrote:
>> On 2012-06-14 23:31, Michael S. Tsirkin wrote:
>>> On Thu, Jun 14, 2012 at 12:15:42PM -0600, Alex Williamson wrote:
>>>> v3:
>>>>  - more patches, smaller diff, must be headed in the right direction
>>>>  - macros for all hardcoded values in msix_init_exclusive_bar
>>>>  - fold msix_add_config into msix_init allowing less churn to moving
>>>>    around msix_uninit
>>>>  - note native endian bug
>>>>  - split msix_mmio_read move to separate patch
>>>>  - split changing return value of msix_uninit to separate patch
>>>>
>>>> Thanks,
>>>>
>>>> Alex
>>>
>>> Thanks, applied all.
>>> Will test/push next week.
>>
>> Could you publish your queue? I'd like to rebase my missing bits.
>>
>> Thanks,
>> Ja
>>
> 
> Will do. FYI Anthony said on irc he objects to the caching approach,
> asked for more time to review it all. Maybe we'll have to
> go back to your original idea of a special API just for
> assigned devices.

Yes, we can still add caching on top.

I really like to have some hook upstream soon as time is running out
quickly for the 1.2 merge window and there is still some work to do on
the qemu-kvm side.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Michael S. Tsirkin]

On Mon, Jun 18, 2012 at 11:23:41AM +0200, Jan Kiszka wrote:
> On 2012-06-18 09:19, Michael S. Tsirkin wrote:
> > On Mon, Jun 18, 2012 at 09:06:01AM +0200, Jan Kiszka wrote:
> >> On 2012-06-14 23:31, Michael S. Tsirkin wrote:
> >>> On Thu, Jun 14, 2012 at 12:15:42PM -0600, Alex Williamson wrote:
> >>>> v3:
> >>>>  - more patches, smaller diff, must be headed in the right direction
> >>>>  - macros for all hardcoded values in msix_init_exclusive_bar
> >>>>  - fold msix_add_config into msix_init allowing less churn to moving
> >>>>    around msix_uninit
> >>>>  - note native endian bug
> >>>>  - split msix_mmio_read move to separate patch
> >>>>  - split changing return value of msix_uninit to separate patch
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Alex
> >>>
> >>> Thanks, applied all.
> >>> Will test/push next week.
> >>
> >> Could you publish your queue? I'd like to rebase my missing bits.
> >>
> >> Thanks,
> >> Ja
> >>
> > 
> > Will do. FYI Anthony said on irc he objects to the caching approach,
> > asked for more time to review it all. Maybe we'll have to
> > go back to your original idea of a special API just for
> > assigned devices.
> 
> Yes, we can still add caching on top.
> 
> I really like to have some hook upstream soon as time is running out
> quickly for the 1.2 merge window and there is still some work to do on
> the qemu-kvm side.
> 
> Jan

Anthony are your ideas for 1.2 timeframe?

> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Jan Kiszka]

On 2012-06-18 11:57, Michael S. Tsirkin wrote:
> On Mon, Jun 18, 2012 at 11:23:41AM +0200, Jan Kiszka wrote:
>> On 2012-06-18 09:19, Michael S. Tsirkin wrote:
>>> On Mon, Jun 18, 2012 at 09:06:01AM +0200, Jan Kiszka wrote:
>>>> On 2012-06-14 23:31, Michael S. Tsirkin wrote:
>>>>> On Thu, Jun 14, 2012 at 12:15:42PM -0600, Alex Williamson wrote:
>>>>>> v3:
>>>>>>  - more patches, smaller diff, must be headed in the right direction
>>>>>>  - macros for all hardcoded values in msix_init_exclusive_bar
>>>>>>  - fold msix_add_config into msix_init allowing less churn to moving
>>>>>>    around msix_uninit
>>>>>>  - note native endian bug
>>>>>>  - split msix_mmio_read move to separate patch
>>>>>>  - split changing return value of msix_uninit to separate patch
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Alex
>>>>>
>>>>> Thanks, applied all.
>>>>> Will test/push next week.
>>>>
>>>> Could you publish your queue? I'd like to rebase my missing bits.
>>>>
>>>> Thanks,
>>>> Ja
>>>>
>>>
>>> Will do. FYI Anthony said on irc he objects to the caching approach,
>>> asked for more time to review it all. Maybe we'll have to
>>> go back to your original idea of a special API just for
>>> assigned devices.
>>
>> Yes, we can still add caching on top.
>>
>> I really like to have some hook upstream soon as time is running out
>> quickly for the 1.2 merge window and there is still some work to do on
>> the qemu-kvm side.
>>
>> Jan
> 
> Anthony are your ideas for 1.2 timeframe?

http://wiki.qemu.org/Planning/1.2

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

On 2012-06-18 12:36, Jan Kiszka wrote:
>>>>> Could you publish your queue? I'd like to rebase my missing bits.
>>>>>
>>>>> Thanks,
>>>>> Ja
>>>>>
>>>>
>>>> Will do. FYI Anthony said on irc he objects to the caching approach,
>>>> asked for more time to review it all. Maybe we'll have to
>>>> go back to your original idea of a special API just for
>>>> assigned devices.
>>>
>>> Yes, we can still add caching on top.
>>>
>>> I really like to have some hook upstream soon as time is running out
>>> quickly for the 1.2 merge window and there is still some work to do on
>>> the qemu-kvm side.
>>>
>>> Jan
>>
>> Anthony are your ideas for 1.2 timeframe?
> 
> http://wiki.qemu.org/Planning/1.2

How to proceed? Your queue currently contains the caching, my add-on
patches make use of it. I can change them to not do this, but I'd like
to learn the preferred direction first.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH v3 0/8] msix: Support specifying offsets, BARs, and capability location

[Michael S. Tsirkin]

On Tue, Jun 19, 2012 at 12:37:13AM +0200, Jan Kiszka wrote:
> On 2012-06-18 12:36, Jan Kiszka wrote:
> >>>>> Could you publish your queue? I'd like to rebase my missing bits.
> >>>>>
> >>>>> Thanks,
> >>>>> Ja
> >>>>>
> >>>>
> >>>> Will do. FYI Anthony said on irc he objects to the caching approach,
> >>>> asked for more time to review it all. Maybe we'll have to
> >>>> go back to your original idea of a special API just for
> >>>> assigned devices.
> >>>
> >>> Yes, we can still add caching on top.
> >>>
> >>> I really like to have some hook upstream soon as time is running out
> >>> quickly for the 1.2 merge window and there is still some work to do on
> >>> the qemu-kvm side.
> >>>
> >>> Jan
> >>
> >> Anthony are your ideas for 1.2 timeframe?
> > 
> > http://wiki.qemu.org/Planning/1.2
> 
> How to proceed? Your queue currently contains the caching, my add-on
> patches make use of it. I can change them to not do this, but I'd like
> to learn the preferred direction first.
> 
> Jan
> 

Let's give Anthony a bit of time to respond, I think he's traveling.