All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] virtio-iommu: config related fixes and qtest
@ 2021-11-27  7:29 Eric Auger
  2021-11-27  7:29 ` [PATCH v6 1/4] virtio-iommu: Remove set_config callback Eric Auger
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Eric Auger @ 2021-11-27  7:29 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, thuth, qemu-arm, qemu-devel,
	jean-philippe, peter.maydell
  Cc: lvivier, pbonzini

Introduce a qtest for the virtio-iommu device. The test
allowed to identify an endianess bug in the get_config().
We also remove the unneeded set_config() and fix the value
for domain_range.end field.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/virtio-iommu-test-v6

History:
v5 -> v6:
- added patches 1-3
- qtest: fix domain_range.end expected value

Eric Auger (4):
  virtio-iommu: Remove set_config callback
  virtio-iommu: Fix endianness in get_config
  virtio-iommu: Fix the domain_range end
  tests: qtest: Add virtio-iommu test

 hw/virtio/trace-events            |   3 +-
 hw/virtio/virtio-iommu.c          |  38 ++--
 tests/qtest/libqos/meson.build    |   1 +
 tests/qtest/libqos/virtio-iommu.c | 126 ++++++++++++
 tests/qtest/libqos/virtio-iommu.h |  40 ++++
 tests/qtest/meson.build           |   1 +
 tests/qtest/virtio-iommu-test.c   | 326 ++++++++++++++++++++++++++++++
 7 files changed, 511 insertions(+), 24 deletions(-)
 create mode 100644 tests/qtest/libqos/virtio-iommu.c
 create mode 100644 tests/qtest/libqos/virtio-iommu.h
 create mode 100644 tests/qtest/virtio-iommu-test.c

-- 
2.26.3



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 1/4] virtio-iommu: Remove set_config callback
  2021-11-27  7:29 [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Eric Auger
@ 2021-11-27  7:29 ` Eric Auger
  2021-11-27  7:50   ` Eric Auger
  2021-11-29 19:04   ` Jean-Philippe Brucker
  2021-11-27  7:29 ` [PATCH v6 2/4] virtio-iommu: Fix endianness in get_config Eric Auger
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Eric Auger @ 2021-11-27  7:29 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, thuth, qemu-arm, qemu-devel,
	jean-philippe, peter.maydell
  Cc: lvivier, pbonzini

The spec says "the driver must not write to device configuration
fields". So remove the set_config() callback which anyway did
not do anything.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/virtio/trace-events   |  1 -
 hw/virtio/virtio-iommu.c | 14 --------------
 2 files changed, 15 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 650e521e351..54bd7da00c8 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -92,7 +92,6 @@ virtio_iommu_device_reset(void) "reset!"
 virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
 virtio_iommu_device_status(uint8_t status) "driver status = %d"
 virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
-virtio_iommu_set_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
 virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
 virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
 virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1b23e8e18c7..645c0aa3997 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -832,19 +832,6 @@ static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
     memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
 }
 
-static void virtio_iommu_set_config(VirtIODevice *vdev,
-                                      const uint8_t *config_data)
-{
-    struct virtio_iommu_config config;
-
-    memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
-    trace_virtio_iommu_set_config(config.page_size_mask,
-                                  config.input_range.start,
-                                  config.input_range.end,
-                                  config.domain_range.end,
-                                  config.probe_size);
-}
-
 static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
                                           Error **errp)
 {
@@ -1185,7 +1172,6 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
     vdc->unrealize = virtio_iommu_device_unrealize;
     vdc->reset = virtio_iommu_device_reset;
     vdc->get_config = virtio_iommu_get_config;
-    vdc->set_config = virtio_iommu_set_config;
     vdc->get_features = virtio_iommu_get_features;
     vdc->set_status = virtio_iommu_set_status;
     vdc->vmsd = &vmstate_virtio_iommu_device;
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 2/4] virtio-iommu: Fix endianness in get_config
  2021-11-27  7:29 [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Eric Auger
  2021-11-27  7:29 ` [PATCH v6 1/4] virtio-iommu: Remove set_config callback Eric Auger
