[Qemu-devel] [PATCH v3 0/9] PCI capability and device assignment improvements

[Qemu-devel] [PATCH v3 0/9] PCI capability and device assignment improvements

[Alex Williamson]

v3:

 - Rework to avoid introducing conflicts with qemu.git hw/pci
 - Drop capability lookup table
 - Add back minimal device assignment PM, EXP, X, VPD, VNDR
   capabilities

This version should do a much better job at not introducing new
differences between qemu-kvm.git and qemu.git hw/pci.  The diff
between the pci files after this series is significantly
reduced.  I added back the previous RFC patch that adds new
capabilities for assigned devices.  I'm sure we'll want to add
passthrough for various fields, but that can come later (along
with figuring out whether we can consolidate emulation should
other drivers want it).  Thanks,

Alex

v2:

 - Fixed the function name in 1/8 per Michael's suggestion.
 - Removed capability specific config read/write registration
 - Added more checks to add_capability
 - Added capability lookup table to PCIDevice

I've dropped the RFC patch to add more capabilities to device
assignment while I do some more work on it.  Please feel free
to comment on the v1 version though.  Patches still against
qemu-kvm, but I hope some of this makes it easier to bring
qemu & qemu-kvm closer here.

v1:

This series attempts to clean up capability support between common
code and device assignment.  In doing so, we can move existing MSI &
MSI-X capabilities to offsets matching real hardware, and further
enable more capabilities to be exposed.

The last patch is only for RFC, I'd like some input on what we should
pass directly and where we should only provide read-only/emulated
access.  Patches 1-7 are submitted for commit.

---

Alex Williamson (9):
      device-assignment: pass through and stub more PCI caps
      device-assignment: Make use of config_map
      pci: Remove capability specific handlers
      device-assignment: Move PCI capabilities to match physical hardware
      pci: Remove cap.length, cap.start, cap.supported
      pci: Replace used bitmap with config byte map
      device-assignment: Use PCI capabilities support
      pci: Remove pci_enable_capability_support()
      pci: pci_default_cap_write_config ignores wmask


 hw/device-assignment.c |  332 +++++++++++++++++++++++++++++++++++++-----------
 hw/pci.c               |  109 ++--------------
 hw/pci.h               |   32 -----
 3 files changed, 276 insertions(+), 197 deletions(-)

[Qemu-devel] [PATCH v3 1/9] pci: pci_default_cap_write_config ignores wmask

[Alex Williamson]

Make use of wmask, just like the rest of config space.

This duplicates code in pci_default_write_config, but we plan to get
rid of this function anyway, so avoid the code churn.

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

 hw/pci.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index c3b5048..8e99746 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1088,16 +1088,6 @@ uint32_t pci_default_read_config(PCIDevice *d,
     return pci_read_config(d, address, len);
 }
 
-static void pci_write_config(PCIDevice *pci_dev,
-                             uint32_t address, uint32_t val, int len)
-{
-    int i;
-    for (i = 0; i < len; i++) {
-        pci_dev->config[address + i] = val & 0xff;
-        val >>= 8;
-    }
-}
-
 int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len)
 {
     if (pci_dev->cap.supported && address >= pci_dev->cap.start &&
@@ -1115,7 +1105,14 @@ uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
 void pci_default_cap_write_config(PCIDevice *pci_dev,
                                   uint32_t address, uint32_t val, int len)
 {
-    pci_write_config(pci_dev, address, val, len);
+    uint32_t config_size = pci_config_size(pci_dev);
+    int i;
+
+    for (i = 0; i < len && address + i < config_size; val >>= 8, ++i) {
+        uint8_t wmask = pci_dev->wmask[address + i];
+        pci_dev->config[address + i] =
+            (pci_dev->config[address + i] & ~wmask) | (val & wmask);
+    }
 }
 
 void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)

[Qemu-devel] [PATCH v3 2/9] pci: Remove pci_enable_capability_support()

[Alex Williamson]

This interface doesn't make much sense, adding a capability can
take care of everything, just provide a means to register
capability read/write handlers.

Device assignment does it's own thing, so requires a couple
ugly hacks that will be cleaned by subsequent patches.

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

 hw/device-assignment.c |   12 ++++++++---
 hw/pci.c               |   52 +++++++++++++++++++++---------------------------
 hw/pci.h               |    9 +++-----
 3 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 369bff9..a297cb4 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1292,7 +1292,12 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
     PCIRegion *pci_region = dev->real_device.regions;
     int next_cap_pt = 0;
 
+    pci_dev->cap.supported = 1;
+    pci_dev->cap.start = PCI_CAPABILITY_CONFIG_DEFAULT_START_ADDR;
     pci_dev->cap.length = 0;
+    pci_dev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+    pci_dev->config[PCI_CAPABILITY_LIST] = pci_dev->cap.start;
+
 #ifdef KVM_CAP_IRQ_ROUTING
 #ifdef KVM_CAP_DEVICE_MSI
     /* Expose MSI capability
@@ -1488,9 +1493,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     dev->h_busnr = dev->host.bus;
     dev->h_devfn = PCI_DEVFN(dev->host.dev, dev->host.func);
 
-    if (pci_enable_capability_support(pci_dev, 0, NULL,
-                    assigned_device_pci_cap_write_config,
-                    assigned_device_pci_cap_init) < 0)
+    pci_register_capability_handlers(pci_dev, NULL,
+                                     assigned_device_pci_cap_write_config);
+
+    if (assigned_device_pci_cap_init(pci_dev) < 0)
         goto out;
 
     /* assign device to guest */
diff --git a/hw/pci.c b/hw/pci.c
index 8e99746..f2896d9 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -770,6 +770,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         config_write = pci_default_write_config;
     pci_dev->config_read = config_read;
     pci_dev->config_write = config_write;
+    pci_dev->cap.config_read = pci_default_cap_read_config;
+    pci_dev->cap.config_write = pci_default_cap_write_config;
     bus->devices[devfn] = pci_dev;
     pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
     pci_dev->version_id = 2; /* Current pci device vmstate version */
@@ -1754,35 +1756,21 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
     return dev;
 }
 
-int pci_enable_capability_support(PCIDevice *pci_dev,
-                                  uint32_t config_start,
-                                  PCICapConfigReadFunc *config_read,
-                                  PCICapConfigWriteFunc *config_write,
-                                  PCICapConfigInitFunc *config_init)
+void pci_register_capability_handlers(PCIDevice *pdev,
+                                      PCICapConfigReadFunc *config_read,
+                                      PCICapConfigWriteFunc *config_write)
 {
-    if (!pci_dev)
-        return -ENODEV;
-
-    pci_dev->config[0x06] |= 0x10; // status = capabilities
-
-    if (config_start == 0)
-	pci_dev->cap.start = PCI_CAPABILITY_CONFIG_DEFAULT_START_ADDR;
-    else if (config_start >= 0x40 && config_start < 0xff)
-        pci_dev->cap.start = config_start;
-    else
-        return -EINVAL;
+    if (config_read) {
+        pdev->cap.config_read = config_read;
+    } else {
+        pdev->cap.config_read = pci_default_cap_read_config;
+    }
 
-    if (config_read)
-        pci_dev->cap.config_read = config_read;
-    else
-        pci_dev->cap.config_read = pci_default_cap_read_config;
-    if (config_write)
-        pci_dev->cap.config_write = config_write;
-    else
-        pci_dev->cap.config_write = pci_default_cap_write_config;
-    pci_dev->cap.supported = 1;
-    pci_dev->config[PCI_CAPABILITY_LIST] = pci_dev->cap.start;
-    return config_init(pci_dev);
+    if (config_write) {
+        pdev->cap.config_write = config_write;
+    } else {
+        pdev->cap.config_write = pci_default_cap_write_config;
+    }
 }
 
 PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name)
