qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device
@ 2017-08-01  9:33 Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 1/8] update-linux-headers: import virtio_iommu.h Eric Auger
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

This series implements the virtio-iommu device.

This v3 mostly is a rebase on top of v2.10-rc0 that uses
IOMMUMmeoryRegion plus some small fixes.

This is a proof of concept based on the virtio-iommu specification
written by Jean-Philippe Brucker [1].

The device gets instantiated using the "-device virtio-iommu-device"
option. It currently works with ARM virt machine only, as the machine
must handle the dt binding between the virtio-mmio "iommu" node and
the PCI host bridge node.

ACPI booting is not yet supported.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/v2.10.0-rc0-virtio-iommu-rfcv3

References:
[1] [RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
[2] [RFC PATCH linux] iommu: Add virtio-iommu driver
[3] [RFC PATCH kvmtool 00/15] Add virtio-iommu

Testing:
- >= 4.12 guest kernel + virtio-iommu driver [2]
- guest using a virtio-net-pci device:
  ,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on

History:
v2 -> v3:
- rebase on top of 2.10-rc0 and especially
  [PATCH qemu v9 0/2] memory/iommu: QOM'fy IOMMU MemoryRegion
- add mutex init
- fix as->mappings deletion using g_tree_ref/unref
- when a dev is attached whereas it is already attached to
  another address space, first detach it
- fix some error values
- page_sizes = TARGET_PAGE_MASK;
- I haven't changed the unmap() semantics yet, waiting for the
  next virtio-iommu spec revision.

v1 -> v2:
- fix redifinition of viommu_as typedef


Eric Auger (8):
  update-linux-headers: import virtio_iommu.h
  linux-headers: Update for virtio-iommu
  virtio_iommu: add skeleton
  virtio-iommu: Decode the command payload
  virtio_iommu: Add the iommu regions
  virtio-iommu: Implement the translation and commands
  hw/arm/virt: Add 2.10 machine type
  hw/arm/virt: Add virtio-iommu the virt board

 hw/arm/virt.c                                 | 116 ++++-
 hw/virtio/Makefile.objs                       |   1 +
 hw/virtio/trace-events                        |  14 +
 hw/virtio/virtio-iommu.c                      | 670 ++++++++++++++++++++++++++
 include/hw/arm/virt.h                         |   5 +
 include/hw/virtio/virtio-iommu.h              |  61 +++
 include/standard-headers/linux/virtio_ids.h   |   1 +
 include/standard-headers/linux/virtio_iommu.h | 142 ++++++
 linux-headers/linux/virtio_iommu.h            |   1 +
 scripts/update-linux-headers.sh               |   3 +
 10 files changed, 1005 insertions(+), 9 deletions(-)
 create mode 100644 hw/virtio/virtio-iommu.c
 create mode 100644 include/hw/virtio/virtio-iommu.h
 create mode 100644 include/standard-headers/linux/virtio_iommu.h
 create mode 100644 linux-headers/linux/virtio_iommu.h

-- 
2.5.5

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

* [Qemu-devel] [RFC v3 1/8] update-linux-headers: import virtio_iommu.h
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 2/8] linux-headers: Update for virtio-iommu Eric Auger
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

Update the script to update the virtio_iommu.h header.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 scripts/update-linux-headers.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 2f906c4..03f6712 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -134,6 +134,9 @@ EOF
 cat <<EOF >$output/linux-headers/linux/virtio_config.h
 #include "standard-headers/linux/virtio_config.h"
 EOF
+cat <<EOF >$output/linux-headers/linux/virtio_iommu.h
+#include "standard-headers/linux/virtio_iommu.h"
+EOF
 cat <<EOF >$output/linux-headers/linux/virtio_ring.h
 #include "standard-headers/linux/virtio_ring.h"
 EOF
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 2/8] linux-headers: Update for virtio-iommu
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 1/8] update-linux-headers: import virtio_iommu.h Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 3/8] virtio_iommu: add skeleton Eric Auger
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

This is a partial linux header update against Jean-Philippe's branch:
git://linux-arm.org/linux-jpb.git virtio-iommu/base (unstable)

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/standard-headers/linux/virtio_ids.h   |   1 +
 include/standard-headers/linux/virtio_iommu.h | 142 ++++++++++++++++++++++++++
 linux-headers/linux/virtio_iommu.h            |   1 +
 3 files changed, 144 insertions(+)
 create mode 100644 include/standard-headers/linux/virtio_iommu.h
 create mode 100644 linux-headers/linux/virtio_iommu.h

diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
index 6d5c3b2..934ed3d 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -43,5 +43,6 @@
 #define VIRTIO_ID_INPUT        18 /* virtio input */
 #define VIRTIO_ID_VSOCK        19 /* virtio vsock transport */
 #define VIRTIO_ID_CRYPTO       20 /* virtio crypto */
+#define VIRTIO_ID_IOMMU	    61216 /* virtio IOMMU (temporary) */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/standard-headers/linux/virtio_iommu.h b/include/standard-headers/linux/virtio_iommu.h
new file mode 100644
index 0000000..e139587
--- /dev/null
+++ b/include/standard-headers/linux/virtio_iommu.h
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2017 ARM Ltd.
+ *
+ * This header is BSD licensed so anyone can use the definitions
+ * to implement compatible drivers/servers:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of ARM Ltd. nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#ifndef _LINUX_VIRTIO_IOMMU_H
+#define _LINUX_VIRTIO_IOMMU_H
+
+/* Feature bits */
+#define VIRTIO_IOMMU_F_INPUT_RANGE		0
+#define VIRTIO_IOMMU_F_IOASID_BITS		1
+#define VIRTIO_IOMMU_F_MAP_UNMAP		2
+#define VIRTIO_IOMMU_F_BYPASS			3
+
+QEMU_PACKED
+struct virtio_iommu_config {
+	/* Supported page sizes */
+	uint64_t					page_sizes;
+	struct virtio_iommu_range {
+		uint64_t				start;
+		uint64_t				end;
+	} input_range;
+	uint8_t 					ioasid_bits;
+};
+
+/* Request types */
+#define VIRTIO_IOMMU_T_ATTACH			0x01
+#define VIRTIO_IOMMU_T_DETACH			0x02
+#define VIRTIO_IOMMU_T_MAP			0x03
+#define VIRTIO_IOMMU_T_UNMAP			0x04
+
+/* Status types */
+#define VIRTIO_IOMMU_S_OK			0x00
+#define VIRTIO_IOMMU_S_IOERR			0x01
+#define VIRTIO_IOMMU_S_UNSUPP			0x02
+#define VIRTIO_IOMMU_S_DEVERR			0x03
+#define VIRTIO_IOMMU_S_INVAL			0x04
+#define VIRTIO_IOMMU_S_RANGE			0x05
+#define VIRTIO_IOMMU_S_NOENT			0x06
+#define VIRTIO_IOMMU_S_FAULT			0x07
+
+QEMU_PACKED
+struct virtio_iommu_req_head {
+	uint8_t					type;
+	uint8_t					reserved[3];
+};
+
+QEMU_PACKED
+struct virtio_iommu_req_tail {
+	uint8_t					status;
+	uint8_t					reserved[3];
+};
+
+QEMU_PACKED
+struct virtio_iommu_req_attach {
+	struct virtio_iommu_req_head		head;
+
+	uint32_t					address_space;
+	uint32_t					device;
+	uint32_t					reserved;
+
+	struct virtio_iommu_req_tail		tail;
+};
+
+QEMU_PACKED
+struct virtio_iommu_req_detach {
+	struct virtio_iommu_req_head		head;
+
+	uint32_t					device;
+	uint32_t					reserved;
+
+	struct virtio_iommu_req_tail		tail;
+};
+
+#define VIRTIO_IOMMU_MAP_F_READ			(1 << 0)
+#define VIRTIO_IOMMU_MAP_F_WRITE		(1 << 1)
+#define VIRTIO_IOMMU_MAP_F_EXEC			(1 << 2)
+
+#define VIRTIO_IOMMU_MAP_F_MASK			(VIRTIO_IOMMU_MAP_F_READ |	\
+						 VIRTIO_IOMMU_MAP_F_WRITE |	\
+						 VIRTIO_IOMMU_MAP_F_EXEC)
+
+QEMU_PACKED
+struct virtio_iommu_req_map {
+	struct virtio_iommu_req_head		head;
+
+	uint32_t					address_space;
+	uint32_t					flags;
+	uint64_t					virt_addr;
+	uint64_t					phys_addr;
+	uint64_t					size;
+
+	struct virtio_iommu_req_tail		tail;
+};
+
+QEMU_PACKED
+struct virtio_iommu_req_unmap {
+	struct virtio_iommu_req_head		head;
+
+	uint32_t					address_space;
+	uint32_t					flags;
+	uint64_t					virt_addr;
+	uint64_t					size;
+
+	struct virtio_iommu_req_tail		tail;
+};
+
+union virtio_iommu_req {
+	struct virtio_iommu_req_head		head;
+
+	struct virtio_iommu_req_attach		attach;
+	struct virtio_iommu_req_detach		detach;
+	struct virtio_iommu_req_map		map;
+	struct virtio_iommu_req_unmap		unmap;
+};
+
+#endif
diff --git a/linux-headers/linux/virtio_iommu.h b/linux-headers/linux/virtio_iommu.h
new file mode 100644
index 0000000..2dc4609
--- /dev/null
+++ b/linux-headers/linux/virtio_iommu.h
@@ -0,0 +1 @@
+#include "standard-headers/linux/virtio_iommu.h"
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 3/8] virtio_iommu: add skeleton
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 1/8] update-linux-headers: import virtio_iommu.h Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 2/8] linux-headers: Update for virtio-iommu Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 4/8] virtio-iommu: Decode the command payload Eric Auger
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

This patchs adds the skeleton for the virtio-iommu device.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v2 -> v3:
- rebase on 2.10-rc0, ie. use IOMMUMemoryRegion and remove
  iommu_ops.
- advertise VIRTIO_IOMMU_F_MAP_UNMAP feature
- page_sizes set to TARGET_PAGE_SIZE
---
 hw/virtio/Makefile.objs          |   1 +
 hw/virtio/virtio-iommu.c         | 248 +++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-iommu.h |  59 ++++++++++
 3 files changed, 308 insertions(+)
 create mode 100644 hw/virtio/virtio-iommu.c
 create mode 100644 include/hw/virtio/virtio-iommu.h

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 765d363..8967a4a 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-y += virtio-mmio.o
 
 obj-y += virtio.o virtio-balloon.o 
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
+obj-$(CONFIG_LINUX) += virtio-iommu.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
 obj-y += virtio-crypto.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
