Re: [Qemu-devel] [PATCH 8/8] libqos: Change PCI accessors to take opaque BAR handle

[Laurent Vivier <lvivier@redhat.com>]

On 18/10/2016 12:52, David Gibson wrote:
> 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.
> 
> A few notes:
>     * Asserts that iomap() returns non-NULL are removed in some places;
> iomap() already asserts if it can't map the BAR
>     * In ide-test.c we change explicit outb() etc. calls to matching
> qpci_io_writeb() calls.  That makes the test more portable, and removes
> assumptions that the test case shouldn't be making about how iomap()'s
> return value is formatted internally.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> # Conflicts:
> #	tests/libqos/virtio-pci.c

A sequel of a cherry-pick?

> ---
>  tests/ahci-test.c         |   4 +-
>  tests/e1000e-test.c       |   7 ++-
>  tests/ide-test.c          |  23 +++++----
>  tests/ivshmem-test.c      |  28 +++++------
>  tests/libqos/ahci.c       |   3 +-
>  tests/libqos/ahci.h       |   6 +--
>  tests/libqos/pci.c        | 125 +++++++++++++++++++++-------------------------
>  tests/libqos/pci.h        |  39 +++++++++------
>  tests/libqos/usb.c        |   6 +--
>  tests/libqos/usb.h        |   2 +-
>  tests/libqos/virtio-pci.c | 113 +++++++++++++++++++++--------------------
>  tests/libqos/virtio-pci.h |   2 +-
>  tests/rtl8139-test.c      |  10 ++--
>  tests/usb-hcd-ehci-test.c |   5 +-

Perhaps you can move tco-test update in this series?