@@ -1920,12 +1908,16 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
     config[PCI_CAP_LIST_ID] = cap_id;
     config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
     pdev->config[PCI_CAPABILITY_LIST] = offset;
-    pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
     memset(pdev->used + offset, 0xFF, size);
     /* Make capability read-only by default */
     memset(pdev->wmask + offset, 0, size);
     /* Check capability by default */
     memset(pdev->cmask + offset, 0xFF, size);
+
+    pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+    pdev->cap.supported = 1;
+    pdev->cap.start = pdev->cap.start ? MIN(pdev->cap.start, offset) : offset;
+
     return offset;
 }
 
@@ -1943,8 +1935,10 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
     memset(pdev->cmask + offset, 0, size);
     memset(pdev->used + offset, 0, size);
 
-    if (!pdev->config[PCI_CAPABILITY_LIST])
+    if (!pdev->config[PCI_CAPABILITY_LIST]) {
         pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
+        pdev->cap.start = pdev->cap.length = 0;
+    }
 }
 
 /* Reserve space for capability at a known offset (to call after load). */
diff --git a/hw/pci.h b/hw/pci.h
index e7a19e5..fafc2bb 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -89,7 +89,6 @@ typedef void PCICapConfigWriteFunc(PCIDevice *pci_dev,
                                    uint32_t address, uint32_t val, int len);
 typedef uint32_t PCICapConfigReadFunc(PCIDevice *pci_dev,
                                       uint32_t address, int len);
-typedef int PCICapConfigInitFunc(PCIDevice *pci_dev);
 
 typedef struct PCIIORegion {
     pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
@@ -240,11 +239,9 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
 void pci_map_option_rom(PCIDevice *pdev, int region_num, pcibus_t addr,
                         pcibus_t size, int type);
 
-int pci_enable_capability_support(PCIDevice *pci_dev,
-                                  uint32_t config_start,
-                                  PCICapConfigReadFunc *config_read,
-                                  PCICapConfigWriteFunc *config_write,
-                                  PCICapConfigInitFunc *config_init);
+void pci_register_capability_handlers(PCIDevice *pci_dev,
+                                      PCICapConfigReadFunc *config_read,
+                                      PCICapConfigWriteFunc *config_write);
 
 int pci_map_irq(PCIDevice *pci_dev, int pin);
 

[Qemu-devel] [PATCH v3 3/9] device-assignment: Use PCI capabilities support

[Alex Williamson]

Convert to use common pci_add_capabilities() rather than creating
our own mess.

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

 hw/device-assignment.c |  112 +++++++++++++++++++++++++++---------------------
 1 files changed, 63 insertions(+), 49 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index a297cb4..76aacac 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -38,6 +38,7 @@
 #include "device-assignment.h"
 #include "loader.h"
 #include "monitor.h"
+#include "range.h"
 #include <pci/header.h>
 
 /* From linux/ioport.h */
@@ -1075,17 +1076,17 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev, unsigned int ctrl_pos)
     }
 
     if (ctrl_byte & PCI_MSI_FLAGS_ENABLE) {
+        int pos = ctrl_pos - PCI_MSI_FLAGS;
         assigned_dev->entry = calloc(1, sizeof(struct kvm_irq_routing_entry));
         if (!assigned_dev->entry) {
             perror("assigned_dev_update_msi: ");
             return;
         }
         assigned_dev->entry->u.msi.address_lo =
-                *(uint32_t *)(pci_dev->config + pci_dev->cap.start +
-                              PCI_MSI_ADDRESS_LO);
+            pci_get_long(pci_dev->config + pos + PCI_MSI_ADDRESS_LO);
         assigned_dev->entry->u.msi.address_hi = 0;
-        assigned_dev->entry->u.msi.data = *(uint16_t *)(pci_dev->config +
-                pci_dev->cap.start + PCI_MSI_DATA_32);
+        assigned_dev->entry->u.msi.data =
+            pci_get_word(pci_dev->config + pos + PCI_MSI_DATA_32);
         assigned_dev->entry->type = KVM_IRQ_ROUTING_MSI;
         r = kvm_get_irq_route_gsi();
         if (r < 0) {
@@ -1123,10 +1124,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     struct kvm_assigned_msix_entry msix_entry;
     void *va = adev->msix_table_page;
 
-    if (adev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
-        pos = pci_dev->cap.start + PCI_CAPABILITY_CONFIG_MSI_LENGTH;
-    else
-        pos = pci_dev->cap.start;
+    pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSIX);
 
     entries_max_nr = *(uint16_t *)(pci_dev->config + pos + 2);
     entries_max_nr &= PCI_MSIX_TABSIZE;
@@ -1260,26 +1258,23 @@ static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t ad
                                                  uint32_t val, int len)
 {
     AssignedDevice *assigned_dev = container_of(pci_dev, AssignedDevice, dev);
-    unsigned int pos = pci_dev->cap.start, ctrl_pos;
 
     pci_default_cap_write_config(pci_dev, address, val, len);
 #ifdef KVM_CAP_IRQ_ROUTING
 #ifdef KVM_CAP_DEVICE_MSI
     if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) {
-        ctrl_pos = pos + PCI_MSI_FLAGS;
-        if (address <= ctrl_pos && address + len > ctrl_pos)
-            assigned_dev_update_msi(pci_dev, ctrl_pos);
-        pos += PCI_CAPABILITY_CONFIG_MSI_LENGTH;
+        int pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSI);
+        if (ranges_overlap(address, len, pos + PCI_MSI_FLAGS, 1)) {
+            assigned_dev_update_msi(pci_dev, pos + PCI_MSI_FLAGS);
+        }
     }
 #endif
 #ifdef KVM_CAP_DEVICE_MSIX
     if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
-        ctrl_pos = pos + 3;
-        if (address <= ctrl_pos && address + len > ctrl_pos) {
-            ctrl_pos--; /* control is word long */
-            assigned_dev_update_msix(pci_dev, ctrl_pos);
+        int pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSIX);
+        if (ranges_overlap(address, len, pos + PCI_MSIX_FLAGS + 1, 1)) {
+            assigned_dev_update_msix(pci_dev, pos + PCI_MSIX_FLAGS);
 	}
-        pos += PCI_CAPABILITY_CONFIG_MSIX_LENGTH;
     }
 #endif
 #endif
