[PATCH 0/6] qtests/libqos: Introduce pci-arm

[PATCH 0/6] qtests/libqos: Introduce pci-arm

[Eric Auger]

Up to now the virt-machine node only contains a virtio-mmio
driver node but no driver that eventually produces any pci-bus
interface.

Hence, PCI libqos tests cannot be run with aarch64 binary.

This series brings the pieces needed to be able to run PCI tests
with the aarch64 binary: a generic-pcihost driver node gets
instantiated by the machine. This later contains a pci-bus-arm
driver which produces a pci-bus interface. Then all tests
consuming the pci-bus interface can be run with the libqos arm
virt machine.

One of the first goal was to be able to run the virtio-iommu-pci
tests as the virtio-iommu was initially targetting ARM and it
was awkard to be run the test with the pc machine. This is now
possible.

Only the tests doing hotplug cannot be run yet as hotplug is
not possible on the root bus. This will be dealt with separately
by adding a root port to the object tree.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/libqos-pci-arm-v1

Eric Auger (6):
  tests/qtest/vhost-user-test.c: Use vhostforce=on
  tests/qtest/libqos/pci: Introduce pio_limit
  tests/qtest/libqos: Skip hotplug tests if pci root bus is not
    hotpluggable
  tests/qtest/vhost-user-blk-test: Setup MSIx to avoid error on aarch64
  tests/qtest/vhost-user-blk-test: Factorize vq setup code
  tests/qtest/libqos: Add pci-arm and add a pci-arm producer in arm-virt
    machine

 tests/qtest/e1000e-test.c             |   6 +
 tests/qtest/libqos/arm-virt-machine.c |  47 +++++-
 tests/qtest/libqos/meson.build        |   3 +
 tests/qtest/libqos/pci-arm.c          | 219 ++++++++++++++++++++++++++
 tests/qtest/libqos/pci-arm.h          |  56 +++++++
 tests/qtest/libqos/pci-pc.c           |   1 +
 tests/qtest/libqos/pci-spapr.c        |   1 +
 tests/qtest/libqos/pci.c              |  78 +++++----
 tests/qtest/libqos/pci.h              |   7 +-
 tests/qtest/libqos/qgraph.c           |   7 +
 tests/qtest/libqos/qgraph.h           |  15 ++
 tests/qtest/vhost-user-blk-test.c     |  39 ++++-
 tests/qtest/vhost-user-test.c         |   2 +-
 tests/qtest/virtio-blk-test.c         |   5 +
 tests/qtest/virtio-net-test.c         |   5 +
 tests/qtest/virtio-rng-test.c         |   5 +
 16 files changed, 452 insertions(+), 44 deletions(-)
 create mode 100644 tests/qtest/libqos/pci-arm.c
 create mode 100644 tests/qtest/libqos/pci-arm.h

-- 
2.26.3

[PATCH 1/6] tests/qtest/vhost-user-test.c: Use vhostforce=on

[Eric Auger]

-netdev vhost-user,vhostforce is deprecated and vhostforce=on
should be used instead.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/vhost-user-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 3d6337fb5c5..6e79935c47e 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -42,7 +42,7 @@
 #define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," \
                         " -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
-#define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce"
+#define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
 
 #define HUGETLBFS_MAGIC       0x958458f6
 
-- 
2.26.3

Re: [PATCH 1/6] tests/qtest/vhost-user-test.c: Use vhostforce=on

[Thomas Huth]

On 10/01/2022 22.19, Eric Auger wrote:
> -netdev vhost-user,vhostforce is deprecated and vhostforce=on
> should be used instead.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/qtest/vhost-user-test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index 3d6337fb5c5..6e79935c47e 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -42,7 +42,7 @@
>   #define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," \
>                           " -numa node,memdev=mem"
>   #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
> -#define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce"
> +#define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
>   
>   #define HUGETLBFS_MAGIC       0x958458f6
>   

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 2/6] tests/qtest/libqos/pci: Introduce pio_limit

[Eric Auger]

At the moment the IO space limit is hardcoded to
QPCI_PIO_LIMIT = 0x10000. When accesses are performed to a bar,
the base address of this latter is compared against the limit
to decide whether we perform an IO or a memory access.

On ARM, we cannot keep this PIO limit as the arm-virt machine
uses [0x3eff0000, 0x3f000000 ] for the IO space map and we
are mandated to allocate at 0x0.

Add a new flag in QPCIBar indicating whether it is an IO bar
or a memory bar. This flag is set on QPCIBar allocation and
provisionned based on the BAR configuration. Then the new flag
is used in access functions and in iomap() function.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/libqos/pci-pc.c    |  1 +
 tests/qtest/libqos/pci-spapr.c |  1 +
 tests/qtest/libqos/pci.c       | 78 ++++++++++++++++++++++------------
 tests/qtest/libqos/pci.h       |  5 +--
 4 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/tests/qtest/libqos/pci-pc.c b/tests/qtest/libqos/pci-pc.c
index f97844289f1..8051a0881a0 100644
--- a/tests/qtest/libqos/pci-pc.c
+++ b/tests/qtest/libqos/pci-pc.c
@@ -150,6 +150,7 @@ void qpci_init_pc(QPCIBusPC *qpci, QTestState *qts, QGuestAllocator *alloc)
 
     qpci->bus.qts = qts;
     qpci->bus.pio_alloc_ptr = 0xc000;
+    qpci->bus.pio_limit = 0x10000;
     qpci->bus.mmio_alloc_ptr = 0xE0000000;
     qpci->bus.mmio_limit = 0x100000000ULL;
 
diff --git a/tests/qtest/libqos/pci-spapr.c b/tests/qtest/libqos/pci-spapr.c
index 262226985f3..870ffdd8b51 100644
--- a/tests/qtest/libqos/pci-spapr.c
+++ b/tests/qtest/libqos/pci-spapr.c
@@ -197,6 +197,7 @@ void qpci_init_spapr(QPCIBusSPAPR *qpci, QTestState *qts,
 
     qpci->bus.qts = qts;
     qpci->bus.pio_alloc_ptr = 0xc000;
+    qpci->bus.pio_limit = 0x10000;
     qpci->bus.mmio_alloc_ptr = qpci->mmio32.pci_base;
     qpci->bus.mmio_limit = qpci->mmio32.pci_base + qpci->mmio32.size;
 
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index 3a9076ae580..b23d72346b6 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -398,44 +398,56 @@ void qpci_config_writel(QPCIDevice *dev, uint8_t offset, uint32_t value)
 
 uint8_t qpci_io_readb(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readb(dev->bus, token.addr + off);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        return bus->pio_readb(bus, token.addr + off);
     } else {
         uint8_t val;
-        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
+
+        bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
         return val;
     }
 }
 
 uint16_t qpci_io_readw(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readw(dev->bus, token.addr + off);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        return bus->pio_readw(bus, token.addr + off);
     } else {
         uint16_t val;
-        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
+
+        bus->memread(bus, token.addr + off, &val, sizeof(val));
         return le16_to_cpu(val);
     }
 }
 
 uint32_t qpci_io_readl(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readl(dev->bus, token.addr + off);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        return bus->pio_readl(bus, token.addr + off);
     } else {
         uint32_t val;
-        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
+
+        bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
         return le32_to_cpu(val);
     }
 }
 
 uint64_t qpci_io_readq(QPCIDevice *dev, QPCIBar token, uint64_t off)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        return dev->bus->pio_readq(dev->bus, token.addr + off);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        return bus->pio_readq(bus, token.addr + off);
     } else {
         uint64_t val;
-        dev->bus->memread(dev->bus, token.addr + off, &val, sizeof(val));
+
+        bus->memread(bus, token.addr + off, &val, sizeof(val));
         return le64_to_cpu(val);
     }
 }
