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]> Signed-off-by: Jishnu Warrier <[email protected]> Signed-off-by: Utkarsh Verma <[email protected]> --- tests/qtest/libqos/ahci.c | 3 ++- tests/qtest/libqos/pci.c | 57 +++++++++++++++++++++++++++++++++++---- tests/qtest/libqos/pci.h | 10 +++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c index a709558aac..53a98bd6cb 100644 --- a/tests/qtest/libqos/ahci.c +++ b/tests/qtest/libqos/ahci.c @@ -225,7 +225,7 @@ void start_ahci_device(AHCIQState *ahci) void stop_ahci_device(AHCIQState *ahci) { - /* Unmap AHCI's ABAR */ + /* Unmap AHCI's ABAR (no-op if not mapped) */ qpci_iounmap(ahci->dev, ahci->hba_bar); } @@ -250,6 +250,7 @@ void ahci_hba_enable(AHCIQState *ahci) uint8_t num_cmd_slots; g_assert(ahci != NULL); + g_assert(ahci->hba_bar.mapped); /* Set GHC.AE to 1 */ ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE); diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c index e78dd4d4c7..365fde1fb9 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) @@ -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,40 @@ 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 +670,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 cfb7c17dd3..2f9555ae4e 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