@@ -1290,58 +1285,77 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
 {
     AssignedDevice *dev = container_of(pci_dev, AssignedDevice, dev);
     PCIRegion *pci_region = dev->real_device.regions;
-    int next_cap_pt = 0;
 
-    pci_dev->cap.supported = 1;
-    pci_dev->cap.start = PCI_CAPABILITY_CONFIG_DEFAULT_START_ADDR;
+    /* Clear initial capabilities pointer and status copied from hw */
+    pci_set_byte(pci_dev->config + PCI_CAPABILITY_LIST, 0);
+    pci_set_word(pci_dev->config + PCI_STATUS,
+                 pci_get_word(pci_dev->config + PCI_STATUS) &
+                 ~PCI_STATUS_CAP_LIST);
+
     pci_dev->cap.length = 0;
-    pci_dev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
-    pci_dev->config[PCI_CAPABILITY_LIST] = pci_dev->cap.start;
 
 #ifdef KVM_CAP_IRQ_ROUTING
 #ifdef KVM_CAP_DEVICE_MSI
     /* Expose MSI capability
      * MSI capability is the 1st capability in capability config */
     if (pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI)) {
+        int vpos, ppos;
+        uint16_t flags;
+
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
-        memset(&pci_dev->config[pci_dev->cap.start + pci_dev->cap.length],
-               0, PCI_CAPABILITY_CONFIG_MSI_LENGTH);
-        pci_dev->config[pci_dev->cap.start + pci_dev->cap.length] =
-                        PCI_CAP_ID_MSI;
+        vpos = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, 0,
+                                  PCI_CAPABILITY_CONFIG_MSI_LENGTH);
+
+        memset(pci_dev->config + vpos + PCI_CAP_FLAGS, 0,
+               PCI_CAPABILITY_CONFIG_MSI_LENGTH - PCI_CAP_FLAGS);
+
+        /* Only 32-bit/no-mask currently supported */
+        ppos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI);
+        flags = assigned_dev_pci_read_word(pci_dev, ppos + PCI_MSI_FLAGS);
+        flags &= PCI_MSI_FLAGS_QMASK;
+        pci_set_word(pci_dev->config + vpos + PCI_MSI_FLAGS, flags);
+
+        /* Set writable fields */
+        pci_set_word(pci_dev->wmask + vpos + PCI_MSI_FLAGS,
+                     PCI_MSI_FLAGS_QSIZE | PCI_MSI_FLAGS_ENABLE);
+        pci_set_long(pci_dev->wmask + vpos + PCI_MSI_ADDRESS_LO, 0xfffffffc);
+        pci_set_long(pci_dev->wmask + vpos + PCI_MSI_DATA_32, 0xffff);
         pci_dev->cap.length += PCI_CAPABILITY_CONFIG_MSI_LENGTH;
-        next_cap_pt = 1;
     }
 #endif
 #ifdef KVM_CAP_DEVICE_MSIX
     /* Expose MSI-X capability */
     if (pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX)) {
-        int pos, entry_nr, bar_nr;
+        int vpos, ppos, entry_nr, bar_nr;
         uint32_t msix_table_entry;
+
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
-        memset(&pci_dev->config[pci_dev->cap.start + pci_dev->cap.length],
-               0, PCI_CAPABILITY_CONFIG_MSIX_LENGTH);
-        pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX);
-        entry_nr = assigned_dev_pci_read_word(pci_dev, pos + 2) &
-                                                             PCI_MSIX_TABSIZE;
-        pci_dev->config[pci_dev->cap.start + pci_dev->cap.length] = 0x11;
-        *(uint16_t *)(pci_dev->config + pci_dev->cap.start +
-                      pci_dev->cap.length + 2) = entry_nr;
+        vpos = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, 0,
+                           PCI_CAPABILITY_CONFIG_MSIX_LENGTH);
+
+        memset(pci_dev->config + vpos + PCI_CAP_FLAGS, 0,
+               PCI_CAPABILITY_CONFIG_MSIX_LENGTH - PCI_CAP_FLAGS);
+
+        /* Only enable and function mask bits are writable */
+        pci_set_word(pci_dev->wmask + vpos + PCI_MSIX_FLAGS,
+                     PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
+
+        ppos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX);
+
+        entry_nr = assigned_dev_pci_read_word(pci_dev, ppos + PCI_MSIX_FLAGS);
+        entry_nr &= PCI_MSIX_TABSIZE;
+        pci_set_word(pci_dev->config + vpos + PCI_MSIX_FLAGS, entry_nr);
+
         msix_table_entry = assigned_dev_pci_read_long(pci_dev,
-                                                      pos + PCI_MSIX_TABLE);
-        *(uint32_t *)(pci_dev->config + pci_dev->cap.start +
-                      pci_dev->cap.length + PCI_MSIX_TABLE) = msix_table_entry;
-        *(uint32_t *)(pci_dev->config + pci_dev->cap.start +
-                      pci_dev->cap.length + PCI_MSIX_PBA) =
-                    assigned_dev_pci_read_long(pci_dev, pos + PCI_MSIX_PBA);
+                                                      ppos + PCI_MSIX_TABLE);
+        pci_set_long(pci_dev->config + vpos + PCI_MSIX_TABLE, msix_table_entry);
+
+        pci_set_long(pci_dev->config + vpos + PCI_MSIX_PBA,
+                     assigned_dev_pci_read_long(pci_dev, ppos + PCI_MSIX_PBA));
+
         bar_nr = msix_table_entry & PCI_MSIX_BIR;
         msix_table_entry &= ~PCI_MSIX_BIR;
         dev->msix_table_addr = pci_region[bar_nr].base_addr + msix_table_entry;
-        if (next_cap_pt != 0) {
-            pci_dev->config[pci_dev->cap.start + next_cap_pt] =
-                pci_dev->cap.start + pci_dev->cap.length;
-            next_cap_pt += PCI_CAPABILITY_CONFIG_MSI_LENGTH;
-        } else
-            next_cap_pt = 1;
         pci_dev->cap.length += PCI_CAPABILITY_CONFIG_MSIX_LENGTH;
     }
 #endif

[Qemu-devel] [PATCH v3 4/9] pci: Replace used bitmap with config byte map

[Alex Williamson]

Capabilities are allocated in bytes, so we can track both whether
a byte is used and by what capability in the same structure.

Remove pci_reserve_capability() as there are no users, remove
pci_access_cap_config() since it's now a trivial lookup.

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

 hw/device-assignment.c |    4 ++--
 hw/pci.c               |   30 +++++++++---------------------
 hw/pci.h               |    8 ++------
 3 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 76aacac..a4fad60 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -423,7 +423,7 @@ static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address,
 
     if ((address >= 0x10 && address <= 0x24) || address == 0x30 ||
         address == 0x34 || address == 0x3c || address == 0x3d ||
-        pci_access_cap_config(d, address, len)) {
+        (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address])) {
         /* used for update-mappings (BAR emulation) */
         pci_default_write_config(d, address, val, len);
         return;
@@ -459,7 +459,7 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address,
     if (address < 0x4 || (pci_dev->need_emulate_cmd && address == 0x4) ||
 	(address >= 0x10 && address <= 0x24) || address == 0x30 ||
         address == 0x34 || address == 0x3c || address == 0x3d ||
-        pci_access_cap_config(d, address, len)) {
+        (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address])) {
         val = pci_default_read_config(d, address, len);
         DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n",
               (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len);
diff --git a/hw/pci.c b/hw/pci.c
index f2896d9..cbe6fb7 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -712,7 +712,7 @@ static void pci_config_alloc(PCIDevice *pci_dev)
     pci_dev->cmask = qemu_mallocz(config_size);
     pci_dev->wmask = qemu_mallocz(config_size);
     pci_dev->w1cmask = qemu_mallocz(config_size);
-    pci_dev->used = qemu_mallocz(config_size);
+    pci_dev->config_map = qemu_mallocz(config_size);
 }
 
 static void pci_config_free(PCIDevice *pci_dev)