@ 2021-11-27  7:29 ` Eric Auger
  2021-11-29 19:10   ` Jean-Philippe Brucker
  2021-11-27  7:29 ` [PATCH v6 3/4] virtio-iommu: Fix the domain_range end Eric Auger
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Eric Auger @ 2021-11-27  7:29 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, thuth, qemu-arm, qemu-devel,
	jean-philippe, peter.maydell
  Cc: lvivier, pbonzini

Endianess is not properly handled when populating
the returned config. Use the cpu_to_le* primitives
for each separate field. Also, while at it, trace
the domain range start.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
---
 hw/virtio/trace-events   |  2 +-
 hw/virtio/virtio-iommu.c | 22 +++++++++++++++-------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 54bd7da00c8..f7ad6be5fbb 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -91,7 +91,7 @@ virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
 virtio_iommu_device_reset(void) "reset!"
 virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
 virtio_iommu_device_status(uint8_t status) "driver status = %d"
-virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d domain range end=%d probe_size=0x%x"
 virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
 virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
 virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 645c0aa3997..30ee09187b8 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -822,14 +822,22 @@ unlock:
 static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
-    struct virtio_iommu_config *config = &dev->config;
+    struct virtio_iommu_config *dev_config = &dev->config;
+    struct virtio_iommu_config *out_config = (void *)config_data;
 
-    trace_virtio_iommu_get_config(config->page_size_mask,
-                                  config->input_range.start,
-                                  config->input_range.end,
-                                  config->domain_range.end,
-                                  config->probe_size);
-    memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
+    out_config->page_size_mask = cpu_to_le64(dev_config->page_size_mask);
+    out_config->input_range.start = cpu_to_le64(dev_config->input_range.start);
+    out_config->input_range.end = cpu_to_le64(dev_config->input_range.end);
+    out_config->domain_range.start = cpu_to_le32(dev_config->domain_range.start);
+    out_config->domain_range.end = cpu_to_le32(dev_config->domain_range.end);
+    out_config->probe_size = cpu_to_le32(dev_config->probe_size);
+
+    trace_virtio_iommu_get_config(dev_config->page_size_mask,
+                                  dev_config->input_range.start,
+                                  dev_config->input_range.end,
+                                  dev_config->domain_range.start,
+                                  dev_config->domain_range.end,
+                                  dev_config->probe_size);
 }
 
 static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 3/4] virtio-iommu: Fix the domain_range end
  2021-11-27  7:29 [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Eric Auger
  2021-11-27  7:29 ` [PATCH v6 1/4] virtio-iommu: Remove set_config callback Eric Auger
  2021-11-27  7:29 ` [PATCH v6 2/4] virtio-iommu: Fix endianness in get_config Eric Auger
@ 2021-11-27  7:29 ` Eric Auger
  2021-11-29 19:12   ` Jean-Philippe Brucker
  2021-11-27  7:29 ` [PATCH v6 4/4] tests: qtest: Add virtio-iommu test Eric Auger
  2021-12-13 10:02 ` [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Thomas Huth
  4 siblings, 1 reply; 11+ messages in thread
From: Eric Auger @ 2021-11-27  7:29 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, thuth, qemu-arm, qemu-devel,
	jean-philippe, peter.maydell
  Cc: lvivier, pbonzini

in old times the domain range was defined by a domain_bits le32.
This was then converted into a domain_range struct. During the
upgrade the original value of '32' (bits) has been kept while
the end field now is the max value of the domain id (UINT32_MAX).
Fix that and also use UINT64_MAX for the input_range.end.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/virtio/virtio-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 30ee09187b8..aa9c16a17b1 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -978,8 +978,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
     s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
 
     s->config.page_size_mask = TARGET_PAGE_MASK;
-    s->config.input_range.end = -1UL;
-    s->config.domain_range.end = 32;
+    s->config.input_range.end = UINT64_MAX;
+    s->config.domain_range.end = UINT32_MAX;
     s->config.probe_size = VIOMMU_PROBE_SIZE;
 
     virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 4/4] tests: qtest: Add virtio-iommu test
  2021-11-27  7:29 [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Eric Auger
                   ` (2 preceding siblings ...)
  2021-11-27  7:29 ` [PATCH v6 3/4] virtio-iommu: Fix the domain_range end Eric Auger
@ 2021-11-27  7:29 ` Eric Auger
  2021-12-08 13:49   ` Thomas Huth
  2021-12-13 10:02 ` [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Thomas Huth
  4 siblings, 1 reply; 11+ messages in thread
From: Eric Auger @ 2021-11-27  7:29 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, thuth, qemu-arm, qemu-devel,
	jean-philippe, peter.maydell
  Cc: lvivier, pbonzini

Add the framework to test the virtio-iommu-pci device
and tests exercising the attach/detach, map/unmap API.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

---

v5 -> v6:
- changed the expected value for domain.end (32 -> MAX_UINT32)
---
 tests/qtest/libqos/meson.build    |   1 +
 tests/qtest/libqos/virtio-iommu.c | 126 ++++++++++++
 tests/qtest/libqos/virtio-iommu.h |  40 ++++
 tests/qtest/meson.build           |   1 +
 tests/qtest/virtio-iommu-test.c   | 326 ++++++++++++++++++++++++++++++
 5 files changed, 494 insertions(+)
 create mode 100644 tests/qtest/libqos/virtio-iommu.c
 create mode 100644 tests/qtest/libqos/virtio-iommu.h
 create mode 100644 tests/qtest/virtio-iommu-test.c

diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 4af1f04787c..e988d157917 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -41,6 +41,7 @@ libqos_srcs = files('../libqtest.c',
         'virtio-rng.c',
         'virtio-scsi.c',
         'virtio-serial.c',
+        'virtio-iommu.c',
 
         # qgraph machines:
         'aarch64-xlnx-zcu102-machine.c',
diff --git a/tests/qtest/libqos/virtio-iommu.c b/tests/qtest/libqos/virtio-iommu.c
new file mode 100644
index 00000000000..18cba4ca36b
--- /dev/null
+++ b/tests/qtest/libqos/virtio-iommu.c
@@ -0,0 +1,126 @@
+/*
+ * libqos driver virtio-iommu-pci framework
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * Authors:
+ *  Eric Auger <eric.auger@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version.  See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "qgraph.h"
+#include "virtio-iommu.h"
+#include "hw/virtio/virtio-iommu.h"
+
+static QGuestAllocator *alloc;
+
+/* virtio-iommu-device */
+static void *qvirtio_iommu_get_driver(QVirtioIOMMU *v_iommu,
+                                      const char *interface)
+{
+    if (!g_strcmp0(interface, "virtio-iommu")) {
+        return v_iommu;
+    }
+    if (!g_strcmp0(interface, "virtio")) {
+        return v_iommu->vdev;
+    }
+
+    fprintf(stderr, "%s not present in virtio-iommu-device\n", interface);
+    g_assert_not_reached();
+}
+
+static void virtio_iommu_cleanup(QVirtioIOMMU *interface)
+{
+    qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);
+}
+
+static void virtio_iommu_setup(QVirtioIOMMU *interface)
+{
+    QVirtioDevice *vdev = interface->vdev;
+    uint64_t features;
+
+    features = qvirtio_get_features(vdev);
+    features &= ~(QVIRTIO_F_BAD_FEATURE |
+                  (1ull << VIRTIO_RING_F_INDIRECT_DESC) |
+                  (1ull << VIRTIO_RING_F_EVENT_IDX) |
+                  (1ull << VIRTIO_IOMMU_F_BYPASS));
+    qvirtio_set_features(vdev, features);
+    interface->vq = qvirtqueue_setup(interface->vdev, alloc, 0);
+    qvirtio_set_driver_ok(interface->vdev);
+}
+
+/* virtio-iommu-pci */
+static void *qvirtio_iommu_pci_get_driver(void *object, const char *interface)
+{
+    QVirtioIOMMUPCI *v_iommu = object;
+    if (!g_strcmp0(interface, "pci-device")) {
+        return v_iommu->pci_vdev.pdev;
+    }
+    return qvirtio_iommu_get_driver(&v_iommu->iommu, interface);
+}
+
+static void qvirtio_iommu_pci_destructor(QOSGraphObject *obj)
+{
+    QVirtioIOMMUPCI *iommu_pci = (QVirtioIOMMUPCI *) obj;
+    QVirtioIOMMU *interface = &iommu_pci->iommu;
+    QOSGraphObject *pci_vobj =  &iommu_pci->pci_vdev.obj;
+
+    virtio_iommu_cleanup(interface);
+    qvirtio_pci_destructor(pci_vobj);
+}
+
+static void qvirtio_iommu_pci_start_hw(QOSGraphObject *obj)
+{
+    QVirtioIOMMUPCI *iommu_pci = (QVirtioIOMMUPCI *) obj;
+    QVirtioIOMMU *interface = &iommu_pci->iommu;
+    QOSGraphObject *pci_vobj =  &iommu_pci->pci_vdev.obj;
+
+    qvirtio_pci_start_hw(pci_vobj);
+    virtio_iommu_setup(interface);
+}
+
+
+static void *virtio_iommu_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
+                                   void *addr)
+{
+    QVirtioIOMMUPCI *virtio_rpci = g_new0(QVirtioIOMMUPCI, 1);
+    QVirtioIOMMU *interface = &virtio_rpci->iommu;
+    QOSGraphObject *obj = &virtio_rpci->pci_vdev.obj;
+
+    virtio_pci_init(&virtio_rpci->pci_vdev, pci_bus, addr);
+    interface->vdev = &virtio_rpci->pci_vdev.vdev;
+    alloc = t_alloc;
+
+    obj->get_driver = qvirtio_iommu_pci_get_driver;
+    obj->start_hw = qvirtio_iommu_pci_start_hw;
+    obj->destructor = qvirtio_iommu_pci_destructor;
+
+    return obj;
+}
+
+static void virtio_iommu_register_nodes(void)
+{
+    QPCIAddress addr = {
+        .devfn = QPCI_DEVFN(4, 0),
+    };
+
+    QOSGraphEdgeOptions opts = {
+        .extra_device_opts = "addr=04.0",
+    };
+
+    /* virtio-iommu-pci */
+    add_qpci_address(&opts, &addr);
+    qos_node_create_driver("virtio-iommu-pci", virtio_iommu_pci_create);
+    qos_node_consumes("virtio-iommu-pci", "pci-bus", &opts);
+    qos_node_produces("virtio-iommu-pci", "pci-device");
+    qos_node_produces("virtio-iommu-pci", "virtio");
+    qos_node_produces("virtio-iommu-pci", "virtio-iommu");
+}
+
+libqos_init(virtio_iommu_register_nodes);
diff --git a/tests/qtest/libqos/virtio-iommu.h b/tests/qtest/libqos/virtio-iommu.h
new file mode 100644
index 00000000000..d753761958a
--- /dev/null
+++ b/tests/qtest/libqos/virtio-iommu.h
@@ -0,0 +1,40 @@
+/*
+ * libqos driver virtio-iommu-pci framework
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * Authors:
+ *  Eric Auger <eric.auger@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version.  See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef TESTS_LIBQOS_VIRTIO_IOMMU_H
+#define TESTS_LIBQOS_VIRTIO_IOMMU_H
+
+#include "qgraph.h"
+#include "virtio.h"
+#include "virtio-pci.h"
+
+typedef struct QVirtioIOMMU QVirtioIOMMU;
+typedef struct QVirtioIOMMUPCI QVirtioIOMMUPCI;
+typedef struct QVirtioIOMMUDevice QVirtioIOMMUDevice;
+
+struct QVirtioIOMMU {
+    QVirtioDevice *vdev;
+    QVirtQueue *vq;
+};
+
+struct QVirtioIOMMUPCI {
+    QVirtioPCIDevice pci_vdev;
+    QVirtioIOMMU iommu;
+};
+
+struct QVirtioIOMMUDevice {
+    QOSGraphObject obj;
+    QVirtioIOMMU iommu;
+};
+
+#endif
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c9d8458062f..982ffb3e38d 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -230,6 +230,7 @@ qos_test_ss.add(
   'virtio-rng-test.c',
   'virtio-scsi-test.c',
   'virtio-serial-test.c',
+  'virtio-iommu-test.c',
   'vmxnet3-test.c',
 )
 if have_virtfs
diff --git a/tests/qtest/virtio-iommu-test.c b/tests/qtest/virtio-iommu-test.c
new file mode 100644
index 00000000000..47e68388a00
--- /dev/null
+++ b/tests/qtest/virtio-iommu-test.c
@@ -0,0 +1,326 @@
+/*
+ * QTest testcase for VirtIO IOMMU
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * Authors:
+ *  Eric Auger <eric.auger@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version.  See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "qemu/module.h"
+#include "libqos/qgraph.h"
+#include "libqos/virtio-iommu.h"
+#include "hw/virtio/virtio-iommu.h"
+
+#define PCI_SLOT_HP             0x06
+#define QVIRTIO_IOMMU_TIMEOUT_US (30 * 1000 * 1000)
+
+static QGuestAllocator *alloc;
+
+static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtioIOMMU *v_iommu = obj;
+    QVirtioDevice *dev = v_iommu->vdev;
+    uint64_t input_range_start = qvirtio_config_readq(dev, 8);
+    uint64_t input_range_end = qvirtio_config_readq(dev, 16);
+    uint32_t domain_range_start = qvirtio_config_readl(dev, 24);
+    uint32_t domain_range_end = qvirtio_config_readl(dev, 28);
+
+    g_assert_cmpint(input_range_start, ==, 0);
+    g_assert_cmphex(input_range_end, ==, UINT64_MAX);
+    g_assert_cmpint(domain_range_start, ==, 0);
+    g_assert_cmpint(domain_range_end, ==, UINT32_MAX);
+}
+
+static int read_tail_status(struct virtio_iommu_req_tail *buffer)
+{
+    int i;
+
+    for (i = 0; i < 3; i++) {
+        g_assert_cmpint(buffer->reserved[i], ==, 0);
+    }
+    return buffer->status;
+}
+
+/**
+ * send_attach_detach - Send an attach/detach command to the device
+ * @type: VIRTIO_IOMMU_T_ATTACH/VIRTIO_IOMMU_T_DETACH
+ * @domain: domain the endpoint is attached to
+ * @ep: endpoint
+ */
+static int send_attach_detach(QTestState *qts, QVirtioIOMMU *v_iommu,
+                              uint8_t type, uint32_t domain, uint32_t ep)
+{
+    QVirtioDevice *dev = v_iommu->vdev;
+    QVirtQueue *vq = v_iommu->vq;
+    uint64_t ro_addr, wr_addr;
+    uint32_t free_head;
+    struct virtio_iommu_req_attach req = {}; /* same layout as detach */
+    size_t ro_size = sizeof(req) - sizeof(struct virtio_iommu_req_tail);
+    size_t wr_size = sizeof(struct virtio_iommu_req_tail);
+    struct virtio_iommu_req_tail buffer;
+    int ret;
+
+    req.head.type = type;
+    req.domain = cpu_to_le32(domain);
+    req.endpoint = cpu_to_le32(ep);
+
+    ro_addr = guest_alloc(alloc, ro_size);
+    wr_addr = guest_alloc(alloc, wr_size);
+
+    qtest_memwrite(qts, ro_addr, &req, ro_size);
+    free_head = qvirtqueue_add(qts, vq, ro_addr, ro_size, false, true);
+    qvirtqueue_add(qts, vq, wr_addr, wr_size, true, false);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_IOMMU_TIMEOUT_US);
+    qtest_memread(qts, wr_addr, &buffer, wr_size);
+    ret = read_tail_status(&buffer);
+    guest_free(alloc, ro_addr);
+    guest_free(alloc, wr_addr);
+    return ret;
+}
+
+/**
+ * send_map - Send a map command to the device
+ * @domain: domain the new mapping is attached to
+ * @virt_start: iova start
+ * @virt_end: iova end
+ * @phys_start: base physical address
+ * @flags: mapping flags
+ */
+static int send_map(QTestState *qts, QVirtioIOMMU *v_iommu,
+                    uint32_t domain, uint64_t virt_start, uint64_t virt_end,
+                    uint64_t phys_start, uint32_t flags)
+{
+    QVirtioDevice *dev = v_iommu->vdev;
+    QVirtQueue *vq = v_iommu->vq;
+    uint64_t ro_addr, wr_addr;
+    uint32_t free_head;
+    struct virtio_iommu_req_map req;
+    size_t ro_size = sizeof(req) - sizeof(struct virtio_iommu_req_tail);
+    size_t wr_size = sizeof(struct virtio_iommu_req_tail);
+    struct virtio_iommu_req_tail buffer;
+    int ret;
+
+    req.head.type = VIRTIO_IOMMU_T_MAP;
+    req.domain = cpu_to_le32(domain);
+    req.virt_start = cpu_to_le64(virt_start);
+    req.virt_end = cpu_to_le64(virt_end);
+    req.phys_start = cpu_to_le64(phys_start);
+    req.flags = cpu_to_le32(flags);
+
+    ro_addr = guest_alloc(alloc, ro_size);
+    wr_addr = guest_alloc(alloc, wr_size);
+
+    qtest_memwrite(qts, ro_addr, &req, ro_size);
+    free_head = qvirtqueue_add(qts, vq, ro_addr, ro_size, false, true);
+    qvirtqueue_add(qts, vq, wr_addr, wr_size, true, false);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_IOMMU_TIMEOUT_US);
+    qtest_memread(qts, wr_addr, &buffer, wr_size);
+    ret = read_tail_status(&buffer);
+    guest_free(alloc, ro_addr);
+    guest_free(alloc, wr_addr);
+    return ret;
+}
+
+/**
+ * send_unmap - Send an unmap command to the device
+ * @domain: domain the new binding is attached to
+ * @virt_start: iova start
+ * @virt_end: iova end
+ */
+static int send_unmap(QTestState *qts, QVirtioIOMMU *v_iommu,
+                      uint32_t domain, uint64_t virt_start, uint64_t virt_end)
+{
+    QVirtioDevice *dev = v_iommu->vdev;
+    QVirtQueue *vq = v_iommu->vq;
+    uint64_t ro_addr, wr_addr;
+    uint32_t free_head;
+    struct virtio_iommu_req_unmap req;
+    size_t ro_size = sizeof(req) - sizeof(struct virtio_iommu_req_tail);
+    size_t wr_size = sizeof(struct virtio_iommu_req_tail);
+    struct virtio_iommu_req_tail buffer;
+    int ret;
+
+    req.head.type = VIRTIO_IOMMU_T_UNMAP;
+    req.domain = cpu_to_le32(domain);
+    req.virt_start = cpu_to_le64(virt_start);
+    req.virt_end = cpu_to_le64(virt_end);
+
+    ro_addr = guest_alloc(alloc, ro_size);
+    wr_addr = guest_alloc(alloc, wr_size);
+
+    qtest_memwrite(qts, ro_addr, &req, ro_size);
+    free_head = qvirtqueue_add(qts, vq, ro_addr, ro_size, false, true);
+    qvirtqueue_add(qts, vq, wr_addr, wr_size, true, false);
+    qvirtqueue_kick(qts, dev, vq, free_head);
+    qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                           QVIRTIO_IOMMU_TIMEOUT_US);
+    qtest_memread(qts, wr_addr, &buffer, wr_size);
+    ret = read_tail_status(&buffer);
+    guest_free(alloc, ro_addr);
+    guest_free(alloc, wr_addr);
+    return ret;
+}
+
+static void test_attach_detach(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtioIOMMU *v_iommu = obj;
+    QTestState *qts = global_qtest;
+    int ret;
+
+    alloc = t_alloc;
+
+    /* type, domain, ep */
+
+    /* attach ep0 to domain 0 */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 0, 0);
+    g_assert_cmpint(ret, ==, 0);
+
+    /* attach a non existing device */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 0, 444);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+    /* detach a non existing device (1) */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 0, 1);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+    /* move ep0 from domain 0 to domain 1 */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 1, 0);
+    g_assert_cmpint(ret, ==, 0);
+
+    /* detach ep0 from domain 0 */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 0, 0);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_INVAL);
+
+    /* detach ep0 from domain 1 */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 1, 0);
+    g_assert_cmpint(ret, ==, 0);
+
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 1, 0);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_map(qts, v_iommu, 1, 0x0, 0xFFF, 0xa1000,
+                   VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_map(qts, v_iommu, 1, 0x2000, 0x2FFF, 0xb1000,
+                   VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 1, 0);
+    g_assert_cmpint(ret, ==, 0);
+}
+
+/* Test map/unmap scenari documented in the spec */
+static void test_map_unmap(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtioIOMMU *v_iommu = obj;
+    QTestState *qts = global_qtest;
+    int ret;
+
+    alloc = t_alloc;
+
+    /* attach ep0 to domain 1 */
+    ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 1, 0);
+    g_assert_cmpint(ret, ==, 0);
+
+    ret = send_map(qts, v_iommu, 0, 0, 0xFFF, 0xa1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+    /* domain, virt start, virt end, phys start, flags */
+    ret = send_map(qts, v_iommu, 1, 0x0, 0xFFF, 0xa1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+
+    /* send a new mapping overlapping the previous one */
+    ret = send_map(qts, v_iommu, 1, 0, 0xFFFF, 0xb1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_INVAL);
+
+    ret = send_unmap(qts, v_iommu, 4, 0x10, 0xFFF);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+    ret = send_unmap(qts, v_iommu, 1, 0x10, 0xFFF);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_RANGE);
+
+    ret = send_unmap(qts, v_iommu, 1, 0, 0x1000);
+    g_assert_cmpint(ret, ==, 0); /* unmap everything */
+
+    /* Spec example sequence */
+
+    /* 1 */
+    ret = send_unmap(qts, v_iommu, 1, 0, 4);
+    g_assert_cmpint(ret, ==, 0); /* doesn't unmap anything */
+
+    /* 2 */
+    ret = send_map(qts, v_iommu, 1, 0, 9, 0xa1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_unmap(qts, v_iommu, 1, 0, 9);
+    g_assert_cmpint(ret, ==, 0); /* unmaps [0,9] */
+
+    /* 3 */
+    ret = send_map(qts, v_iommu, 1, 0, 4, 0xb1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_map(qts, v_iommu, 1, 5, 9, 0xb2000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_unmap(qts, v_iommu, 1, 0, 9);
+    g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] and [5,9] */
+
+    /* 4 */
+    ret = send_map(qts, v_iommu, 1, 0, 9, 0xc1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+
+    ret = send_unmap(qts, v_iommu, 1, 0, 4);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_RANGE); /* doesn't unmap anything */
+
+    ret = send_unmap(qts, v_iommu, 1, 0, 10);
+    g_assert_cmpint(ret, ==, 0);
+
+    /* 5 */
+    ret = send_map(qts, v_iommu, 1, 0, 4, 0xd1000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_map(qts, v_iommu, 1, 5, 9, 0xd2000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_unmap(qts, v_iommu, 1, 0, 4);
+    g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] */
+
+    ret = send_unmap(qts, v_iommu, 1, 5, 9);
+    g_assert_cmpint(ret, ==, 0);
+
+    /* 6 */
+    ret = send_map(qts, v_iommu, 1, 0, 4, 0xe2000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_unmap(qts, v_iommu, 1, 0, 9);
+    g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] */
+
+    /* 7 */
+    ret = send_map(qts, v_iommu, 1, 0, 4, 0xf2000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_map(qts, v_iommu, 1, 10, 14, 0xf3000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_unmap(qts, v_iommu, 1, 0, 14);
+    g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] and [10,14] */
+
+    ret = send_map(qts, v_iommu, 1, 10, 14, 0xf3000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_map(qts, v_iommu, 1, 0, 4, 0xf2000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, 0);
+    ret = send_unmap(qts, v_iommu, 1, 0, 4);
+    g_assert_cmpint(ret, ==, 0); /* only unmaps [0,4] */
+    ret = send_map(qts, v_iommu, 1, 10, 14, 0xf3000, VIRTIO_IOMMU_MAP_F_READ);
+    g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_INVAL); /* 10-14 still is mapped */
+}
+
+static void register_virtio_iommu_test(void)
+{
+    qos_add_test("config", "virtio-iommu", pci_config, NULL);
+    qos_add_test("attach_detach", "virtio-iommu", test_attach_detach, NULL);
+    qos_add_test("map_unmap", "virtio-iommu", test_map_unmap, NULL);
+}
+
+libqos_init(register_virtio_iommu_test);
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/4] virtio-iommu: Remove set_config callback
  2021-11-27  7:29 ` [PATCH v6 1/4] virtio-iommu: Remove set_config callback Eric Auger
@ 2021-11-27  7:50   ` Eric Auger
  2021-11-29 19:04   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 11+ messages in thread
From: Eric Auger @ 2021-11-27  7:50 UTC (permalink / raw)
  To: eric.auger.pro, thuth, qemu-arm, qemu-devel, jean-philippe,
	peter.maydell
  Cc: lvivier, pbonzini

Hi,
On 11/27/21 8:29 AM, Eric Auger wrote:
> The spec says "the driver must not write to device configuration
> fields". So remove the set_config() callback which anyway did
> not do anything.
Forgot to mention that with the advent of  VIRTIO_IOMMU_F_BYPASS_CONFIG
feature and bypass field in struct virtio_iommu_config coming, this will
change soon. Only the bypass field will be settable. But this is not yet
available in the imported linux header.

Thanks

Eric

>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/virtio/trace-events   |  1 -
>  hw/virtio/virtio-iommu.c | 14 --------------
>  2 files changed, 15 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 650e521e351..54bd7da00c8 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -92,7 +92,6 @@ virtio_iommu_device_reset(void) "reset!"
>  virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
>  virtio_iommu_device_status(uint8_t status) "driver status = %d"
>  virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
> -virtio_iommu_set_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
>  virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>  virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>  virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1b23e8e18c7..645c0aa3997 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -832,19 +832,6 @@ static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
>      memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
>  }
>  
> -static void virtio_iommu_set_config(VirtIODevice *vdev,
> -                                      const uint8_t *config_data)
> -{
> -    struct virtio_iommu_config config;
> -
> -    memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
> -    trace_virtio_iommu_set_config(config.page_size_mask,
> -                                  config.input_range.start,
> -                                  config.input_range.end,
> -                                  config.domain_range.end,
> -                                  config.probe_size);
> -}
> -
>  static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
>                                            Error **errp)
>  {
> @@ -1185,7 +1172,6 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
>      vdc->unrealize = virtio_iommu_device_unrealize;
>      vdc->reset = virtio_iommu_device_reset;
>      vdc->get_config = virtio_iommu_get_config;
> -    vdc->set_config = virtio_iommu_set_config;
>      vdc->get_features = virtio_iommu_get_features;
>      vdc->set_status = virtio_iommu_set_status;
>      vdc->vmsd = &vmstate_virtio_iommu_device;



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/4] virtio-iommu: Remove set_config callback
  2021-11-27  7:29 ` [PATCH v6 1/4] virtio-iommu: Remove set_config callback Eric Auger
  2021-11-27  7:50   ` Eric Auger
@ 2021-11-29 19:04   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 11+ messages in thread
From: Jean-Philippe Brucker @ 2021-11-29 19:04 UTC (permalink / raw)
  To: Eric Auger
  Cc: lvivier, peter.maydell, thuth, qemu-devel, qemu-arm, pbonzini,
	eric.auger.pro

On Sat, Nov 27, 2021 at 08:29:07AM +0100, Eric Auger wrote:
> The spec says "the driver must not write to device configuration
> fields". So remove the set_config() callback which anyway did
> not do anything.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Removing this makes sense. For bypass, I'll add the function back with a
reduced trace

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  hw/virtio/trace-events   |  1 -
>  hw/virtio/virtio-iommu.c | 14 --------------
>  2 files changed, 15 deletions(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 650e521e351..54bd7da00c8 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -92,7 +92,6 @@ virtio_iommu_device_reset(void) "reset!"
>  virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
>  virtio_iommu_device_status(uint8_t status) "driver status = %d"
>  virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
> -virtio_iommu_set_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
>  virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>  virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>  virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1b23e8e18c7..645c0aa3997 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -832,19 +832,6 @@ static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
>      memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
>  }
>  
> -static void virtio_iommu_set_config(VirtIODevice *vdev,
> -                                      const uint8_t *config_data)
> -{
> -    struct virtio_iommu_config config;
> -
> -    memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
> -    trace_virtio_iommu_set_config(config.page_size_mask,
> -                                  config.input_range.start,
> -                                  config.input_range.end,
> -                                  config.domain_range.end,
> -                                  config.probe_size);
> -}
> -
>  static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
>                                            Error **errp)
>  {
> @@ -1185,7 +1172,6 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
>      vdc->unrealize = virtio_iommu_device_unrealize;
>      vdc->reset = virtio_iommu_device_reset;
>      vdc->get_config = virtio_iommu_get_config;
> -    vdc->set_config = virtio_iommu_set_config;
>      vdc->get_features = virtio_iommu_get_features;
>      vdc->set_status = virtio_iommu_set_status;
>      vdc->vmsd = &vmstate_virtio_iommu_device;
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 2/4] virtio-iommu: Fix endianness in get_config
  2021-11-27  7:29 ` [PATCH v6 2/4] virtio-iommu: Fix endianness in get_config Eric Auger
@ 2021-11-29 19:10   ` Jean-Philippe Brucker
  0 siblings, 0 replies; 11+ messages in thread
