From: Nicholas Piggin <npiggin@gmail.com>
To: Fabiano Rosas <farosas@suse.de>
Cc: Nicholas Piggin <npiggin@gmail.com>, John Snow <jsnow@redhat.com>,
Laurent Vivier <lvivier@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Akihiko Odaki <akihiko.odaki@daynix.com>,
"Michael S . Tsirkin" <mst@redhat.com>,
Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
qemu-block@nongnu.org, qemu-devel@nongnu.org
Subject: [PATCH v3 3/4] qtest/libqos/pci: Fix qpci_msix_enable sharing bar0
Date: Sat, 18 Jan 2025 03:22:42 +1000 [thread overview]
Message-ID: <20250117172244.406206-4-npiggin@gmail.com> (raw)
In-Reply-To: <20250117172244.406206-1-npiggin@gmail.com>
Devices where the MSI-X addresses are shared with other MMIO on BAR0
can not use msi_enable because it unmaps and remaps BAR0, which
interferes with device MMIO mappings. xhci-nec is one such device we
would like to test msix with.
Use the BAR iomap tracking structure introduced in the previous change
to have qpci_misx_enable() use existing iomaps if msix bars are
already mapped.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
tests/qtest/libqos/pci.h | 1 +
tests/qtest/libqos/pci.c | 40 ++++++++++++++++++++++++++++++++++------
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index 9dc82ea723a..5a7b2454ad5 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -68,6 +68,7 @@ struct QPCIDevice
bool bars_mapped[6];
QPCIBar bars[6];
bool msix_enabled;
+ bool msix_table_bar_iomap, msix_pba_bar_iomap;
QPCIBar msix_table_bar, msix_pba_bar;
uint64_t msix_table_off, msix_pba_off;
};
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index 05089a5f24f..a187349d30a 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -288,15 +288,21 @@ void qpci_msix_enable(QPCIDevice *dev)
table = qpci_config_readl(dev, addr + PCI_MSIX_TABLE);
bir_table = table & PCI_MSIX_FLAGS_BIRMASK;
- dev->msix_table_bar = qpci_iomap(dev, bir_table, NULL);
+ if (dev->bars_mapped[bir_table]) {
+ dev->msix_table_bar = dev->bars[bir_table];
+ } else {
+ dev->msix_table_bar_iomap = true;
+ 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) {
- dev->msix_pba_bar = qpci_iomap(dev, bir_pba, NULL);
+ if (dev->bars_mapped[bir_pba]) {
+ dev->msix_pba_bar = dev->bars[bir_pba];
} else {
- dev->msix_pba_bar = dev->msix_table_bar;
+ dev->msix_pba_bar_iomap = true;
+ dev->msix_pba_bar = qpci_iomap(dev, bir_pba, NULL);
}
dev->msix_pba_off = table & ~PCI_MSIX_FLAGS_BIRMASK;
@@ -307,6 +313,7 @@ void qpci_msix_disable(QPCIDevice *dev)
{
uint8_t addr;
uint16_t val;
+ uint32_t table;
g_assert(dev->msix_enabled);
addr = qpci_find_capability(dev, PCI_CAP_ID_MSIX, 0);
@@ -315,10 +322,31 @@ void qpci_msix_disable(QPCIDevice *dev)
qpci_config_writew(dev, addr + PCI_MSIX_FLAGS,
val & ~PCI_MSIX_FLAGS_ENABLE);
- if (dev->msix_pba_bar.addr != dev->msix_table_bar.addr) {
+ if (dev->msix_pba_bar_iomap) {
+ dev->msix_pba_bar_iomap = false;
qpci_iounmap(dev, dev->msix_pba_bar);
+ } else {
+ /*
+ * If we had reused an existing iomap, ensure it is still mapped
+ * otherwise it would be a bug if it were unmapped before msix is
+ * disabled. A refcounting iomap implementation could avoid this
+ * issue entirely, but let's wait until that's needed.
+ */
+ uint8_t bir_pba;
+ table = qpci_config_readl(dev, addr + PCI_MSIX_PBA);
+ bir_pba = table & PCI_MSIX_FLAGS_BIRMASK;
+ g_assert(dev->bars_mapped[bir_pba]);
+ }
+
+ if (dev->msix_table_bar_iomap) {
+ dev->msix_table_bar_iomap = false;
+ qpci_iounmap(dev, dev->msix_table_bar);
+ } else {
+ uint8_t bir_table;
+ table = qpci_config_readl(dev, addr + PCI_MSIX_TABLE);
+ bir_table = table & PCI_MSIX_FLAGS_BIRMASK;
+ g_assert(dev->bars_mapped[bir_table]);
}
- qpci_iounmap(dev, dev->msix_table_bar);
dev->msix_enabled = 0;
dev->msix_table_off = 0;
--
2.45.2
next prev parent reply other threads:[~2025-01-17 17:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-17 17:22 [PATCH v3 0/4] qtest/libqos/pci: pci and msix fixes Nicholas Piggin
2025-01-17 17:22 ` [PATCH v3 1/4] qtest/libqos/pci: Do not write to PBA memory Nicholas Piggin
2025-01-17 17:22 ` [PATCH v3 2/4] qtest/libqos/pci: Enforce balanced iomap/unmap Nicholas Piggin
2025-01-20 5:29 ` Philippe Mathieu-Daudé
2025-01-21 4:39 ` Nicholas Piggin
2025-01-21 6:53 ` Philippe Mathieu-Daudé
2025-01-17 17:22 ` Nicholas Piggin [this message]
2025-01-17 17:22 ` [PATCH v3 4/4] qtest/libqos/pci: Factor msix entry helpers into pci common code Nicholas Piggin
2025-01-18 9:58 ` [PATCH v3 0/4] qtest/libqos/pci: pci and msix fixes Akihiko Odaki
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=20250117172244.406206-4-npiggin@gmail.com \
--to=npiggin@gmail.com \
--cc=akihiko.odaki@daynix.com \
--cc=farosas@suse.de \
--cc=jsnow@redhat.com \
--cc=lvivier@redhat.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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.