Patchwork [6/8] device-assignment: Move PCI capabilities to match physical hardware

login
register
mail settings
Submitter Alex Williamson
Date Nov. 12, 2010, 2:56 a.m.
Message ID <20101112025602.31423.95572.stgit@s20.home>
Download mbox | patch
Permalink /patch/70923/
State New
Headers show

Comments

Alex Williamson - Nov. 12, 2010, 2:56 a.m.
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 |   67 +++++++++++++++---------------------------------
 1 files changed, 21 insertions(+), 46 deletions(-)
Michael S. Tsirkin - Nov. 12, 2010, 9:20 a.m.
On Thu, Nov 11, 2010 at 07:56:13PM -0700, Alex Williamson wrote:
> Now that common PCI code doesn't have a hangup on capabilities
> being contiguous,

Hmm, this comment confused me : there's no requirement of
contigious allocations in current code in pci.c, is there?
Alex Williamson - Nov. 12, 2010, 1:53 p.m.
On Fri, 2010-11-12 at 11:20 +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 11, 2010 at 07:56:13PM -0700, Alex Williamson wrote:
> > Now that common PCI code doesn't have a hangup on capabilities
> > being contiguous,
> 
> Hmm, this comment confused me : there's no requirement of
> contigious allocations in current code in pci.c, is there?

Exactly, but the code used to have cap.start and cap.length, which
implied it was contiguous.  Since those were removed in 5/8, we don't
need to worry about where the physical capabilities land in config
space.  Thanks,

Alex

Patch

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 322fa9f..39f19be 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,
-                                  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_at_offset(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,
-                           PCI_CAPABILITY_CONFIG_MSIX_LENGTH);
+        pci_add_capability_at_offset(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;