From: Jean-Philippe Brucker @ 2021-11-29 19:10 UTC (permalink / raw)
  To: Eric Auger
  Cc: lvivier, peter.maydell, thuth, qemu-devel, qemu-arm, pbonzini,
	eric.auger.pro

On Sat, Nov 27, 2021 at 08:29:08AM +0100, Eric Auger wrote:
> Endianess is not properly handled when populating
> the returned config. Use the cpu_to_le* primitives
> for each separate field. Also, while at it, trace
> the domain range start.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  hw/virtio/trace-events   |  2 +-
>  hw/virtio/virtio-iommu.c | 22 +++++++++++++++-------
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 54bd7da00c8..f7ad6be5fbb 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -91,7 +91,7 @@ virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
>  virtio_iommu_device_reset(void) "reset!"
>  virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
>  virtio_iommu_device_status(uint8_t status) "driver status = %d"
> -virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
> +virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d domain range end=%d probe_size=0x%x"
>  virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>  virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>  virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 645c0aa3997..30ee09187b8 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -822,14 +822,22 @@ unlock:
>  static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
>  {
>      VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
> -    struct virtio_iommu_config *config = &dev->config;
> +    struct virtio_iommu_config *dev_config = &dev->config;
> +    struct virtio_iommu_config *out_config = (void *)config_data;
>  
> -    trace_virtio_iommu_get_config(config->page_size_mask,
> -                                  config->input_range.start,
> -                                  config->input_range.end,
> -                                  config->domain_range.end,
> -                                  config->probe_size);
> -    memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
> +    out_config->page_size_mask = cpu_to_le64(dev_config->page_size_mask);
> +    out_config->input_range.start = cpu_to_le64(dev_config->input_range.start);
> +    out_config->input_range.end = cpu_to_le64(dev_config->input_range.end);
> +    out_config->domain_range.start = cpu_to_le32(dev_config->domain_range.start);
> +    out_config->domain_range.end = cpu_to_le32(dev_config->domain_range.end);
> +    out_config->probe_size = cpu_to_le32(dev_config->probe_size);
> +
> +    trace_virtio_iommu_get_config(dev_config->page_size_mask,
> +                                  dev_config->input_range.start,
> +                                  dev_config->input_range.end,
> +                                  dev_config->domain_range.start,
> +                                  dev_config->domain_range.end,
> +                                  dev_config->probe_size);
>  }
>  
>  static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 3/4] virtio-iommu: Fix the domain_range end
  2021-11-27  7:29 ` [PATCH v6 3/4] virtio-iommu: Fix the domain_range end Eric Auger
@ 2021-11-29 19:12   ` Jean-Philippe Brucker
  0 siblings, 0 replies; 11+ messages in thread