@@ -721,7 +721,7 @@ static void pci_config_free(PCIDevice *pci_dev)
     qemu_free(pci_dev->cmask);
     qemu_free(pci_dev->wmask);
     qemu_free(pci_dev->w1cmask);
-    qemu_free(pci_dev->used);
+    qemu_free(pci_dev->config_map);
 }
 
 /* -1 for devfn means auto assign */
@@ -751,6 +751,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     pci_dev->irq_state = 0;
     pci_config_alloc(pci_dev);
 
+    memset(pci_dev->config_map, 0xff, PCI_CONFIG_HEADER_SIZE);
+
     if (!is_bridge) {
         pci_set_default_subsystem_id(pci_dev);
     }
@@ -1083,21 +1085,13 @@ uint32_t pci_default_read_config(PCIDevice *d,
 {
     assert(len == 1 || len == 2 || len == 4);
 
-    if (pci_access_cap_config(d, address, len)) {
+    if (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address]) {
         return d->cap.config_read(d, address, len);
     }
 
     return pci_read_config(d, address, len);
 }
 
-int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len)
-{
-    if (pci_dev->cap.supported && address >= pci_dev->cap.start &&
-            (address + len) < pci_dev->cap.start + pci_dev->cap.length)
-        return 1;
-    return 0;
-}
-
 uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
                                      uint32_t address, int len)
 {
@@ -1122,7 +1116,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
     int i, was_irq_disabled = pci_irq_disabled(d);
     uint32_t config_size = pci_config_size(d);
 
-    if (pci_access_cap_config(d, addr, l)) {
+    if (addr > PCI_CONFIG_HEADER_SIZE && d->config_map[addr]) {
         d->cap.config_write(d, addr, val, l);
         return;
     }
@@ -1789,7 +1783,7 @@ static int pci_find_space(PCIDevice *pdev, uint8_t size)
     int offset = PCI_CONFIG_HEADER_SIZE;
     int i;
     for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i)
-        if (pdev->used[i])
+        if (pdev->config_map[i])
             offset = i + 1;
         else if (i - offset + 1 == size)
             return offset;
@@ -1908,7 +1902,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
     config[PCI_CAP_LIST_ID] = cap_id;
     config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
     pdev->config[PCI_CAPABILITY_LIST] = offset;
-    memset(pdev->used + offset, 0xFF, size);
+    memset(pdev->config_map + offset, cap_id, size);
     /* Make capability read-only by default */
     memset(pdev->wmask + offset, 0, size);
     /* Check capability by default */
@@ -1933,7 +1927,7 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
     memset(pdev->w1cmask + offset, 0, size);
     /* Clear cmask as device-specific registers can't be checked */
     memset(pdev->cmask + offset, 0, size);
-    memset(pdev->used + offset, 0, size);
+    memset(pdev->config_map + offset, 0, size);
 
     if (!pdev->config[PCI_CAPABILITY_LIST]) {
         pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
@@ -1941,12 +1935,6 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
     }
 }
 
-/* Reserve space for capability at a known offset (to call after load). */
-void pci_reserve_capability(PCIDevice *pdev, uint8_t offset, uint8_t size)
-{
-    memset(pdev->used + offset, 0xff, size);
-}
-
 uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id)
 {
     return pci_find_capability_list(pdev, cap_id, NULL);
diff --git a/hw/pci.h b/hw/pci.h
index fafc2bb..bd15b43 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -157,8 +157,8 @@ struct PCIDevice {
     /* Used to implement RW1C(Write 1 to Clear) bytes */
     uint8_t *w1cmask;
 
-    /* Used to allocate config space for capabilities. */
-    uint8_t *used;
+    /* Used to allocate config space and track capabilities. */
+    uint8_t *config_map;
 
     /* the following fields are read only */
     PCIBus *bus;
@@ -250,8 +250,6 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
 
 void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
 
-void pci_reserve_capability(PCIDevice *pci_dev, uint8_t offset, uint8_t size);
-
 uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
 
 uint32_t pci_default_read_config(PCIDevice *d,
@@ -264,8 +262,6 @@ uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
                                      uint32_t address, int len);
 void pci_default_cap_write_config(PCIDevice *pci_dev,
                                   uint32_t address, uint32_t val, int len);
-int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len);
-
 typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev, int state);

[Qemu-devel] [PATCH v3 5/9] pci: Remove cap.length, cap.start, cap.supported

[Alex Williamson]

Capabilities aren't required to be contiguous, so cap.length never
really made much sense.  Likewise, cap.start is mostly meaningless
too.  Both of these are better served by the capability map.  We
can also get rid of cap.supported, since it's really now unused
and redundant with flag in the status word anyway.

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

 hw/device-assignment.c |    4 ----
 hw/pci.c               |    3 ---
 hw/pci.h               |    2 --
 3 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index a4fad60..975d3cb 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1292,8 +1292,6 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
                  pci_get_word(pci_dev->config + PCI_STATUS) &
                  ~PCI_STATUS_CAP_LIST);
 
-    pci_dev->cap.length = 0;
-
 #ifdef KVM_CAP_IRQ_ROUTING
 #ifdef KVM_CAP_DEVICE_MSI
     /* Expose MSI capability
@@ -1320,7 +1318,6 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
                      PCI_MSI_FLAGS_QSIZE | PCI_MSI_FLAGS_ENABLE);
         pci_set_long(pci_dev->wmask + vpos + PCI_MSI_ADDRESS_LO, 0xfffffffc);
         pci_set_long(pci_dev->wmask + vpos + PCI_MSI_DATA_32, 0xffff);
-        pci_dev->cap.length += PCI_CAPABILITY_CONFIG_MSI_LENGTH;
     }
 #endif
 #ifdef KVM_CAP_DEVICE_MSIX
@@ -1356,7 +1353,6 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         bar_nr = msix_table_entry & PCI_MSIX_BIR;
         msix_table_entry &= ~PCI_MSIX_BIR;
         dev->msix_table_addr = pci_region[bar_nr].base_addr + msix_table_entry;
-        pci_dev->cap.length += PCI_CAPABILITY_CONFIG_MSIX_LENGTH;
     }
 #endif
 #endif
diff --git a/hw/pci.c b/hw/pci.c
index cbe6fb7..1cf62b6 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1909,8 +1909,6 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
     memset(pdev->cmask + offset, 0xFF, size);
 
     pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
-    pdev->cap.supported = 1;
-    pdev->cap.start = pdev->cap.start ? MIN(pdev->cap.start, offset) : offset;
 
     return offset;
 }
@@ -1931,7 +1929,6 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
 
     if (!pdev->config[PCI_CAPABILITY_LIST]) {
         pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
-        pdev->cap.start = pdev->cap.length = 0;
     }
 }
 
diff --git a/hw/pci.h b/hw/pci.h
index bd15b43..146f81d 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -220,8 +220,6 @@ struct PCIDevice {
 
     /* Device capability configuration space */
     struct {
-        int supported;
-        unsigned int start, length;
         PCICapConfigReadFunc *config_read;
         PCICapConfigWriteFunc *config_write;
     } cap;

