libqos: Change PCI accessors to take opaque BAR handle

The usual use model for the libqos PCI functions is to map a specific PCI
BAR using qpci_iomap() then pass the returned token into IO accessor
functions.  This, and the fact that iomap() returns a (void *) which
actually contains a PCI space address, kind of suggests that the return
value from iomap is supposed to be an opaque token.

..except that the callers expect to be able to add offsets to it.  Which
also assumes the compiler will support pointer arithmetic on a (void *),
and treat it as working with byte offsets.

To clarify this situation change iomap() and the IO accessors to take
a definitely opaque BAR handle (enforced with a wrapper struct) along with
an offset within the BAR.  This changes both the functions and all the
callers.

There were a number of places that checked if iomap() returned non-NULL,
and or initialized it to NULL before hand.  Since iomap() already assert()s
if it fails to map the BAR, these tests were mostly pointless and are
removed.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>

1 files changed, 68 insertions, 81 deletions

diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index 3021651ee4..2dcdeade2a 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -104,7 +104,6 @@ void qpci_msix_enable(QPCIDevice *dev)
     uint32_t table;
     uint8_t bir_table;
     uint8_t bir_pba;
-    void *offset;
 
     addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
     g_assert_cmphex(addr, !=, 0);
@@ -114,18 +113,16 @@ void qpci_msix_enable(QPCIDevice *dev)
 
     table = qpci_config_readl(dev, addr + PCI_MSIX_TABLE);
     bir_table = table & PCI_MSIX_FLAGS_BIRMASK;
-    offset = qpci_iomap(dev, bir_table, NULL);
-    dev->msix_table = offset + (table & ~PCI_MSIX_FLAGS_BIRMASK);
+    dev->msix_table_bar = qpci_iomap(dev, bir_table, NULL);
+    dev->msix_table_off = table & ~PCI_MSIX_FLAGS_BIRMASK;
 
     table = qpci_config_readl(dev, addr + PCI_MSIX_PBA);
     bir_pba = table & PCI_MSIX_FLAGS_BIRMASK;
     if (bir_pba != bir_table) {
-        offset = qpci_iomap(dev, bir_pba, NULL);
+        dev->msix_pba_bar = qpci_iomap(dev, bir_pba, NULL);
     }
-    dev->msix_pba = offset + (table & ~PCI_MSIX_FLAGS_BIRMASK);
+    dev->msix_pba_off = table & ~PCI_MSIX_FLAGS_BIRMASK;
 
-    g_assert(dev->msix_table != NULL);
-    g_assert(dev->msix_pba != NULL);
     dev->msix_enabled = true;
 }
 
@@ -141,22 +138,23 @@ void qpci_msix_disable(QPCIDevice *dev)
     qpci_config_writew(dev, addr + PCI_MSIX_FLAGS,
                                                 val & ~PCI_MSIX_FLAGS_ENABLE);
 
-    qpci_iounmap(dev, dev->msix_table);
-    qpci_iounmap(dev, dev->msix_pba);
+    qpci_iounmap(dev, dev->msix_table_bar);
+    qpci_iounmap(dev, dev->msix_pba_bar);
     dev->msix_enabled = 0;
-    dev->msix_table = NULL;
-    dev->msix_pba = NULL;
+    dev->msix_table_off = 0;
+    dev->msix_pba_off = 0;
 }
 
 bool qpci_msix_pending(QPCIDevice *dev, uint16_t entry)
 {
     uint32_t pba_entry;
     uint8_t bit_n = entry % 32;
-    void *addr = dev->msix_pba + (entry / 32) * PCI_MSIX_ENTRY_SIZE / 4;
+    uint64_t  off = (entry / 32) * PCI_MSIX_ENTRY_SIZE / 4;
 
     g_assert(dev->msix_enabled);
-    pba_entry = qpci_io_readl(dev, addr);
-    qpci_io_writel(dev, addr, pba_entry & ~(1 << bit_n));
+    pba_entry = qpci_io_readl(dev, dev->msix_pba_bar, dev->msix_pba_off + off);
+    qpci_io_writel(dev, dev->msix_pba_bar, dev->msix_pba_off + off,
+                   pba_entry & ~(1 << bit_n));
     return (pba_entry & (1 << bit_n)) != 0;
 }
 
@@ -164,7 +162,7 @@ bool qpci_msix_masked(QPCIDevice *dev, uint16_t entry)
 {
     uint8_t addr;
     uint16_t val;
-    void *vector_addr = dev->msix_table + (entry * PCI_MSIX_ENTRY_SIZE);
+    uint64_t vector_off = dev->msix_table_off + entry * PCI_MSIX_ENTRY_SIZE;
 
     g_assert(dev->msix_enabled);
     addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX);
@@ -174,8 +172,9 @@ bool qpci_msix_masked(QPCIDevice *dev, uint16_t entry)
     if (val & PCI_MSIX_FLAGS_MASKALL) {
         return true;
     } else {
-        return (qpci_io_readl(dev, vector_addr + PCI_MSIX_ENTRY_VECTOR_CTRL)
-                                            & PCI_MSIX_ENTRY_CTRL_MASKBIT) != 0;
+        return (qpci_io_readl(dev, dev->msix_table_bar,
+                              vector_off + PCI_MSIX_ENTRY_VECTOR_CTRL)
+                & PCI_MSIX_ENTRY_CTRL_MASKBIT) != 0;
     }
 }
 