new file mode 100644
index 0000000..4570e19
--- /dev/null
+++ b/hw/virtio/virtio-iommu.c
@@ -0,0 +1,248 @@
+/*
+ * virtio-iommu device
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu-common.h"
+#include "hw/virtio/virtio.h"
+#include "sysemu/kvm.h"
+#include "qapi-event.h"
+#include "trace.h"
+
+#include "standard-headers/linux/virtio_ids.h"
+#include <linux/virtio_iommu.h>
+
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-iommu.h"
+
+/* Max size */
+#define VIOMMU_DEFAULT_QUEUE_SIZE 256
+
+static int virtio_iommu_handle_attach(VirtIOIOMMU *s,
+                                      struct iovec *iov,
+                                      unsigned int iov_cnt)
+{
+    return -ENOENT;
+}
+static int virtio_iommu_handle_detach(VirtIOIOMMU *s,
+                                      struct iovec *iov,
+                                      unsigned int iov_cnt)
+{
+    return -ENOENT;
+}
+static int virtio_iommu_handle_map(VirtIOIOMMU *s,
+                                   struct iovec *iov,
+                                   unsigned int iov_cnt)
+{
+    return -ENOENT;
+}
+static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
+                                     struct iovec *iov,
+                                     unsigned int iov_cnt)
+{
+    return -ENOENT;
+}
+
+static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
+    VirtQueueElement *elem;
+    struct virtio_iommu_req_head head;
+    struct virtio_iommu_req_tail tail;
+    unsigned int iov_cnt;
+    struct iovec *iov;
+    size_t sz;
+
+    for (;;) {
+        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+        if (!elem) {
+            return;
+        }
+
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(tail) ||
+            iov_size(elem->out_sg, elem->out_num) < sizeof(head)) {
+            virtio_error(vdev, "virtio-iommu erroneous head or tail");
+            virtqueue_detach_element(vq, elem, 0);
+            g_free(elem);
+            break;
+        }
+
+        iov_cnt = elem->out_num;
+        iov = g_memdup(elem->out_sg, sizeof(struct iovec) * elem->out_num);
+        sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
+        if (sz != sizeof(head)) {
+            tail.status = VIRTIO_IOMMU_S_UNSUPP;
+        }
+        qemu_mutex_lock(&s->mutex);
+        switch (head.type) {
+        case VIRTIO_IOMMU_T_ATTACH:
+            tail.status = virtio_iommu_handle_attach(s, iov, iov_cnt);
+            break;
+        case VIRTIO_IOMMU_T_DETACH:
+            tail.status = virtio_iommu_handle_detach(s, iov, iov_cnt);
+            break;
+        case VIRTIO_IOMMU_T_MAP:
+            tail.status = virtio_iommu_handle_map(s, iov, iov_cnt);
+            break;
+        case VIRTIO_IOMMU_T_UNMAP:
+            tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
+            break;
+        default:
+            tail.status = VIRTIO_IOMMU_S_UNSUPP;
+        }
+        qemu_mutex_unlock(&s->mutex);
+
+        sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
+                          &tail, sizeof(tail));
+        assert(sz == sizeof(tail));
+
+        virtqueue_push(vq, elem, sizeof(tail));
+        virtio_notify(vdev, vq);
+        g_free(elem);
+    }
+}
+
+static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
+{
+    VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
+
+    memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
+}
+
+static void virtio_iommu_set_config(VirtIODevice *vdev,
+                                      const uint8_t *config_data)
+{
+    VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
+    struct virtio_iommu_config config;
+
+    memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
+
+    dev->config.page_sizes = le64_to_cpu(config.page_sizes);
+    dev->config.input_range.end = le64_to_cpu(config.input_range.end);
+}
+
+static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
+                                            Error **errp)
+{
+    VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
+    f |= dev->host_features;
+    virtio_add_feature(&f, VIRTIO_RING_F_EVENT_IDX);
+    virtio_add_feature(&f, VIRTIO_RING_F_INDIRECT_DESC);
+    virtio_add_feature(&f, VIRTIO_IOMMU_F_INPUT_RANGE);
+    virtio_add_feature(&f, VIRTIO_IOMMU_F_MAP_UNMAP);
+    return f;
+}
+
+static int virtio_iommu_post_load_device(void *opaque, int version_id)
+{
+    return 0;
+}
+
+static const VMStateDescription vmstate_virtio_iommu_device = {
+    .name = "virtio-iommu-device",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = virtio_iommu_post_load_device,
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
+
+    virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
+                sizeof(struct virtio_iommu_config));
+
+    s->vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
+                             virtio_iommu_handle_command);
+
+    s->config.page_sizes = TARGET_PAGE_MASK;
+    s->config.input_range.end = -1UL;
+}
+
+static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+    virtio_cleanup(vdev);
+}
+
+static void virtio_iommu_device_reset(VirtIODevice *vdev)
+{
+}
+
+static void virtio_iommu_set_status(VirtIODevice *vdev, uint8_t status)
+{
+}
+
+static void virtio_iommu_instance_init(Object *obj)
+{
+}
+
+static const VMStateDescription vmstate_virtio_iommu = {
+    .name = "virtio-iommu",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static Property virtio_iommu_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_iommu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    dc->props = virtio_iommu_properties;
+    dc->vmsd = &vmstate_virtio_iommu;
+
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    vdc->realize = virtio_iommu_device_realize;
+    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;
+}
+
+static const TypeInfo virtio_iommu_info = {
+    .name = TYPE_VIRTIO_IOMMU,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOIOMMU),
+    .instance_init = virtio_iommu_instance_init,
+    .class_init = virtio_iommu_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_iommu_info);
+}
+
+type_init(virtio_register_types)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
new file mode 100644
index 0000000..6716cdb
--- /dev/null
+++ b/include/hw/virtio/virtio-iommu.h
@@ -0,0 +1,59 @@
+/*
+ * virtio-iommu device
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QEMU_VIRTIO_IOMMU_H
+#define QEMU_VIRTIO_IOMMU_H
+
+#include "standard-headers/linux/virtio_iommu.h"
+#include "hw/virtio/virtio.h"
+#include "hw/pci/pci.h"
+
+#define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
+#define VIRTIO_IOMMU(obj) \
+        OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
+
+#define IOMMU_PCI_BUS_MAX      256
+#define IOMMU_PCI_DEVFN_MAX    256
+
+typedef struct IOMMUDevice {
+    void         *viommu;
+    PCIBus       *bus;
+    int           devfn;
+    IOMMUMemoryRegion  iommu_mr;
+    AddressSpace  as;
+} IOMMUDevice;
+
+typedef struct IOMMUPciBus {
+    PCIBus       *bus;
+    IOMMUDevice  *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
+} IOMMUPciBus;
+
+typedef struct VirtIOIOMMU {
+    VirtIODevice parent_obj;
+    VirtQueue *vq;
+    struct virtio_iommu_config config;
+    uint32_t host_features;
+    GHashTable *as_by_busptr;
+    IOMMUPciBus *as_by_bus_num[IOMMU_PCI_BUS_MAX];
+    GTree *address_spaces;
+    QemuMutex mutex;
+    GTree *devices;
+} VirtIOIOMMU;
+
+#endif
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 4/8] virtio-iommu: Decode the command payload
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
                   ` (2 preceding siblings ...)
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 3/8] virtio_iommu: add skeleton Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 5/8] virtio_iommu: Add the iommu regions Eric Auger
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

This patch adds the command payload decoding and
introduces the functions that will do the actual
command handling. Those functions are not yet implemented.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/virtio/trace-events   |  7 ++++
 hw/virtio/virtio-iommu.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index e24d8fa..fba1da6 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -25,3 +25,10 @@ virtio_balloon_handle_output(const char *name, uint64_t gpa) "section name: %s g
 virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) "num_pages: %d actual: %d"
 virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: %d oldactual: %d"
 virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon target: %"PRIx64" num_pages: %d"
+
+# hw/virtio/virtio-iommu.c
+#
+virtio_iommu_attach(uint32_t as, uint32_t dev, uint32_t flags) "as=%d dev=%d flags=%d"
+virtio_iommu_detach(uint32_t dev, uint32_t flags) "dev=%d flags=%d"
+virtio_iommu_map(uint32_t as, uint64_t phys_addr, uint64_t virt_addr, uint64_t size, uint32_t flags) "as= %d phys_addr=0x%"PRIx64" virt_addr=0x%"PRIx64" size=0x%"PRIx64" flags=%d"
+virtio_iommu_unmap(uint32_t as, uint64_t virt_addr, uint64_t size, uint32_t reserved) "as= %d virt_addr=0x%"PRIx64" size=0x%"PRIx64" reserved=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 4570e19..6f5b71c 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -35,29 +35,118 @@
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
+static int virtio_iommu_attach(VirtIOIOMMU *s,
+                               struct virtio_iommu_req_attach *req)
+{
+    uint32_t asid = le32_to_cpu(req->address_space);
+    uint32_t devid = le32_to_cpu(req->device);
+    uint32_t reserved = le32_to_cpu(req->reserved);
+
+    trace_virtio_iommu_attach(asid, devid, reserved);
+
+    return VIRTIO_IOMMU_S_UNSUPP;
+}
+
+static int virtio_iommu_detach(VirtIOIOMMU *s,
+                               struct virtio_iommu_req_detach *req)
+{
+    uint32_t devid = le32_to_cpu(req->device);
+    uint32_t reserved = le32_to_cpu(req->reserved);
+
+    trace_virtio_iommu_detach(devid, reserved);
+
+    return VIRTIO_IOMMU_S_UNSUPP;
+}
+
+static int virtio_iommu_map(VirtIOIOMMU *s,
+                            struct virtio_iommu_req_map *req)
+{
+    uint32_t asid = le32_to_cpu(req->address_space);
+    uint64_t phys_addr = le64_to_cpu(req->phys_addr);
+    uint64_t virt_addr = le64_to_cpu(req->virt_addr);
+    uint64_t size = le64_to_cpu(req->size);
+    uint32_t flags = le32_to_cpu(req->flags);
+
+    trace_virtio_iommu_map(asid, phys_addr, virt_addr, size, flags);
+
+    return VIRTIO_IOMMU_S_UNSUPP;
+}
+
+static int virtio_iommu_unmap(VirtIOIOMMU *s,
+                              struct virtio_iommu_req_unmap *req)
+{
+    uint32_t asid = le32_to_cpu(req->address_space);
+    uint64_t virt_addr = le64_to_cpu(req->virt_addr);
+    uint64_t size = le64_to_cpu(req->size);
+    uint32_t flags = le32_to_cpu(req->flags);
+
+    trace_virtio_iommu_unmap(asid, virt_addr, size, flags);
+
+    return VIRTIO_IOMMU_S_UNSUPP;
+}
+
+#define get_payload_size(req) (\
+sizeof((req)) - sizeof(struct virtio_iommu_req_tail))
+
 static int virtio_iommu_handle_attach(VirtIOIOMMU *s,
                                       struct iovec *iov,
                                       unsigned int iov_cnt)
 {
-    return -ENOENT;
+    struct virtio_iommu_req_attach req;
+    size_t sz, payload_sz;
+
+    payload_sz = get_payload_size(req);
+
+    sz = iov_to_buf(iov, iov_cnt, 0, &req, payload_sz);
+    if (sz != payload_sz) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+    return virtio_iommu_attach(s, &req);
 }
 static int virtio_iommu_handle_detach(VirtIOIOMMU *s,
                                       struct iovec *iov,
                                       unsigned int iov_cnt)
 {
-    return -ENOENT;
+    struct virtio_iommu_req_detach req;
+    size_t sz, payload_sz;
+
+    payload_sz = get_payload_size(req);
+
+    sz = iov_to_buf(iov, iov_cnt, 0, &req, payload_sz);
+    if (sz != payload_sz) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+    return virtio_iommu_detach(s, &req);
 }
 static int virtio_iommu_handle_map(VirtIOIOMMU *s,
                                    struct iovec *iov,
                                    unsigned int iov_cnt)
 {
-    return -ENOENT;
+    struct virtio_iommu_req_map req;
+    size_t sz, payload_sz;
+
+    payload_sz = get_payload_size(req);
+
+    sz = iov_to_buf(iov, iov_cnt, 0, &req, payload_sz);
+    if (sz != payload_sz) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+    return virtio_iommu_map(s, &req);
 }
 static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
                                      struct iovec *iov,
                                      unsigned int iov_cnt)
 {
-    return -ENOENT;
+    struct virtio_iommu_req_unmap req;
+    size_t sz, payload_sz;
+
+    payload_sz = get_payload_size(req);
+
+    sz = iov_to_buf(iov, iov_cnt, 0, &req, payload_sz);
+    if (sz != payload_sz) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+    return virtio_iommu_unmap(s, &req);
 }
 
 static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 5/8] virtio_iommu: Add the iommu regions
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
                   ` (3 preceding siblings ...)
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 4/8] virtio-iommu: Decode the command payload Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 6/8] virtio-iommu: Implement the translation and commands Eric Auger
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

This patch initializes the iommu memory regions so that
PCIe end point transactions get translated. The translation function
is not yet implemented at that stage.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v2 -> v3:
- use IOMMUMemoryRegion
- iommu mr name built with BDF
- rename smmu_get_sid into virtio_iommu_get_sid and use PCI_BUILD_BDF
---
 hw/virtio/trace-events           |   1 +
 hw/virtio/virtio-iommu.c         | 117 +++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-iommu.h |   2 +
 3 files changed, 120 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index fba1da6..341dbdf 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -32,3 +32,4 @@ virtio_iommu_attach(uint32_t as, uint32_t dev, uint32_t flags) "as=%d dev=%d fla
 virtio_iommu_detach(uint32_t dev, uint32_t flags) "dev=%d flags=%d"
 virtio_iommu_map(uint32_t as, uint64_t phys_addr, uint64_t virt_addr, uint64_t size, uint32_t flags) "as= %d phys_addr=0x%"PRIx64" virt_addr=0x%"PRIx64" size=0x%"PRIx64" flags=%d"
 virtio_iommu_unmap(uint32_t as, uint64_t virt_addr, uint64_t size, uint32_t reserved) "as= %d virt_addr=0x%"PRIx64" size=0x%"PRIx64" reserved=%d"
+virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 6f5b71c..e663d9e 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -23,6 +23,7 @@
 #include "hw/virtio/virtio.h"
 #include "sysemu/kvm.h"
 #include "qapi-event.h"
+#include "qemu/error-report.h"
 #include "trace.h"
 
 #include "standard-headers/linux/virtio_ids.h"
@@ -35,6 +36,64 @@
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
+static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
+{
+    return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
+}
+
+static AddressSpace *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
+                                              int devfn)
+{
+    VirtIOIOMMU *s = opaque;
+    uintptr_t key = (uintptr_t)bus;
+    IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, &key);
+    IOMMUDevice *sdev;
+
+    if (!sbus) {
+        uintptr_t *new_key = g_malloc(sizeof(*new_key));
+
+        *new_key = (uintptr_t)bus;
+        sbus = g_malloc0(sizeof(IOMMUPciBus) +
+                         sizeof(IOMMUDevice *) * IOMMU_PCI_DEVFN_MAX);
+        sbus->bus = bus;
+        g_hash_table_insert(s->as_by_busptr, new_key, sbus);
+    }
+
+    sdev = sbus->pbdev[devfn];
+    if (!sdev) {
+        char *name = g_strdup_printf("%s-%d-%d",
+                                     TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+                                     pci_bus_num(bus), devfn);
+        sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
+
+        sdev->viommu = s;
+        sdev->bus = bus;
+        sdev->devfn = devfn;
+
+        memory_region_init_iommu(&sdev->iommu_mr, sizeof(sdev->iommu_mr),
+                                 TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+                                 OBJECT(s), name,
+                                 UINT64_MAX);
+        address_space_init(&sdev->as,
+                           MEMORY_REGION(&sdev->iommu_mr), TYPE_VIRTIO_IOMMU);
+    }
+
+    return &sdev->as;
+
+}
+
+static void virtio_iommu_init_as(VirtIOIOMMU *s)
+{
+    PCIBus *pcibus = pci_find_primary_bus();
+
+    if (pcibus) {
+        pci_setup_iommu(pcibus, virtio_iommu_find_add_as, s);
+    } else {
+        error_report("No PCI bus, virtio-iommu is not registered");
+    }
+}
+
+
 static int virtio_iommu_attach(VirtIOIOMMU *s,
                                struct virtio_iommu_req_attach *req)
 {
@@ -208,6 +267,26 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
+                                            IOMMUAccessFlags flag)
+{
+    IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+    uint32_t sid;
+
+    IOMMUTLBEntry entry = {
+        .target_as = &address_space_memory,
+        .iova = addr,
+        .translated_addr = addr,
+        .addr_mask = ~(hwaddr)0,
+        .perm = IOMMU_NONE,
+    };
+
+    sid = virtio_iommu_get_sid(sdev);
+
+    trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
+    return entry;
+}
+
 static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
@@ -254,6 +333,21 @@ static const VMStateDescription vmstate_virtio_iommu_device = {
     },
 };
 
+/*****************************
+ * Hash Table
+ *****************************/
+
+static inline gboolean as_uint64_equal(gconstpointer v1, gconstpointer v2)
+{
+    return *((const uint64_t *)v1) == *((const uint64_t *)v2);
+}
+
+static inline guint as_uint64_hash(gconstpointer v)
+{
+    return (guint)*(const uint64_t *)v;
+}
+
+
 static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -267,6 +361,13 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
 
     s->config.page_sizes = TARGET_PAGE_MASK;
     s->config.input_range.end = -1UL;