[Qemu-devel] [PATCH v3 6/9] device-assignment: Move PCI capabilities to match physical hardware

[Alex Williamson]

Now that common PCI code doesn't have a hangup on capabilities
being contiguous, move assigned device capabilities to match
their offset on physical hardware.  This helps for drivers that
assume a capability configuration and don't bother searching.

We can also remove several calls to assigned_dev_pci_read_* because
we're overlaying the capability at the same location as the initial
copy we made of config space.  We can therefore just use pci_get_*.

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

 hw/device-assignment.c |   65 +++++++++++++++---------------------------------
 1 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 975d3cb..6314773 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -366,16 +366,6 @@ static uint8_t assigned_dev_pci_read_byte(PCIDevice *d, int pos)
     return (uint8_t)assigned_dev_pci_read(d, pos, 1);
 }
 
-static uint16_t assigned_dev_pci_read_word(PCIDevice *d, int pos)
-{
-    return (uint16_t)assigned_dev_pci_read(d, pos, 2);
-}
-
-static uint32_t assigned_dev_pci_read_long(PCIDevice *d, int pos)
-{
-    return assigned_dev_pci_read(d, pos, 4);
-}
-
 static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap)
 {
     int id;
@@ -1285,6 +1275,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
 {
     AssignedDevice *dev = container_of(pci_dev, AssignedDevice, dev);
     PCIRegion *pci_region = dev->real_device.regions;
+    int pos;
 
     /* Clear initial capabilities pointer and status copied from hw */
     pci_set_byte(pci_dev->config + PCI_CAPABILITY_LIST, 0);
@@ -1296,60 +1287,44 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
 #ifdef KVM_CAP_DEVICE_MSI
     /* Expose MSI capability
      * MSI capability is the 1st capability in capability config */
-    if (pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI)) {
-        int vpos, ppos;
-        uint16_t flags;
-
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI))) {
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
-        vpos = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, 0,
-                                  PCI_CAPABILITY_CONFIG_MSI_LENGTH);
-
-        memset(pci_dev->config + vpos + PCI_CAP_FLAGS, 0,
-               PCI_CAPABILITY_CONFIG_MSI_LENGTH - PCI_CAP_FLAGS);
+        pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos,
+                           PCI_CAPABILITY_CONFIG_MSI_LENGTH);
 
         /* Only 32-bit/no-mask currently supported */
-        ppos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI);
-        flags = assigned_dev_pci_read_word(pci_dev, ppos + PCI_MSI_FLAGS);
-        flags &= PCI_MSI_FLAGS_QMASK;
-        pci_set_word(pci_dev->config + vpos + PCI_MSI_FLAGS, flags);
+        pci_set_word(pci_dev->config + pos + PCI_MSI_FLAGS,
+                     pci_get_word(pci_dev->config + pos + PCI_MSI_FLAGS) &
+                     PCI_MSI_FLAGS_QMASK);
+        pci_set_long(pci_dev->config + pos + PCI_MSI_ADDRESS_LO, 0);
+        pci_set_word(pci_dev->config + pos + PCI_MSI_DATA_32, 0);
 
         /* Set writable fields */
-        pci_set_word(pci_dev->wmask + vpos + PCI_MSI_FLAGS,
+        pci_set_word(pci_dev->wmask + pos + PCI_MSI_FLAGS,
                      PCI_MSI_FLAGS_QSIZE | PCI_MSI_FLAGS_ENABLE);
-        pci_set_long(pci_dev->wmask + vpos + PCI_MSI_ADDRESS_LO, 0xfffffffc);
-        pci_set_long(pci_dev->wmask + vpos + PCI_MSI_DATA_32, 0xffff);
+        pci_set_long(pci_dev->wmask + pos + PCI_MSI_ADDRESS_LO, 0xfffffffc);
+        pci_set_word(pci_dev->wmask + pos + PCI_MSI_DATA_32, 0xffff);
     }
 #endif
 #ifdef KVM_CAP_DEVICE_MSIX
     /* Expose MSI-X capability */
-    if (pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX)) {
-        int vpos, ppos, entry_nr, bar_nr;
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX))) {
+        int bar_nr;
         uint32_t msix_table_entry;
 
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
-        vpos = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, 0,
+        pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos,
                            PCI_CAPABILITY_CONFIG_MSIX_LENGTH);
 
-        memset(pci_dev->config + vpos + PCI_CAP_FLAGS, 0,
-               PCI_CAPABILITY_CONFIG_MSIX_LENGTH - PCI_CAP_FLAGS);
+        pci_set_word(pci_dev->config + pos + PCI_MSIX_FLAGS,
+                     pci_get_word(pci_dev->config + pos + PCI_MSIX_FLAGS) &
+                     PCI_MSIX_TABSIZE);
 
         /* Only enable and function mask bits are writable */
-        pci_set_word(pci_dev->wmask + vpos + PCI_MSIX_FLAGS,
+        pci_set_word(pci_dev->wmask + pos + PCI_MSIX_FLAGS,
                      PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
 
-        ppos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX);
-
-        entry_nr = assigned_dev_pci_read_word(pci_dev, ppos + PCI_MSIX_FLAGS);
-        entry_nr &= PCI_MSIX_TABSIZE;
-        pci_set_word(pci_dev->config + vpos + PCI_MSIX_FLAGS, entry_nr);
-
-        msix_table_entry = assigned_dev_pci_read_long(pci_dev,
-                                                      ppos + PCI_MSIX_TABLE);
-        pci_set_long(pci_dev->config + vpos + PCI_MSIX_TABLE, msix_table_entry);
-
-        pci_set_long(pci_dev->config + vpos + PCI_MSIX_PBA,
-                     assigned_dev_pci_read_long(pci_dev, ppos + PCI_MSIX_PBA));
-
+        msix_table_entry = pci_get_long(pci_dev->config + pos + PCI_MSIX_TABLE);
         bar_nr = msix_table_entry & PCI_MSIX_BIR;
         msix_table_entry &= ~PCI_MSIX_BIR;
         dev->msix_table_addr = pci_region[bar_nr].base_addr + msix_table_entry;

[Qemu-devel] [PATCH v3 7/9] pci: Remove capability specific handlers

[Alex Williamson]

Drivers can break these out on their own if they need to.

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

 hw/device-assignment.c |   16 ++++++++-----
 hw/pci.c               |   61 ++----------------------------------------------
 hw/pci.h               |   19 ---------------
 3 files changed, 13 insertions(+), 83 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 6314773..970ffa1 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -63,6 +63,10 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev);
 
 static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev);
 
+static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev,
+                                                 uint32_t address,
+                                                 uint32_t val, int len);
+
 static uint32_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
                                        uint32_t addr, int len, uint32_t *val)
 {
@@ -406,14 +410,17 @@ static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address,
           ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7),
           (uint16_t) address, val, len);
 