From: Jean-Philippe Brucker @ 2021-11-29 19:12 UTC (permalink / raw)
  To: Eric Auger
  Cc: lvivier, peter.maydell, thuth, qemu-devel, qemu-arm, pbonzini,
	eric.auger.pro

On Sat, Nov 27, 2021 at 08:29:09AM +0100, Eric Auger wrote:
> in old times the domain range was defined by a domain_bits le32.
> This was then converted into a domain_range struct. During the
> upgrade the original value of '32' (bits) has been kept while
> the end field now is the max value of the domain id (UINT32_MAX).
> Fix that and also use UINT64_MAX for the input_range.end.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  hw/virtio/virtio-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 30ee09187b8..aa9c16a17b1 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -978,8 +978,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>      s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
>  
>      s->config.page_size_mask = TARGET_PAGE_MASK;
> -    s->config.input_range.end = -1UL;
> -    s->config.domain_range.end = 32;
> +    s->config.input_range.end = UINT64_MAX;
> +    s->config.domain_range.end = UINT32_MAX;
>      s->config.probe_size = VIOMMU_PROBE_SIZE;
>  
>      virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
> -- 
> 2.26.3
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 4/4] tests: qtest: Add virtio-iommu test
  2021-11-27  7:29 ` [PATCH v6 4/4] tests: qtest: Add virtio-iommu test Eric Auger
@ 2021-12-08 13:49   ` Thomas Huth
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2021-12-08 13:49 UTC (permalink / raw)
  To: Eric Auger, eric.auger.pro, qemu-arm, qemu-devel, jean-philippe,
	peter.maydell
  Cc: lvivier, pbonzini