+
+    memset(s->as_by_bus_num, 0, sizeof(s->as_by_bus_num));
+    s->as_by_busptr = g_hash_table_new_full(as_uint64_hash,
+                                            as_uint64_equal,
+                                            g_free, g_free);
+
+    virtio_iommu_init_as(s);
 }
 
 static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
@@ -321,6 +422,14 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
     vdc->vmsd = &vmstate_virtio_iommu_device;
 }
 
+static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
+                                                  void *data)
+{
+    IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
+
+    imrc->translate = virtio_iommu_translate;
+}
+
 static const TypeInfo virtio_iommu_info = {
     .name = TYPE_VIRTIO_IOMMU,
     .parent = TYPE_VIRTIO_DEVICE,
@@ -329,9 +438,17 @@ static const TypeInfo virtio_iommu_info = {
     .class_init = virtio_iommu_class_init,
 };
 
+static const TypeInfo virtio_iommu_memory_region_info = {
+    .parent = TYPE_IOMMU_MEMORY_REGION,
+    .name = TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+    .class_init = virtio_iommu_memory_region_class_init,
+};
+
+
 static void virtio_register_types(void)
 {
     type_register_static(&virtio_iommu_info);
+    type_register_static(&virtio_iommu_memory_region_info);
 }
 
 type_init(virtio_register_types)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 6716cdb..f9c988f 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -28,6 +28,8 @@
 #define VIRTIO_IOMMU(obj) \
         OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
 
+#define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region"
+
 #define IOMMU_PCI_BUS_MAX      256
 #define IOMMU_PCI_DEVFN_MAX    256
 
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 6/8] virtio-iommu: Implement the translation and commands
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
                   ` (4 preceding siblings ...)
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 5/8] virtio_iommu: Add the iommu regions Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-03 10:15   ` Bharat Bhushan
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 7/8] hw/arm/virt: Add 2.10 machine type Eric Auger
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

This patch adds the actual implementation for the translation routine
and the virtio-iommu commands.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
v2 -> v3:
- init the mutex
- return VIRTIO_IOMMU_S_INVAL is reserved field is not null on
  attach/detach commands
- on attach, when the device is already attached to an address space,
  detach it first instead of returning an error
- remove nr_devices and use g_tree_ref/unref to destroy the as->mappings
  on last device detach
- map/unmap: return NOENT instead of INVAL error if as does not exist
- remove flags argument from attach/detach trace functions

v1 -> v2:
- fix compilation issue reported by autobuild system
---
 hw/virtio/trace-events   |  10 +-
 hw/virtio/virtio-iommu.c | 232 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 232 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 341dbdf..8db3d91 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -28,8 +28,14 @@ virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon target: %
 
 # hw/virtio/virtio-iommu.c
 #
-virtio_iommu_attach(uint32_t as, uint32_t dev, uint32_t flags) "as=%d dev=%d flags=%d"
-virtio_iommu_detach(uint32_t dev, uint32_t flags) "dev=%d flags=%d"
+virtio_iommu_attach(uint32_t as, uint32_t dev) "as=%d dev=%d"
+virtio_iommu_detach(uint32_t dev) "dev=%d"
 virtio_iommu_map(uint32_t as, uint64_t phys_addr, uint64_t virt_addr, uint64_t size, uint32_t flags) "as= %d phys_addr=0x%"PRIx64" virt_addr=0x%"PRIx64" size=0x%"PRIx64" flags=%d"
 virtio_iommu_unmap(uint32_t as, uint64_t virt_addr, uint64_t size, uint32_t reserved) "as= %d virt_addr=0x%"PRIx64" size=0x%"PRIx64" reserved=%d"
 virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
+virtio_iommu_new_asid(uint32_t asid) "Allocate a new asid=%d"
+virtio_iommu_new_devid(uint32_t devid) "Allocate a new devid=%d"
+virtio_iommu_unmap_left_interval(uint64_t low, uint64_t high, uint64_t next_low, uint64_t next_high) "Unmap left [0x%"PRIx64",0x%"PRIx64"], new interval=[0x%"PRIx64",0x%"PRIx64"]"
+virtio_iommu_unmap_right_interval(uint64_t low, uint64_t high, uint64_t next_low, uint64_t next_high) "Unmap right [0x%"PRIx64",0x%"PRIx64"], new interval=[0x%"PRIx64",0x%"PRIx64"]"
+virtio_iommu_unmap_inc_interval(uint64_t low, uint64_t high) "Unmap inc [0x%"PRIx64",0x%"PRIx64"]"
+virtio_iommu_translate_result(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index e663d9e..9217587 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -32,10 +32,36 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 #include "hw/virtio/virtio-iommu.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci.h"
 
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
+typedef struct viommu_as viommu_as;
+
+typedef struct viommu_mapping {
+    uint64_t virt_addr;
+    uint64_t phys_addr;
+    uint64_t size;
+    uint32_t flags;
+} viommu_mapping;
+
+typedef struct viommu_interval {
+    uint64_t low;
+    uint64_t high;
+} viommu_interval;
+
+typedef struct viommu_dev {
+    uint32_t id;
+    viommu_as *as;
+} viommu_dev;
+
+struct viommu_as {
+    uint32_t id;
+    GTree *mappings;
+};
+
 static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
 {
     return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
@@ -93,6 +119,29 @@ static void virtio_iommu_init_as(VirtIOIOMMU *s)
     }
 }
 
