[RFC PATCH 0/2] tests/qtest: Enable spapr dma tests

[RFC PATCH 0/2] tests/qtest: Enable spapr dma tests

[Nicholas Piggin]

I took a look at the spapr buggy msix fixme in qtests... AFAIKS
it turns out to be DMA entirely broken due to iommu not set up.
Adding a quick workaround for spapr gets everything working
properly other than exposing bugs in msi message endianness
conversions.

Thanks,
Nick

Nicholas Piggin (2):
  tests/qtest: Fix virtio msix message endianness
  tests/qtest: Enable spapr dma with linear iommu map

 tests/qtest/libqos/pci.h               |  4 ----
 hw/ppc/spapr_iommu.c                   |  9 ++++++++-
 tests/qtest/e1000e-test.c              | 23 +++--------------------
 tests/qtest/igb-test.c                 | 21 ---------------------
 tests/qtest/libqos/generic-pcihost.c   |  1 -
 tests/qtest/libqos/pci-pc.c            |  3 ---
 tests/qtest/libqos/pci-spapr.c         |  7 ++++---
 tests/qtest/libqos/pci.c               | 14 --------------
 tests/qtest/libqos/virtio-pci-modern.c |  9 +++++++--
 tests/qtest/libqos/virtio-pci.c        |  7 +++++--
 tests/qtest/vhost-user-blk-test.c      |  6 ------
 tests/qtest/virtio-blk-test.c          | 12 ------------
 12 files changed, 27 insertions(+), 89 deletions(-)

-- 
2.47.1

[RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness

[Nicholas Piggin]

msix messages are written to memory in little-endian order, so they
should not be byteswapped depending on target endianness, but read
as le and converted to host endian by the qtest.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/qtest/libqos/virtio-pci-modern.c | 9 +++++++--
 tests/qtest/libqos/virtio-pci.c        | 7 +++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/libqos/virtio-pci-modern.c b/tests/qtest/libqos/virtio-pci-modern.c
index 4e67fcbd5d3..67aa2af0bd7 100644
--- a/tests/qtest/libqos/virtio-pci-modern.c
+++ b/tests/qtest/libqos/virtio-pci-modern.c
@@ -8,6 +8,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bswap.h"
 #include "standard-headers/linux/pci_regs.h"
 #include "standard-headers/linux/virtio_pci.h"
 #include "standard-headers/linux/virtio_config.h"
@@ -136,12 +137,16 @@ static bool get_msix_status(QVirtioPCIDevice *dev, uint32_t msix_entry,
         return qpci_msix_pending(dev->pdev, msix_entry);
     }
 
-    data = qtest_readl(dev->pdev->bus->qts, msix_addr);
+    qtest_memread(dev->pdev->bus->qts, msix_addr, &data, 4);
+    data = le32_to_cpu(data);
     if (data == msix_data) {
         qtest_writel(dev->pdev->bus->qts, msix_addr, 0);
         return true;
-    } else {
+    } else if (data == 0) {
         return false;
+    } else {
+        /* Must only be either 0 (no interrupt) or the msix data. */
+        g_assert_not_reached();
     }
 }
 
