[Qemu-devel] Re: [PATCH 4/8] pci: Replace used bitmap with capability byte map

[Michael S. Tsirkin]

On Thu, Nov 11, 2010 at 11:07:15PM -0700, Alex Williamson wrote:
> On Fri, 2010-11-12 at 07:40 +0200, Michael S. Tsirkin wrote:
> > On Thu, Nov 11, 2010 at 07:55:43PM -0700, Alex Williamson wrote:
> > > 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.
> > > 
> > > Signed-off-by: Alex Williamson <address@hidden>
> > 
> > I actually wanted to remove the used array completely and ask
> > all users to add offsets directly.
> > Will this be needed then?
> 
> Can you give an example, I don't understand what you mean by asking
> users to add offsets directly.

Here's a dump of patch in progress.
Something like the below (untested).  After applying this we can remove
the whole used array allocator.

>  I think some kind of tracking what's
> where in config space needs to be done somewhere and the common PCI code
> seems like it'd be the place.

Why do we need it? config lets us scan the capability list
readily enough. I had this idea that we should pack
capabilities in config space, but now I think it's silly.

>  Thanks,
> 
> Alex

pci: remove config space allocator

pci supports allocating caps in config space so they are packed tightly.
This doesn't seem to be useful, especially since caps must stay at fixed
offsets to ensure backwards compatibility (e.g. for migration).

Remove this support, and make virtio-pci supply the offset to
the MSIX capability explicitly.

Signed-off-by: Michael S. Tsirkin <address@hidden>

diff --git a/hw/msix.c b/hw/msix.c
index f66d255..6fd3791 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -52,7 +52,7 @@ int msix_supported;
  * 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)
+                           unsigned offset, unsigned bar_nr, unsigned bar_size)
 {
     int config_offset;
     uint8_t *config;
@@ -75,7 +75,7 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned 
short nentries,
 
     pdev->msix_bar_size = new_size;
     config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
-                                       0, MSIX_CAP_LENGTH);
+                                       offset, MSIX_CAP_LENGTH);
     if (config_offset < 0)
         return config_offset;
     config = pdev->config + config_offset;
@@ -237,7 +237,7 @@ 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. */
 int msix_init(struct PCIDevice *dev, unsigned short nentries,
-              unsigned bar_nr, unsigned bar_size)
+              unsigned offset, unsigned bar_nr, unsigned bar_size)
 {
     int ret;
     /* Nothing to do if MSI is not supported by interrupt controller */
@@ -261,7 +261,7 @@ int msix_init(struct PCIDevice *dev, unsigned short 
nentries,
     }
 
     dev->msix_entries_nr = nentries;
-    ret = msix_add_config(dev, nentries, bar_nr, bar_size);
+    ret = msix_add_config(dev, nentries, offset, bar_nr, bar_size);
     if (ret)
         goto err_config;
 
diff --git a/hw/msix.h b/hw/msix.h
index a9f7993..b61e42e 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -5,7 +5,7 @@
 #include "pci.h"
 
 int msix_init(PCIDevice *pdev, unsigned short nentries,
-              unsigned bar_nr, unsigned bar_size);
+              unsigned offset, unsigned bar_nr, unsigned bar_size);
 
 void msix_write_config(PCIDevice *pci_dev, uint32_t address,
                        uint32_t val, int len);
diff --git a/hw/pci.c b/hw/pci.c
index aed2d42..e411f12 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1737,12 +1737,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
                        uint8_t offset, uint8_t size)
 {
     uint8_t *config;
-    if (!offset) {
-        offset = pci_find_space(pdev, size);
-        if (!offset) {
-            return -ENOSPC;
-        }
-    }
+    assert(offset >= PCI_CONFIG_HEADER_SIZE);
 
     config = pdev->config + offset;
     config[PCI_CAP_LIST_ID] = cap_id;
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 729917d..bcb0266 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -543,7 +543,8 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, 
VirtIODevice *vdev,
 
     config[0x3d] = 1;
 
-    if (vdev->nvectors && !msix_init(&proxy->pci_dev, vdev->nvectors, 1, 0)) {
+    if (vdev->nvectors && !msix_init(&proxy->pci_dev, vdev->nvectors,
+                                     PCI_CONFIG_HEADER_SIZE, 1, 0)) {
         pci_register_bar(&proxy->pci_dev, 1,
                          msix_bar_size(&proxy->pci_dev),
                          PCI_BASE_ADDRESS_SPACE_MEMORY,