+static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+    viommu_interval *inta = (viommu_interval *)a;
+    viommu_interval *intb = (viommu_interval *)b;
+
+    if (inta->high <= intb->low) {
+        return -1;
+    } else if (intb->high <= inta->low) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
+static void virtio_iommu_detach_dev(VirtIOIOMMU *s, viommu_dev *dev)
+{
+    viommu_as *as = dev->as;
+
+    trace_virtio_iommu_detach(dev->id);
+
+    g_tree_remove(s->devices, GUINT_TO_POINTER(dev->id));
+    g_tree_unref(as->mappings);
+}
 
 static int virtio_iommu_attach(VirtIOIOMMU *s,
                                struct virtio_iommu_req_attach *req)
@@ -100,10 +149,42 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
     uint32_t asid = le32_to_cpu(req->address_space);
     uint32_t devid = le32_to_cpu(req->device);
     uint32_t reserved = le32_to_cpu(req->reserved);
+    viommu_as *as;
+    viommu_dev *dev;
+
+    trace_virtio_iommu_attach(asid, devid);
+
+    if (reserved) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+
+    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
+    if (dev) {
+        /*
+         * the device is already attached to an address space,
+         * detach it first
+         */
+         virtio_iommu_detach_dev(s, dev);
+    }
 
-    trace_virtio_iommu_attach(asid, devid, reserved);
+    as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
+    if (!as) {
+        as = g_malloc0(sizeof(*as));
+        as->id = asid;
+        as->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
+                                         NULL, NULL, (GDestroyNotify)g_free);
+        g_tree_insert(s->address_spaces, GUINT_TO_POINTER(asid), as);
+        trace_virtio_iommu_new_asid(asid);
+    }
 
-    return VIRTIO_IOMMU_S_UNSUPP;
+    dev = g_malloc0(sizeof(*dev));
+    dev->as = as;
+    dev->id = devid;
+    trace_virtio_iommu_new_devid(devid);
+    g_tree_insert(s->devices, GUINT_TO_POINTER(devid), dev);
+    g_tree_ref(as->mappings);
+
+    return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_detach(VirtIOIOMMU *s,
@@ -111,10 +192,20 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
 {
     uint32_t devid = le32_to_cpu(req->device);
     uint32_t reserved = le32_to_cpu(req->reserved);
+    viommu_dev *dev;
+
+    if (reserved) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+
+    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
+    if (!dev) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
 
-    trace_virtio_iommu_detach(devid, reserved);
+    virtio_iommu_detach_dev(s, dev);
 
-    return VIRTIO_IOMMU_S_UNSUPP;
+    return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_map(VirtIOIOMMU *s,
@@ -125,10 +216,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
     uint64_t virt_addr = le64_to_cpu(req->virt_addr);
     uint64_t size = le64_to_cpu(req->size);
     uint32_t flags = le32_to_cpu(req->flags);
+    viommu_as *as;
+    viommu_interval *interval;
+    viommu_mapping *mapping;
+
+    interval = g_malloc0(sizeof(*interval));
+
+    interval->low = virt_addr;
+    interval->high = virt_addr + size - 1;
+
+    as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
+    if (!as) {
+        return VIRTIO_IOMMU_S_NOENT;
+    }
+
+    mapping = g_tree_lookup(as->mappings, (gpointer)interval);
+    if (mapping) {
+        g_free(interval);
+        return VIRTIO_IOMMU_S_INVAL;
+    }
 
     trace_virtio_iommu_map(asid, phys_addr, virt_addr, size, flags);
 
-    return VIRTIO_IOMMU_S_UNSUPP;
+    mapping = g_malloc0(sizeof(*mapping));
+    mapping->virt_addr = virt_addr;
+    mapping->phys_addr = phys_addr;
+    mapping->size = size;
+    mapping->flags = flags;
+
+    g_tree_insert(as->mappings, interval, mapping);
+
+    return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_unmap(VirtIOIOMMU *s,
@@ -138,10 +256,64 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
     uint64_t virt_addr = le64_to_cpu(req->virt_addr);
     uint64_t size = le64_to_cpu(req->size);
     uint32_t flags = le32_to_cpu(req->flags);
+    viommu_mapping *mapping;
+    viommu_interval interval;
+    viommu_as *as;
 
     trace_virtio_iommu_unmap(asid, virt_addr, size, flags);
 
-    return VIRTIO_IOMMU_S_UNSUPP;
+    as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
+    if (!as) {
+        error_report("%s: no as", __func__);
+        return VIRTIO_IOMMU_S_NOENT;
+    }
+    interval.low = virt_addr;
+    interval.high = virt_addr + size - 1;
+
+    mapping = g_tree_lookup(as->mappings, (gpointer)&interval);
+
+    while (mapping) {
+        viommu_interval current;
+        uint64_t low  = mapping->virt_addr;
+        uint64_t high = mapping->virt_addr + mapping->size - 1;
+
+        current.low = low;
+        current.high = high;
+
+        if (low == interval.low && size >= mapping->size) {
+            g_tree_remove(as->mappings, (gpointer)&current);
+            interval.low = high + 1;
+            trace_virtio_iommu_unmap_left_interval(current.low, current.high,
+                interval.low, interval.high);
+        } else if (high == interval.high && size >= mapping->size) {
+            trace_virtio_iommu_unmap_right_interval(current.low, current.high,
+                interval.low, interval.high);
+            g_tree_remove(as->mappings, (gpointer)&current);
+            interval.high = low - 1;
+        } else if (low > interval.low && high < interval.high) {
+            trace_virtio_iommu_unmap_inc_interval(current.low, current.high);
+            g_tree_remove(as->mappings, (gpointer)&current);
+        } else {
+            break;
+        }
+        if (interval.low >= interval.high) {
+            return VIRTIO_IOMMU_S_OK;
+        } else {
+            mapping = g_tree_lookup(as->mappings, (gpointer)&interval);
+        }
+    }
+
+    if (mapping) {
+        error_report("****** %s: Unmap 0x%"PRIx64" size=0x%"PRIx64
+                     " from 0x%"PRIx64" size=0x%"PRIx64" is not supported",
+                     __func__, interval.low, size,
+                     mapping->virt_addr, mapping->size);
+    } else {
+        error_report("****** %s: no mapping for [0x%"PRIx64",0x%"PRIx64"]",
+                     __func__, interval.low, interval.high);
+    }
+
+    return VIRTIO_IOMMU_S_INVAL;
 }
 
 #define get_payload_size(req) (\
@@ -271,19 +443,46 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
                                             IOMMUAccessFlags flag)
 {
     IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+    VirtIOIOMMU *s = sdev->viommu;
     uint32_t sid;
+    viommu_dev *dev;
+    viommu_mapping *mapping;
+    viommu_interval interval;
+
+    interval.low = addr;
+    interval.high = addr + 1;
 
     IOMMUTLBEntry entry = {
         .target_as = &address_space_memory,
         .iova = addr,
         .translated_addr = addr,
-        .addr_mask = ~(hwaddr)0,
-        .perm = IOMMU_NONE,
+        .addr_mask = (1 << 12) - 1, /* TODO */
+        .perm = 3,
     };
 
     sid = virtio_iommu_get_sid(sdev);
 
     trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
+    qemu_mutex_lock(&s->mutex);
+
+    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(sid));
+    if (!dev) {
+        /* device cannot be attached to another as */
+        printf("%s sid=%d is not known!!\n", __func__, sid);
+        goto unlock;
+    }
+
+    mapping = g_tree_lookup(dev->as->mappings, (gpointer)&interval);
+    if (!mapping) {
+        printf("%s no mapping for 0x%"PRIx64" for sid=%d\n", __func__,
+               addr, sid);
+        goto unlock;
+    }
+    entry.translated_addr = addr - mapping->virt_addr + mapping->phys_addr,
+    trace_virtio_iommu_translate_result(addr, entry.translated_addr, sid);
+
+unlock:
+    qemu_mutex_unlock(&s->mutex);
     return entry;
 }
 
@@ -347,6 +546,12 @@ static inline guint as_uint64_hash(gconstpointer v)
     return (guint)*(const uint64_t *)v;
 }
 
+static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+    uint ua = GPOINTER_TO_UINT(a);
+    uint ub = GPOINTER_TO_UINT(b);
+    return (ua > ub) - (ua < ub);
+}
 
 static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
 {
@@ -362,17 +567,28 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
     s->config.page_sizes = TARGET_PAGE_MASK;
     s->config.input_range.end = -1UL;
 
+    qemu_mutex_init(&s->mutex);
+
     memset(s->as_by_bus_num, 0, sizeof(s->as_by_bus_num));
     s->as_by_busptr = g_hash_table_new_full(as_uint64_hash,
                                             as_uint64_equal,
                                             g_free, g_free);
 
+    s->address_spaces = g_tree_new_full((GCompareDataFunc)int_cmp,
+                                         NULL, NULL, (GDestroyNotify)g_free);
+    s->devices = g_tree_new_full((GCompareDataFunc)int_cmp,
+                                         NULL, NULL, (GDestroyNotify)g_free);
+
     virtio_iommu_init_as(s);
 }
 
 static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
+
+    g_tree_destroy(s->address_spaces);
+    g_tree_destroy(s->devices);
 
     virtio_cleanup(vdev);
 }
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 7/8] hw/arm/virt: Add 2.10 machine type
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
                   ` (5 preceding siblings ...)
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 6/8] virtio-iommu: Implement the translation and commands Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 8/8] hw/arm/virt: Add virtio-iommu the virt board Eric Auger
  2017-08-17 11:26 ` [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device Linu Cherian
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

The new machine type allows virtio-iommu instantiation.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

a Veuillez saisir le message de validation pour vos modifications. Les lignes
---
 hw/arm/virt.c         | 24 ++++++++++++++++++++++--
 include/hw/arm/virt.h |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 31739d7..93c4ab2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1639,7 +1639,7 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
-static void virt_2_9_instance_init(Object *obj)
+static void virt_2_10_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
@@ -1699,10 +1699,30 @@ static void virt_2_9_instance_init(Object *obj)
     vms->irqmap = a15irqmap;
 }
 
+static void virt_machine_2_10_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(2, 10)
+
+#define VIRT_COMPAT_2_9 \
+    HW_COMPAT_2_9
+
+static void virt_2_9_instance_init(Object *obj)
+{
+    virt_2_10_instance_init(obj);
+}
+
 static void virt_machine_2_9_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
+    virt_machine_2_10_options(mc);
+    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_9);
+
+    vmc->no_iommu = true;
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(2, 9)
+DEFINE_VIRT_MACHINE(2, 9)
+
 
 #define VIRT_COMPAT_2_8 \
     HW_COMPAT_2_8
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 33b0ff3..ff27551 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -84,6 +84,7 @@ typedef struct {
     bool disallow_affinity_adjustment;
     bool no_its;
     bool no_pmu;
+    bool no_iommu;
     bool claim_edge_triggered_timers;
 } VirtMachineClass;
 
-- 
2.5.5

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

* [Qemu-devel] [RFC v3 8/8] hw/arm/virt: Add virtio-iommu the virt board
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
                   ` (6 preceding siblings ...)
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 7/8] hw/arm/virt: Add 2.10 machine type Eric Auger
@ 2017-08-01  9:33 ` Eric Auger
  2017-08-17 11:26 ` [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device Linu Cherian
  8 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2017-08-01  9:33 UTC (permalink / raw)
  To: eric.auger.pro, eric.auger, peter.maydell, alex.williamson, mst,
	qemu-arm, qemu-devel, jean-philippe.brucker
  Cc: will.deacon, kevin.tian, marc.zyngier, christoffer.dall, drjones,
	wei, tn, bharat.bhushan, peterx

The specific virtio-mmio node is inconditionally added on
machine init while the binding between this latter and the
PCIe host bridge is done on machine init done notifier, only
if -device virtio-iommu-device was added to the qemu command
line.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
---
 hw/arm/virt.c         | 92 +++++++++++++++++++++++++++++++++++++++++++++++----
 include/hw/arm/virt.h |  4 +++
 2 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 93c4ab2..9509399 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -52,6 +52,7 @@
 #include "hw/arm/fdt.h"
 #include "hw/intc/arm_gic.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/virtio/virtio-iommu.h"
 #include "kvm_arm.h"
 #include "hw/smbios/smbios.h"
 #include "qapi/visitor.h"
@@ -139,6 +140,7 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
     [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
     [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
+    [VIRT_SMMU] =               { 0x09050000, 0x00000200 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -159,6 +161,7 @@ static const int a15irqmap[] = {
     [VIRT_SECURE_UART] = 8,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
     [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
+    [VIRT_SMMU] = 74,
     [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
 };
 
@@ -991,7 +994,81 @@ static void create_pcie_irq_map(const VirtMachineState *vms,
                            0x7           /* PCI irq */);
 }
 
-static void create_pcie(const VirtMachineState *vms, qemu_irq *pic)
+static int bind_virtio_iommu_device(Object *obj, void *opaque)
+{
+    VirtMachineState *vms = (VirtMachineState *)opaque;
+    struct arm_boot_info *info = &vms->bootinfo;
+    int dtb_size;
+    void *fdt = info->get_dtb(info, &dtb_size);
+    Object *dev;
+
+    dev = object_dynamic_cast(obj, TYPE_VIRTIO_IOMMU);
+
+    if (!dev) {
+        /* Container, traverse it for children */
+        return object_child_foreach(obj, bind_virtio_iommu_device, opaque);
+    }
+
+    qemu_fdt_setprop_cells(fdt, vms->pcie_host_nodename, "iommu-map",
+                           0x0, vms->smmu_phandle, 0x0, 0x10000);
+
+    return true;
+}
+
+static
+void virtio_iommu_notifier(Notifier *notifier, void *data)
+{
+    VirtMachineState *vms = container_of(notifier, VirtMachineState,
+                                         virtio_iommu_done);
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
+    Object *container;
+
+
+    if (vmc->no_iommu) {
+        return;
+    }
+
+    container = container_get(qdev_get_machine(), "/peripheral");
+    bind_virtio_iommu_device(container, vms);
+    container = container_get(qdev_get_machine(), "/peripheral-anon");
+    bind_virtio_iommu_device(container, vms);
+}
+
+static void create_virtio_iommu(VirtMachineState *vms, qemu_irq *pic)
+{
+    char *smmu;
+    const char compat[] = "virtio,mmio";
+    int irq =  vms->irqmap[VIRT_SMMU];
+    hwaddr base = vms->memmap[VIRT_SMMU].base;
+    hwaddr size = vms->memmap[VIRT_SMMU].size;
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
+
+    if (vmc->no_iommu) {
+        return;
+    }
+
+    vms->smmu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
+
+    sysbus_create_simple("virtio-mmio", base, pic[irq]);
+
+    smmu = g_strdup_printf("/virtio_mmio@%" PRIx64, base);
+    qemu_fdt_add_subnode(vms->fdt, smmu);
+    qemu_fdt_setprop(vms->fdt, smmu, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vms->fdt, smmu, "reg", 2, base, 2, size);
+
+    qemu_fdt_setprop_cells(vms->fdt, smmu, "interrupts",
+            GIC_FDT_IRQ_TYPE_SPI, irq, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+
+    qemu_fdt_setprop(vms->fdt, smmu, "dma-coherent", NULL, 0);
+    qemu_fdt_setprop_cell(vms->fdt, smmu, "#iommu-cells", 1);
+    qemu_fdt_setprop_cell(vms->fdt, smmu, "phandle", vms->smmu_phandle);
+    g_free(smmu);
+
+    vms->virtio_iommu_done.notify = virtio_iommu_notifier;
+    qemu_add_machine_init_done_notifier(&vms->virtio_iommu_done);
+}
+
+static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
 {
     hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base;
     hwaddr size_mmio = vms->memmap[VIRT_PCIE_MMIO].size;
@@ -1064,7 +1141,8 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic)
         }
     }
 
-    nodename = g_strdup_printf("/pcie@%" PRIx64, base);
+    vms->pcie_host_nodename = g_strdup_printf("/pcie@%" PRIx64, base);
+    nodename = vms->pcie_host_nodename;
     qemu_fdt_add_subnode(vms->fdt, nodename);
     qemu_fdt_setprop_string(vms->fdt, nodename,
                             "compatible", "pci-host-ecam-generic");
@@ -1103,7 +1181,6 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic)
     qemu_fdt_setprop_cell(vms->fdt, nodename, "#interrupt-cells", 1);
     create_pcie_irq_map(vms, vms->gic_phandle, irq, nodename);
 
-    g_free(nodename);
 }
 
 static void create_platform_bus(VirtMachineState *vms, qemu_irq *pic)
@@ -1448,16 +1525,16 @@ static void machvirt_init(MachineState *machine)
 
     create_rtc(vms, pic);
 
-    create_pcie(vms, pic);
-
-    create_gpio(vms, pic);
-
     /* Create mmio transports, so the user can create virtio backends
      * (which will be automatically plugged in to the transports). If
      * no backend is created the transport will just sit harmlessly idle.
      */
     create_virtio_devices(vms, pic);
 
+    create_pcie(vms, pic);
+
+    create_gpio(vms, pic);
+
     vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
     rom_set_fw(vms->fw_cfg);
 
@@ -1482,6 +1559,7 @@ static void machvirt_init(MachineState *machine)
      * Notifiers are executed in registration reverse order.
      */
     create_platform_bus(vms, pic);
+    create_virtio_iommu(vms, pic);
 }
 
 static bool virt_get_secure(Object *obj, Error **errp)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index ff27551..070cb39 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -59,6 +59,7 @@ enum {
     VIRT_GIC_V2M,
     VIRT_GIC_ITS,
     VIRT_GIC_REDIST,
+    VIRT_SMMU,
     VIRT_UART,
     VIRT_MMIO,
     VIRT_RTC,
@@ -91,6 +92,7 @@ typedef struct {
 typedef struct {
     MachineState parent;
     Notifier machine_done;
+    Notifier virtio_iommu_done;
     FWCfgState *fw_cfg;
     bool secure;
     bool highmem;
@@ -106,6 +108,8 @@ typedef struct {
     uint32_t clock_phandle;
     uint32_t gic_phandle;
     uint32_t msi_phandle;
+    uint32_t smmu_phandle;
+    char *pcie_host_nodename;
     int psci_conduit;
 } VirtMachineState;
 
-- 
2.5.5

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

* Re: [Qemu-devel] [RFC v3 6/8] virtio-iommu: Implement the translation and commands
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 6/8] virtio-iommu: Implement the translation and commands Eric Auger
@ 2017-08-03 10:15   ` Bharat Bhushan
  0 siblings, 0 replies; 15+ messages in thread
From: Bharat Bhushan @ 2017-08-03 10:15 UTC (permalink / raw)
  To: Eric Auger, eric.auger.pro@gmail.com, peter.maydell@linaro.org,
	alex.williamson@redhat.com, mst@redhat.com, qemu-arm@nongnu.org,
	qemu-devel@nongnu.org, jean-philippe.brucker@arm.com
  Cc: will.deacon@arm.com, kevin.tian@intel.com, marc.zyngier@arm.com,
	christoffer.dall@linaro.org, drjones@redhat.com, wei@redhat.com,
	tn@semihalf.com, peterx@redhat.com