diff --git a/tests/qtest/libqos/virtio-pci.c b/tests/qtest/libqos/virtio-pci.c
index 002bf8b8c2d..6b421a4d859 100644
--- a/tests/qtest/libqos/virtio-pci.c
+++ b/tests/qtest/libqos/virtio-pci.c
@@ -131,12 +131,15 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
             /* No ISR checking should be done if masked, but read anyway */
             return qpci_msix_pending(dev->pdev, vqpci->msix_entry);
         } else {
-            data = qtest_readl(dev->pdev->bus->qts, vqpci->msix_addr);
+            qtest_memread(dev->pdev->bus->qts, vqpci->msix_addr, &data, 4);
+            data = le32_to_cpu(data);
             if (data == vqpci->msix_data) {
                 qtest_writel(dev->pdev->bus->qts, vqpci->msix_addr, 0);
                 return true;
-            } else {
+            } else if (data == 0) {
                 return false;
+            } else {
+                g_assert_not_reached();
             }
         }
     } else {
-- 
2.47.1

Re: [RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness

[Fabiano Rosas]

Nicholas Piggin <npiggin@gmail.com> writes:

> msix messages are written to memory in little-endian order, so they
> should not be byteswapped depending on target endianness, but read
> as le and converted to host endian by the qtest.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness

[Philippe Mathieu-Daudé]

Hi Nick,

On 15/4/25 10:19, Nicholas Piggin wrote:
> msix messages are written to memory in little-endian order, so they
> should not be byteswapped depending on target endianness, but read
> as le and converted to host endian by the qtest.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   tests/qtest/libqos/virtio-pci-modern.c | 9 +++++++--
>   tests/qtest/libqos/virtio-pci.c        | 7 +++++--
>   2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/libqos/virtio-pci-modern.c b/tests/qtest/libqos/virtio-pci-modern.c
> index 4e67fcbd5d3..67aa2af0bd7 100644
> --- a/tests/qtest/libqos/virtio-pci-modern.c
> +++ b/tests/qtest/libqos/virtio-pci-modern.c
> @@ -8,6 +8,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/bswap.h"
>   #include "standard-headers/linux/pci_regs.h"
>   #include "standard-headers/linux/virtio_pci.h"
>   #include "standard-headers/linux/virtio_config.h"
> @@ -136,12 +137,16 @@ static bool get_msix_status(QVirtioPCIDevice *dev, uint32_t msix_entry,
>           return qpci_msix_pending(dev->pdev, msix_entry);
>       }
>   
> -    data = qtest_readl(dev->pdev->bus->qts, msix_addr);
> +    qtest_memread(dev->pdev->bus->qts, msix_addr, &data, 4);
> +    data = le32_to_cpu(data);
>       if (data == msix_data) {
>           qtest_writel(dev->pdev->bus->qts, msix_addr, 0);
>           return true;
> -    } else {
> +    } else if (data == 0) {
>           return false;
> +    } else {
> +        /* Must only be either 0 (no interrupt) or the msix data. */
> +        g_assert_not_reached();

This distinct change belong to a different preliminary patch.

>       }
>   }
>   
> diff --git a/tests/qtest/libqos/virtio-pci.c b/tests/qtest/libqos/virtio-pci.c
> index 002bf8b8c2d..6b421a4d859 100644
> --- a/tests/qtest/libqos/virtio-pci.c
> +++ b/tests/qtest/libqos/virtio-pci.c
> @@ -131,12 +131,15 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
>               /* No ISR checking should be done if masked, but read anyway */
>               return qpci_msix_pending(dev->pdev, vqpci->msix_entry);
>           } else {
> -            data = qtest_readl(dev->pdev->bus->qts, vqpci->msix_addr);
> +            qtest_memread(dev->pdev->bus->qts, vqpci->msix_addr, &data, 4);
> +            data = le32_to_cpu(data);
>               if (data == vqpci->msix_data) {
>                   qtest_writel(dev->pdev->bus->qts, vqpci->msix_addr, 0);
>                   return true;
> -            } else {
> +            } else if (data == 0) {
>                   return false;
> +            } else {
> +                g_assert_not_reached();

Ditto.

>               }
>           }