@@ -443,57 +455,65 @@ uint64_t qpci_io_readq(QPCIDevice *dev, QPCIBar token, uint64_t off)
 void qpci_io_writeb(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint8_t value)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writeb(dev->bus, token.addr + off, value);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        bus->pio_writeb(bus, token.addr + off, value);
     } else {
-        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
+        bus->memwrite(bus, token.addr + off, &value, sizeof(value));
     }
 }
 
 void qpci_io_writew(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint16_t value)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writew(dev->bus, token.addr + off, value);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        bus->pio_writew(bus, token.addr + off, value);
     } else {
         value = cpu_to_le16(value);
-        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
+        bus->memwrite(bus, token.addr + off, &value, sizeof(value));
     }
 }
 
 void qpci_io_writel(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint32_t value)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writel(dev->bus, token.addr + off, value);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        bus->pio_writel(bus, token.addr + off, value);
     } else {
         value = cpu_to_le32(value);
-        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
+        bus->memwrite(bus, token.addr + off, &value, sizeof(value));
     }
 }
 
 void qpci_io_writeq(QPCIDevice *dev, QPCIBar token, uint64_t off,
                     uint64_t value)
 {
-    if (token.addr < QPCI_PIO_LIMIT) {
-        dev->bus->pio_writeq(dev->bus, token.addr + off, value);
+    QPCIBus *bus = dev->bus;
+
+    if (token.is_io) {
+        bus->pio_writeq(bus, token.addr + off, value);
     } else {
         value = cpu_to_le64(value);
-        dev->bus->memwrite(dev->bus, token.addr + off, &value, sizeof(value));
+        bus->memwrite(bus, token.addr + off, &value, sizeof(value));
     }
 }
 
 void qpci_memread(QPCIDevice *dev, QPCIBar token, uint64_t off,
                   void *buf, size_t len)
 {
-    g_assert(token.addr >= QPCI_PIO_LIMIT);
+    g_assert(!token.is_io);
     dev->bus->memread(dev->bus, token.addr + off, buf, len);
 }
 
 void qpci_memwrite(QPCIDevice *dev, QPCIBar token, uint64_t off,
                    const void *buf, size_t len)
 {
-    g_assert(token.addr >= QPCI_PIO_LIMIT);
+    g_assert(!token.is_io);
     dev->bus->memwrite(dev->bus, token.addr + off, buf, len);
 }
 
@@ -534,9 +554,10 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
         loc = QEMU_ALIGN_UP(bus->pio_alloc_ptr, size);
 
         g_assert(loc >= bus->pio_alloc_ptr);
-        g_assert(loc + size <= QPCI_PIO_LIMIT); /* Keep PIO below 64kiB */
+        g_assert(loc + size <= bus->pio_limit);
 
         bus->pio_alloc_ptr = loc + size;
+        bar.is_io = true;
 
         qpci_config_writel(dev, bar_reg, loc | PCI_BASE_ADDRESS_SPACE_IO);
     } else {
@@ -547,6 +568,7 @@ QPCIBar qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
         g_assert(loc + size <= bus->mmio_limit);
 
         bus->mmio_alloc_ptr = loc + size;
+        bar.is_io = false;
 
         qpci_config_writel(dev, bar_reg, loc);
     }
@@ -562,7 +584,7 @@ void qpci_iounmap(QPCIDevice *dev, QPCIBar bar)
 
 QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
 {
-    QPCIBar bar = { .addr = addr };
+    QPCIBar bar = { .addr = addr, .is_io = true };
     return bar;
 }
 
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index becb800f9e6..44f6806fe44 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -16,8 +16,6 @@
 #include "libqtest.h"
 #include "qgraph.h"
 
-#define QPCI_PIO_LIMIT    0x10000
-
 #define QPCI_DEVFN(dev, fn) (((dev) << 3) | (fn))
 
 typedef struct QPCIDevice QPCIDevice;
@@ -51,7 +49,7 @@ struct QPCIBus {
                           uint8_t offset, uint32_t value);
 
     QTestState *qts;
-    uint16_t pio_alloc_ptr;
+    uint64_t pio_alloc_ptr, pio_limit;
     uint64_t mmio_alloc_ptr, mmio_limit;
     bool has_buggy_msi; /* TRUE for spapr, FALSE for pci */
 
@@ -59,6 +57,7 @@ struct QPCIBus {
 
 struct QPCIBar {
     uint64_t addr;
+    bool is_io;
 };
 
 struct QPCIDevice
-- 
2.26.3

Re: [PATCH 2/6] tests/qtest/libqos/pci: Introduce pio_limit

[Thomas Huth]

On 10/01/2022 22.19, Eric Auger wrote:
> At the moment the IO space limit is hardcoded to
> QPCI_PIO_LIMIT = 0x10000. When accesses are performed to a bar,
> the base address of this latter is compared against the limit
> to decide whether we perform an IO or a memory access.
> 
> On ARM, we cannot keep this PIO limit as the arm-virt machine
> uses [0x3eff0000, 0x3f000000 ] for the IO space map and we
> are mandated to allocate at 0x0.
> 
> Add a new flag in QPCIBar indicating whether it is an IO bar
> or a memory bar. This flag is set on QPCIBar allocation and
> provisionned based on the BAR configuration. Then the new flag
> is used in access functions and in iomap() function.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/qtest/libqos/pci-pc.c    |  1 +
>   tests/qtest/libqos/pci-spapr.c |  1 +
>   tests/qtest/libqos/pci.c       | 78 ++++++++++++++++++++++------------
>   tests/qtest/libqos/pci.h       |  5 +--
>   4 files changed, 54 insertions(+), 31 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 3/6] tests/qtest/libqos: Skip hotplug tests if pci root bus is not hotpluggable

[Eric Auger]

ARM does not not support hotplug on pcie.0. Add a flag on the bus
which tells if devices can be hotplugged and skip hotplug tests
if the bus cannot be hotplugged. This is a temporary solution to
enable the other pci tests on aarch64.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/e1000e-test.c         |  6 ++++++
 tests/qtest/libqos/pci.h          |  1 +
 tests/qtest/vhost-user-blk-test.c | 10 ++++++++++
 tests/qtest/virtio-blk-test.c     |  5 +++++
 tests/qtest/virtio-net-test.c     |  5 +++++
 tests/qtest/virtio-rng-test.c     |  5 +++++
 6 files changed, 32 insertions(+)

diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
index 0273fe4c156..0d656d3af63 100644
--- a/tests/qtest/e1000e-test.c
+++ b/tests/qtest/e1000e-test.c
@@ -235,6 +235,12 @@ static void test_e1000e_multiple_transfers(void *obj, void *data,
 static void test_e1000e_hotplug(void *obj, void *data, QGuestAllocator * alloc)
 {
     QTestState *qts = global_qtest;  /* TODO: get rid of global_qtest here */
+    QE1000E_PCI *dev = obj;
+
+    if (dev->pci_dev.bus->not_hotpluggable) {
+        g_test_skip("bus pci.0 does not support hotplug");
+        return;
+    }
 
     qtest_qmp_device_add(qts, "e1000e", "e1000e_net", "{'addr': '0x06'}");
     qpci_unplug_acpi_device_test(qts, "e1000e_net", 0x06);
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index 44f6806fe44..6a28b405221 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -52,6 +52,7 @@ struct QPCIBus {
     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 */
 
 };
 
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 62e670f39be..cea2570884b 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -676,6 +676,11 @@ static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtioPCIDevice *dev;
     QTestState *qts = dev1->pdev->bus->qts;
 
+    if (dev1->pdev->bus->not_hotpluggable) {
+        g_test_skip("bus pci.0 does not support hotplug");
+        return;
+    }
+
     /* plug secondary disk */
     qtest_qmp_device_add(qts, "vhost-user-blk-pci", "drv1",
                          "{'addr': %s, 'chardev': 'char2'}",
@@ -703,6 +708,11 @@ static void multiqueue(void *obj, void *data, QGuestAllocator *t_alloc)
     uint64_t features;
     uint16_t num_queues;
 
+    if (pdev1->pdev->bus->not_hotpluggable) {
+        g_test_skip("bus pci.0 does not support hotplug");
+        return;
+    }
+
     /*
      * The primary device has 1 queue and VIRTIO_BLK_F_MQ is not enabled. The
      * VIRTIO specification allows VIRTIO_BLK_F_MQ to be enabled when there is
diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c
index 2a236982118..0d5fa5b3247 100644
--- a/tests/qtest/virtio-blk-test.c
+++ b/tests/qtest/virtio-blk-test.c
@@ -701,6 +701,11 @@ static void pci_hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtioPCIDevice *dev;
     QTestState *qts = dev1->pdev->bus->qts;
 
+    if (dev1->pdev->bus->not_hotpluggable) {
+        g_test_skip("bus pci.0 does not support hotplug");
+        return;
+    }
+
     /* plug secondary disk */
     qtest_qmp_device_add(qts, "virtio-blk-pci", "drv1",
                          "{'addr': %s, 'drive': 'drive1'}",
diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index 8bf74e516cc..fdf7dd750b9 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -174,6 +174,11 @@ static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
     QTestState *qts = dev->pdev->bus->qts;
     const char *arch = qtest_get_arch();
 
+    if (dev->pdev->bus->not_hotpluggable) {
+        g_test_skip("bus pci.0 does not support hotplug");
+        return;
+    }
+
     qtest_qmp_device_add(qts, "virtio-net-pci", "net1",
                          "{'addr': %s}", stringify(PCI_SLOT_HP));
 
diff --git a/tests/qtest/virtio-rng-test.c b/tests/qtest/virtio-rng-test.c
index e6b8cd8e0cf..6b7d2b9eea4 100644
--- a/tests/qtest/virtio-rng-test.c
+++ b/tests/qtest/virtio-rng-test.c
@@ -20,6 +20,11 @@ static void rng_hotplug(void *obj, void *data, QGuestAllocator *alloc)
     QVirtioPCIDevice *dev = obj;
     QTestState *qts = dev->pdev->bus->qts;
 
+   if (dev->pdev->bus->not_hotpluggable) {
+        g_test_skip("bus pci.0 does not support hotplug");
+        return;
+    }
+
     const char *arch = qtest_get_arch();
 
     qtest_qmp_device_add(qts, "virtio-rng-pci", "rng1",
-- 
2.26.3

Re: [PATCH 3/6] tests/qtest/libqos: Skip hotplug tests if pci root bus is not hotpluggable

[Thomas Huth]

On 10/01/2022 22.19, Eric Auger wrote:
> ARM does not not support hotplug on pcie.0. Add a flag on the bus
> which tells if devices can be hotplugged and skip hotplug tests
> if the bus cannot be hotplugged. This is a temporary solution to
> enable the other pci tests on aarch64.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/qtest/e1000e-test.c         |  6 ++++++
>   tests/qtest/libqos/pci.h          |  1 +
>   tests/qtest/vhost-user-blk-test.c | 10 ++++++++++
>   tests/qtest/virtio-blk-test.c     |  5 +++++
>   tests/qtest/virtio-net-test.c     |  5 +++++
>   tests/qtest/virtio-rng-test.c     |  5 +++++
>   6 files changed, 32 insertions(+)
> 
> diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
> index 0273fe4c156..0d656d3af63 100644
> --- a/tests/qtest/e1000e-test.c
> +++ b/tests/qtest/e1000e-test.c
> @@ -235,6 +235,12 @@ static void test_e1000e_multiple_transfers(void *obj, void *data,
>   static void test_e1000e_hotplug(void *obj, void *data, QGuestAllocator * alloc)
>   {
>       QTestState *qts = global_qtest;  /* TODO: get rid of global_qtest here */
> +    QE1000E_PCI *dev = obj;
> +
> +    if (dev->pci_dev.bus->not_hotpluggable) {
> +        g_test_skip("bus pci.0 does not support hotplug");

I'd maybe rather say "pci bus does not support hotplug", i.e. avoid "pci.0" 
here, in case this is ever used for another port, too.

Apart from that:
Acked-by: Thomas Huth <thuth@redhat.com>


> +        return;
> +    }

[PATCH 4/6] tests/qtest/vhost-user-blk-test: Setup MSIx to avoid error on aarch64

[Eric Auger]

When run on ARM, basic and indirect tests currently fail with the
following error:

ERROR:../tests/qtest/libqos/virtio.c:224:qvirtio_wait_used_elem:
assertion failed (got_desc_idx == desc_idx): (50331648 == 0)
Bail out! ERROR:../tests/qtest/libqos/virtio.c:224: qvirtio_wait_used_elem:
assertion failed (got_desc_idx == desc_idx): (50331648 == 0)

Setting up and enabling MSIX fixes the issue.

Also remove the useless libqos/libqos-pc.h header inclusion.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/vhost-user-blk-test.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index cea2570884b..2024aa5923e 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -19,7 +19,6 @@
 #include "standard-headers/linux/virtio_pci.h"
 #include "libqos/qgraph.h"
 #include "libqos/vhost-user-blk.h"
-#include "libqos/libqos-pc.h"
 
 #define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
 #define QVIRTIO_BLK_TIMEOUT_US  (30 * 1000 * 1000)
@@ -224,6 +223,10 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
     char *data;
     QTestState *qts = global_qtest;
     QVirtQueue *vq;
+    QVirtioPCIDevice *vpcidev = container_of(dev, QVirtioPCIDevice, vdev);
+
+    qpci_msix_enable(vpcidev->pdev);
+    qvirtio_pci_set_msix_configuration_vector(vpcidev, alloc, 0);
 
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
@@ -236,9 +239,12 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     vq = qvirtqueue_setup(dev, alloc, 0);
+    qvirtqueue_pci_msix_setup(vpcidev, (QVirtQueuePCI *)vq, alloc, 1);
 
     qvirtio_set_driver_ok(dev);
 
+    qvirtio_wait_queue_isr(qts, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+
     /* Write and read with 3 descriptor layout */
     /* Write request */
     req.type = VIRTIO_BLK_T_OUT;
@@ -468,6 +474,10 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     uint8_t status;
     char *data;
     QTestState *qts = global_qtest;
+    QVirtioPCIDevice *vpcidev = container_of(dev, QVirtioPCIDevice, vdev);
+
+    qpci_msix_enable(vpcidev->pdev);
+    qvirtio_pci_set_msix_configuration_vector(vpcidev, t_alloc, 0);
 
     features = qvirtio_get_features(dev);
     g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
@@ -480,8 +490,12 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     vq = qvirtqueue_setup(dev, t_alloc, 0);
+    qvirtqueue_pci_msix_setup(vpcidev, (QVirtQueuePCI *)vq, t_alloc, 1);
+
     qvirtio_set_driver_ok(dev);
 
+    qvirtio_wait_queue_isr(qts, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+
     /* Write request */
     req.type = VIRTIO_BLK_T_OUT;
     req.ioprio = 1;
-- 
2.26.3

Re: [PATCH 4/6] tests/qtest/vhost-user-blk-test: Setup MSIx to avoid error on aarch64

[Thomas Huth]

On 10/01/2022 22.19, Eric Auger wrote:
> When run on ARM, basic and indirect tests currently fail with the
> following error:
> 
> ERROR:../tests/qtest/libqos/virtio.c:224:qvirtio_wait_used_elem:
> assertion failed (got_desc_idx == desc_idx): (50331648 == 0)
> Bail out! ERROR:../tests/qtest/libqos/virtio.c:224: qvirtio_wait_used_elem:
> assertion failed (got_desc_idx == desc_idx): (50331648 == 0)
> 
> Setting up and enabling MSIX fixes the issue.
> 
> Also remove the useless libqos/libqos-pc.h header inclusion.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/qtest/vhost-user-blk-test.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)

Acked-by: Thomas Huth <thuth@redhat.com>

[PATCH 5/6] tests/qtest/vhost-user-blk-test: Factorize vq setup code

[Eric Auger]

The vq setup code is repeated several times and can be
easily factorized.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/vhost-user-blk-test.c | 33 +++++++++++++++----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 2024aa5923e..236af80e4ad 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -211,6 +211,19 @@ static void test_invalid_discard_write_zeroes(QVirtioDevice *dev,
     guest_free(alloc, req_addr);
 }
 
+static QVirtQueue *setup_vq(QVirtioDevice *dev, QGuestAllocator *alloc)
+{
+    QVirtioPCIDevice *vpcidev = container_of(dev, QVirtioPCIDevice, vdev);
+    QVirtQueue *vq;
+
+    qpci_msix_enable(vpcidev->pdev);
+    qvirtio_pci_set_msix_configuration_vector(vpcidev, alloc, 0);
+
+    vq = qvirtqueue_setup(dev, alloc, 0);
+    qvirtqueue_pci_msix_setup(vpcidev, (QVirtQueuePCI *)vq, alloc, 1);
+    return vq;
+}
+
 /* Returns the request virtqueue so the caller can perform further tests */
 static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
 {
@@ -223,10 +236,6 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
     char *data;
     QTestState *qts = global_qtest;
     QVirtQueue *vq;
-    QVirtioPCIDevice *vpcidev = container_of(dev, QVirtioPCIDevice, vdev);
-
-    qpci_msix_enable(vpcidev->pdev);
-    qvirtio_pci_set_msix_configuration_vector(vpcidev, alloc, 0);
 
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
@@ -238,8 +247,7 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
     capacity = qvirtio_config_readq(dev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
-    vq = qvirtqueue_setup(dev, alloc, 0);
-    qvirtqueue_pci_msix_setup(vpcidev, (QVirtQueuePCI *)vq, alloc, 1);
+    vq = setup_vq(dev, alloc);
 
     qvirtio_set_driver_ok(dev);
 
@@ -474,10 +482,6 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     uint8_t status;
     char *data;
     QTestState *qts = global_qtest;
-    QVirtioPCIDevice *vpcidev = container_of(dev, QVirtioPCIDevice, vdev);
-
-    qpci_msix_enable(vpcidev->pdev);
-    qvirtio_pci_set_msix_configuration_vector(vpcidev, t_alloc, 0);
 
     features = qvirtio_get_features(dev);
     g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
@@ -489,8 +493,7 @@ static void indirect(void *obj, void *u_data, QGuestAllocator *t_alloc)
     capacity = qvirtio_config_readq(dev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
-    vq = qvirtqueue_setup(dev, t_alloc, 0);
-    qvirtqueue_pci_msix_setup(vpcidev, (QVirtQueuePCI *)vq, t_alloc, 1);
+    vq = setup_vq(dev, t_alloc);
 
     qvirtio_set_driver_ok(dev);
 
@@ -576,9 +579,6 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
         return;
     }
 
-    qpci_msix_enable(pdev->pdev);
-    qvirtio_pci_set_msix_configuration_vector(pdev, t_alloc, 0);
-
     features = qvirtio_get_features(dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             (1u << VIRTIO_RING_F_INDIRECT_DESC) |
@@ -589,8 +589,7 @@ static void idx(void *obj, void *u_data, QGuestAllocator *t_alloc)
     capacity = qvirtio_config_readq(dev, 0);
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
-    vq = qvirtqueue_setup(dev, t_alloc, 0);
-    qvirtqueue_pci_msix_setup(pdev, (QVirtQueuePCI *)vq, t_alloc, 1);
+    vq = setup_vq(dev, t_alloc);
 
     qvirtio_set_driver_ok(dev);
 
-- 
2.26.3

Re: [PATCH 5/6] tests/qtest/vhost-user-blk-test: Factorize vq setup code

[Thomas Huth]

On 10/01/2022 22.19, Eric Auger wrote:
> The vq setup code is repeated several times and can be
> easily factorized.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/qtest/vhost-user-blk-test.c | 33 +++++++++++++++----------------
>   1 file changed, 16 insertions(+), 17 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 6/6] tests/qtest/libqos: Add pci-arm and add a pci-arm producer in arm-virt machine

[Eric Auger]

Up to now the virt-machine node contains a virtio-mmio node.
However no driver produces any PCI interface node. Hence, PCI
tests cannot be run with aarch64 binary.

Add a GPEX driver node that produces a pci interface node. This latter
then can be consumed by all the pci tests. One of the first motivation
was to be able to run the virtio-iommu-pci tests.

We still face an issue with pci hotplug tests as hotplug cannot happen
on the pcie root bus and require a generic root port. This will be
addressed later on.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/libqos/arm-virt-machine.c |  47 +++++-
 tests/qtest/libqos/meson.build        |   3 +
 tests/qtest/libqos/pci-arm.c          | 219 ++++++++++++++++++++++++++
 tests/qtest/libqos/pci-arm.h          |  56 +++++++
 tests/qtest/libqos/pci.h              |   1 +
 tests/qtest/libqos/qgraph.c           |   7 +
 tests/qtest/libqos/qgraph.h           |  15 ++
 7 files changed, 344 insertions(+), 4 deletions(-)
 create mode 100644 tests/qtest/libqos/pci-arm.c
 create mode 100644 tests/qtest/libqos/pci-arm.h

diff --git a/tests/qtest/libqos/arm-virt-machine.c b/tests/qtest/libqos/arm-virt-machine.c
index e0f59322845..130c45c51e2 100644
--- a/tests/qtest/libqos/arm-virt-machine.c
+++ b/tests/qtest/libqos/arm-virt-machine.c
@@ -22,6 +22,8 @@
 #include "malloc.h"
 #include "qgraph.h"
 #include "virtio-mmio.h"
+#include "pci-arm.h"
+#include "hw/pci/pci_regs.h"
 
 #define ARM_PAGE_SIZE               4096
 #define VIRTIO_MMIO_BASE_ADDR       0x0A003E00
@@ -30,13 +32,40 @@
 #define VIRTIO_MMIO_SIZE            0x00000200
 
 typedef struct QVirtMachine QVirtMachine;
+typedef struct QGenericPCIHost QGenericPCIHost;
+
+struct QGenericPCIHost {
+    QOSGraphObject obj;
+    QPCIBusARM pci;
+};
 
 struct QVirtMachine {
     QOSGraphObject obj;
     QGuestAllocator alloc;
     QVirtioMMIODevice virtio_mmio;
+    QGenericPCIHost bridge;
 };
 
+/* QGenericPCIHost */
+
+static QOSGraphObject *generic_pcihost_get_device(void *obj, const char *device)
+{
+    QGenericPCIHost *host = obj;
+    if (!g_strcmp0(device, "pci-bus-arm")) {
+        return &host->pci.obj;
+    }
+    fprintf(stderr, "%s not present in generic-pcihost\n", device);
+    g_assert_not_reached();
+}
+
+static void qos_create_generic_pcihost(QGenericPCIHost *host,
+                                       QTestState *qts,
+                                       QGuestAllocator *alloc)
+{
+    host->obj.get_device = generic_pcihost_get_device;
+    qpci_init_arm(&host->pci, qts, alloc, false);
+}
+
 static void virt_destructor(QOSGraphObject *obj)
 {
     QVirtMachine *machine = (QVirtMachine *) obj;
@@ -57,11 +86,13 @@ static void *virt_get_driver(void *object, const char *interface)
 static QOSGraphObject *virt_get_device(void *obj, const char *device)
 {
     QVirtMachine *machine = obj;
-    if (!g_strcmp0(device, "virtio-mmio")) {
+    if (!g_strcmp0(device, "generic-pcihost")) {
+        return &machine->bridge.obj;
+    } else if (!g_strcmp0(device, "virtio-mmio")) {
         return &machine->virtio_mmio.obj;
     }
 
-    fprintf(stderr, "%s not present in arm/virtio\n", device);
+    fprintf(stderr, "%s not present in arm/virt\n", device);
     g_assert_not_reached();
 }
 
@@ -76,16 +107,24 @@ static void *qos_create_machine_arm_virt(QTestState *qts)
     qvirtio_mmio_init_device(&machine->virtio_mmio, qts, VIRTIO_MMIO_BASE_ADDR,
                              VIRTIO_MMIO_SIZE);
 
+    qos_create_generic_pcihost(&machine->bridge, qts, &machine->alloc);
+
     machine->obj.get_device = virt_get_device;
     machine->obj.get_driver = virt_get_driver;
     machine->obj.destructor = virt_destructor;
     return machine;
 }
 
-static void virtio_mmio_register_nodes(void)
+static void virt_machine_register_nodes(void)
 {
     qos_node_create_machine("arm/virt", qos_create_machine_arm_virt);
     qos_node_contains("arm/virt", "virtio-mmio", NULL);
+
+    qos_node_create_machine("aarch64/virt", qos_create_machine_arm_virt);
+    qos_node_contains("aarch64/virt", "generic-pcihost", NULL);
+
+    qos_node_create_driver("generic-pcihost", NULL);
+    qos_node_contains("generic-pcihost", "pci-bus-arm", NULL);
 }
 
-libqos_init(virtio_mmio_register_nodes);
+libqos_init(virt_machine_register_nodes);
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index e988d157917..af1bc723737 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -19,6 +19,9 @@ libqos_srcs = files('../libqtest.c',
         'libqos-pc.c',
         'ahci.c',
 
+        # arm
+        'pci-arm.c',
+
         # usb
         'usb.c',
 
diff --git a/tests/qtest/libqos/pci-arm.c b/tests/qtest/libqos/pci-arm.c
new file mode 100644
index 00000000000..64d826bb3c7
--- /dev/null
+++ b/tests/qtest/libqos/pci-arm.c
@@ -0,0 +1,219 @@
+/*
+ * libqos PCI bindings for ARM
+ *
+ * Copyright Red Hat Inc., 2021
+ *
+ * Authors:
+ *  Eric Auger   <eric.auger@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "pci-arm.h"
+#include "qapi/qmp/qdict.h"
+#include "hw/pci/pci_regs.h"
+
+#include "qemu/module.h"
+
+static uint8_t qpci_arm_pio_readb(QPCIBus *bus, uint32_t addr)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    return qtest_readb(bus->qts, s->gpex_pio_base + addr);
+}
+
+static void qpci_arm_pio_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    qtest_writeb(bus->qts, s->gpex_pio_base + addr,  val);
+}
+
+static uint16_t qpci_arm_pio_readw(QPCIBus *bus, uint32_t addr)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    return qtest_readw(bus->qts, s->gpex_pio_base + addr);
+}
+
+static void qpci_arm_pio_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    qtest_writew(bus->qts, s->gpex_pio_base + addr, val);
+}
+
+static uint32_t qpci_arm_pio_readl(QPCIBus *bus, uint32_t addr)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    return qtest_readl(bus->qts, s->gpex_pio_base + addr);
+}
+
+static void qpci_arm_pio_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    qtest_writel(bus->qts, s->gpex_pio_base + addr, val);
+}
+
+static uint64_t qpci_arm_pio_readq(QPCIBus *bus, uint32_t addr)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    return qtest_readq(bus->qts, s->gpex_pio_base + addr);
+}
+
+static void qpci_arm_pio_writeq(QPCIBus *bus, uint32_t addr, uint64_t val)
+{
+    QPCIBusARM *s = container_of(bus, QPCIBusARM, bus);
+
+    qtest_writeq(bus->qts, s->gpex_pio_base + addr, val);
+}
+
+static void qpci_arm_memread(QPCIBus *bus, uint32_t addr, void *buf, size_t len)
+{
+    qtest_memread(bus->qts, addr, buf, len);
+}
+
+static void qpci_arm_memwrite(QPCIBus *bus, uint32_t addr,
+                             const void *buf, size_t len)
+{
+    qtest_memwrite(bus->qts, addr, buf, len);
+}
+
+static uint8_t qpci_arm_config_readb(QPCIBus *bus, int devfn, uint8_t offset)
+{
+    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);
+    uint8_t val;
+
+    qtest_memread(bus->qts, addr, &val, 1);
+    return val;
+}
+
+static uint16_t qpci_arm_config_readw(QPCIBus *bus, int devfn, uint8_t offset)
+{
+    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);
+    uint16_t val;
+
+    qtest_memread(bus->qts, addr, &val, 2);
+    return val;
+}
+
+static uint32_t qpci_arm_config_readl(QPCIBus *bus, int devfn, uint8_t offset)
+{
+    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);
+    uint32_t val;
+
+    qtest_memread(bus->qts, addr, &val, 4);
+    return val;
+}
+
+static void
+qpci_arm_config_writeb(QPCIBus *bus, int devfn, uint8_t offset, uint8_t value)
+{
+    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);
+    uint32_t val = value;
+
+    qtest_memwrite(bus->qts, addr, &val, 1);
+}
+
+static void
+qpci_arm_config_writew(QPCIBus *bus, int devfn, uint8_t offset, uint16_t value)
+{
+    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);
+    uint32_t val = value;
+
+    qtest_memwrite(bus->qts, addr, &val, 2);
+}
+
+static void
+qpci_arm_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint32_t value)
+{
+    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);
+    uint32_t val = value;
+
+    qtest_memwrite(bus->qts, addr, &val, 4);
+}
+
+static void *qpci_arm_get_driver(void *obj, const char *interface)
+{
+    QPCIBusARM *qpci = obj;
+    if (!g_strcmp0(interface, "pci-bus")) {
+        return &qpci->bus;
+    }
+    fprintf(stderr, "%s not present in pci-bus-arm\n", interface);
+    g_assert_not_reached();
+}
+
+void qpci_init_arm(QPCIBusARM *qpci, QTestState *qts,
+                   QGuestAllocator *alloc, bool hotpluggable)
+{
+    assert(qts);
+
+    qpci->gpex_pio_base = 0x3eff0000;
+    qpci->bus.not_hotpluggable = !hotpluggable;
+    qpci->bus.has_buggy_msi = false;
+
+    qpci->bus.pio_readb = qpci_arm_pio_readb;
+    qpci->bus.pio_readw = qpci_arm_pio_readw;
+    qpci->bus.pio_readl = qpci_arm_pio_readl;
+    qpci->bus.pio_readq = qpci_arm_pio_readq;
+
+    qpci->bus.pio_writeb = qpci_arm_pio_writeb;
+    qpci->bus.pio_writew = qpci_arm_pio_writew;
+    qpci->bus.pio_writel = qpci_arm_pio_writel;
+    qpci->bus.pio_writeq = qpci_arm_pio_writeq;
+
+    qpci->bus.memread = qpci_arm_memread;
+    qpci->bus.memwrite = qpci_arm_memwrite;
+
+    qpci->bus.config_readb = qpci_arm_config_readb;
+    qpci->bus.config_readw = qpci_arm_config_readw;
+    qpci->bus.config_readl = qpci_arm_config_readl;
+
+    qpci->bus.config_writeb = qpci_arm_config_writeb;
+    qpci->bus.config_writew = qpci_arm_config_writew;
+    qpci->bus.config_writel = qpci_arm_config_writel;
+
+    qpci->bus.qts = qts;
+    qpci->bus.pio_alloc_ptr = 0x0000;
+    qpci->bus.pio_limit = 0x10000;
+    qpci->bus.mmio_alloc_ptr = 0x10000000;
+    qpci->bus.mmio_limit = 0x2eff0000;
+    qpci->bus.ecam_alloc_ptr = 0x4010000000;
+
+    qpci->obj.get_driver = qpci_arm_get_driver;
+}
+
+QPCIBus *qpci_new_arm(QTestState *qts, QGuestAllocator *alloc,
+                      bool hotpluggable)
+{
+    QPCIBusARM *qpci = g_new0(QPCIBusARM, 1);
+    qpci_init_arm(qpci, qts, alloc, hotpluggable);
+
+    return &qpci->bus;
+}
+
+void qpci_free_arm(QPCIBus *bus)
+{
+    QPCIBusARM *s;
+
+    if (!bus) {
+        return;
+    }
+    s = container_of(bus, QPCIBusARM, bus);
+
+    g_free(s);
+}
+
+static void qpci_arm_register_nodes(void)
+{
+    qos_node_create_driver("pci-bus-arm", NULL);
+    qos_node_produces_opts("pci-bus-arm", "pci-bus", NULL);
+}
+
+libqos_init(qpci_arm_register_nodes);
diff --git a/tests/qtest/libqos/pci-arm.h b/tests/qtest/libqos/pci-arm.h
new file mode 100644
index 00000000000..8cd49ec2969
--- /dev/null
+++ b/tests/qtest/libqos/pci-arm.h
@@ -0,0 +1,56 @@
+/*
+ * libqos PCI bindings for ARM
+ *
+ * Copyright Red Hat Inc., 2021
+ *
+ * Authors:
+ *  Eric Auger   <eric.auger@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_PCI_ARM_H
+#define LIBQOS_PCI_ARM_H
+
+#include "pci.h"
+#include "malloc.h"
+#include "qgraph.h"
+
+typedef struct QPCIBusARM {
+    QOSGraphObject obj;
+    QPCIBus bus;
+    uint64_t gpex_pio_base;
+} QPCIBusARM;
+
+/*
+ * qpci_init_arm():
+ * @ret: A valid QPCIBusARM * pointer
+ * @qts: The %QTestState for this ARM machine
+ * @alloc: A previously initialized @alloc providing memory for @qts
+ * @bool: devices can be hotplugged on this bus
+ *
+ * This function initializes an already allocated
+ * QPCIBusARM object.
+ */
+void qpci_init_arm(QPCIBusARM *ret, QTestState *qts,
+                   QGuestAllocator *alloc, bool hotpluggable);
+
+/*
+ * qpci_arm_new():
+ * @qts: The %QTestState for this ARM machine
+ * @alloc: A previously initialized @alloc providing memory for @qts
+ * @hotpluggable: the pci bus is hotpluggable
+ *
+ * This function creates a new QPCIBusARM object,
+ * and properly initialize its fields.
+ *
+ * Returns the QPCIBus *bus field of a newly
+ * allocated QPCIBusARM.
+ */
+QPCIBus *qpci_new_arm(QTestState *qts, QGuestAllocator *alloc,
+                      bool hotpluggable);
+
+void qpci_free_arm(QPCIBus *bus);
+
+#endif
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index 6a28b405221..07393dda4ec 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -51,6 +51,7 @@ struct QPCIBus {
     QTestState *qts;
     uint64_t pio_alloc_ptr, pio_limit;
     uint64_t mmio_alloc_ptr, mmio_limit;
+    uint64_t ecam_alloc_ptr;
     bool has_buggy_msi; /* TRUE for spapr, FALSE for pci */
     bool not_hotpluggable; /* TRUE if devices cannot be hotplugged */
 
diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c
index 109ff04e1e8..e03fad35241 100644
--- a/tests/qtest/libqos/qgraph.c
+++ b/tests/qtest/libqos/qgraph.c
@@ -667,6 +667,13 @@ void qos_node_produces(const char *producer, const char *interface)
     add_edge(producer, interface, QEDGE_PRODUCES, NULL);
 }
 
+void qos_node_produces_opts(const char *producer, const char *interface,
+                            QOSGraphEdgeOptions *opts)
+{
+    create_interface(interface);
+    add_edge(producer, interface, QEDGE_PRODUCES, opts);
+}
+
 void qos_node_consumes(const char *consumer, const char *interface,
                        QOSGraphEdgeOptions *opts)
 {
diff --git a/tests/qtest/libqos/qgraph.h b/tests/qtest/libqos/qgraph.h
index 871740c0dc8..9879af118d4 100644
--- a/tests/qtest/libqos/qgraph.h
+++ b/tests/qtest/libqos/qgraph.h
@@ -284,6 +284,21 @@ void qos_node_contains(const char *container, const char *contained,
  */
 void qos_node_produces(const char *producer, const char *interface);
 
+/**
+ * qos_node_produces(): creates an edge of type QEDGE_PRODUCES and
+ * adds it to the edge list mapped to @producer in the
+ * edge hash table.
+ * @producer: Source node that "produces"
+ * @interface: Interface node that "is produced"
+ * @ops: Facultative options (see %QOSGraphEdgeOptions
+ *
+ * This edge will have @producer as source and @interface as destination.
+ * It also has the possibility to add an optional @opts
+ * (see %QOSGraphEdgeOptions)
+ */
+void qos_node_produces_opts(const char *producer, const char *interface,
+                            QOSGraphEdgeOptions *opts);
+
 /**
  * qos_node_consumes():  creates an edge of type QEDGE_CONSUMED_BY and
  * adds it to the edge list mapped to @interface in the
-- 
2.26.3

Re: [PATCH 6/6] tests/qtest/libqos: Add pci-arm and add a pci-arm producer in arm-virt machine

[Thomas Huth]

On 10/01/2022 22.19, Eric Auger wrote:
> Up to now the virt-machine node contains a virtio-mmio node.
> However no driver produces any PCI interface node. Hence, PCI
> tests cannot be run with aarch64 binary.
> 
> Add a GPEX driver node that produces a pci interface node. This latter
> then can be consumed by all the pci tests. One of the first motivation
> was to be able to run the virtio-iommu-pci tests.
> 
> We still face an issue with pci hotplug tests as hotplug cannot happen
> on the pcie root bus and require a generic root port. This will be
> addressed later on.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/qtest/libqos/arm-virt-machine.c |  47 +++++-
>   tests/qtest/libqos/meson.build        |   3 +
>   tests/qtest/libqos/pci-arm.c          | 219 ++++++++++++++++++++++++++
>   tests/qtest/libqos/pci-arm.h          |  56 +++++++
>   tests/qtest/libqos/pci.h              |   1 +
>   tests/qtest/libqos/qgraph.c           |   7 +
>   tests/qtest/libqos/qgraph.h           |  15 ++
>   7 files changed, 344 insertions(+), 4 deletions(-)
>   create mode 100644 tests/qtest/libqos/pci-arm.c
>   create mode 100644 tests/qtest/libqos/pci-arm.h
[...]
> diff --git a/tests/qtest/libqos/pci-arm.c b/tests/qtest/libqos/pci-arm.c
> new file mode 100644
> index 00000000000..64d826bb3c7
> --- /dev/null
> +++ b/tests/qtest/libqos/pci-arm.c
> @@ -0,0 +1,219 @@
> +/*
> + * libqos PCI bindings for ARM
> + *
> + * Copyright Red Hat Inc., 2021

You might want to update to 2022 now.

Apart from that:
Acked-by: Thomas Huth <thuth@redhat.com>

Let me know if I should take this through my qtest branch - otherwise I'll 
assume it will go via Peter's arm branch.

  Thomas

Re: [PATCH 6/6] tests/qtest/libqos: Add pci-arm and add a pci-arm producer in arm-virt machine

[Paolo Bonzini]

On 1/10/22 22:19, Eric Auger wrote:
> Up to now the virt-machine node contains a virtio-mmio node.
> However no driver produces any PCI interface node. Hence, PCI
> tests cannot be run with aarch64 binary.
> 
> Add a GPEX driver node that produces a pci interface node. This latter
> then can be consumed by all the pci tests. One of the first motivation
> was to be able to run the virtio-iommu-pci tests.
> 
> We still face an issue with pci hotplug tests as hotplug cannot happen
> on the pcie root bus and require a generic root port. This will be
> addressed later on.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Hey Eric,

it's great to have gpex support in libqos/qgraph!  On the next versions 
you might also Cc Emanuele since he was the author of the framework.

> ---
>   tests/qtest/libqos/arm-virt-machine.c |  47 +++++-
>   tests/qtest/libqos/meson.build        |   3 +
>   tests/qtest/libqos/pci-arm.c          | 219 ++++++++++++++++++++++++++
>   tests/qtest/libqos/pci-arm.h          |  56 +++++++
>   tests/qtest/libqos/pci.h              |   1 +
>   tests/qtest/libqos/qgraph.c           |   7 +
>   tests/qtest/libqos/qgraph.h           |  15 ++
>   7 files changed, 344 insertions(+), 4 deletions(-)
>   create mode 100644 tests/qtest/libqos/pci-arm.c
>   create mode 100644 tests/qtest/libqos/pci-arm.h
> 
> diff --git a/tests/qtest/libqos/arm-virt-machine.c b/tests/qtest/libqos/arm-virt-machine.c
> index e0f59322845..130c45c51e2 100644
> --- a/tests/qtest/libqos/arm-virt-machine.c
> +++ b/tests/qtest/libqos/arm-virt-machine.c
> @@ -22,6 +22,8 @@
>   #include "malloc.h"
>   #include "qgraph.h"
>   #include "virtio-mmio.h"
> +#include "pci-arm.h"
> +#include "hw/pci/pci_regs.h"
>   
>   #define ARM_PAGE_SIZE               4096
>   #define VIRTIO_MMIO_BASE_ADDR       0x0A003E00
> @@ -30,13 +32,40 @@
>   #define VIRTIO_MMIO_SIZE            0x00000200
>   
>   typedef struct QVirtMachine QVirtMachine;
> +typedef struct QGenericPCIHost QGenericPCIHost;
> +
> +struct QGenericPCIHost {
> +    QOSGraphObject obj;
> +    QPCIBusARM pci;
> +};

You can rename QPCIBusARM to QGenericPCIBus and move QGenericPCIHost to 
the same file.  There's nothing ARM specific in either file, and nothing 
specific to -M virt in QGenericPCIHost.

>   struct QVirtMachine {
>       QOSGraphObject obj;
>       QGuestAllocator alloc;
>       QVirtioMMIODevice virtio_mmio;
> +    QGenericPCIHost bridge;
>   };
>   
> +/* QGenericPCIHost */
> +
> +static QOSGraphObject *generic_pcihost_get_device(void *obj, const char *device)
> +{
> +    QGenericPCIHost *host = obj;
> +    if (!g_strcmp0(device, "pci-bus-arm")) {
> +        return &host->pci.obj;
> +    }
> +    fprintf(stderr, "%s not present in generic-pcihost\n", device);
> +    g_assert_not_reached();
> +}
> +
> +static void qos_create_generic_pcihost(QGenericPCIHost *host,
> +                                       QTestState *qts,
> +                                       QGuestAllocator *alloc)
> +{
> +    host->obj.get_device = generic_pcihost_get_device;
> +    qpci_init_arm(&host->pci, qts, alloc, false);
> +}
> +
>   static void virt_destructor(QOSGraphObject *obj)
>   {
>       QVirtMachine *machine = (QVirtMachine *) obj;

This should also be in the same file as the bus implementation.

> +    qos_node_create_driver("generic-pcihost", NULL);
> +    qos_node_contains("generic-pcihost", "pci-bus-arm", NULL);

This too, with a new libqos_init.


> +static uint8_t qpci_arm_config_readb(QPCIBus *bus, int devfn, uint8_t offset)
> +{
> +    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12) | offset);

ecam_alloc_ptr should be in QPCIBusARM (to be renamed to 
QGenericPCIBus), which you can retrieve from the "bus" QPCIBus* via 
container_of.

> diff --git a/tests/qtest/libqos/pci-arm.h b/tests/qtest/libqos/pci-arm.h
> new file mode 100644
> index 00000000000..8cd49ec2969
> --- /dev/null
> +++ b/tests/qtest/libqos/pci-arm.h
> @@ -0,0 +1,56 @@
> +/*
> + * libqos PCI bindings for ARM
> + *
> + * Copyright Red Hat Inc., 2021
> + *
> + * Authors:
> + *  Eric Auger   <eric.auger@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_PCI_ARM_H
> +#define LIBQOS_PCI_ARM_H
> +
> +#include "pci.h"
> +#include "malloc.h"
> +#include "qgraph.h"
> +
> +typedef struct QPCIBusARM {
> +    QOSGraphObject obj;
> +    QPCIBus bus;
> +    uint64_t gpex_pio_base;
> +} QPCIBusARM;
> +
> +/*
> + * qpci_init_arm():
> + * @ret: A valid QPCIBusARM * pointer
> + * @qts: The %QTestState for this ARM machine
> + * @alloc: A previously initialized @alloc providing memory for @qts
> + * @bool: devices can be hotplugged on this bus
> + *
> + * This function initializes an already allocated
> + * QPCIBusARM object.
> + */
> +void qpci_init_arm(QPCIBusARM *ret, QTestState *qts,
> +                   QGuestAllocator *alloc, bool hotpluggable);
> +
> +/*
> + * qpci_arm_new():
> + * @qts: The %QTestState for this ARM machine
> + * @alloc: A previously initialized @alloc providing memory for @qts
> + * @hotpluggable: the pci bus is hotpluggable
> + *
> + * This function creates a new QPCIBusARM object,
> + * and properly initialize its fields.
> + *
> + * Returns the QPCIBus *bus field of a newly
> + * allocated QPCIBusARM.
> + */
> +QPCIBus *qpci_new_arm(QTestState *qts, QGuestAllocator *alloc,
> +                      bool hotpluggable);
> +
> +void qpci_free_arm(QPCIBus *bus);

These two functions are not needed now.  I'm not opposing non-qgraph 
tests that use the gpex device, but the functions should be introduced 
together with their users.

> diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c
> index 109ff04e1e8..e03fad35241 100644
> --- a/tests/qtest/libqos/qgraph.c
> +++ b/tests/qtest/libqos/qgraph.c
> @@ -667,6 +667,13 @@ void qos_node_produces(const char *producer, const char *interface)
>       add_edge(producer, interface, QEDGE_PRODUCES, NULL);
>   }
>   
> +void qos_node_produces_opts(const char *producer, const char *interface,
> +                            QOSGraphEdgeOptions *opts)
> +{
> +    create_interface(interface);
> +    add_edge(producer, interface, QEDGE_PRODUCES, opts);
> +}
> +

This is not needed, I think.

Paolo

Re: [PATCH 6/6] tests/qtest/libqos: Add pci-arm and add a pci-arm producer in arm-virt machine

[Eric Auger]

Hi Paolo,

On 1/15/22 5:01 PM, Paolo Bonzini wrote:
> On 1/10/22 22:19, Eric Auger wrote:
>> Up to now the virt-machine node contains a virtio-mmio node.
>> However no driver produces any PCI interface node. Hence, PCI
>> tests cannot be run with aarch64 binary.
>>
>> Add a GPEX driver node that produces a pci interface node. This latter
>> then can be consumed by all the pci tests. One of the first motivation
>> was to be able to run the virtio-iommu-pci tests.
>>
>> We still face an issue with pci hotplug tests as hotplug cannot happen
>> on the pcie root bus and require a generic root port. This will be
>> addressed later on.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> Hey Eric,
>
> it's great to have gpex support in libqos/qgraph!  On the next
> versions you might also Cc Emanuele since he was the author of the
> framework.
sure!
>
>> ---
>>   tests/qtest/libqos/arm-virt-machine.c |  47 +++++-
>>   tests/qtest/libqos/meson.build        |   3 +
>>   tests/qtest/libqos/pci-arm.c          | 219 ++++++++++++++++++++++++++
>>   tests/qtest/libqos/pci-arm.h          |  56 +++++++
>>   tests/qtest/libqos/pci.h              |   1 +
>>   tests/qtest/libqos/qgraph.c           |   7 +
>>   tests/qtest/libqos/qgraph.h           |  15 ++
>>   7 files changed, 344 insertions(+), 4 deletions(-)
>>   create mode 100644 tests/qtest/libqos/pci-arm.c
>>   create mode 100644 tests/qtest/libqos/pci-arm.h
>>
>> diff --git a/tests/qtest/libqos/arm-virt-machine.c
>> b/tests/qtest/libqos/arm-virt-machine.c
>> index e0f59322845..130c45c51e2 100644
>> --- a/tests/qtest/libqos/arm-virt-machine.c
>> +++ b/tests/qtest/libqos/arm-virt-machine.c
>> @@ -22,6 +22,8 @@
>>   #include "malloc.h"
>>   #include "qgraph.h"
>>   #include "virtio-mmio.h"
>> +#include "pci-arm.h"
>> +#include "hw/pci/pci_regs.h"
>>     #define ARM_PAGE_SIZE               4096
>>   #define VIRTIO_MMIO_BASE_ADDR       0x0A003E00
>> @@ -30,13 +32,40 @@
>>   #define VIRTIO_MMIO_SIZE            0x00000200
>>     typedef struct QVirtMachine QVirtMachine;
>> +typedef struct QGenericPCIHost QGenericPCIHost;
>> +
>> +struct QGenericPCIHost {
>> +    QOSGraphObject obj;
>> +    QPCIBusARM pci;
>> +};
>
> You can rename QPCIBusARM to QGenericPCIBus and move QGenericPCIHost
> to the same file.  There's nothing ARM specific in either file, and
> nothing specific to -M virt in QGenericPCIHost.

done
>
>>   struct QVirtMachine {
>>       QOSGraphObject obj;
>>       QGuestAllocator alloc;
>>       QVirtioMMIODevice virtio_mmio;
>> +    QGenericPCIHost bridge;
>>   };
>>   +/* QGenericPCIHost */
>> +
>> +static QOSGraphObject *generic_pcihost_get_device(void *obj, const
>> char *device)
>> +{
>> +    QGenericPCIHost *host = obj;
>> +    if (!g_strcmp0(device, "pci-bus-arm")) {
>> +        return &host->pci.obj;
>> +    }
>> +    fprintf(stderr, "%s not present in generic-pcihost\n", device);
>> +    g_assert_not_reached();
>> +}
>> +
>> +static void qos_create_generic_pcihost(QGenericPCIHost *host,
>> +                                       QTestState *qts,
>> +                                       QGuestAllocator *alloc)
>> +{
>> +    host->obj.get_device = generic_pcihost_get_device;
>> +    qpci_init_arm(&host->pci, qts, alloc, false);
>> +}
>> +
>>   static void virt_destructor(QOSGraphObject *obj)
>>   {
>>       QVirtMachine *machine = (QVirtMachine *) obj;
>
> This should also be in the same file as the bus implementation.
done
>
>> +    qos_node_create_driver("generic-pcihost", NULL);
>> +    qos_node_contains("generic-pcihost", "pci-bus-arm", NULL);
>
> This too, with a new libqos_init.
done
>
>
>> +static uint8_t qpci_arm_config_readb(QPCIBus *bus, int devfn,
>> uint8_t offset)
>> +{
>> +    uint64_t addr = bus->ecam_alloc_ptr + ((0 << 20) | (devfn << 12)
>> | offset);
>
> ecam_alloc_ptr should be in QPCIBusARM (to be renamed to
> QGenericPCIBus), which you can retrieve from the "bus" QPCIBus* via
> container_of.
done
>
>> diff --git a/tests/qtest/libqos/pci-arm.h b/tests/qtest/libqos/pci-arm.h
>> new file mode 100644
>> index 00000000000..8cd49ec2969
>> --- /dev/null
>> +++ b/tests/qtest/libqos/pci-arm.h
>> @@ -0,0 +1,56 @@
>> +/*
>> + * libqos PCI bindings for ARM
>> + *
>> + * Copyright Red Hat Inc., 2021
>> + *
>> + * Authors:
>> + *  Eric Auger   <eric.auger@redhat.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2
>> or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef LIBQOS_PCI_ARM_H
>> +#define LIBQOS_PCI_ARM_H
>> +
>> +#include "pci.h"
>> +#include "malloc.h"
>> +#include "qgraph.h"
>> +
>> +typedef struct QPCIBusARM {
>> +    QOSGraphObject obj;
>> +    QPCIBus bus;
>> +    uint64_t gpex_pio_base;
>> +} QPCIBusARM;
>> +
>> +/*
>> + * qpci_init_arm():
>> + * @ret: A valid QPCIBusARM * pointer
>> + * @qts: The %QTestState for this ARM machine
>> + * @alloc: A previously initialized @alloc providing memory for @qts
>> + * @bool: devices can be hotplugged on this bus
>> + *
>> + * This function initializes an already allocated
>> + * QPCIBusARM object.
>> + */
>> +void qpci_init_arm(QPCIBusARM *ret, QTestState *qts,
>> +                   QGuestAllocator *alloc, bool hotpluggable);
>> +
>> +/*
>> + * qpci_arm_new():
>> + * @qts: The %QTestState for this ARM machine
>> + * @alloc: A previously initialized @alloc providing memory for @qts
>> + * @hotpluggable: the pci bus is hotpluggable
>> + *
>> + * This function creates a new QPCIBusARM object,
>> + * and properly initialize its fields.
>> + *
>> + * Returns the QPCIBus *bus field of a newly
>> + * allocated QPCIBusARM.
>> + */
>> +QPCIBus *qpci_new_arm(QTestState *qts, QGuestAllocator *alloc,
>> +                      bool hotpluggable);
>> +
>> +void qpci_free_arm(QPCIBus *bus);
>
> These two functions are not needed now.  I'm not opposing non-qgraph
> tests that use the gpex device, but the functions should be introduced
> together with their users.
removed
>
>> diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c
>> index 109ff04e1e8..e03fad35241 100644
>> --- a/tests/qtest/libqos/qgraph.c
>> +++ b/tests/qtest/libqos/qgraph.c
>> @@ -667,6 +667,13 @@ void qos_node_produces(const char *producer,
>> const char *interface)
>>       add_edge(producer, interface, QEDGE_PRODUCES, NULL);
>>   }
>>   +void qos_node_produces_opts(const char *producer, const char
>> *interface,
>> +                            QOSGraphEdgeOptions *opts)
>> +{
>> +    create_interface(interface);
>> +    add_edge(producer, interface, QEDGE_PRODUCES, opts);
>> +}
>> +
>
> This is not needed, I think.
indeed, removed.

So you should see the above comments taken into account in just posted v2.

Thank you for the review!

Eric
>
> Paolo
>