* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).