@@ -222,129 +221,115 @@ void qpci_config_writel(QPCIDevice *dev, uint8_t offset, uint32_t value)
     dev->bus->config_writel(dev->bus, dev->devfn, offset, value);
 }
 
-
-uint8_t qpci_io_readb(QPCIDevice *dev, void *data)
+uint8_t qpci_io_readb(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readb(dev->bus, addr);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        return dev->bus->pio_readb(dev->bus, token.addr + off);
     } else {
         uint8_t val;
-        dev->bus->memread(dev->bus, addr, &val, sizeof(val));
+        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
         return val;
     }
 }
 
-uint16_t qpci_io_readw(QPCIDevice *dev, void *data)
+uint16_t qpci_io_readw(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readw(dev->bus, addr);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        return dev->bus->pio_readw(dev->bus, token.addr + off);
     } else {
         uint16_t val;
-        dev->bus->memread(dev->bus, addr, &val, sizeof(val));
+        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
         return le16_to_cpu(val);
     }
 }
 
-uint32_t qpci_io_readl(QPCIDevice *dev, void *data)
+uint32_t qpci_io_readl(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readl(dev->bus, addr);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        return dev->bus->pio_readl(dev->bus, token.addr + off);
     } else {
         uint32_t val;
-        dev->bus->memread(dev->bus, addr, &val, sizeof(val));
+        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
         return le32_to_cpu(val);
     }
 }
 
-uint64_t qpci_io_readq(QPCIDevice *dev, void *data)
+uint64_t qpci_io_readq(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readq(dev->bus, addr);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        return dev->bus->pio_readq(dev->bus, token.addr + off);
     } else {
         uint64_t val;
-        dev->bus->memread(dev->bus, addr, &val, sizeof(val));
+        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
         return le64_to_cpu(val);
     }
 }
 
-void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t value)
+void qpci_io_writeb(QPCIDevice *dev, QPCIBar token, uint64_t off,
+                    uint8_t value)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writeb(dev->bus, addr, value);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        dev->bus->pio_writeb(dev->bus, token.addr + off, value);
     } else {
-        dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
+        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
     }
 }
 
-void qpci_io_writew(QPCIDevice *dev, void *data, uint16_t value)
+void qpci_io_writew(QPCIDevice *dev, QPCIBar token, uint64_t off,
+                    uint16_t value)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writew(dev->bus, addr, value);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        dev->bus->pio_writew(dev->bus, token.addr + off, value);
     } else {
         value = cpu_to_le16(value);
-        dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
+        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
     }
 }
 
-void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value)
+void qpci_io_writel(QPCIDevice *dev, QPCIBar token, uint64_t off,
+                    uint32_t value)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writel(dev->bus, addr, value);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        dev->bus->pio_writel(dev->bus, token.addr + off, value);
     } else {
         value = cpu_to_le32(value);
-        dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
+        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
     }
 }
 
-void qpci_io_writeq(QPCIDevice *dev, void *data, uint64_t value)
+void qpci_io_writeq(QPCIDevice *dev, QPCIBar token, uint64_t off,
+                    uint64_t value)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    if (addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writeq(dev->bus, addr, value);
+    if (token.addr < QPCI_PIO_LIMIT) {
+        dev->bus->pio_writeq(dev->bus, token.addr + off, value);
     } else {
         value = cpu_to_le64(value);
-        dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
+        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
     }
 }
 
-void qpci_memread(QPCIDevice *dev, void *data, void *buf, size_t len)
+void qpci_memread(QPCIDevice *dev, QPCIBar token, uint64_t off,
+                  void *buf, size_t len)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    g_assert(addr >= QPCI_PIO_LIMIT);
-    dev->bus->memread(dev->bus, addr, buf, len);
+    g_assert(token.addr >= QPCI_PIO_LIMIT);
+    dev->bus->memread(dev->bus, token.addr + off, buf, len);
 }
 
-void qpci_memwrite(QPCIDevice *dev, void *data, const void *buf, size_t len)
+void qpci_memwrite(QPCIDevice *dev, QPCIBar token, uint64_t off,
+                   const void *buf, size_t len)
 {
-    uintptr_t addr = (uintptr_t)data;
-
-    g_assert(addr >= QPCI_PIO_LIMIT);
-    dev->bus->memwrite(dev->bus, addr, buf, len);
+    g_assert(token.addr >= QPCI_PIO_LIMIT);
+    dev->bus->memwrite(dev->bus, token.addr + off, buf, len);
 }
 
-void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
+QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
 {
     QPCIBus *bus = dev->bus;
     static const int bar_reg_map[] = {
         PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1, PCI_BASE_ADDRESS_2,
         PCI_BASE_ADDRESS_3, PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5,
     };
+    QPCIBar bar;
     int bar_reg;
     uint32_t addr, size;
     uint32_t io_type;
@@ -391,17 +376,19 @@ void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
         qpci_config_writel(dev, bar_reg, loc);
     }
 
-    return (void *)(uintptr_t)loc;
+    bar.addr = loc;
+    return bar;
 }
 
-void qpci_iounmap(QPCIDevice *dev, void *data)
+void qpci_iounmap(QPCIDevice *dev, QPCIBar bar)
 {
     /* FIXME */
 }
 
-void *qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
+QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
 {
-    return (void *)(uintptr_t)addr;
+    QPCIBar bar = { .addr = addr };
+    return bar;
 }
 
 void qpci_plug_device_test(const char *driver, const char *id,