On Tue, Oct 18, 2016 at 06:48:16PM +0200, Laurent Vivier wrote: > > > 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 > > > > # 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() Good idea. Well, actually, I thinl I'll add 64-bit PCI accessors. I'm sure we'll want them in other places soon enough. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson