From: Nicholas Piggin <[email protected]> Add assertions to ensure a BAR is not mapped twice, and that only previously mapped BARs are unmapped. This can help catch bugs and fragile coding.
Cc: Michael S. Tsirkin <[email protected]> Cc: Marcel Apfelbaum <[email protected]> Reviewed-by: Akihiko Odaki <[email protected]> Reviewed-by: Fabiano Rosas <[email protected]> Signed-off-by: Nicholas Piggin <[email protected]> --- tests/qtest/libqos/pci.c | 75 ++++++++++++++++++++++++++++++++-------- tests/qtest/libqos/pci.h | 10 ++++++ 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c index cda3c56c..3d953d23 100644 --- a/tests/qtest/libqos/pci.c +++ b/tests/qtest/libqos/pci.c @@ -79,12 +79,17 @@ QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn) void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr) { uint16_t vendor_id, device_id; + int i; qpci_device_set(dev, bus, addr->devfn); vendor_id = qpci_config_readw(dev, PCI_VENDOR_ID); device_id = qpci_config_readw(dev, PCI_DEVICE_ID); g_assert(!addr->vendor_id || vendor_id == addr->vendor_id); g_assert(!addr->device_id || device_id == addr->device_id); + + for (i = 0; i < QPCI_NUM_REGIONS; i++) { + g_assert(!dev->bars_mapped[i]); + } } static uint8_t qpci_find_resource_reserve_capability(QPCIDevice *dev) @@ -338,21 +343,21 @@ bool qpci_msix_masked(QPCIDevice *dev, uint16_t entry) } /** - * qpci_msix_test_interrupt - test whether msix interrupt has been raised + * qpci_msix_test_interrupt - test whether MSI-X interrupt has been raised * @dev: PCI device - * @msix_entry: msix entry to test - * @msix_addr: address of msix message - * @msix_data: expected msix message payload + * @msix_entry: MSI-X entry to test + * @msix_addr: address of MSI-X message + * @msix_data: expected MSI-X message payload * - * This tests whether the msix source has raised an interrupt. If the msix + * This tests whether the MSI-X source has raised an interrupt. If the MSI-X * entry is masked, it tests the pending bit array for a pending message * and @msix_addr and @msix_data need not be supplied. If the entry is not * masked, it tests the address for corresponding data to see if the interrupt * fired. * * Note that this does not lower the interrupt, however it does clear the - * msix message address to 0 if it is found set. This must be called with - * the msix address memory containing either 0 or the value of data, otherwise + * MSI-X message address to 0 if it is found set. This must be called with + * the MSI-X address memory containing either 0 or the value of data, otherwise * it will assert on incorrect message. */ bool qpci_msix_test_interrupt(QPCIDevice *dev, uint32_t msix_entry, @@ -376,8 +381,8 @@ bool qpci_msix_test_interrupt(QPCIDevice *dev, uint32_t msix_entry, g_assert_cmpint(msix_addr, !=, 0); g_assert_cmpint(msix_data, !=, 0); - /* msix payload is written in little-endian format */ - qtest_memread(dev->bus->qts, msix_addr, &data, 4); + /* MSI-X payload is written in little-endian format */ + qtest_memread(dev->bus->qts, msix_addr, &data, sizeof(data)); data = le32_to_cpu(data); if (data == 0) { return false; @@ -385,7 +390,7 @@ bool qpci_msix_test_interrupt(QPCIDevice *dev, uint32_t msix_entry, /* got a message, ensure it matches expected value then clear it. */ g_assert_cmphex(data, ==, msix_data); - qtest_memset(dev->bus->qts, msix_addr, 0, 4); + qtest_memset(dev->bus->qts, msix_addr, 0, sizeof(data)); return true; } @@ -554,21 +559,31 @@ void qpci_memwrite(QPCIDevice *dev, QPCIBar token, uint64_t off, dev->bus->memwrite(dev->bus, token.addr + off, buf, len); } -QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr) +static uint8_t qpci_bar_reg(int barno) { - 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, }; + + g_assert(barno >= 0 && barno < QPCI_NUM_REGIONS); + + return bar_reg_map[barno]; +} + +QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr) +{ + QPCIBus *bus = dev->bus; QPCIBar bar; int bar_reg; uint32_t addr, size; uint32_t io_type; uint64_t loc; - g_assert(barno >= 0 && barno <= 5); - bar_reg = bar_reg_map[barno]; + g_assert(barno >= 0 && barno < QPCI_NUM_REGIONS); + g_assert(!dev->bars_mapped[barno]); + + bar_reg = qpci_bar_reg(barno); qpci_config_writel(dev, bar_reg, 0xFFFFFFFF); addr = qpci_config_readl(dev, bar_reg); @@ -611,12 +626,38 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr) } bar.addr = loc; + bar.mapped = true; + + dev->bars_mapped[barno] = true; + dev->bars[barno] = bar; + return bar; } void qpci_iounmap(QPCIDevice *dev, QPCIBar bar) { - /* FIXME */ + int bar_reg; + int i; + + if (!bar.mapped) { + return; /* bar was never mapped; no-op */ + } + + for (i = 0; i < QPCI_NUM_REGIONS; i++) { + if (!dev->bars_mapped[i]) { + continue; + } + if (dev->bars[i].addr == bar.addr) { + dev->bars_mapped[i] = false; + dev->bars[i].mapped = false; + bar_reg = qpci_bar_reg(i); + qpci_config_writel(dev, bar_reg, 0xFFFFFFFF); + /* FIXME: the address space is leaked */ + return; + } + } + /* bar was not iomap()ed; treat as no-op for callers that may + * call iounmap unconditionally during cleanup paths. */ } QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr) @@ -627,6 +668,10 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr) void qpci_migrate_fixup(QPCIDevice *to, QPCIDevice *from) { + memcpy(to->bars_mapped, from->bars_mapped, sizeof(from->bars_mapped)); + memset(from->bars_mapped, 0, sizeof(from->bars_mapped)); + memcpy(to->bars, from->bars, sizeof(from->bars)); + memset(from->bars, 0, sizeof(from->bars)); } void add_qpci_address(QOSGraphEdgeOptions *opts, QPCIAddress *addr) diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h index 19f1dd13..73739afe 100644 --- a/tests/qtest/libqos/pci.h +++ b/tests/qtest/libqos/pci.h @@ -58,12 +58,22 @@ struct QPCIBus { struct QPCIBar { uint64_t addr; bool is_io; + bool mapped; }; +/* + * hw/pci permits 7 (PCI_NUM_REGIONS) regions, the last for PCI_ROM_SLOT. + * libqos does not implement PCI_ROM_SLOT at the moment, and as such it + * permits 6. + */ +#define QPCI_NUM_REGIONS 6 + struct QPCIDevice { QPCIBus *bus; int devfn; + bool bars_mapped[QPCI_NUM_REGIONS]; + QPCIBar bars[QPCI_NUM_REGIONS]; bool msix_enabled; QPCIBar msix_table_bar, msix_pba_bar; uint64_t msix_table_off, msix_pba_off; -- 2.55.0