Hi Eric,

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: Tuesday, August 01, 2017 3:03 PM
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org; jean-
> philippe.brucker@arm.com
> Cc: will.deacon@arm.com; kevin.tian@intel.com; marc.zyngier@arm.com;
> christoffer.dall@linaro.org; drjones@redhat.com; wei@redhat.com;
> tn@semihalf.com; Bharat Bhushan <bharat.bhushan@nxp.com>;
> peterx@redhat.com
> Subject: [RFC v3 6/8] virtio-iommu: Implement the translation and
> commands
> 
> This patch adds the actual implementation for the translation routine
> and the virtio-iommu commands.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v2 -> v3:
> - init the mutex
> - return VIRTIO_IOMMU_S_INVAL is reserved field is not null on
>   attach/detach commands
> - on attach, when the device is already attached to an address space,
>   detach it first instead of returning an error
> - remove nr_devices and use g_tree_ref/unref to destroy the as->mappings
>   on last device detach
> - map/unmap: return NOENT instead of INVAL error if as does not exist
> - remove flags argument from attach/detach trace functions
> 
> v1 -> v2:
> - fix compilation issue reported by autobuild system
> ---
>  hw/virtio/trace-events   |  10 +-
>  hw/virtio/virtio-iommu.c | 232
> +++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 232 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 341dbdf..8db3d91 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -28,8 +28,14 @@ virtio_balloon_to_target(uint64_t target, uint32_t
> num_pages) "balloon target: %
> 
>  # hw/virtio/virtio-iommu.c
>  #
> -virtio_iommu_attach(uint32_t as, uint32_t dev, uint32_t flags) "as=%d
> dev=%d flags=%d"
> -virtio_iommu_detach(uint32_t dev, uint32_t flags) "dev=%d flags=%d"
> +virtio_iommu_attach(uint32_t as, uint32_t dev) "as=%d dev=%d"
> +virtio_iommu_detach(uint32_t dev) "dev=%d"
>  virtio_iommu_map(uint32_t as, uint64_t phys_addr, uint64_t virt_addr,
> uint64_t size, uint32_t flags) "as= %d phys_addr=0x%"PRIx64"
> virt_addr=0x%"PRIx64" size=0x%"PRIx64" flags=%d"
>  virtio_iommu_unmap(uint32_t as, uint64_t virt_addr, uint64_t size, uint32_t
> reserved) "as= %d virt_addr=0x%"PRIx64" size=0x%"PRIx64" reserved=%d"
>  virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int
> flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
> +virtio_iommu_new_asid(uint32_t asid) "Allocate a new asid=%d"
> +virtio_iommu_new_devid(uint32_t devid) "Allocate a new devid=%d"
> +virtio_iommu_unmap_left_interval(uint64_t low, uint64_t high, uint64_t
> next_low, uint64_t next_high) "Unmap left [0x%"PRIx64",0x%"PRIx64"],
> new interval=[0x%"PRIx64",0x%"PRIx64"]"
> +virtio_iommu_unmap_right_interval(uint64_t low, uint64_t high, uint64_t
> next_low, uint64_t next_high) "Unmap right [0x%"PRIx64",0x%"PRIx64"],
> new interval=[0x%"PRIx64",0x%"PRIx64"]"
> +virtio_iommu_unmap_inc_interval(uint64_t low, uint64_t high) "Unmap inc
> [0x%"PRIx64",0x%"PRIx64"]"
> +virtio_iommu_translate_result(uint64_t virt_addr, uint64_t phys_addr,
> uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index e663d9e..9217587 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -32,10 +32,36 @@
>  #include "hw/virtio/virtio-bus.h"
>  #include "hw/virtio/virtio-access.h"
>  #include "hw/virtio/virtio-iommu.h"
> +#include "hw/pci/pci_bus.h"
> +#include "hw/pci/pci.h"
> 
>  /* Max size */
>  #define VIOMMU_DEFAULT_QUEUE_SIZE 256
> 
> +typedef struct viommu_as viommu_as;
> +
> +typedef struct viommu_mapping {
> +    uint64_t virt_addr;
> +    uint64_t phys_addr;
> +    uint64_t size;
> +    uint32_t flags;
> +} viommu_mapping;
> +
> +typedef struct viommu_interval {
> +    uint64_t low;
> +    uint64_t high;
> +} viommu_interval;
> +
> +typedef struct viommu_dev {
> +    uint32_t id;
> +    viommu_as *as;
> +} viommu_dev;
> +
> +struct viommu_as {
> +    uint32_t id;
> +    GTree *mappings;
> +};
> +
>  static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
>  {
>      return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
> @@ -93,6 +119,29 @@ static void virtio_iommu_init_as(VirtIOIOMMU *s)
>      }
>  }
> 
> +static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer
> user_data)
> +{
> +    viommu_interval *inta = (viommu_interval *)a;
> +    viommu_interval *intb = (viommu_interval *)b;
> +
> +    if (inta->high <= intb->low) {
> +        return -1;
> +    } else if (intb->high <= inta->low) {
> +        return 1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +static void virtio_iommu_detach_dev(VirtIOIOMMU *s, viommu_dev *dev)
> +{
> +    viommu_as *as = dev->as;
> +
> +    trace_virtio_iommu_detach(dev->id);
> +
> +    g_tree_remove(s->devices, GUINT_TO_POINTER(dev->id));
> +    g_tree_unref(as->mappings);
> +}
> 
>  static int virtio_iommu_attach(VirtIOIOMMU *s,
>                                 struct virtio_iommu_req_attach *req)
> @@ -100,10 +149,42 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
>      uint32_t asid = le32_to_cpu(req->address_space);
>      uint32_t devid = le32_to_cpu(req->device);
>      uint32_t reserved = le32_to_cpu(req->reserved);
> +    viommu_as *as;
> +    viommu_dev *dev;
> +
> +    trace_virtio_iommu_attach(asid, devid);
> +
> +    if (reserved) {
> +        return VIRTIO_IOMMU_S_INVAL;
> +    }
> +
> +    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
> +    if (dev) {
> +        /*
> +         * the device is already attached to an address space,
> +         * detach it first
> +         */
> +         virtio_iommu_detach_dev(s, dev);
> +    }
> 
> -    trace_virtio_iommu_attach(asid, devid, reserved);
> +    as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
> +    if (!as) {
> +        as = g_malloc0(sizeof(*as));
> +        as->id = asid;
> +        as->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
> +                                         NULL, NULL, (GDestroyNotify)g_free);

"key" free function is not provided while "value" free function is provided (continue ..)

> +        g_tree_insert(s->address_spaces, GUINT_TO_POINTER(asid), as);
> +        trace_virtio_iommu_new_asid(asid);
> +    }
> 
> -    return VIRTIO_IOMMU_S_UNSUPP;
> +    dev = g_malloc0(sizeof(*dev));
> +    dev->as = as;
> +    dev->id = devid;
> +    trace_virtio_iommu_new_devid(devid);
> +    g_tree_insert(s->devices, GUINT_TO_POINTER(devid), dev);
> +    g_tree_ref(as->mappings);
> +
> +    return VIRTIO_IOMMU_S_OK;
>  }
> 
>  static int virtio_iommu_detach(VirtIOIOMMU *s,
> @@ -111,10 +192,20 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
>  {
>      uint32_t devid = le32_to_cpu(req->device);
>      uint32_t reserved = le32_to_cpu(req->reserved);
> +    viommu_dev *dev;
> +
> +    if (reserved) {
> +        return VIRTIO_IOMMU_S_INVAL;
> +    }
> +
> +    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
> +    if (!dev) {
> +        return VIRTIO_IOMMU_S_INVAL;
> +    }
> 
> -    trace_virtio_iommu_detach(devid, reserved);
> +    virtio_iommu_detach_dev(s, dev);
> 
> -    return VIRTIO_IOMMU_S_UNSUPP;
> +    return VIRTIO_IOMMU_S_OK;
>  }
> 
>  static int virtio_iommu_map(VirtIOIOMMU *s,
> @@ -125,10 +216,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
>      uint64_t virt_addr = le64_to_cpu(req->virt_addr);
>      uint64_t size = le64_to_cpu(req->size);
>      uint32_t flags = le32_to_cpu(req->flags);
> +    viommu_as *as;
> +    viommu_interval *interval;
> +    viommu_mapping *mapping;
> +
> +    interval = g_malloc0(sizeof(*interval));

"Key" is allocated here (continue below ...)

> +
> +    interval->low = virt_addr;
> +    interval->high = virt_addr + size - 1;
> +
> +    as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
> +    if (!as) {
> +        return VIRTIO_IOMMU_S_NOENT;
> +    }
> +
> +    mapping = g_tree_lookup(as->mappings, (gpointer)interval);
> +    if (mapping) {
> +        g_free(interval);
> +        return VIRTIO_IOMMU_S_INVAL;
> +    }
> 
>      trace_virtio_iommu_map(asid, phys_addr, virt_addr, size, flags);
> 
> -    return VIRTIO_IOMMU_S_UNSUPP;
> +    mapping = g_malloc0(sizeof(*mapping));

"Value" for b-tree is allocated here (continue below ..)

> +    mapping->virt_addr = virt_addr;
> +    mapping->phys_addr = phys_addr;
> +    mapping->size = size;
> +    mapping->flags = flags;
> +
> +    g_tree_insert(as->mappings, interval, mapping);
> +
> +    return VIRTIO_IOMMU_S_OK;
>  }
> 
>  static int virtio_iommu_unmap(VirtIOIOMMU *s,
> @@ -138,10 +256,64 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
>      uint64_t virt_addr = le64_to_cpu(req->virt_addr);
>      uint64_t size = le64_to_cpu(req->size);
>      uint32_t flags = le32_to_cpu(req->flags);
> +    viommu_mapping *mapping;
> +    viommu_interval interval;
> +    viommu_as *as;
> 
>      trace_virtio_iommu_unmap(asid, virt_addr, size, flags);
> 
> -    return VIRTIO_IOMMU_S_UNSUPP;
> +    as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
> +    if (!as) {
> +        error_report("%s: no as", __func__);
> +        return VIRTIO_IOMMU_S_NOENT;
> +    }
> +    interval.low = virt_addr;
> +    interval.high = virt_addr + size - 1;
> +
> +    mapping = g_tree_lookup(as->mappings, (gpointer)&interval);
> +
> +    while (mapping) {
> +        viommu_interval current;
> +        uint64_t low  = mapping->virt_addr;
> +        uint64_t high = mapping->virt_addr + mapping->size - 1;
> +
> +        current.low = low;
> +        current.high = high;
> +
> +        if (low == interval.low && size >= mapping->size) {
> +            g_tree_remove(as->mappings, (gpointer)&current);
> +            interval.low = high + 1;
> +            trace_virtio_iommu_unmap_left_interval(current.low, current.high,
> +                interval.low, interval.high);
> +        } else if (high == interval.high && size >= mapping->size) {
> +            trace_virtio_iommu_unmap_right_interval(current.low, current.high,
> +                interval.low, interval.high);
> +            g_tree_remove(as->mappings, (gpointer)&current);
> +            interval.high = low - 1;
> +        } else if (low > interval.low && high < interval.high) {
> +            trace_virtio_iommu_unmap_inc_interval(current.low, current.high);
> +            g_tree_remove(as->mappings, (gpointer)&current);
> +        } else {
> +            break;
> +        }

"value" will be destroyed here but seems like "Key" are not freed.
Looks like memory allocated for "key" are never freed. Hopefully I am understanding it correctly

Thanks
-Bharat

> +        if (interval.low >= interval.high) {
> +            return VIRTIO_IOMMU_S_OK;
> +        } else {
> +            mapping = g_tree_lookup(as->mappings, (gpointer)&interval);
> +        }
> +    }
> +
> +    if (mapping) {
> +        error_report("****** %s: Unmap 0x%"PRIx64" size=0x%"PRIx64
> +                     " from 0x%"PRIx64" size=0x%"PRIx64" is not supported",
> +                     __func__, interval.low, size,
> +                     mapping->virt_addr, mapping->size);
> +    } else {
> +        error_report("****** %s: no mapping for
> [0x%"PRIx64",0x%"PRIx64"]",
> +                     __func__, interval.low, interval.high);
> +    }
> +
> +    return VIRTIO_IOMMU_S_INVAL;
>  }
> 
>  #define get_payload_size(req) (\
> @@ -271,19 +443,46 @@ static IOMMUTLBEntry
> virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>                                              IOMMUAccessFlags flag)
>  {
>      IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
> +    VirtIOIOMMU *s = sdev->viommu;
>      uint32_t sid;
> +    viommu_dev *dev;
> +    viommu_mapping *mapping;
> +    viommu_interval interval;
> +
> +    interval.low = addr;
> +    interval.high = addr + 1;
> 
>      IOMMUTLBEntry entry = {
>          .target_as = &address_space_memory,
>          .iova = addr,
>          .translated_addr = addr,
> -        .addr_mask = ~(hwaddr)0,
> -        .perm = IOMMU_NONE,
> +        .addr_mask = (1 << 12) - 1, /* TODO */
> +        .perm = 3,
>      };
> 
>      sid = virtio_iommu_get_sid(sdev);
> 
>      trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
> +    qemu_mutex_lock(&s->mutex);
> +
> +    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(sid));
> +    if (!dev) {
> +        /* device cannot be attached to another as */
> +        printf("%s sid=%d is not known!!\n", __func__, sid);
> +        goto unlock;
> +    }
> +
> +    mapping = g_tree_lookup(dev->as->mappings, (gpointer)&interval);
> +    if (!mapping) {
> +        printf("%s no mapping for 0x%"PRIx64" for sid=%d\n", __func__,
> +               addr, sid);
> +        goto unlock;
> +    }
> +    entry.translated_addr = addr - mapping->virt_addr + mapping-
> >phys_addr,
> +    trace_virtio_iommu_translate_result(addr, entry.translated_addr, sid);
> +
> +unlock:
> +    qemu_mutex_unlock(&s->mutex);
>      return entry;
>  }
> 
> @@ -347,6 +546,12 @@ static inline guint as_uint64_hash(gconstpointer v)
>      return (guint)*(const uint64_t *)v;
>  }
> 
> +static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> +{
> +    uint ua = GPOINTER_TO_UINT(a);
> +    uint ub = GPOINTER_TO_UINT(b);
> +    return (ua > ub) - (ua < ub);
> +}
> 
>  static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>  {
> @@ -362,17 +567,28 @@ static void
> virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>      s->config.page_sizes = TARGET_PAGE_MASK;
>      s->config.input_range.end = -1UL;
> 
> +    qemu_mutex_init(&s->mutex);
> +
>      memset(s->as_by_bus_num, 0, sizeof(s->as_by_bus_num));
>      s->as_by_busptr = g_hash_table_new_full(as_uint64_hash,
>                                              as_uint64_equal,
>                                              g_free, g_free);
> 
> +    s->address_spaces = g_tree_new_full((GCompareDataFunc)int_cmp,
> +                                         NULL, NULL, (GDestroyNotify)g_free);
> +    s->devices = g_tree_new_full((GCompareDataFunc)int_cmp,
> +                                         NULL, NULL, (GDestroyNotify)g_free);
> +
>      virtio_iommu_init_as(s);
>  }
> 
>  static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
> +
> +    g_tree_destroy(s->address_spaces);
> +    g_tree_destroy(s->devices);
> 
>      virtio_cleanup(vdev);
>  }
> --
> 2.5.5

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