+    if (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address]) {
+        return assigned_device_pci_cap_write_config(d, address, val, len);
+    }
+
     if (address == 0x4) {
         pci_default_write_config(d, address, val, len);
         /* Continue to program the card */
     }
 
     if ((address >= 0x10 && address <= 0x24) || address == 0x30 ||
-        address == 0x34 || address == 0x3c || address == 0x3d ||
-        (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address])) {
+        address == 0x34 || address == 0x3c || address == 0x3d) {
         /* used for update-mappings (BAR emulation) */
         pci_default_write_config(d, address, val, len);
         return;
@@ -1249,7 +1256,7 @@ static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t ad
 {
     AssignedDevice *assigned_dev = container_of(pci_dev, AssignedDevice, dev);
 
-    pci_default_cap_write_config(pci_dev, address, val, len);
+    pci_default_write_config(pci_dev, address, val, len);
 #ifdef KVM_CAP_IRQ_ROUTING
 #ifdef KVM_CAP_DEVICE_MSI
     if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) {
@@ -1478,9 +1485,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     dev->h_busnr = dev->host.bus;
     dev->h_devfn = PCI_DEVFN(dev->host.dev, dev->host.func);
 
-    pci_register_capability_handlers(pci_dev, NULL,
-                                     assigned_device_pci_cap_write_config);
-
     if (assigned_device_pci_cap_init(pci_dev) < 0)
         goto out;
 
diff --git a/hw/pci.c b/hw/pci.c
index 1cf62b6..e529793 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -772,8 +772,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         config_write = pci_default_write_config;
     pci_dev->config_read = config_read;
     pci_dev->config_write = config_write;
-    pci_dev->cap.config_read = pci_default_cap_read_config;
-    pci_dev->cap.config_write = pci_default_cap_write_config;
     bus->devices[devfn] = pci_dev;
     pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
     pci_dev->version_id = 2; /* Current pci device vmstate version */
@@ -1070,57 +1068,21 @@ static void pci_update_irq_disabled(PCIDevice *d, int was_irq_disabled)
     }
 }
 
-static uint32_t pci_read_config(PCIDevice *d,
-                                uint32_t address, int len)
+uint32_t pci_default_read_config(PCIDevice *d,
+                                 uint32_t address, int len)
 {
     uint32_t val = 0;
-
+    assert(len == 1 || len == 2 || len == 4);
     len = MIN(len, pci_config_size(d) - address);
     memcpy(&val, d->config + address, len);
     return le32_to_cpu(val);
 }
 
-uint32_t pci_default_read_config(PCIDevice *d,
-                                 uint32_t address, int len)
-{
-    assert(len == 1 || len == 2 || len == 4);
-
-    if (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address]) {
-        return d->cap.config_read(d, address, len);
-    }
-
-    return pci_read_config(d, address, len);
-}
-
-uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
-                                     uint32_t address, int len)
-{
-    return pci_read_config(pci_dev, address, len);
-}
-
-void pci_default_cap_write_config(PCIDevice *pci_dev,
-                                  uint32_t address, uint32_t val, int len)
-{
-    uint32_t config_size = pci_config_size(pci_dev);
-    int i;
-
-    for (i = 0; i < len && address + i < config_size; val >>= 8, ++i) {
-        uint8_t wmask = pci_dev->wmask[address + i];
-        pci_dev->config[address + i] =
-            (pci_dev->config[address + i] & ~wmask) | (val & wmask);
-    }
-}
-
 void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
 {
     int i, was_irq_disabled = pci_irq_disabled(d);
     uint32_t config_size = pci_config_size(d);
 
-    if (addr > PCI_CONFIG_HEADER_SIZE && d->config_map[addr]) {
-        d->cap.config_write(d, addr, val, l);
-        return;
-    }
-
     for (i = 0; i < l && addr + i < config_size; val >>= 8, ++i) {
         uint8_t wmask = d->wmask[addr + i];
         uint8_t w1cmask = d->w1cmask[addr + i];
@@ -1750,23 +1712,6 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
     return dev;
 }
 
-void pci_register_capability_handlers(PCIDevice *pdev,
-                                      PCICapConfigReadFunc *config_read,
-                                      PCICapConfigWriteFunc *config_write)
-{
-    if (config_read) {
-        pdev->cap.config_read = config_read;
-    } else {
-        pdev->cap.config_read = pci_default_cap_read_config;
-    }
-
-    if (config_write) {
-        pdev->cap.config_write = config_write;
-    } else {
-        pdev->cap.config_write = pci_default_cap_write_config;
-    }
-}
-
 PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name)
 {
     return pci_create_multifunction(bus, devfn, false, name);
diff --git a/hw/pci.h b/hw/pci.h
index 146f81d..1fe6e49 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -85,11 +85,6 @@ typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
                                 pcibus_t addr, pcibus_t size, int type);
 typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
 
-typedef void PCICapConfigWriteFunc(PCIDevice *pci_dev,
-                                   uint32_t address, uint32_t val, int len);
-typedef uint32_t PCICapConfigReadFunc(PCIDevice *pci_dev,
-                                      uint32_t address, int len);
-
 typedef struct PCIIORegion {
     pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
 #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
@@ -217,12 +212,6 @@ struct PCIDevice {
     struct kvm_msix_message *msix_irq_entries;
 
     msix_mask_notifier_func msix_mask_notifier;
-
-    /* Device capability configuration space */
-    struct {
-        PCICapConfigReadFunc *config_read;
-        PCICapConfigWriteFunc *config_write;
-    } cap;
 };
 
 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
@@ -237,10 +226,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
 void pci_map_option_rom(PCIDevice *pdev, int region_num, pcibus_t addr,
                         pcibus_t size, int type);
 
-void pci_register_capability_handlers(PCIDevice *pci_dev,
-                                      PCICapConfigReadFunc *config_read,
-                                      PCICapConfigWriteFunc *config_write);
-
 int pci_map_irq(PCIDevice *pci_dev, int pin);
 
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
@@ -256,10 +241,6 @@ void pci_default_write_config(PCIDevice *d,
                               uint32_t address, uint32_t val, int len);
 void pci_device_save(PCIDevice *s, QEMUFile *f);
 int pci_device_load(PCIDevice *s, QEMUFile *f);
-uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
-                                     uint32_t address, int len);
-void pci_default_cap_write_config(PCIDevice *pci_dev,
-                                  uint32_t address, uint32_t val, int len);
 typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev, int state);

[Qemu-devel] [PATCH v3 8/9] device-assignment: Make use of config_map

[Alex Williamson]

We can figure out the capability being touched much more quickly
and efficiently with the config_map.  Use it.

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

 hw/device-assignment.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 970ffa1..832c236 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1254,28 +1254,34 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos)
 static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t address,
                                                  uint32_t val, int len)
 {
-    AssignedDevice *assigned_dev = container_of(pci_dev, AssignedDevice, dev);
+    uint8_t cap_id = pci_dev->config_map[address];
 
     pci_default_write_config(pci_dev, address, val, len);
+    switch (cap_id) {
 #ifdef KVM_CAP_IRQ_ROUTING
+    case PCI_CAP_ID_MSI:
 #ifdef KVM_CAP_DEVICE_MSI
-    if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) {
-        int pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSI);
-        if (ranges_overlap(address, len, pos + PCI_MSI_FLAGS, 1)) {
-            assigned_dev_update_msi(pci_dev, pos + PCI_MSI_FLAGS);
+        {
+            uint8_t cap = pci_find_capability(pci_dev, cap_id);
+            if (ranges_overlap(address - cap, len, PCI_MSI_FLAGS, 1)) {
+                assigned_dev_update_msi(pci_dev, cap + PCI_MSI_FLAGS);
+            }
         }
-    }
 #endif
+        break;
+
+    case PCI_CAP_ID_MSIX:
 #ifdef KVM_CAP_DEVICE_MSIX
-    if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
-        int pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSIX);
-        if (ranges_overlap(address, len, pos + PCI_MSIX_FLAGS + 1, 1)) {
-            assigned_dev_update_msix(pci_dev, pos + PCI_MSIX_FLAGS);
-	}
-    }
+        {
+            uint8_t cap = pci_find_capability(pci_dev, cap_id);
+            if (ranges_overlap(address - cap, len, PCI_MSIX_FLAGS + 1, 1)) {
+                assigned_dev_update_msix(pci_dev, cap + PCI_MSIX_FLAGS);
+            }
+        }
 #endif
