[Qemu-devel] [PATCH] pci: Length-align config space accesses

[Jan Kiszka]

From: Jan Kiszka <address@hidden>

Introduce pci_config_read/write helpers to split up config space
accesses that are not length-aligned. This particularly avoids that each
and every device needs to check for config space overruns. Also move the
access length assertion to the new helpers.

Signed-off-by: Jan Kiszka <address@hidden>
---

This will also simplify my cap refactorings for the device-assignment
code in qemu-kvm.

 hw/pci.c       |   43 +++++++++++++++++++++++++++++++++++++++----
 hw/pci.h       |    5 +++++
 hw/pci_host.c  |   14 ++++++--------
 hw/pcie_host.c |   12 ++++++------
 4 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index b904a4e..6957ece 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1104,12 +1104,48 @@ static void pci_update_irq_disabled(PCIDevice *d, int 
was_irq_disabled)
     }
 }
 
+void pci_config_write(PCIDevice *pci_dev, uint32_t addr, uint32_t addr_mask,
+                      uint32_t val, uint32_t len)
+{
+    assert(len == 1 || len == 2 || len == 4);
+
+    addr &= addr_mask;
+
+    if (addr & (len - 1)) {
+        len >>= 1;
+        pci_config_write(pci_dev, addr, addr_mask, val, len);
+        pci_config_write(pci_dev, addr + len, addr_mask,
+                         val >> (len * 8), len);
+    } else {
+        pci_dev->config_write(pci_dev, addr, val, len);
+    }
+}
+
+uint32_t pci_config_read(PCIDevice *pci_dev, uint32_t addr, uint32_t addr_mask,
+                         uint32_t len)
+{
+    uint32_t val;
+
+    assert(len == 1 || len == 2 || len == 4);
+
+    addr &= addr_mask;
+
+    if (addr & (len - 1)) {
+        len >>= 1;
+        val = pci_config_read(pci_dev, addr, addr_mask, len);
+        val |= pci_config_read(pci_dev, addr + len,
+                               addr_mask, len) << (len * 8);
+    } else {
+        val = pci_dev->config_read(pci_dev, addr, len);
+    }
+    return val;
+}
+
 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);
 }
@@ -1117,9 +1153,8 @@ uint32_t pci_default_read_config(PCIDevice *d,
 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);
 
-    for (i = 0; i < l && addr + i < config_size; val >>= 8, ++i) {
+    for (i = 0; i < l; val >>= 8, ++i) {
         uint8_t wmask = d->wmask[addr + i];
         uint8_t w1cmask = d->w1cmask[addr + i];
         assert(!(wmask & w1cmask));
diff --git a/hw/pci.h b/hw/pci.h
index c220745..3d5b39c 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -482,4 +482,9 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
     return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
 }
 
+void pci_config_write(PCIDevice *pci_dev, uint32_t addr, uint32_t addr_mask,
+                      uint32_t val, uint32_t len);
+uint32_t pci_config_read(PCIDevice *pci_dev, uint32_t addr, uint32_t addr_mask,
+                         uint32_t len);
+
 #endif
diff --git a/hw/pci_host.c b/hw/pci_host.c
index 728e2d4..db888fb 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -50,30 +50,28 @@ static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, 
uint32_t addr)
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len)
 {
     PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
-    uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
 
-    if (!pci_dev)
+    if (!pci_dev) {
         return;
+    }
 
     PCI_DPRINTF("%s: %s: addr=%02" PRIx32 " val=%08" PRIx32 " len=%d\n",
-                __func__, pci_dev->name, config_addr, val, len);
-    pci_dev->config_write(pci_dev, config_addr, val, len);
+                __func__, pci_dev->name, addr, val, len);
+    pci_config_write(pci_dev, addr, PCI_CONFIG_SPACE_SIZE - 1, val, len);
 }
 
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
 {
     PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
-    uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
     uint32_t val;
 
-    assert(len == 1 || len == 2 || len == 4);
     if (!pci_dev) {
         return ~0x0;
     }
 
-    val = pci_dev->config_read(pci_dev, config_addr, len);
+    val = pci_config_read(pci_dev, addr, PCI_CONFIG_SPACE_SIZE - 1, len);
     PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
-                __func__, pci_dev->name, config_addr, val, len);
+                __func__, pci_dev->name, addr, val, len);
 
     return val;
 }
diff --git a/hw/pcie_host.c b/hw/pcie_host.c
index b749865..e10d7a3 100644
--- a/hw/pcie_host.c
+++ b/hw/pcie_host.c
@@ -57,22 +57,22 @@ static void pcie_mmcfg_data_write(PCIBus *s,
 {
     PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
 
-    if (!pci_dev)
+    if (!pci_dev) {
         return;
-
-    pci_dev->config_write(pci_dev,
-                          PCIE_MMCFG_CONFOFFSET(mmcfg_addr), val, len);
+    }
+    pci_config_write(pci_dev, PCIE_MMCFG_CONFOFFSET(mmcfg_addr),
+                     PCIE_MMCFG_CONFOFFSET_MASK, val, len);
 }
 
 static uint32_t pcie_mmcfg_data_read(PCIBus *s, uint32_t addr, int len)
 {
     PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, addr);
 
-    assert(len == 1 || len == 2 || len == 4);
     if (!pci_dev) {
         return ~0x0;
     }
-    return pci_dev->config_read(pci_dev, PCIE_MMCFG_CONFOFFSET(addr), len);
+    return pci_config_read(pci_dev, PCIE_MMCFG_CONFOFFSET(addr),
+                           PCIE_MMCFG_CONFOFFSET_MASK, len);
 }
 
 static void pcie_mmcfg_data_writeb(void *opaque,
-- 
1.7.3.4