>  14 files changed, 186 insertions(+), 187 deletions(-)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index 9c0adce..4358631 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -90,12 +90,12 @@ static void verify_state(AHCIQState *ahci)
>      g_assert_cmphex(ahci_fingerprint, ==, ahci->fingerprint);
>  
>      /* If we haven't initialized, this is as much as can be validated. */
> -    if (!ahci->hba_base) {
> +    if (!ahci->hba_bar.addr) {
>          return;
>      }
>  
>      hba_base = (uint64_t)qpci_config_readl(ahci->dev, PCI_BASE_ADDRESS_5);
> -    hba_stored = (uint64_t)(uintptr_t)ahci->hba_base;
> +    hba_stored = ahci->hba_bar.addr;
>      g_assert_cmphex(hba_base, ==, hba_stored);
>  
>      g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci->cap);
> diff --git a/tests/e1000e-test.c b/tests/e1000e-test.c
> index 3979b20..8c42ca9 100644
> --- a/tests/e1000e-test.c
> +++ b/tests/e1000e-test.c
> @@ -87,7 +87,7 @@
>  
>  typedef struct e1000e_device {
>      QPCIDevice *pci_dev;
> -    void *mac_regs;
> +    QPCIBar mac_regs;
>  
>      uint64_t tx_ring;
>      uint64_t rx_ring;
> @@ -119,12 +119,12 @@ static QPCIDevice *e1000e_device_find(QPCIBus *bus)
>  
>  static void e1000e_macreg_write(e1000e_device *d, uint32_t reg, uint32_t val)
>  {
> -    qpci_io_writel(d->pci_dev, d->mac_regs + reg, val);
> +    qpci_io_writel(d->pci_dev, d->mac_regs, reg, val);
>  }
>  
>  static uint32_t e1000e_macreg_read(e1000e_device *d, uint32_t reg)
>  {
> -    return qpci_io_readl(d->pci_dev, d->mac_regs + reg);
> +    return qpci_io_readl(d->pci_dev, d->mac_regs, reg);
>  }
>  
>  static void e1000e_device_init(QPCIBus *bus, e1000e_device *d)
> @@ -138,7 +138,6 @@ static void e1000e_device_init(QPCIBus *bus, e1000e_device *d)
>  
>      /* Map BAR0 (mac registers) */
>      d->mac_regs = qpci_iomap(d->pci_dev, 0, NULL);
> -    g_assert_nonnull(d->mac_regs);
>  
>      /* Reset the device */
>      val = e1000e_macreg_read(d, E1000E_CTRL);
> diff --git a/tests/ide-test.c b/tests/ide-test.c
> index a8a4081..dc08536 100644
> --- a/tests/ide-test.c
> +++ b/tests/ide-test.c
> @@ -137,7 +137,7 @@ static void ide_test_quit(void)
>      qtest_end();
>  }
>  
> -static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
> +static QPCIDevice *get_pci_device(QPCIBar *bmdma_bar)
>  {
>      QPCIDevice *dev;
>      uint16_t vendor_id, device_id;
> @@ -156,7 +156,7 @@ static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
>      g_assert(device_id == PCI_DEVICE_ID_INTEL_82371SB_1);
>  
>      /* Map bmdma BAR */
> -    *bmdma_base = (uint16_t)(uintptr_t) qpci_iomap(dev, 4, NULL);
> +    *bmdma_bar = qpci_iomap(dev, 4, NULL);
>  
>      qpci_device_enable(dev);
>  
> @@ -182,14 +182,14 @@ static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
>                              void(*post_exec)(uint64_t sector, int nb_sectors))
>  {
>      QPCIDevice *dev;
> -    uint16_t bmdma_base;
> +    QPCIBar bmdma_bar;
>      uintptr_t guest_prdt;
>      size_t len;
>      bool from_dev;
>      uint8_t status;
>      int flags;
>  
> -    dev = get_pci_device(&bmdma_base);
> +    dev = get_pci_device(&bmdma_bar);
>  
>      flags = cmd & ~0xff;
>      cmd &= 0xff;
> @@ -217,14 +217,14 @@ static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
>      outb(IDE_BASE + reg_device, 0 | LBA);
>  
>      /* Stop any running transfer, clear any pending interrupt */
> -    outb(bmdma_base + bmreg_cmd, 0);
> -    outb(bmdma_base + bmreg_status, BM_STS_INTR);
> +    qpci_io_writeb(dev, bmdma_bar, bmreg_cmd, 0);
> +    qpci_io_writeb(dev, bmdma_bar, bmreg_status, BM_STS_INTR);
>  
>      /* Setup PRDT */
>      len = sizeof(*prdt) * prdt_entries;
>      guest_prdt = guest_alloc(guest_malloc, len);
>      memwrite(guest_prdt, prdt, len);
> -    outl(bmdma_base + bmreg_prdt, guest_prdt);
> +    qpci_io_writel(dev, bmdma_bar, bmreg_prdt, guest_prdt);
>  
>      /* ATA DMA command */
>      if (cmd == CMD_PACKET) {
> @@ -244,15 +244,16 @@ static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
>      }
>  
>      /* Start DMA transfer */
> -    outb(bmdma_base + bmreg_cmd, BM_CMD_START | (from_dev ? BM_CMD_WRITE : 0));
> +    qpci_io_writeb(dev, bmdma_bar, bmreg_cmd,
> +                   BM_CMD_START | (from_dev ? BM_CMD_WRITE : 0));
>  
>      if (flags & CMDF_ABORT) {
> -        outb(bmdma_base + bmreg_cmd, 0);
> +        qpci_io_writeb(dev, bmdma_bar, bmreg_cmd, 0);
>      }
>  
>      /* Wait for the DMA transfer to complete */
>      do {
> -        status = inb(bmdma_base + bmreg_status);
> +        status = qpci_io_readb(dev, bmdma_bar, bmreg_status);
>      } while ((status & (BM_STS_ACTIVE | BM_STS_INTR)) == BM_STS_ACTIVE);
>  
>      g_assert_cmpint(get_irq(IDE_PRIMARY_IRQ), ==, !!(status & BM_STS_INTR));
> @@ -266,7 +267,7 @@ static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
>  
>      /* Stop DMA transfer if still active */
>      if (status & BM_STS_ACTIVE) {
> -        outb(bmdma_base + bmreg_cmd, 0);
> +        qpci_io_writeb(dev, bmdma_bar, bmreg_cmd, 0);
>      }
>  
>      free_pci_device(dev);
> diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> index 97a887e..f7c5bf4 100644
> --- a/tests/ivshmem-test.c
> +++ b/tests/ivshmem-test.c
> @@ -41,7 +41,7 @@ static QPCIDevice *get_device(QPCIBus *pcibus)
>  
>  typedef struct _IVState {
>      QTestState *qtest;
> -    void *reg_base, *mem_base;
> +    QPCIBar reg_bar, mem_bar;
>      QPCIBus *pcibus;
>      QPCIDevice *dev;
>  } IVState;
> @@ -75,7 +75,7 @@ static inline unsigned in_reg(IVState *s, enum Reg reg)
>      unsigned res;
>  
>      global_qtest = s->qtest;
> -    res = qpci_io_readl(s->dev, s->reg_base + reg);
> +    res = qpci_io_readl(s->dev, s->reg_bar, reg);
>      g_test_message("*%s -> %x\n", name, res);
>      global_qtest = qtest;
>  
> @@ -89,7 +89,7 @@ static inline void out_reg(IVState *s, enum Reg reg, unsigned v)
>  
>      global_qtest = s->qtest;
>      g_test_message("%x -> *%s\n", v, name);
> -    qpci_io_writel(s->dev, s->reg_base + reg, v);
> +    qpci_io_writel(s->dev, s->reg_bar, reg, v);
>      global_qtest = qtest;
>  }
>  
> @@ -108,16 +108,14 @@ static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
>      s->pcibus = qpci_init_pc(NULL);
>      s->dev = get_device(s->pcibus);
>  
> -    s->reg_base = qpci_iomap(s->dev, 0, &barsize);
> -    g_assert_nonnull(s->reg_base);
> +    s->reg_bar = qpci_iomap(s->dev, 0, &barsize);
>      g_assert_cmpuint(barsize, ==, 256);
>  
>      if (msix) {
>          qpci_msix_enable(s->dev);
>      }
>  
> -    s->mem_base = qpci_iomap(s->dev, 2, &barsize);
> -    g_assert_nonnull(s->mem_base);
> +    s->mem_bar = qpci_iomap(s->dev, 2, &barsize);
>      g_assert_cmpuint(barsize, ==, TMPSHMSIZE);
>  
>      qpci_device_enable(s->dev);
> @@ -169,7 +167,7 @@ static void test_ivshmem_single(void)
>      for (i = 0; i < G_N_ELEMENTS(data); i++) {
>          data[i] = i;
>      }
> -    qpci_memwrite(s->dev, s->mem_base, data, sizeof(data));
> +    qpci_memwrite(s->dev, s->mem_bar, 0, data, sizeof(data));
>  
>      /* verify write */
>      for (i = 0; i < G_N_ELEMENTS(data); i++) {
> @@ -178,7 +176,7 @@ static void test_ivshmem_single(void)
>  
>      /* read it back and verify read */
>      memset(data, 0, sizeof(data));
> -    qpci_memread(s->dev, s->mem_base, data, sizeof(data));
> +    qpci_memread(s->dev, s->mem_bar, 0, data, sizeof(data));
>      for (i = 0; i < G_N_ELEMENTS(data); i++) {
>          g_assert_cmpuint(data[i], ==, i);
>      }
> @@ -201,29 +199,29 @@ static void test_ivshmem_pair(void)
>  
>      /* host write, guest 1 & 2 read */
>      memset(tmpshmem, 0x42, TMPSHMSIZE);
> -    qpci_memread(s1->dev, s1->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s1->dev, s1->mem_bar, 0, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x42);
>      }
> -    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s2->dev, s2->mem_bar, 0, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x42);
>      }
>  
>      /* guest 1 write, guest 2 read */
>      memset(data, 0x43, TMPSHMSIZE);
> -    qpci_memwrite(s1->dev, s1->mem_base, data, TMPSHMSIZE);
> +    qpci_memwrite(s1->dev, s1->mem_bar, 0, data, TMPSHMSIZE);
>      memset(data, 0, TMPSHMSIZE);
> -    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s2->dev, s2->mem_bar, 0, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x43);
>      }
>  
>      /* guest 2 write, guest 1 read */
>      memset(data, 0x44, TMPSHMSIZE);
> -    qpci_memwrite(s2->dev, s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memwrite(s2->dev, s2->mem_bar, 0, data, TMPSHMSIZE);
>      memset(data, 0, TMPSHMSIZE);
> -    qpci_memread(s1->dev, s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s1->dev, s2->mem_bar, 0, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x44);
>      }
> diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
> index 716ab79..43b6695 100644
> --- a/tests/libqos/ahci.c
> +++ b/tests/libqos/ahci.c
> @@ -210,8 +210,7 @@ void ahci_pci_enable(AHCIQState *ahci)
>  void start_ahci_device(AHCIQState *ahci)
>  {
>      /* Map AHCI's ABAR (BAR5) */
> -    ahci->hba_base = qpci_iomap(ahci->dev, 5, &ahci->barsize);
> -    g_assert(ahci->hba_base);
> +    ahci->hba_bar = qpci_iomap(ahci->dev, 5, &ahci->barsize);
>  
>      /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
>      qpci_device_enable(ahci->dev);
> diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
> index c69fb5a..6c4abf6 100644
> --- a/tests/libqos/ahci.h
> +++ b/tests/libqos/ahci.h
> @@ -321,7 +321,7 @@ typedef struct AHCIPortQState {
>  typedef struct AHCIQState {
>      QOSState *parent;
>      QPCIDevice *dev;
> -    void *hba_base;
> +    QPCIBar hba_bar;
>      uint64_t barsize;
>      uint32_t fingerprint;
>      uint32_t cap;
> @@ -488,12 +488,12 @@ typedef struct AHCIOpts {
>  
>  static inline uint32_t ahci_mread(AHCIQState *ahci, size_t offset)
>  {
> -    return qpci_io_readl(ahci->dev, ahci->hba_base + offset);
> +    return qpci_io_readl(ahci->dev, ahci->hba_bar, offset);
>  }
>  
>  static inline void ahci_mwrite(AHCIQState *ahci, size_t offset, uint32_t value)
>  {
> -    qpci_io_writel(ahci->dev, ahci->hba_base + offset, value);
> +    qpci_io_writel(ahci->dev, ahci->hba_bar, offset, value);
>  }
>  
>  static inline uint32_t ahci_rreg(AHCIQState *ahci, uint32_t reg_num)
> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
> index bbacbcf..875c517 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,25 @@ 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;
> +    memset(&dev->msix_table_bar, 0, sizeof(dev->msix_table_bar));
> +    memset(&dev->msix_pba_bar, 0, sizeof(dev->msix_pba_bar));
> +    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 +164,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 +174,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,104 +223,93 @@ 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);
>      }
>  }
>  
> -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_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;
> @@ -366,10 +356,11 @@ 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 */
>  }
> diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
> index 59fa3da..40b277c 100644
> --- a/tests/libqos/pci.h
> +++ b/tests/libqos/pci.h
> @@ -21,6 +21,7 @@
>  
>  typedef struct QPCIDevice QPCIDevice;
>  typedef struct QPCIBus QPCIBus;
> +typedef struct QPCIBar QPCIBar;
>  
>  struct QPCIBus {
>      uint8_t (*pio_readb)(QPCIBus *bus, uint32_t addr);
> @@ -49,13 +50,17 @@ struct QPCIBus {
>      uint64_t mmio_alloc_ptr, mmio_limit;
>  };
>  
> +struct QPCIBar {
> +    uint64_t addr;
> +};
> +
>  struct QPCIDevice
>  {
>      QPCIBus *bus;
>      int devfn;
>      bool msix_enabled;
> -    void *msix_table;
> -    void *msix_pba;
> +    QPCIBar msix_table_bar, msix_pba_bar;
> +    uint64_t msix_table_off, msix_pba_off;
>  };
>  
>  void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
> @@ -79,19 +84,23 @@ void qpci_config_writeb(QPCIDevice *dev, uint8_t offset, uint8_t value);
>  void qpci_config_writew(QPCIDevice *dev, uint8_t offset, uint16_t value);
>  void qpci_config_writel(QPCIDevice *dev, uint8_t offset, uint32_t value);
>  
> -uint8_t qpci_io_readb(QPCIDevice *dev, void *data);
> -uint16_t qpci_io_readw(QPCIDevice *dev, void *data);
> -uint32_t qpci_io_readl(QPCIDevice *dev, void *data);
> -
> -void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t value);
> -void qpci_io_writew(QPCIDevice *dev, void *data, uint16_t value);
> -void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value);
> -
> -void qpci_memread(QPCIDevice *bus, void *data, void *buf, size_t len);
> -void qpci_memwrite(QPCIDevice *bus, void *data, const void *buf, size_t len);
> -
> -void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr);
> -void qpci_iounmap(QPCIDevice *dev, void *data);
> +uint8_t qpci_io_readb(QPCIDevice *dev, QPCIBar token, uint64_t off);
> +uint16_t qpci_io_readw(QPCIDevice *dev, QPCIBar token, uint64_t off);
> +uint32_t qpci_io_readl(QPCIDevice *dev, QPCIBar token, uint64_t off);
> +
> +void qpci_io_writeb(QPCIDevice *dev, QPCIBar token, uint64_t off,
> +                    uint8_t value);
> +void qpci_io_writew(QPCIDevice *dev, QPCIBar token, uint64_t off,
> +                    uint16_t value);
> +void qpci_io_writel(QPCIDevice *dev, QPCIBar token, uint64_t off,
> +                    uint32_t value);
> +
> +void qpci_memread(QPCIDevice *bus, QPCIBar token, uint64_t off,
> +                  void *buf, size_t len);
> +void qpci_memwrite(QPCIDevice *bus, QPCIBar token, uint64_t off,
> +                   const void *buf, size_t len);
> +QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr);
> +void qpci_iounmap(QPCIDevice *dev, QPCIBar addr);
>  
>  void qpci_plug_device_test(const char *driver, const char *id,
>                             uint8_t slot, const char *opts);
> diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c
> index f794d92..72d7a96 100644
> --- a/tests/libqos/usb.c
> +++ b/tests/libqos/usb.c
> @@ -21,14 +21,12 @@ void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc, uint32_t devfn, int bar)
>      hc->dev = qpci_device_find(pcibus, devfn);
>      g_assert(hc->dev != NULL);
>      qpci_device_enable(hc->dev);
> -    hc->base = qpci_iomap(hc->dev, bar, NULL);
> -    g_assert(hc->base != NULL);
> +    hc->bar = qpci_iomap(hc->dev, bar, NULL);
>  }
>  
>  void uhci_port_test(struct qhc *hc, int port, uint16_t expect)
>  {
> -    void *addr = hc->base + 0x10 + 2 * port;
> -    uint16_t value = qpci_io_readw(hc->dev, addr);
> +    uint16_t value = qpci_io_readw(hc->dev, hc->bar, 0x10 + 2 * port);
>      uint16_t mask = ~(UHCI_PORT_WRITE_CLEAR | UHCI_PORT_RSVD1);
>  
>      g_assert((value & mask) == (expect & mask));
> diff --git a/tests/libqos/usb.h b/tests/libqos/usb.h
> index 8fe5687..423dcfd 100644
> --- a/tests/libqos/usb.h
> +++ b/tests/libqos/usb.h
> @@ -5,7 +5,7 @@
>  
>  struct qhc {
>      QPCIDevice *dev;
> -    void *base;
> +    QPCIBar bar;
>  };
>  
>  void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc,
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> index 8037724..5b78d30 100644
> --- a/tests/libqos/virtio-pci.c
> +++ b/tests/libqos/virtio-pci.c
> @@ -65,22 +65,22 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
>  static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t off)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    void *base = dev->addr + VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> -    return qpci_io_readb(dev->pdev, base + off);
> +    uint64_t base = VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> +    return qpci_io_readb(dev->pdev, dev->bar, base + off);
>  }
>  
>  static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t off)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    void *base = dev->addr + VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> -    return qpci_io_readw(dev->pdev, base + off);
> +    uint64_t base = VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> +    return qpci_io_readw(dev->pdev, dev->bar, base + off);
>  }
>  
>  static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t off)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    void *base = dev->addr + VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> -    return qpci_io_readl(dev->pdev, base + off);
> +    uint64_t base = VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> +    return qpci_io_readl(dev->pdev, dev->bar, base + off);
>  }
>  
>  static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t off)
> @@ -88,17 +88,17 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t off)
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
>      int i;
>      uint64_t u64 = 0;
> -    void *base = dev->addr + VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled);
> +    uint64_t base = VIRTIO_PCI_CONFIG_OFF(dev->pdev->msix_enabled) + off;
>  
>      if (target_big_endian()) {
>          for (i = 0; i < 8; ++i) {
> -            u64 |= (uint64_t)qpci_io_readb(dev->pdev,
> -                                           base + off + i) << (7 - i) * 8;
> +            u64 |= (uint64_t)qpci_io_readb(dev->pdev, dev->bar,
> +                                           base + i) << (7 - i) * 8;
>          }
>      } else {
>          for (i = 0; i < 8; ++i) {
> -            u64 |= (uint64_t)qpci_io_readb(dev->pdev,
> -                                           base + off + i) << i * 8;
> +            u64 |= (uint64_t)qpci_io_readb(dev->pdev, dev->bar,
> +                                           base + i) << i * 8;
>          }

You should update this part with qpci_memread() + bswap64()

Thanks,
Laurent