+        break;
 #endif
-    return;
+    }
 }
 
 static int assigned_device_pci_cap_init(PCIDevice *pci_dev)

[Qemu-devel] [PATCH v3 9/9] device-assignment: pass through and stub more PCI caps

[Alex Williamson]

Some drivers depend on finding capabilities like power management,
PCI express/X, vital product data, or vendor specific fields.  Now
that we have better capability support, we can pass more of these
tables through to the guest.  Note that VPD and VNDR are direct pass
through capabilies, the rest are mostly empty shells with a few
writable bits where necessary.

It may be possible to consolidate dummy capabilities into common files
for other drivers to use, but I prefer to leave them here for now as
we figure out what bits to handle directly with hardware and what bits
are purely emulated.

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

 hw/device-assignment.c |  213 +++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 199 insertions(+), 14 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 832c236..cd62b5a 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -67,6 +67,9 @@ static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev,
                                                  uint32_t address,
                                                  uint32_t val, int len);
 
+static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev,
+                                                    uint32_t address, int len);
+
 static uint32_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
                                        uint32_t addr, int len, uint32_t *val)
 {
@@ -370,6 +373,27 @@ static uint8_t assigned_dev_pci_read_byte(PCIDevice *d, int pos)
     return (uint8_t)assigned_dev_pci_read(d, pos, 1);
 }
 
+static void assigned_dev_pci_write(PCIDevice *d, int pos, uint32_t val, int len)
+{
+    AssignedDevice *pci_dev = container_of(d, AssignedDevice, dev);
+    ssize_t ret;
+    int fd = pci_dev->real_device.config_fd;
+
+again:
+    ret = pwrite(fd, &val, len, pos);
+    if (ret != len) {
+	if ((ret < 0) && (errno == EINTR || errno == EAGAIN))
+	    goto again;
+
+	fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n",
+		__func__, ret, errno);
+
+	exit(1);
+    }
+
+    return;
+}
+
 static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap)
 {
     int id;
@@ -453,10 +477,13 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address,
     ssize_t ret;
     AssignedDevice *pci_dev = container_of(d, AssignedDevice, dev);
 
+    if (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address]) {
+        return assigned_device_pci_cap_read_config(d, address, len);
+    }
+
     if (address < 0x4 || (pci_dev->need_emulate_cmd && address == 0x4) ||
 	(address >= 0x10 && address <= 0x24) || address == 0x30 ||
-        address == 0x34 || address == 0x3c || address == 0x3d ||
-        (address > PCI_CONFIG_HEADER_SIZE && d->config_map[address])) {
+        address == 0x34 || address == 0x3c || address == 0x3d) {
         val = pci_default_read_config(d, address, len);
         DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n",
               (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len);
@@ -1251,36 +1278,72 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos)
 #endif
 #endif
 
