From: Nicholas Piggin <npiggin@gmail.com>
To: qemu-devel@nongnu.org
Cc: Nicholas Piggin <npiggin@gmail.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
Fabiano Rosas <farosas@suse.de>,
Laurent Vivier <lvivier@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Dmitry Fleytman <dmitry.fleytman@gmail.com>,
Akihiko Odaki <akihiko.odaki@daynix.com>,
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
Subject: [PATCH 1/8] qtest/pci: Enforce balanced iomap/unmap
Date: Thu, 12 Dec 2024 18:34:54 +1000 [thread overview]
Message-ID: <20241212083502.1439033-2-npiggin@gmail.com> (raw)
In-Reply-To: <20241212083502.1439033-1-npiggin@gmail.com>
Add assertions to ensure a BAR is not mapped twice, and only
previously mapped BARs are unmapped. This can help catch some
bugs.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
tests/qtest/libqos/ahci.h | 1 +
tests/qtest/libqos/pci.h | 2 ++
tests/qtest/libqos/virtio-pci.h | 1 +
tests/qtest/ahci-test.c | 2 ++
tests/qtest/libqos/ahci.c | 6 ++++++
tests/qtest/libqos/pci.c | 32 +++++++++++++++++++++++++++++++-
tests/qtest/libqos/virtio-pci.c | 6 +++++-
7 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/libqos/ahci.h b/tests/qtest/libqos/ahci.h
index a0487a1557d..5d7e26aee2a 100644
--- a/tests/qtest/libqos/ahci.h
+++ b/tests/qtest/libqos/ahci.h
@@ -575,6 +575,7 @@ QPCIDevice *get_ahci_device(QTestState *qts, uint32_t *fingerprint);
void free_ahci_device(QPCIDevice *dev);
void ahci_pci_enable(AHCIQState *ahci);
void start_ahci_device(AHCIQState *ahci);
+void stop_ahci_device(AHCIQState *ahci);
void ahci_hba_enable(AHCIQState *ahci);
/* Port Management */
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index 83896145235..9dc82ea723a 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -65,6 +65,8 @@ struct QPCIDevice
{
QPCIBus *bus;
int devfn;
+ bool bars_mapped[6];
+ QPCIBar bars[6];
bool msix_enabled;
QPCIBar msix_table_bar, msix_pba_bar;
uint64_t msix_table_off, msix_pba_off;
diff --git a/tests/qtest/libqos/virtio-pci.h b/tests/qtest/libqos/virtio-pci.h
index f5115cacba2..efdf904b254 100644
--- a/tests/qtest/libqos/virtio-pci.h
+++ b/tests/qtest/libqos/virtio-pci.h
@@ -26,6 +26,7 @@ typedef struct QVirtioPCIDevice {
uint64_t config_msix_addr;
uint32_t config_msix_data;
+ bool enabled;
int bar_idx;
/* VIRTIO 1.0 */
diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 5a1923f721b..b3dae7a8ce4 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1483,6 +1483,8 @@ static void test_reset_pending_callback(void)
/* Wait for throttled write to finish. */
sleep(1);
+ stop_ahci_device(ahci);
+
/* Start again. */
ahci_clean_mem(ahci);
ahci_pci_enable(ahci);
diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
index 34a75b7f43b..cfc435b6663 100644
--- a/tests/qtest/libqos/ahci.c
+++ b/tests/qtest/libqos/ahci.c
@@ -217,6 +217,12 @@ void start_ahci_device(AHCIQState *ahci)
qpci_device_enable(ahci->dev);
}
+void stop_ahci_device(AHCIQState *ahci)
+{
+ /* Map AHCI's ABAR (BAR5) */
+ qpci_iounmap(ahci->dev, ahci->hba_bar);
+}
+
/**
* Test and initialize the AHCI's HBA memory areas.
* Initialize and start any ports with devices attached.
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index b23d72346b6..a42ca08261d 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -93,12 +93,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 < 6; i++) {
+ g_assert(!dev->bars_mapped[i]);
+ }
}
static uint8_t qpci_find_resource_reserve_capability(QPCIDevice *dev)
@@ -531,6 +536,8 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
uint64_t loc;
g_assert(barno >= 0 && barno <= 5);
+ g_assert(!dev->bars_mapped[barno]);
+
bar_reg = bar_reg_map[barno];
qpci_config_writel(dev, bar_reg, 0xFFFFFFFF);
@@ -574,12 +581,35 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
}
bar.addr = loc;
+
+ dev->bars_mapped[barno] = true;
+ dev->bars[barno] = bar;
+
return bar;
}
void qpci_iounmap(QPCIDevice *dev, QPCIBar bar)
{
- /* FIXME */
+ 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,
+ };
+ int bar_reg;
+ int i;
+
+ for (i = 0; i < 6; i++) {
+ if (!dev->bars_mapped[i]) {
+ continue;
+ }
+ if (dev->bars[i].addr == bar.addr) {
+ dev->bars_mapped[i] = false;
+ bar_reg = bar_reg_map[i];
+ qpci_config_writel(dev, bar_reg, 0xFFFFFFFF);
+ /* FIXME: the address space is leaked */
+ return;
+ }
+ }
+ g_assert_not_reached();
}
QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
diff --git a/tests/qtest/libqos/virtio-pci.c b/tests/qtest/libqos/virtio-pci.c
index 485b8f6b7e0..2b59fb181c9 100644
--- a/tests/qtest/libqos/virtio-pci.c
+++ b/tests/qtest/libqos/virtio-pci.c
@@ -304,11 +304,15 @@ void qvirtio_pci_device_enable(QVirtioPCIDevice *d)
{
qpci_device_enable(d->pdev);
d->bar = qpci_iomap(d->pdev, d->bar_idx, NULL);
+ d->enabled = true;
}
void qvirtio_pci_device_disable(QVirtioPCIDevice *d)
{
- qpci_iounmap(d->pdev, d->bar);
+ if (d->enabled) {
+ qpci_iounmap(d->pdev, d->bar);
+ d->enabled = false;
+ }
}
void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
--
2.45.2
next prev parent reply other threads:[~2024-12-12 8:36 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-12 8:34 [PATCH 0/8] Add XHCI TR NOOP support, plus PCI, MSIX changes Nicholas Piggin
2024-12-12 8:34 ` Nicholas Piggin [this message]
2024-12-12 8:34 ` [PATCH 2/8] qtest/libqos/pci: Fix qpci_msix_enable sharing bar0 Nicholas Piggin
2024-12-12 8:34 ` [PATCH 3/8] pci/msix: Implement PBA writes Nicholas Piggin
2024-12-13 5:14 ` Akihiko Odaki
2024-12-18 1:44 ` Nicholas Piggin
2024-12-12 8:34 ` [PATCH 4/8] tests/qtest/e1000e|igb: Fix e1000e and igb tests to re-trigger interrupts Nicholas Piggin
2024-12-12 8:34 ` [PATCH 5/8] hw/usb/xhci: Move HCD constants to a header and add register constants Nicholas Piggin
2024-12-18 15:08 ` Phil Dennis-Jordan
2024-12-19 1:50 ` Nicholas Piggin
2024-12-20 14:11 ` Phil Dennis-Jordan
2024-12-12 8:34 ` [PATCH 6/8] qtest/xhci: Add controller and device setup and ring tests Nicholas Piggin
2024-12-12 8:35 ` [PATCH 7/8] hw/usb/xhci: Support TR NOOP commands Nicholas Piggin
2024-12-12 8:35 ` [PATCH 8/8] qtest/xhci: add a test for " Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241212083502.1439033-2-npiggin@gmail.com \
--to=npiggin@gmail.com \
--cc=akihiko.odaki@daynix.com \
--cc=dmitry.fleytman@gmail.com \
--cc=farosas@suse.de \
--cc=lvivier@redhat.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=sriram.yagnaraman@ericsson.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.