Splitting in 2:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>       } else {

[RFC PATCH 2/2] tests/qtest: Enable spapr dma with linear iommu map

[Nicholas Piggin]

qtests spapr dma was broken because the iommu was not set up.

spapr requires hypercalls to set up the iommu (TCE tables), but
there is no support for that or a side-channel to the iommu in
qtests at the moment, so add a quick workaround in QEMU to have
the spapr iommu provide a linear map to memory when running
qtests.

The buggy msix checks can all be removed since the tests all work
now.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/qtest/libqos/pci.h             |  4 ----
 hw/ppc/spapr_iommu.c                 |  9 ++++++++-
 tests/qtest/e1000e-test.c            | 23 +++--------------------
 tests/qtest/igb-test.c               | 21 ---------------------
 tests/qtest/libqos/generic-pcihost.c |  1 -
 tests/qtest/libqos/pci-pc.c          |  3 ---
 tests/qtest/libqos/pci-spapr.c       |  7 ++++---
 tests/qtest/libqos/pci.c             | 14 --------------
 tests/qtest/vhost-user-blk-test.c    |  6 ------
 tests/qtest/virtio-blk-test.c        | 12 ------------
 10 files changed, 15 insertions(+), 85 deletions(-)

diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index 83896145235..9b0b365a0d2 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -51,7 +51,6 @@ struct QPCIBus {
     QTestState *qts;
     uint64_t pio_alloc_ptr, pio_limit;
     uint64_t mmio_alloc_ptr, mmio_limit;
-    bool has_buggy_msi; /* TRUE for spapr, FALSE for pci */
     bool not_hotpluggable; /* TRUE if devices cannot be hotplugged */
 
 };
@@ -83,9 +82,6 @@ QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn);
 void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr);
 int qpci_secondary_buses_init(QPCIBus *bus);
 
-bool qpci_has_buggy_msi(QPCIDevice *dev);
-bool qpci_check_buggy_msi(QPCIDevice *dev);
-
 void qpci_device_enable(QPCIDevice *dev);
 uint8_t qpci_find_capability(QPCIDevice *dev, uint8_t id, uint8_t start_addr);
 void qpci_msix_enable(QPCIDevice *dev);
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index db3a14c1dfd..77895c597df 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -22,6 +22,7 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "system/kvm.h"
+#include "system/qtest.h"
 #include "kvm_ppc.h"
 #include "migration/vmstate.h"
 #include "system/dma.h"
@@ -125,7 +126,13 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,
         .perm = IOMMU_NONE,
     };
 