On 27/11/2021 08.29, Eric Auger wrote:
> Add the framework to test the virtio-iommu-pci device
> and tests exercising the attach/detach, map/unmap API.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> 
> ---
> 
> v5 -> v6:
> - changed the expected value for domain.end (32 -> MAX_UINT32)
> ---
>   tests/qtest/libqos/meson.build    |   1 +
>   tests/qtest/libqos/virtio-iommu.c | 126 ++++++++++++
>   tests/qtest/libqos/virtio-iommu.h |  40 ++++
>   tests/qtest/meson.build           |   1 +
>   tests/qtest/virtio-iommu-test.c   | 326 ++++++++++++++++++++++++++++++
>   5 files changed, 494 insertions(+)
>   create mode 100644 tests/qtest/libqos/virtio-iommu.c
>   create mode 100644 tests/qtest/libqos/virtio-iommu.h
>   create mode 100644 tests/qtest/virtio-iommu-test.c

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 0/4] virtio-iommu: config related fixes and qtest
  2021-11-27  7:29 [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Eric Auger
                   ` (3 preceding siblings ...)
  2021-11-27  7:29 ` [PATCH v6 4/4] tests: qtest: Add virtio-iommu test Eric Auger
@ 2021-12-13 10:02 ` Thomas Huth
  4 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2021-12-13 10:02 UTC (permalink / raw)
  To: Eric Auger, eric.auger.pro, qemu-arm, qemu-devel, jean-philippe,
	peter.maydell
  Cc: lvivier, pbonzini

On 27/11/2021 08.29, Eric Auger wrote:
> Introduce a qtest for the virtio-iommu device. The test
> allowed to identify an endianess bug in the get_config().
> We also remove the unneeded set_config() and fix the value
> for domain_range.end field.
> 
> Best Regards
> 
> Eric

Thanks, I've queued the series now to my testing-next branch:

  https://gitlab.com/thuth/qemu/-/commits/testing-next

  Thomas




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-13 10:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-27  7:29 [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Eric Auger
2021-11-27  7:29 ` [PATCH v6 1/4] virtio-iommu: Remove set_config callback Eric Auger
2021-11-27  7:50   ` Eric Auger
2021-11-29 19:04   ` Jean-Philippe Brucker
2021-11-27  7:29 ` [PATCH v6 2/4] virtio-iommu: Fix endianness in get_config Eric Auger
2021-11-29 19:10   ` Jean-Philippe Brucker
2021-11-27  7:29 ` [PATCH v6 3/4] virtio-iommu: Fix the domain_range end Eric Auger
2021-11-29 19:12   ` Jean-Philippe Brucker
2021-11-27  7:29 ` [PATCH v6 4/4] tests: qtest: Add virtio-iommu test Eric Auger
2021-12-08 13:49   ` Thomas Huth
2021-12-13 10:02 ` [PATCH v6 0/4] virtio-iommu: config related fixes and qtest Thomas Huth

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.