* Re: [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device
  2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
                   ` (7 preceding siblings ...)
  2017-08-01  9:33 ` [Qemu-devel] [RFC v3 8/8] hw/arm/virt: Add virtio-iommu the virt board Eric Auger
@ 2017-08-17 11:26 ` Linu Cherian
  2017-08-17 13:39   ` Jean-Philippe Brucker
  8 siblings, 1 reply; 15+ messages in thread
From: Linu Cherian @ 2017-08-17 11:26 UTC (permalink / raw)
  To: Eric Auger
  Cc: eric.auger.pro, peter.maydell, alex.williamson, mst, qemu-arm,
	qemu-devel, jean-philippe.brucker, kevin.tian, marc.zyngier, tn,
	will.deacon, drjones, peterx, bharat.bhushan, christoffer.dall,
	linu.cherian

Hi Eric,

On Tue Aug 01, 2017 at 11:33:06AM +0200, Eric Auger wrote:
> This series implements the virtio-iommu device.
> 
> This v3 mostly is a rebase on top of v2.10-rc0 that uses
> IOMMUMmeoryRegion plus some small fixes.
> 
> This is a proof of concept based on the virtio-iommu specification
> written by Jean-Philippe Brucker [1].
> 
> The device gets instantiated using the "-device virtio-iommu-device"
> option. It currently works with ARM virt machine only, as the machine
> must handle the dt binding between the virtio-mmio "iommu" node and
> the PCI host bridge node.
> 
> ACPI booting is not yet supported.
> 
> Best Regards
> 
> Eric
> 
> This series can be found at:
> https://github.com/eauger/qemu/tree/v2.10.0-rc0-virtio-iommu-rfcv3
> 
> References:
> [1] [RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
> [2] [RFC PATCH linux] iommu: Add virtio-iommu driver
> [3] [RFC PATCH kvmtool 00/15] Add virtio-iommu
> 
> Testing:
> - >= 4.12 guest kernel + virtio-iommu driver [2]
> - guest using a virtio-net-pci device:
>   ,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on

Was trying to test this out and facing issues.
Guest and Host Kernel - git://linux-arm.org/linux-jpb.git, Branch virtio-iommu/v0.4
Qemu - As mentioned above.

Relevant Qemu command - -device virtio-iommu-device \
-netdev type=tap,id=eth0,ifname=tap0,vhost=off,script=no,downscript=no \
-device virtio-net-pci,netdev=eth0,iommu_platform,disable-modern=off,disable-legacy=on

Issue:
Though guest boots with virtio-iommu device probe succuessful,
virtio net device probe gets failed.

928763] virtio_iommu virtio0: aperture: 0x0-0xffffffffffffffff
[    0.933145] virtio_iommu virtio0: page mask: 0xfffffffffffff000
[    0.937333] virtio_iommu virtio0: probe successful

...

virtio_net: probe of virtio2 failed with error -12"


Even before this failure, i did saw couple of repeated warnings.

73085] ------------[ cut here ]------------
[    2.576330] WARNING: CPU: 0 PID: 49 at drivers/iommu/dma-iommu.c:904 iommu_dma_map_msi_msg+0x1dc/0x1f4
[    2.582963] Modules linked in:
[    2.585210] CPU: 0 PID: 49 Comm: kworker/0:1 Tainted: G        W       4.13.0-rc1-gd1949df-dirty #14
[    2.591733] Hardware name: linux,dummy-virt (DT)
[    2.595062] Workqueue: events deferred_probe_work_func
[    2.598777] task: ffff80003e0c0d80 task.stack: ffff80003e104000
[    2.603036] PC is at iommu_dma_map_msi_msg+0x1dc/0x1f4
[    2.606745] LR is at iommu_dma_map_msi_msg+0x1dc/0x1f4
....
snip
....
   2.735988] [<ffff000008658b18>] iommu_dma_map_msi_msg+0x1dc/0x1f4
[    2.740429] [<ffff00000858846c>] its_irq_compose_msi_msg+0x5c/0x68
[    2.744869] [<ffff00000811e3f4>] irq_chip_compose_msi_msg+0x5c/0x74
[    2.749378] [<ffff000008123d04>] msi_domain_activate+0x24/0x4c
[    2.753577] [<ffff0000081202fc>] __irq_domain_activate_irq+0x48/0x54
[    2.758151] [<ffff00000812231c>] irq_domain_activate_irq+0x2c/0x48
[    2.762593] [<ffff0000081242cc>] msi_domain_alloc_irqs+0x150/0x214
[    2.767038] [<ffff0000085c4edc>] pci_msi_setup_msi_irqs+0x5c/0x68
[    2.771560] [<ffff0000085c5544>] __pci_enable_msix+0x41c/0x4c0
[    2.775778] [<ffff0000085c5d1c>] pci_alloc_irq_vectors_affinity+0xac/0x160
[    2.780709] [<ffff0000085ff6e4>] vp_find_vqs_msix+0x148/0x440
[    2.784848] [<ffff0000085ffa8c>] vp_find_vqs+0xb0/0x1b8
[    2.788624] [<ffff0000085fe798>] vp_modern_find_vqs+0x50/0xa8
[    2.792768] [<ffff000008723454>] init_vqs+0x394/0x600
[    2.796417] [<ffff000008723b50>] virtnet_probe+0x3c4/0x77c
[    2.800370] [<ffff0000085fa738>] virtio_dev_probe+0x198/0x230
[    2.804507] [<ffff00000866aa38>] driver_probe_device+0x298/0x440
[    2.808827] [<ffff00000866ae00>] __device_attach_driver+0x9c/0x140
[    2.813269] [<ffff0000086686c8>] bus_for_each_drv+0x68/0xa8
[    2.817283] [<ffff00000866a5e4>] __device_attach+0xcc/0x15c
[    2.821299] [<ffff00000866aec8>] device_initial_probe+0x24/0x30
[    2.825558] [<ffff00000866994c>] bus_probe_device+0x9c/0xa4
[    2.829574] [<ffff000008667328>] device_add+0x378/0x5c8
[    2.833344] [<ffff0000086675a0>] device_register+0x28/0x34
[    2.837302] [<ffff0000085fa22c>] register_virtio_device+0xbc/0x10c
[    2.841747] [<ffff0000085ff2cc>] virtio_pci_probe+0xdc/0x14c
[    2.845822] [<ffff0000085abf8c>] local_pci_probe+0x50/0xb4
[    2.849784] [<ffff0000085acd8c>] pci_device_probe+0x168/0x178
[    2.853926] [<ffff00000866aa38>] driver_probe_device+0x298/0x440
[    2.858243] [<ffff00000866ae00>] __device_attach_driver+0x9c/0x140
[    2.862681] [<ffff0000086686c8>] bus_for_each_drv+0x68/0xa8
[    2.866692] [<ffff00000866a5e4>] __device_attach+0xcc/0x15c
[    2.870707] [<ffff00000866aec8>] device_initial_probe+0x24/0x30
[    2.874966] [<ffff00000866994c>] bus_probe_device+0x9c/0xa4
[    2.878982] [<ffff000008669f68>] deferred_probe_work_func+0xa0/0xec
[    2.883487] [<ffff0000080db09c>] process_one_work+0x160/0x384
[    2.887623] [<ffff0000080db4b4>] worker_thread+0x1f4/0x3d8
[    2.891577] [<ffff0000080e1b48>] kthread+0x10c/0x138
[    2.895165] [<ffff0000080833b0>] ret_from_fork+0x10/0x20


Am i missing something ?


> 
> History:
> v2 -> v3:
> - rebase on top of 2.10-rc0 and especially
>   [PATCH qemu v9 0/2] memory/iommu: QOM'fy IOMMU MemoryRegion
> - add mutex init
> - fix as->mappings deletion using g_tree_ref/unref
> - when a dev is attached whereas it is already attached to
>   another address space, first detach it
> - fix some error values
> - page_sizes = TARGET_PAGE_MASK;
> - I haven't changed the unmap() semantics yet, waiting for the
>   next virtio-iommu spec revision.
> 
> v1 -> v2:
> - fix redifinition of viommu_as typedef
> 
> 
> Eric Auger (8):
>   update-linux-headers: import virtio_iommu.h
>   linux-headers: Update for virtio-iommu
>   virtio_iommu: add skeleton
>   virtio-iommu: Decode the command payload
>   virtio_iommu: Add the iommu regions
>   virtio-iommu: Implement the translation and commands
>   hw/arm/virt: Add 2.10 machine type
>   hw/arm/virt: Add virtio-iommu the virt board
> 
>  hw/arm/virt.c                                 | 116 ++++-
>  hw/virtio/Makefile.objs                       |   1 +
>  hw/virtio/trace-events                        |  14 +
>  hw/virtio/virtio-iommu.c                      | 670 ++++++++++++++++++++++++++
>  include/hw/arm/virt.h                         |   5 +
>  include/hw/virtio/virtio-iommu.h              |  61 +++
>  include/standard-headers/linux/virtio_ids.h   |   1 +
>  include/standard-headers/linux/virtio_iommu.h | 142 ++++++
>  linux-headers/linux/virtio_iommu.h            |   1 +
>  scripts/update-linux-headers.sh               |   3 +
>  10 files changed, 1005 insertions(+), 9 deletions(-)
>  create mode 100644 hw/virtio/virtio-iommu.c
>  create mode 100644 include/hw/virtio/virtio-iommu.h
>  create mode 100644 include/standard-headers/linux/virtio_iommu.h
>  create mode 100644 linux-headers/linux/virtio_iommu.h
> 
> -- 
> 2.5.5
> 
> 

-- 
Linu cherian

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