-    if ((addr >> tcet->page_shift) < tcet->nb_table) {
+    if (qtest_enabled()) {
+        /* spapr qtests does not set up the IOMMU, shortcut a linear map */
+        ret.iova = addr & TARGET_PAGE_MASK;
+        ret.translated_addr = addr & TARGET_PAGE_MASK;
+        ret.addr_mask = ~TARGET_PAGE_MASK;
+        ret.perm = IOMMU_RW;
+    } else if ((addr >> tcet->page_shift) < tcet->nb_table) {
         /* Check if we are in bound */
         hwaddr page_mask = IOMMU_PAGE_MASK(tcet->page_shift);
 
diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
index de9738fdb74..a8d6d3eb013 100644
--- a/tests/qtest/e1000e-test.c
+++ b/tests/qtest/e1000e-test.c
@@ -139,13 +139,6 @@ static void test_e1000e_tx(void *obj, void *data, QGuestAllocator * alloc)
 {
     QE1000E_PCI *e1000e = obj;
     QE1000E *d = &e1000e->e1000e;
-    QOSGraphObject *e_object = obj;
-    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
-
-    /* FIXME: add spapr support */
-    if (qpci_check_buggy_msi(dev)) {
-        return;
-    }
 
     e1000e_send_verify(d, data, alloc);
 }
@@ -154,13 +147,6 @@ static void test_e1000e_rx(void *obj, void *data, QGuestAllocator * alloc)
 {
     QE1000E_PCI *e1000e = obj;
     QE1000E *d = &e1000e->e1000e;
-    QOSGraphObject *e_object = obj;
-    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
-
-    /* FIXME: add spapr support */
-    if (qpci_check_buggy_msi(dev)) {
-        return;
-    }
 
     e1000e_receive_verify(d, data, alloc);
 }
@@ -173,13 +159,10 @@ static void test_e1000e_multiple_transfers(void *obj, void *data,
 
     QE1000E_PCI *e1000e = obj;
     QE1000E *d = &e1000e->e1000e;
-    QOSGraphObject *e_object = obj;
-    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
 
-    /* FIXME: add spapr support */
-    if (qpci_check_buggy_msi(dev)) {
-        return;
-    }
+    /* Use EITR for one irq and disable it for the other, for testing */
+    e1000e_macreg_write(d, E1000_EITR + E1000E_RX0_MSG_ID * 4, 500);
+    e1000e_macreg_write(d, E1000_EITR + E1000E_TX0_MSG_ID * 4, 0);
 
     for (i = 0; i < iterations; i++) {
         e1000e_send_verify(d, data, alloc);
diff --git a/tests/qtest/igb-test.c b/tests/qtest/igb-test.c
index 3d397ea6973..1b3b5aa6c76 100644
--- a/tests/qtest/igb-test.c
+++ b/tests/qtest/igb-test.c
@@ -142,13 +142,6 @@ static void test_igb_tx(void *obj, void *data, QGuestAllocator * alloc)
 {
     QE1000E_PCI *e1000e = obj;
     QE1000E *d = &e1000e->e1000e;
-    QOSGraphObject *e_object = obj;
-    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
-
-    /* FIXME: add spapr support */
-    if (qpci_check_buggy_msi(dev)) {
-        return;
-    }
 
     igb_send_verify(d, data, alloc);
 }
@@ -157,13 +150,6 @@ static void test_igb_rx(void *obj, void *data, QGuestAllocator * alloc)
 {
     QE1000E_PCI *e1000e = obj;
     QE1000E *d = &e1000e->e1000e;
-    QOSGraphObject *e_object = obj;
-    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
-
-    /* FIXME: add spapr support */
-    if (qpci_check_buggy_msi(dev)) {
-        return;
-    }
 
     igb_receive_verify(d, data, alloc);
 }
@@ -176,13 +162,6 @@ static void test_igb_multiple_transfers(void *obj, void *data,
 
     QE1000E_PCI *e1000e = obj;
     QE1000E *d = &e1000e->e1000e;
-    QOSGraphObject *e_object = obj;
-    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
-
-    /* FIXME: add spapr support */
-    if (qpci_check_buggy_msi(dev)) {
-        return;
-    }
 
     for (i = 0; i < iterations; i++) {
         igb_send_verify(d, data, alloc);
diff --git a/tests/qtest/libqos/generic-pcihost.c b/tests/qtest/libqos/generic-pcihost.c
index 4bbeb5ff508..568897e0ecc 100644
--- a/tests/qtest/libqos/generic-pcihost.c
+++ b/tests/qtest/libqos/generic-pcihost.c
@@ -182,7 +182,6 @@ void qpci_init_generic(QGenericPCIBus *qpci, QTestState *qts,
 
     qpci->gpex_pio_base = 0x3eff0000;
     qpci->bus.not_hotpluggable = !hotpluggable;
-    qpci->bus.has_buggy_msi = false;
 
     qpci->bus.pio_readb = qpci_generic_pio_readb;
     qpci->bus.pio_readw = qpci_generic_pio_readw;
diff --git a/tests/qtest/libqos/pci-pc.c b/tests/qtest/libqos/pci-pc.c
index 147009f4f44..8b79d858bd5 100644
--- a/tests/qtest/libqos/pci-pc.c
+++ b/tests/qtest/libqos/pci-pc.c
@@ -124,9 +124,6 @@ void qpci_init_pc(QPCIBusPC *qpci, QTestState *qts, QGuestAllocator *alloc)
 {
     assert(qts);
 
-    /* tests can use pci-bus */
-    qpci->bus.has_buggy_msi = false;
-
     qpci->bus.pio_readb = qpci_pc_pio_readb;
     qpci->bus.pio_readw = qpci_pc_pio_readw;
     qpci->bus.pio_readl = qpci_pc_pio_readl;
diff --git a/tests/qtest/libqos/pci-spapr.c b/tests/qtest/libqos/pci-spapr.c
index 0f1023e4a73..dfa2087a599 100644
--- a/tests/qtest/libqos/pci-spapr.c
+++ b/tests/qtest/libqos/pci-spapr.c
@@ -20,6 +20,10 @@
  * PCI devices are always little-endian
  * SPAPR by default is big-endian
  * so PCI accessors need to swap data endianness
+ *
+ * The spapr iommu model has a qtest_enabled() check that short-cuts
+ * the TCE table and provides a linear map for DMA, since qtests does
+ * not have a way to make hcalls to set up the TCE table.
  */
 
 static uint8_t qpci_spapr_pio_readb(QPCIBus *bus, uint32_t addr)
@@ -155,9 +159,6 @@ void qpci_init_spapr(QPCIBusSPAPR *qpci, QTestState *qts,
 {
     assert(qts);
 
-    /* tests cannot use spapr, needs to be fixed first */
-    qpci->bus.has_buggy_msi = true;
-
     qpci->alloc = alloc;
 
     qpci->bus.pio_readb = qpci_spapr_pio_readb;
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index a59197b9922..3bf6a0e0127 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -53,20 +53,6 @@ void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
     }
 }
 
-bool qpci_has_buggy_msi(QPCIDevice *dev)
-{
-    return dev->bus->has_buggy_msi;
-}
-
-bool qpci_check_buggy_msi(QPCIDevice *dev)
-{
-    if (qpci_has_buggy_msi(dev)) {
-        g_test_skip("Skipping due to incomplete support for MSI");
-        return true;
-    }
-    return false;
-}
-
 static void qpci_device_set(QPCIDevice *dev, QPCIBus *bus, int devfn)
 {
     g_assert(dev);
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index ea90d41232e..3e71fdb9d78 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -554,14 +554,8 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
     uint32_t desc_idx;
     uint8_t status;
     char *data;
-    QOSGraphObject *blk_object = obj;
-    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
     QTestState *qts = global_qtest;
 
-    if (qpci_check_buggy_msi(pci_dev)) {
-        return;
-    }
-
     qpci_msix_enable(pdev->pdev);
     qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
 
diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c
index 98c906ebb4a..3a005d600c1 100644
--- a/tests/qtest/virtio-blk-test.c
+++ b/tests/qtest/virtio-blk-test.c
@@ -474,14 +474,8 @@ static void msix(void *obj, void *u_data, QGuestAllocator *t_alloc)
     uint32_t free_head;
     uint8_t status;
     char *data;
-    QOSGraphObject *blk_object = obj;
-    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
     QTestState *qts = global_qtest;
 
-    if (qpci_check_buggy_msi(pci_dev)) {
-        return;
-    }
-
     qpci_msix_enable(pdev->pdev);
     qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
 
@@ -584,14 +578,8 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
     uint32_t desc_idx;
     uint8_t status;
     char *data;
-    QOSGraphObject *blk_object = obj;
-    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
     QTestState *qts = global_qtest;
 
-    if (qpci_check_buggy_msi(pci_dev)) {
-        return;
-    }
-
     qpci_msix_enable(pdev->pdev);
     qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
 
-- 
2.47.1

Re: [RFC PATCH 2/2] tests/qtest: Enable spapr dma with linear iommu map

[Fabiano Rosas]

Nicholas Piggin <npiggin@gmail.com> writes:

> qtests spapr dma was broken because the iommu was not set up.
>
> spapr requires hypercalls to set up the iommu (TCE tables), but
> there is no support for that or a side-channel to the iommu in
> qtests at the moment, so add a quick workaround in QEMU to have
> the spapr iommu provide a linear map to memory when running
> qtests.

That's fine.

But what would it take to add support? Add another callback such as
qtest_rtas_call() to handle hcalls and call H_PUT_TCE from the test? Or
is there some other complication?

>
> The buggy msix checks can all be removed since the tests all work
> now.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  tests/qtest/libqos/pci.h             |  4 ----
>  hw/ppc/spapr_iommu.c                 |  9 ++++++++-
>  tests/qtest/e1000e-test.c            | 23 +++--------------------
>  tests/qtest/igb-test.c               | 21 ---------------------
>  tests/qtest/libqos/generic-pcihost.c |  1 -
>  tests/qtest/libqos/pci-pc.c          |  3 ---
>  tests/qtest/libqos/pci-spapr.c       |  7 ++++---
>  tests/qtest/libqos/pci.c             | 14 --------------
>  tests/qtest/vhost-user-blk-test.c    |  6 ------
>  tests/qtest/virtio-blk-test.c        | 12 ------------
>  10 files changed, 15 insertions(+), 85 deletions(-)
>

...

> @@ -173,13 +159,10 @@ static void test_e1000e_multiple_transfers(void *obj, void *data,
>  
>      QE1000E_PCI *e1000e = obj;
>      QE1000E *d = &e1000e->e1000e;
> -    QOSGraphObject *e_object = obj;
> -    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
>  
> -    /* FIXME: add spapr support */
> -    if (qpci_check_buggy_msi(dev)) {
> -        return;
> -    }
> +    /* Use EITR for one irq and disable it for the other, for testing */
> +    e1000e_macreg_write(d, E1000_EITR + E1000E_RX0_MSG_ID * 4, 500);
> +    e1000e_macreg_write(d, E1000_EITR + E1000E_TX0_MSG_ID * 4, 0);

What's this about?

>  
>      for (i = 0; i < iterations; i++) {
>          e1000e_send_verify(d, data, alloc);
> diff --git a/tests/qtest/igb-test.c b/tests/qtest/igb-test.c
> index 3d397ea6973..1b3b5aa6c76 100644
> --- a/tests/qtest/igb-test.c
> +++ b/tests/qtest/igb-test.c
> @@ -142,13 +142,6 @@ static void test_igb_tx(void *obj, void *data, QGuestAllocator * alloc)
>  {
>      QE1000E_PCI *e1000e = obj;
>      QE1000E *d = &e1000e->e1000e;
> -    QOSGraphObject *e_object = obj;
> -    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
> -
> -    /* FIXME: add spapr support */
> -    if (qpci_check_buggy_msi(dev)) {
> -        return;
> -    }
>  
>      igb_send_verify(d, data, alloc);
>  }
> @@ -157,13 +150,6 @@ static void test_igb_rx(void *obj, void *data, QGuestAllocator * alloc)
>  {
>      QE1000E_PCI *e1000e = obj;
>      QE1000E *d = &e1000e->e1000e;
> -    QOSGraphObject *e_object = obj;
> -    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
> -
> -    /* FIXME: add spapr support */
> -    if (qpci_check_buggy_msi(dev)) {
> -        return;
> -    }
>  
>      igb_receive_verify(d, data, alloc);
>  }
> @@ -176,13 +162,6 @@ static void test_igb_multiple_transfers(void *obj, void *data,
>  
>      QE1000E_PCI *e1000e = obj;
>      QE1000E *d = &e1000e->e1000e;
> -    QOSGraphObject *e_object = obj;
> -    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
> -
> -    /* FIXME: add spapr support */
> -    if (qpci_check_buggy_msi(dev)) {
> -        return;
> -    }
>  
>      for (i = 0; i < iterations; i++) {
>          igb_send_verify(d, data, alloc);
> diff --git a/tests/qtest/libqos/generic-pcihost.c b/tests/qtest/libqos/generic-pcihost.c
> index 4bbeb5ff508..568897e0ecc 100644
> --- a/tests/qtest/libqos/generic-pcihost.c
> +++ b/tests/qtest/libqos/generic-pcihost.c
> @@ -182,7 +182,6 @@ void qpci_init_generic(QGenericPCIBus *qpci, QTestState *qts,
>  
>      qpci->gpex_pio_base = 0x3eff0000;
>      qpci->bus.not_hotpluggable = !hotpluggable;
> -    qpci->bus.has_buggy_msi = false;
>  
>      qpci->bus.pio_readb = qpci_generic_pio_readb;
>      qpci->bus.pio_readw = qpci_generic_pio_readw;
> diff --git a/tests/qtest/libqos/pci-pc.c b/tests/qtest/libqos/pci-pc.c
> index 147009f4f44..8b79d858bd5 100644
> --- a/tests/qtest/libqos/pci-pc.c
> +++ b/tests/qtest/libqos/pci-pc.c
> @@ -124,9 +124,6 @@ void qpci_init_pc(QPCIBusPC *qpci, QTestState *qts, QGuestAllocator *alloc)
>  {
>      assert(qts);
>  
> -    /* tests can use pci-bus */
> -    qpci->bus.has_buggy_msi = false;
> -
>      qpci->bus.pio_readb = qpci_pc_pio_readb;
>      qpci->bus.pio_readw = qpci_pc_pio_readw;
>      qpci->bus.pio_readl = qpci_pc_pio_readl;
> diff --git a/tests/qtest/libqos/pci-spapr.c b/tests/qtest/libqos/pci-spapr.c
> index 0f1023e4a73..dfa2087a599 100644
> --- a/tests/qtest/libqos/pci-spapr.c
> +++ b/tests/qtest/libqos/pci-spapr.c
> @@ -20,6 +20,10 @@
>   * PCI devices are always little-endian
>   * SPAPR by default is big-endian
>   * so PCI accessors need to swap data endianness
> + *
> + * The spapr iommu model has a qtest_enabled() check that short-cuts
> + * the TCE table and provides a linear map for DMA, since qtests does
> + * not have a way to make hcalls to set up the TCE table.
>   */
>  
>  static uint8_t qpci_spapr_pio_readb(QPCIBus *bus, uint32_t addr)
> @@ -155,9 +159,6 @@ void qpci_init_spapr(QPCIBusSPAPR *qpci, QTestState *qts,
>  {
>      assert(qts);
>  
> -    /* tests cannot use spapr, needs to be fixed first */
> -    qpci->bus.has_buggy_msi = true;
> -
>      qpci->alloc = alloc;
>  
>      qpci->bus.pio_readb = qpci_spapr_pio_readb;
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index a59197b9922..3bf6a0e0127 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -53,20 +53,6 @@ void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
>      }
>  }
>  
> -bool qpci_has_buggy_msi(QPCIDevice *dev)
> -{
> -    return dev->bus->has_buggy_msi;
> -}
> -
> -bool qpci_check_buggy_msi(QPCIDevice *dev)
> -{
> -    if (qpci_has_buggy_msi(dev)) {
> -        g_test_skip("Skipping due to incomplete support for MSI");
> -        return true;
> -    }
> -    return false;
> -}
> -
>  static void qpci_device_set(QPCIDevice *dev, QPCIBus *bus, int devfn)
>  {
>      g_assert(dev);
> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> index ea90d41232e..3e71fdb9d78 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -554,14 +554,8 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
>      uint32_t desc_idx;
>      uint8_t status;
>      char *data;
> -    QOSGraphObject *blk_object = obj;
> -    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
>      QTestState *qts = global_qtest;
>  
> -    if (qpci_check_buggy_msi(pci_dev)) {
> -        return;
> -    }
> -
>      qpci_msix_enable(pdev->pdev);
>      qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
>  
> diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c
> index 98c906ebb4a..3a005d600c1 100644
> --- a/tests/qtest/virtio-blk-test.c
> +++ b/tests/qtest/virtio-blk-test.c
> @@ -474,14 +474,8 @@ static void msix(void *obj, void *u_data, QGuestAllocator *t_alloc)
>      uint32_t free_head;
>      uint8_t status;
>      char *data;
> -    QOSGraphObject *blk_object = obj;
> -    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
>      QTestState *qts = global_qtest;
>  
> -    if (qpci_check_buggy_msi(pci_dev)) {
> -        return;
> -    }
> -
>      qpci_msix_enable(pdev->pdev);
>      qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
>  
> @@ -584,14 +578,8 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
>      uint32_t desc_idx;
>      uint8_t status;
>      char *data;
> -    QOSGraphObject *blk_object = obj;
> -    QPCIDevice *pci_dev = blk_object->get_driver(blk_object, "pci-device");
>      QTestState *qts = global_qtest;
>  
> -    if (qpci_check_buggy_msi(pci_dev)) {
> -        return;
> -    }
> -
>      qpci_msix_enable(pdev->pdev);
>      qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);

Re: [RFC PATCH 2/2] tests/qtest: Enable spapr dma with linear iommu map

[Nicholas Piggin]

On Wed Apr 16, 2025 at 4:07 AM AEST, Fabiano Rosas wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> qtests spapr dma was broken because the iommu was not set up.
>>
>> spapr requires hypercalls to set up the iommu (TCE tables), but
>> there is no support for that or a side-channel to the iommu in
>> qtests at the moment, so add a quick workaround in QEMU to have
>> the spapr iommu provide a linear map to memory when running
>> qtests.
>
> That's fine.
>
> But what would it take to add support? Add another callback such as
> qtest_rtas_call() to handle hcalls and call H_PUT_TCE from the test? Or
> is there some other complication?

Yeah, exactly. qtest_ppc_spapr_rtas_call() and/or _spapr_hcall() in
the qtest interface I think would do it.

It would be nice to do that in general to be able to test various spapr
bits with qtests. It's just getting all the pieces together would be a
bit more work than this simple bandaid.

>
>>
>> The buggy msix checks can all be removed since the tests all work
>> now.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  tests/qtest/libqos/pci.h             |  4 ----
>>  hw/ppc/spapr_iommu.c                 |  9 ++++++++-
>>  tests/qtest/e1000e-test.c            | 23 +++--------------------
>>  tests/qtest/igb-test.c               | 21 ---------------------
>>  tests/qtest/libqos/generic-pcihost.c |  1 -
>>  tests/qtest/libqos/pci-pc.c          |  3 ---
>>  tests/qtest/libqos/pci-spapr.c       |  7 ++++---
>>  tests/qtest/libqos/pci.c             | 14 --------------
>>  tests/qtest/vhost-user-blk-test.c    |  6 ------
>>  tests/qtest/virtio-blk-test.c        | 12 ------------
>>  10 files changed, 15 insertions(+), 85 deletions(-)
>>
>
> ...
>
>> @@ -173,13 +159,10 @@ static void test_e1000e_multiple_transfers(void *obj, void *data,
>>  
>>      QE1000E_PCI *e1000e = obj;
>>      QE1000E *d = &e1000e->e1000e;
>> -    QOSGraphObject *e_object = obj;
>> -    QPCIDevice *dev = e_object->get_driver(e_object, "pci-device");
>>  
>> -    /* FIXME: add spapr support */
>> -    if (qpci_check_buggy_msi(dev)) {
>> -        return;
>> -    }
>> +    /* Use EITR for one irq and disable it for the other, for testing */
>> +    e1000e_macreg_write(d, E1000_EITR + E1000E_RX0_MSG_ID * 4, 500);
>> +    e1000e_macreg_write(d, E1000_EITR + E1000E_TX0_MSG_ID * 4, 0);
>
> What's this about?

It's a rebase bug, thanks for catching it.

Thanks,
Nick