-static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t address,
+static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev,
+                                                    uint32_t address, int len)
+{
+    uint8_t cap, cap_id = pci_dev->config_map[address];
+
+    switch (cap_id) {
+
+    case PCI_CAP_ID_VPD:
+        cap = pci_find_capability(pci_dev, cap_id);
+        if (!ranges_overlap(address, len, cap, PCI_CAP_FLAGS)) {
+            return assigned_dev_pci_read(pci_dev, address, len);
+        }
+        break;
+
+    case PCI_CAP_ID_VNDR:
+        cap = pci_find_capability(pci_dev, cap_id);
+        if (!ranges_overlap(address, len, cap, PCI_CAP_FLAGS + 1)) {
+            return assigned_dev_pci_read(pci_dev, address, len);
+        }
+        break;
+    }
+
+    return pci_default_read_config(pci_dev, address, len);
+}
+
+static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev,
+                                                 uint32_t address,
                                                  uint32_t val, int len)
 {
-    uint8_t cap_id = pci_dev->config_map[address];
+    uint8_t cap, cap_id = pci_dev->config_map[address];
 
     pci_default_write_config(pci_dev, address, val, len);
     switch (cap_id) {
 #ifdef KVM_CAP_IRQ_ROUTING
     case PCI_CAP_ID_MSI:
 #ifdef KVM_CAP_DEVICE_MSI
-        {
-            uint8_t cap = pci_find_capability(pci_dev, cap_id);
-            if (ranges_overlap(address - cap, len, PCI_MSI_FLAGS, 1)) {
-                assigned_dev_update_msi(pci_dev, cap + PCI_MSI_FLAGS);
-            }
+        cap = pci_find_capability(pci_dev, cap_id);
+        if (ranges_overlap(address - cap, len, PCI_MSI_FLAGS, 1)) {
+            assigned_dev_update_msi(pci_dev, cap + PCI_MSI_FLAGS);
         }
 #endif
         break;
 
     case PCI_CAP_ID_MSIX:
 #ifdef KVM_CAP_DEVICE_MSIX
-        {
-            uint8_t cap = pci_find_capability(pci_dev, cap_id);
-            if (ranges_overlap(address - cap, len, PCI_MSIX_FLAGS + 1, 1)) {
-                assigned_dev_update_msix(pci_dev, cap + PCI_MSIX_FLAGS);
-            }
+        cap = pci_find_capability(pci_dev, cap_id);
+        if (ranges_overlap(address - cap, len, PCI_MSIX_FLAGS + 1, 1)) {
+            assigned_dev_update_msix(pci_dev, cap + PCI_MSIX_FLAGS);
         }
 #endif
         break;
 #endif
+
+    case PCI_CAP_ID_VPD:
+        cap = pci_find_capability(pci_dev, cap_id);
+        if (!ranges_overlap(address, len, cap, PCI_CAP_FLAGS)) {
+            assigned_dev_pci_write(pci_dev, address, val, len);
+        }
+        break;
+
+    case PCI_CAP_ID_VNDR:
+        cap = pci_find_capability(pci_dev, cap_id);
+        if (!ranges_overlap(address, len, cap, PCI_CAP_FLAGS + 1)) {
+            assigned_dev_pci_write(pci_dev, address, val, len);
+        }
+        break;
     }
 }
 
@@ -1345,6 +1408,128 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
 #endif
 #endif
 
+    /* Minimal PM support, nothing writable, device appears to NAK changes */
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_PM))) {
+        uint16_t pmc, pmcsr;
+        pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF);
+
+        pmc = pci_get_word(pci_dev->config + pos + PCI_CAP_FLAGS);
+        pmc &= (PCI_PM_CAP_VER_MASK | PCI_PM_CAP_DSI);
+        pci_set_word(pci_dev->config + pos + PCI_CAP_FLAGS, pmc);
+
+        pmcsr = pci_get_word(pci_dev->config + pos + PCI_PM_CTRL);
+        pmcsr &= (PCI_PM_CTRL_STATE_MASK);
+        pmcsr |= PCI_PM_CTRL_NO_SOFT_RST;
+        pci_set_word(pci_dev->config + pos + PCI_PM_CTRL, pmcsr);
+
+        pci_set_byte(pci_dev->config + pos + PCI_PM_PPB_EXTENSIONS, 0);
+        pci_set_byte(pci_dev->config + pos + PCI_PM_DATA_REGISTER, 0);
+    }
+
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_EXP))) {
+        uint16_t type, devctl, lnkcap, lnksta;
+        uint32_t devcap;
+
+        pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, 0x40);
+
+        type = pci_get_word(pci_dev->config + pos + PCI_EXP_FLAGS);
+        type = (type & PCI_EXP_FLAGS_TYPE) >> 8;
+        if (type != PCI_EXP_TYPE_ENDPOINT &&
+            type != PCI_EXP_TYPE_LEG_END && type != PCI_EXP_TYPE_RC_END) { 
+            fprintf(stderr,
+                    "Device assignment only supports endpoint assignment, "
+                    "device type %d\n", type);
+            return -EINVAL;
+        }
+
+        /* capabilities, pass existing read-only copy
+         * PCI_EXP_FLAGS_IRQ: updated by hardware, should be direct read */
+
+        /* device capabilities: hide FLR */
+        devcap = pci_get_long(pci_dev->config + pos + PCI_EXP_DEVCAP);
+        devcap &= ~PCI_EXP_DEVCAP_FLR;
+        pci_set_long(pci_dev->config + pos + PCI_EXP_DEVCAP, devcap);
+
+        /* device control: clear all error reporting enable bits, leaving
+         *                 leaving only a few host values.  Note, these are
+         *                 all writable, but not passed to hw.
+         */
+        devctl = pci_get_word(pci_dev->config + pos + PCI_EXP_DEVCTL);
+        devctl = (devctl & (PCI_EXP_DEVCTL_READRQ | PCI_EXP_DEVCTL_PAYLOAD)) |
+                  PCI_EXP_DEVCTL_RELAX_EN | PCI_EXP_DEVCTL_NOSNOOP_EN;
+        pci_set_word(pci_dev->config + pos + PCI_EXP_DEVCTL, devctl);
+        devctl = PCI_EXP_DEVCTL_BCR_FLR | PCI_EXP_DEVCTL_AUX_PME;
+        pci_set_word(pci_dev->wmask + pos + PCI_EXP_DEVCTL, ~devctl);
+
+        /* Clear device status */
+        pci_set_word(pci_dev->config + pos + PCI_EXP_DEVSTA, 0);
+
+        /* Link capabilities, expose links and latencues, clear reporting */
+        lnkcap = pci_get_word(pci_dev->config + pos + PCI_EXP_LNKCAP);
+        lnkcap &= (PCI_EXP_LNKCAP_SLS | PCI_EXP_LNKCAP_MLW |
+                   PCI_EXP_LNKCAP_ASPMS | PCI_EXP_LNKCAP_L0SEL |
+                   PCI_EXP_LNKCAP_L1EL);
+        pci_set_word(pci_dev->config + pos + PCI_EXP_LNKCAP, lnkcap);
+        pci_set_word(pci_dev->wmask + pos + PCI_EXP_LNKCAP,
+                     PCI_EXP_LNKCTL_ASPMC | PCI_EXP_LNKCTL_RCB |
+                     PCI_EXP_LNKCTL_CCC | PCI_EXP_LNKCTL_ES |
+                     PCI_EXP_LNKCTL_CLKREQ_EN | PCI_EXP_LNKCTL_HAWD);
+        
+        /* Link control, pass existing read-only copy.  Should be writable? */
+
+        /* Link status, only expose current speed and width */
+        lnksta = pci_get_word(pci_dev->config + pos + PCI_EXP_LNKSTA);
+        lnksta &= (PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW);
+        pci_set_word(pci_dev->config + pos + PCI_EXP_LNKSTA, lnksta);
+
+        /* Slot capabilities, control, status - not needed for endpoints */
+        pci_set_long(pci_dev->config + pos + PCI_EXP_SLTCAP, 0);
+        pci_set_word(pci_dev->config + pos + PCI_EXP_SLTCTL, 0);
+        pci_set_word(pci_dev->config + pos + PCI_EXP_SLTSTA, 0);
+
+        /* Root control, capabilities, status - not needed for endpoints */
+        pci_set_word(pci_dev->config + pos + PCI_EXP_RTCTL, 0);
+        pci_set_word(pci_dev->config + pos + PCI_EXP_RTCAP, 0);
+        pci_set_long(pci_dev->config + pos + PCI_EXP_RTSTA, 0);
+
+        /* Device capabilities/control 2, pass existing read-only copy */
+        /* Link control 2, pass existing read-only copy */
+    }
+
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_PCIX))) {
+        uint16_t cmd;
+        uint32_t status;
+
+        /* Only expose the minimum, 8 byte capability */
+        pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8);
+
+        /* Command register, clear upper bits, including extended modes */
+        cmd = pci_get_word(pci_dev->config + pos + PCI_X_CMD);
+        cmd &= (PCI_X_CMD_DPERR_E | PCI_X_CMD_ERO | PCI_X_CMD_MAX_READ |
+                PCI_X_CMD_MAX_SPLIT);
+        pci_set_word(pci_dev->config + pos + PCI_X_CMD, cmd);
+
+        /* Status register, update with emulated PCI bus location, clear
+         * error bits, leave the rest. */
+        status = pci_get_long(pci_dev->config + pos + PCI_X_STATUS);
+        status &= ~(PCI_X_STATUS_BUS | PCI_X_STATUS_DEVFN);
+        status |= (pci_bus_num(pci_dev->bus) << 8) | pci_dev->devfn;
+        status &= ~(PCI_X_STATUS_SPL_DISC | PCI_X_STATUS_UNX_SPL |
+                    PCI_X_STATUS_SPL_ERR);
+        pci_set_long(pci_dev->config + pos + PCI_X_STATUS, status);
+    }
+
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD))) {
+        /* Direct R/W passthrough */
+        pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8);
+    }
+
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VNDR))) {
+        uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS);
+        /* Direct R/W passthrough */
+        pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len);
+    }
+
     return 0;
 }
 

[Qemu-devel] Re: [PATCH v3 0/9] PCI capability and device assignment improvements

[Marcelo Tosatti]

On Fri, Nov 19, 2010 at 04:19:01PM -0700, Alex Williamson wrote:
> v3:
> 
>  - Rework to avoid introducing conflicts with qemu.git hw/pci
>  - Drop capability lookup table
>  - Add back minimal device assignment PM, EXP, X, VPD, VNDR
>    capabilities
> 
> This version should do a much better job at not introducing new
> differences between qemu-kvm.git and qemu.git hw/pci.  The diff
> between the pci files after this series is significantly
> reduced.  I added back the previous RFC patch that adds new
> capabilities for assigned devices.  I'm sure we'll want to add
> passthrough for various fields, but that can come later (along
> with figuring out whether we can consolidate emulation should
> other drivers want it).  Thanks,
> 
> Alex

Applied, thanks.