* Re: [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device
  2017-08-17 11:26 ` [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device Linu Cherian
@ 2017-08-17 13:39   ` Jean-Philippe Brucker
  2017-08-17 15:26     ` Auger Eric
  0 siblings, 1 reply; 15+ messages in thread
From: Jean-Philippe Brucker @ 2017-08-17 13:39 UTC (permalink / raw)
  To: Linu Cherian, Eric Auger
  Cc: eric.auger.pro, peter.maydell, alex.williamson, mst, qemu-arm,
	qemu-devel, kevin.tian, marc.zyngier, tn, will.deacon, drjones,
	peterx, bharat.bhushan, christoffer.dall, linu.cherian

Hi Linu,

On 17/08/17 12:26, Linu Cherian wrote:
> Hi Eric,
> 
> On Tue Aug 01, 2017 at 11:33:06AM +0200, Eric Auger wrote:
>> This series implements the virtio-iommu device.
>>
>> This v3 mostly is a rebase on top of v2.10-rc0 that uses
>> IOMMUMmeoryRegion plus some small fixes.
>>
>> This is a proof of concept based on the virtio-iommu specification
>> written by Jean-Philippe Brucker [1].
>>
>> The device gets instantiated using the "-device virtio-iommu-device"
>> option. It currently works with ARM virt machine only, as the machine
>> must handle the dt binding between the virtio-mmio "iommu" node and
>> the PCI host bridge node.
>>
>> ACPI booting is not yet supported.
>>
>> Best Regards
>>
>> Eric
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/v2.10.0-rc0-virtio-iommu-rfcv3
>>
>> References:
>> [1] [RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
>> [2] [RFC PATCH linux] iommu: Add virtio-iommu driver
>> [3] [RFC PATCH kvmtool 00/15] Add virtio-iommu
>>
>> Testing:
>> - >= 4.12 guest kernel + virtio-iommu driver [2]
>> - guest using a virtio-net-pci device:
>>   ,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on
> 
> Was trying to test this out and facing issues.
> Guest and Host Kernel - git://linux-arm.org/linux-jpb.git, Branch virtio-iommu/v0.4
> Qemu - As mentioned above.

Could you try branch virtio-iommu/v0.1? It contains the UAPI headers
compatible with this RFC.

Thanks,
Jean

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

* Re: [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device
  2017-08-17 13:39   ` Jean-Philippe Brucker
@ 2017-08-17 15:26     ` Auger Eric
  2017-08-17 15:36       ` Jean-Philippe Brucker
  2017-08-17 16:22       ` Linu Cherian
  0 siblings, 2 replies; 15+ messages in thread
From: Auger Eric @ 2017-08-17 15:26 UTC (permalink / raw)
  To: Jean-Philippe Brucker, Linu Cherian
  Cc: peter.maydell, kevin.tian, drjones, mst, marc.zyngier, tn,
	will.deacon, qemu-devel, peterx, alex.williamson, qemu-arm,
	linu.cherian, bharat.bhushan, christoffer.dall, eric.auger.pro

Hi Linu, Jean,

On 17/08/2017 15:39, Jean-Philippe Brucker wrote:
> Hi Linu,
> 
> On 17/08/17 12:26, Linu Cherian wrote:
>> Hi Eric,
>>
>> On Tue Aug 01, 2017 at 11:33:06AM +0200, Eric Auger wrote:
>>> This series implements the virtio-iommu device.
>>>
>>> This v3 mostly is a rebase on top of v2.10-rc0 that uses
>>> IOMMUMmeoryRegion plus some small fixes.
>>>
>>> This is a proof of concept based on the virtio-iommu specification
>>> written by Jean-Philippe Brucker [1].
>>>
>>> The device gets instantiated using the "-device virtio-iommu-device"
>>> option. It currently works with ARM virt machine only, as the machine
>>> must handle the dt binding between the virtio-mmio "iommu" node and
>>> the PCI host bridge node.
>>>
>>> ACPI booting is not yet supported.
>>>
>>> Best Regards
>>>
>>> Eric
>>>
>>> This series can be found at:
>>> https://github.com/eauger/qemu/tree/v2.10.0-rc0-virtio-iommu-rfcv3
>>>
>>> References:
>>> [1] [RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
>>> [2] [RFC PATCH linux] iommu: Add virtio-iommu driver
>>> [3] [RFC PATCH kvmtool 00/15] Add virtio-iommu
>>>
>>> Testing:
>>> - >= 4.12 guest kernel + virtio-iommu driver [2]
>>> - guest using a virtio-net-pci device:
>>>   ,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on
>>
>> Was trying to test this out and facing issues.
>> Guest and Host Kernel - git://linux-arm.org/linux-jpb.git, Branch virtio-iommu/v0.4
>> Qemu - As mentioned above.
> 
> Could you try branch virtio-iommu/v0.1? It contains the UAPI headers
> compatible with this RFC.
Thank you Jean. Yes the QEMU virtio-iommu device is based on the first
user API written in [2]. I plan to rebase on v0.4 in short delay. Jean
can I rebase on virtio-iommu/v0.4 or shall I wait a bit more?

Thanks

Eric
> 
> Thanks,
> Jean
> 

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

* Re: [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device
  2017-08-17 15:26     ` Auger Eric
@ 2017-08-17 15:36       ` Jean-Philippe Brucker
  2017-08-17 16:22       ` Linu Cherian
  1 sibling, 0 replies; 15+ messages in thread
From: Jean-Philippe Brucker @ 2017-08-17 15:36 UTC (permalink / raw)
  To: Auger Eric, Linu Cherian
  Cc: peter.maydell, kevin.tian, drjones, mst, marc.zyngier, tn,
	will.deacon, qemu-devel, peterx, alex.williamson, qemu-arm,
	linu.cherian, bharat.bhushan, christoffer.dall, eric.auger.pro

On 17/08/17 16:26, Auger Eric wrote:
> Hi Linu, Jean,
> 
> On 17/08/2017 15:39, Jean-Philippe Brucker wrote:
>> Hi Linu,
>>
>> On 17/08/17 12:26, Linu Cherian wrote:
>>> Hi Eric,
>>>
>>> On Tue Aug 01, 2017 at 11:33:06AM +0200, Eric Auger wrote:
>>>> This series implements the virtio-iommu device.
>>>>
>>>> This v3 mostly is a rebase on top of v2.10-rc0 that uses
>>>> IOMMUMmeoryRegion plus some small fixes.
>>>>
>>>> This is a proof of concept based on the virtio-iommu specification
>>>> written by Jean-Philippe Brucker [1].
>>>>
>>>> The device gets instantiated using the "-device virtio-iommu-device"
>>>> option. It currently works with ARM virt machine only, as the machine
>>>> must handle the dt binding between the virtio-mmio "iommu" node and
>>>> the PCI host bridge node.
>>>>
>>>> ACPI booting is not yet supported.
>>>>
>>>> Best Regards
>>>>
>>>> Eric
>>>>
>>>> This series can be found at:
>>>> https://github.com/eauger/qemu/tree/v2.10.0-rc0-virtio-iommu-rfcv3
>>>>
>>>> References:
>>>> [1] [RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
>>>> [2] [RFC PATCH linux] iommu: Add virtio-iommu driver
>>>> [3] [RFC PATCH kvmtool 00/15] Add virtio-iommu
>>>>
>>>> Testing:
>>>> - >= 4.12 guest kernel + virtio-iommu driver [2]
>>>> - guest using a virtio-net-pci device:
>>>>   ,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on
>>>
>>> Was trying to test this out and facing issues.
>>> Guest and Host Kernel - git://linux-arm.org/linux-jpb.git, Branch virtio-iommu/v0.4
>>> Qemu - As mentioned above.
>>
>> Could you try branch virtio-iommu/v0.1? It contains the UAPI headers
>> compatible with this RFC.
> Thank you Jean. Yes the QEMU virtio-iommu device is based on the first
> user API written in [2]. I plan to rebase on v0.4 in short delay. Jean
> can I rebase on virtio-iommu/v0.4 or shall I wait a bit more?

Please go ahead, I don't have any pending changes at the moment :)

Thanks,
Jean

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

* Re: [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device
  2017-08-17 15:26     ` Auger Eric
  2017-08-17 15:36       ` Jean-Philippe Brucker
@ 2017-08-17 16:22       ` Linu Cherian
  1 sibling, 0 replies; 15+ messages in thread
From: Linu Cherian @ 2017-08-17 16:22 UTC (permalink / raw)
  To: Auger Eric
  Cc: Jean-Philippe Brucker, Linu Cherian, peter.maydell, kevin.tian,
	drjones, mst, marc.zyngier, tn, will.deacon, qemu-devel, peterx,
	alex.williamson, qemu-arm, bharat.bhushan, christoffer.dall,
	eric.auger.pro

On Thu Aug 17, 2017 at 05:26:53PM +0200, Auger Eric wrote:
> Hi Linu, Jean,
> 
> On 17/08/2017 15:39, Jean-Philippe Brucker wrote:
> > Hi Linu,
> > 
> > On 17/08/17 12:26, Linu Cherian wrote:
> >> Hi Eric,
> >>
> >> On Tue Aug 01, 2017 at 11:33:06AM +0200, Eric Auger wrote:
> >>> This series implements the virtio-iommu device.
> >>>
> >>> This v3 mostly is a rebase on top of v2.10-rc0 that uses
> >>> IOMMUMmeoryRegion plus some small fixes.
> >>>
> >>> This is a proof of concept based on the virtio-iommu specification
> >>> written by Jean-Philippe Brucker [1].
> >>>
> >>> The device gets instantiated using the "-device virtio-iommu-device"
> >>> option. It currently works with ARM virt machine only, as the machine
> >>> must handle the dt binding between the virtio-mmio "iommu" node and
> >>> the PCI host bridge node.
> >>>
> >>> ACPI booting is not yet supported.
> >>>
> >>> Best Regards
> >>>
> >>> Eric
> >>>
> >>> This series can be found at:
> >>> https://github.com/eauger/qemu/tree/v2.10.0-rc0-virtio-iommu-rfcv3
> >>>
> >>> References:
> >>> [1] [RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
> >>> [2] [RFC PATCH linux] iommu: Add virtio-iommu driver
> >>> [3] [RFC PATCH kvmtool 00/15] Add virtio-iommu
> >>>
> >>> Testing:
> >>> - >= 4.12 guest kernel + virtio-iommu driver [2]
> >>> - guest using a virtio-net-pci device:
> >>>   ,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on
> >>
> >> Was trying to test this out and facing issues.
> >> Guest and Host Kernel - git://linux-arm.org/linux-jpb.git, Branch virtio-iommu/v0.4
> >> Qemu - As mentioned above.
> > 
> > Could you try branch virtio-iommu/v0.1? It contains the UAPI headers
> > compatible with this RFC.
> Thank you Jean. Yes the QEMU virtio-iommu device is based on the first
> user API written in [2]. I plan to rebase on v0.4 in short delay. Jean
> can I rebase on virtio-iommu/v0.4 or shall I wait a bit more?
> 
> Thanks

Yes, virtio-iommu/v0.1 works for me. Thanks.

> 
> Eric
> > 
> > Thanks,
> > Jean
> > 

-- 
Linu cherian

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

end of thread, other threads:[~2017-08-17 16:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-01  9:33 [Qemu-devel] [RFC v3 0/8] VIRTIO-IOMMU device Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 1/8] update-linux-headers: import virtio_iommu.h Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 2/8] linux-headers: Update for virtio-iommu Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 3/8] virtio_iommu: add skeleton Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 4/8] virtio-iommu: Decode the command payload Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 5/8] virtio_iommu: Add the iommu regions Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 6/8] virtio-iommu: Implement the translation and commands Eric Auger
2017-08-03 10:15   ` Bharat Bhushan
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 7/8] hw/arm/virt: Add 2.10 machine type Eric Auger
2017-08-01  9:33 ` [Qemu-devel] [RFC v3 8/8] hw/arm/virt: Add virtio-iommu the virt board Eric Auger
2017-08-17 11:26 ` [Qemu-devel] [Qemu-arm] [RFC v3 0/8] VIRTIO-IOMMU device Linu Cherian
2017-08-17 13:39   ` Jean-Philippe Brucker
2017-08-17 15:26     ` Auger Eric
2017-08-17 15:36       ` Jean-Philippe Brucker
2017-08-17 16:22       ` Linu Cherian

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).