[Qemu-devel] [PATCH v2 9/9] pci: Store capability offsets in PCIDevice

[Alex Williamson]

This not only makes pci_find_capability a directly lookup, but also
allows us to better track added capabilities and avoids the proliferation
of random additional capability offset markers.

Signed-off-by: Alex Williamson <address@hidden>
---

 hw/msix.c |   15 +++++++--------
 hw/pci.c  |   20 ++++++++++++++++++--
 hw/pci.h  |    5 +++--
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index b98b34a..060f27b 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -204,7 +204,6 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned 
short nentries,
         pci_set_long(config + MSIX_PBA_OFFSET, (bar_size + MSIX_PAGE_PENDING) |
                      bar_nr);
     }
-    pdev->msix_cap = config_offset;
     /* Make flags bit writeable. */
     pdev->wmask[config_offset + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK |
            MSIX_MASKALL_MASK;
@@ -253,7 +252,8 @@ static void msix_clr_pending(PCIDevice *dev, int vector)
 
 static int msix_function_masked(PCIDevice *dev)
 {
-    return dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] & 
MSIX_MASKALL_MASK;
+    return dev->config[dev->caps[PCI_CAP_ID_MSIX] +
+                       MSIX_CONTROL_OFFSET] & MSIX_MASKALL_MASK;
 }
 
 static int msix_is_masked(PCIDevice *dev, int vector)
@@ -275,7 +275,7 @@ static void msix_handle_mask_update(PCIDevice *dev, int 
vector)
 void msix_write_config(PCIDevice *dev, uint32_t addr,
                        uint32_t val, int len)
 {
-    unsigned enable_pos = dev->msix_cap + MSIX_CONTROL_OFFSET;
+    unsigned enable_pos = dev->caps[PCI_CAP_ID_MSIX] + MSIX_CONTROL_OFFSET;
     int vector;
 
     if (!range_covers_byte(addr, len, enable_pos)) {
@@ -334,7 +334,7 @@ static CPUReadMemoryFunc * const msix_mmio_read[] = {
 void msix_mmio_map(PCIDevice *d, int region_num,
                    pcibus_t addr, pcibus_t size, int type)
 {
-    uint8_t *config = d->config + d->msix_cap;
+    uint8_t *config = d->config + d->caps[PCI_CAP_ID_MSIX];
     uint32_t table = pci_get_long(config + MSIX_TABLE_OFFSET);
     uint32_t offset = table & ~(MSIX_PAGE_SIZE - 1);
     /* TODO: for assigned devices, we'll want to make it possible to map
@@ -437,7 +437,6 @@ int msix_uninit(PCIDevice *dev)
     if (!(dev->cap_present & QEMU_PCI_CAP_MSIX))
         return 0;
     pci_del_capability(dev, PCI_CAP_ID_MSIX, MSIX_CAP_LENGTH);
-    dev->msix_cap = 0;
     msix_free_irq_entries(dev);
     dev->msix_entries_nr = 0;
     cpu_unregister_io_memory(dev->msix_mmio_index);
@@ -493,7 +492,7 @@ int msix_present(PCIDevice *dev)
 int msix_enabled(PCIDevice *dev)
 {
     return (dev->cap_present & QEMU_PCI_CAP_MSIX) &&
-        (dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
+        (dev->config[dev->caps[PCI_CAP_ID_MSIX] + MSIX_CONTROL_OFFSET] &
          MSIX_ENABLE_MASK);
 }
 
@@ -534,8 +533,8 @@ void msix_reset(PCIDevice *dev)
     if (!(dev->cap_present & QEMU_PCI_CAP_MSIX))
         return;
     msix_free_irq_entries(dev);
-    dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &=
-           ~dev->wmask[dev->msix_cap + MSIX_CONTROL_OFFSET];
+    dev->config[dev->caps[PCI_CAP_ID_MSIX] + MSIX_CONTROL_OFFSET] &=
+           ~dev->wmask[dev->caps[PCI_CAP_ID_MSIX] + MSIX_CONTROL_OFFSET];
     memset(dev->msix_table_page, 0, MSIX_PAGE_SIZE);
     msix_mask_all(dev, dev->msix_entries_nr);
 }
diff --git a/hw/pci.c b/hw/pci.c
index bc25be7..773afa5 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1990,15 +1990,24 @@ int pci_add_capability_at_offset(PCIDevice *pdev, 
uint8_t cap_id,
 {
     uint8_t i, *config = pdev->config + offset;
 
+    /* Check overlap with existing capabilities, valid cap, already added */
     for (i = 0; i < size; i++) {
         if (pdev->config_map[offset + i]) {
             return -EFAULT;
         }
     }
 
+    if (!cap_id || cap_id > PCI_CAP_ID_MAX) {
+        return -EINVAL;
+    }
+
+    if (pdev->caps[cap_id]) {
+        return -EFAULT;
+    }
+
     config[PCI_CAP_LIST_ID] = cap_id;
     config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
-    pdev->config[PCI_CAPABILITY_LIST] = offset;
+    pdev->caps[cap_id] = pdev->config[PCI_CAPABILITY_LIST] = offset;
     memset(pdev->config_map + offset, cap_id, size);
     /* Make capability read-only by default */
     memset(pdev->wmask + offset, 0, size);
@@ -2033,6 +2042,7 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, 
uint8_t size)
     /* Clear cmask as device-specific registers can't be checked */
     memset(pdev->cmask + offset, 0, size);
     memset(pdev->config_map + offset, 0, size);
+    pdev->caps[cap_id] = 0;
 
     if (!pdev->config[PCI_CAPABILITY_LIST]) {
         pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
@@ -2041,7 +2051,13 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, 
uint8_t size)
 
 uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id)
 {
-    return pci_find_capability_list(pdev, cap_id, NULL);
+    if (cap_id == PCI_CAP_ID_BASIC) {
+        return 0;
+    }
+    if (cap_id && cap_id <= PCI_CAP_ID_MAX) {
+        return pdev->caps[cap_id];
+    }
+    return 0xff;
 }
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
diff --git a/hw/pci.h b/hw/pci.h
index cea1c3a..b4c19ba 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -110,6 +110,7 @@ typedef struct PCIIORegion {
 #define PCI_NUM_PINS 4 /* A-D */
 
 #define PCI_CAP_ID_BASIC 0xff
+#define PCI_CAP_ID_MAX PCI_CAP_ID_AF
 
 /* Bits in cap_present field. */
 enum {
@@ -170,8 +171,8 @@ struct PCIDevice {
     /* Capability bits */
     uint32_t cap_present;
 
-    /* Offset of MSI-X capability in config space */
-    uint8_t msix_cap;
+    /* Offset capabilities in config space */
+    uint8_t caps[PCI_CAP_ID_MAX + 1];
 
     /* MSI-X entries */
     int msix_entries_nr;