[RFC v2 0/8] VIRTIO-IOMMU device

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 0/8] VIRTIO-IOMMU device

Eric Auger-2
This series implements the virtio-iommu device. This is a proof
of concept based on the virtio-iommu specification written by
Jean-Philippe Brucker [1]. This was tested with a guest using
the virtio-iommu driver [2] and exposed with a virtio-net-pci
using dma ops.

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.

This should allow to start some benchmarking activities against
pure emulated IOMMU (especially ARM SMMU).

Best Regards

Eric

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

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

History:
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                      | 623 ++++++++++++++++++++++++++
 include/hw/arm/virt.h                         |   5 +
 include/hw/virtio/virtio-iommu.h              |  60 +++
 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, 957 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 1/8] update-linux-headers: import virtio_iommu.h

Eric Auger-2
Update the script to update the virtio_iommu.h header.

Signed-off-by: Eric Auger <[hidden email]>
---
 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 2/8] linux-headers: Update for virtio-iommu

Eric Auger-2
In reply to this post by Eric Auger-2
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 <[hidden email]>
---
 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 3/8] virtio_iommu: add skeleton

Eric Auger-2
In reply to this post by Eric Auger-2
This patchs adds the skeleton for the virtio-iommu device.

Signed-off-by: Eric Auger <[hidden email]>
---
 hw/virtio/Makefile.objs          |   1 +
 hw/virtio/virtio-iommu.c         | 247 +++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-iommu.h |  60 ++++++++++
 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..86129ef
--- /dev/null
+++ b/hw/virtio/virtio-iommu.c
@@ -0,0 +1,247 @@
+/*
+ * 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);
+    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 = ~((1ULL << 12) - 1);
+    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..2259413
--- /dev/null
+++ b/include/hw/virtio/virtio-iommu.h
@@ -0,0 +1,60 @@
+/*
+ * 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;
+    MemoryRegion  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;
+    MemoryRegionIOMMUOps iommu_ops;
+    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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 4/8] virtio-iommu: Decode the command payload

Eric Auger-2
In reply to this post by Eric Auger-2
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 <[hidden email]>
---
 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 86129ef..ea1caa7 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 5/8] virtio_iommu: Add the iommu regions

Eric Auger-2
In reply to this post by Eric Auger-2
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 <[hidden email]>
---
 hw/virtio/trace-events   |  1 +
 hw/virtio/virtio-iommu.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 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 ea1caa7..902c779 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,59 @@
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
 
+static inline uint16_t smmu_get_sid(IOMMUDevice *dev)
+{
+    return  ((pci_bus_num(dev->bus) & 0xff) << 8) | 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) {
+        sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
+
+        sdev->viommu = s;
+        sdev->bus = bus;
+        sdev->devfn = devfn;
+
+        memory_region_init_iommu(&sdev->iommu_mr, OBJECT(s),
+                                 &s->iommu_ops, TYPE_VIRTIO_IOMMU,
+                                 UINT64_MAX);
+        address_space_init(&sdev->as, &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 +262,26 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static IOMMUTLBEntry virtio_iommu_translate(MemoryRegion *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 = smmu_get_sid(sdev);
+
+    trace_virtio_iommu_translate(mr->name, sid, addr, flag);
+    return entry;
+}
+
 static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
@@ -253,6 +327,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);
@@ -266,6 +355,14 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
 
     s->config.page_sizes = ~((1ULL << 12) - 1);
     s->config.input_range.end = -1UL;
+
+    s->iommu_ops.translate = virtio_iommu_translate;
+    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)
--
2.5.5


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 6/8] virtio-iommu: Implement the translation and commands

Eric Auger-2
In reply to this post by Eric Auger-2
This patch adds the actual implementation for the translation routine
and the virtio-iommu commands.

Signed-off-by: Eric Auger <[hidden email]>

---

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

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 341dbdf..9196b63 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -33,3 +33,9 @@ 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"
+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 902c779..cd188fc 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -32,10 +32,37 @@
 #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;
+    uint32_t nr_devices;
+    GTree *mappings;
+};
+
 static inline uint16_t smmu_get_sid(IOMMUDevice *dev)
 {
     return  ((pci_bus_num(dev->bus) & 0xff) << 8) | dev->devfn;
@@ -88,6 +115,19 @@ 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 int virtio_iommu_attach(VirtIOIOMMU *s,
                                struct virtio_iommu_req_attach *req)
@@ -95,10 +135,34 @@ 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, reserved);
 
-    return VIRTIO_IOMMU_S_UNSUPP;
+    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
+    if (dev) {
+        return -1;
+    }
+
+    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);
+    }
+
+    dev = g_malloc0(sizeof(*dev));
+    dev->as = as;
+    dev->id = devid;
+    as->nr_devices++;
+    trace_virtio_iommu_new_devid(devid);
+    g_tree_insert(s->devices, GUINT_TO_POINTER(devid), dev);
+
+    return VIRTIO_IOMMU_S_OK;
 }
 
 static int virtio_iommu_detach(VirtIOIOMMU *s,
@@ -106,10 +170,13 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
 {
     uint32_t devid = le32_to_cpu(req->device);
     uint32_t reserved = le32_to_cpu(req->reserved);
+    int ret;
 
     trace_virtio_iommu_detach(devid, reserved);
 
-    return VIRTIO_IOMMU_S_UNSUPP;
+    ret = g_tree_remove(s->devices, GUINT_TO_POINTER(devid));
+
+    return ret ? VIRTIO_IOMMU_S_OK : VIRTIO_IOMMU_S_INVAL;
 }
 
 static int virtio_iommu_map(VirtIOIOMMU *s,
@@ -120,10 +187,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_INVAL;
+    }
+
+    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,
@@ -133,10 +227,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_INVAL;
+    }
+    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) (\
@@ -266,19 +414,46 @@ static IOMMUTLBEntry virtio_iommu_translate(MemoryRegion *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 = smmu_get_sid(sdev);
 
     trace_virtio_iommu_translate(mr->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;
 }
 
@@ -341,6 +516,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,12 +543,21 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
                                             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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 7/8] hw/arm/virt: Add 2.10 machine type

Eric Auger-2
In reply to this post by Eric Auger-2
The new machine type allows virtio-iommu instantiation.

Signed-off-by: Eric Auger <[hidden email]>

---

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 010f724..6eb0d2a 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[RFC v2 8/8] hw/arm/virt: Add virtio-iommu the virt board

Eric Auger-2
In reply to this post by Eric Auger-2
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 <[hidden email]>

---
---
 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 6eb0d2a..6bcfbcd 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 3/8] virtio_iommu: add skeleton

Bharat Bhushan-2
In reply to this post by Eric Auger-2


> -----Original Message-----
> From: Eric Auger [mailto:[hidden email]]
> Sent: Wednesday, June 07, 2017 9:31 PM
> To: [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; jean-
> [hidden email]
> Cc: [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; Bharat Bhushan
> <[hidden email]>
> Subject: [RFC v2 3/8] virtio_iommu: add skeleton
>
> This patchs adds the skeleton for the virtio-iommu device.
>
> Signed-off-by: Eric Auger <[hidden email]>
> ---
>  hw/virtio/Makefile.objs          |   1 +
>  hw/virtio/virtio-iommu.c         | 247
> +++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-iommu.h |  60 ++++++++++
>  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..86129ef
> --- /dev/null
> +++ b/hw/virtio/virtio-iommu.c
> @@ -0,0 +1,247 @@
> +/*
> + * 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);
> +    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 = ~((1ULL << 12) - 1);

This is hardcoded to 4K, Should this be aligned to Host-page size ?

Thanks
-Bharat

> +    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..2259413
> --- /dev/null
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -0,0 +1,60 @@
> +/*
> + * 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;
> +    MemoryRegion  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;
> +    MemoryRegionIOMMUOps iommu_ops;
> +    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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Bharat Bhushan-2
In reply to this post by Eric Auger-2
Hi Eric,

> -----Original Message-----
> From: Eric Auger [mailto:[hidden email]]
> Sent: Wednesday, June 07, 2017 9:31 PM
> To: [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; jean-
> [hidden email]
> Cc: [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; Bharat Bhushan
> <[hidden email]>
> Subject: [RFC v2 0/8] VIRTIO-IOMMU device
>
> This series implements the virtio-iommu device. This is a proof of concept
> based on the virtio-iommu specification written by Jean-Philippe Brucker [1].
> This was tested with a guest using the virtio-iommu driver [2] and exposed
> with a virtio-net-pci using dma ops.
>
> 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.
>
> This should allow to start some benchmarking activities against pure
> emulated IOMMU (especially ARM SMMU).

I am testing this on ARM64 and see below continuous error prints:

        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!
        virtio_iommu_translate sid=8 is not known!!


Also in guest I do not see device-tree node with virtio-iommu.
I am using qemu-tree you mentioned below and iommu-driver patches published by Jean-P.
Qemu command line have additional ""-device virtio-iommu-device". What I am missing ?

Thanks
-Bharat

>
> Best Regards
>
> Eric
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/virtio-iommu-rfcv2
>
> 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
>
> History:
> 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                      | 623 ++++++++++++++++++++++++++
>  include/hw/arm/virt.h                         |   5 +
>  include/hw/virtio/virtio-iommu.h              |  60 +++
>  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, 957 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Eric Auger-2
Hi Bharat,

On 09/06/2017 08:16, Bharat Bhushan wrote:

> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger [mailto:[hidden email]]
>> Sent: Wednesday, June 07, 2017 9:31 PM
>> To: [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; jean-
>> [hidden email]
>> Cc: [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; Bharat Bhushan
>> <[hidden email]>
>> Subject: [RFC v2 0/8] VIRTIO-IOMMU device
>>
>> This series implements the virtio-iommu device. This is a proof of concept
>> based on the virtio-iommu specification written by Jean-Philippe Brucker [1].
>> This was tested with a guest using the virtio-iommu driver [2] and exposed
>> with a virtio-net-pci using dma ops.
>>
>> 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.
>>
>> This should allow to start some benchmarking activities against pure
>> emulated IOMMU (especially ARM SMMU).
>
> I am testing this on ARM64 and see below continuous error prints:
>
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
> virtio_iommu_translate sid=8 is not known!!
>
>
> Also in guest I do not see device-tree node with virtio-iommu.
do you mean the virtio-mmio with #iommu-cells property?

This one is created statically by virt machine. I would be surprised if
it were not there. Are you using the virt = virt2.10 machine. Machines
before do not support its instantiation.

Please can you add a printf in hw/arm/virt.c create_virtio_mmio() at the
moment when this node is created. Also you can add a printf in
bind_virtio_iommu_device() to make sure the binding with the PCI host
bridge is added on machine init done.

Also worth to check, CONFIG_VIRTIO_IOMMU=y on guest side.

Thanks

Eric

> I am using qemu-tree you mentioned below and iommu-driver patches published by Jean-P.
> Qemu command line have additional ""-device virtio-iommu-device". What I am missing ?


>
> Thanks
> -Bharat
>
>>
>> Best Regards
>>
>> Eric
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/virtio-iommu-rfcv2
>>
>> 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
>>
>> History:
>> 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                      | 623 ++++++++++++++++++++++++++
>>  include/hw/arm/virt.h                         |   5 +
>>  include/hw/virtio/virtio-iommu.h              |  60 +++
>>  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, 957 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Bharat Bhushan-2
Hi Eric,

> -----Original Message-----
> From: Auger Eric [mailto:[hidden email]]
> Sent: Friday, June 09, 2017 12:14 PM
> To: Bharat Bhushan <[hidden email]>;
> [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]
> Cc: [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]
> Subject: Re: [RFC v2 0/8] VIRTIO-IOMMU device
>
> Hi Bharat,
>
> On 09/06/2017 08:16, Bharat Bhushan wrote:
> > Hi Eric,
> >
> >> -----Original Message-----
> >> From: Eric Auger [mailto:[hidden email]]
> >> Sent: Wednesday, June 07, 2017 9:31 PM
> >> To: [hidden email]; [hidden email];
> >> [hidden email]; [hidden email];
> [hidden email];
> >> [hidden email]; [hidden email]; jean-
> >> [hidden email]
> >> Cc: [hidden email]; [hidden email];
> [hidden email];
> >> [hidden email]; [hidden email];
> >> [hidden email]; [hidden email]; [hidden email]; Bharat
> Bhushan
> >> <[hidden email]>
> >> Subject: [RFC v2 0/8] VIRTIO-IOMMU device
> >>
> >> This series implements the virtio-iommu device. This is a proof of
> >> concept based on the virtio-iommu specification written by Jean-Philippe
> Brucker [1].
> >> This was tested with a guest using the virtio-iommu driver [2] and
> >> exposed with a virtio-net-pci using dma ops.
> >>
> >> 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.
> >>
> >> This should allow to start some benchmarking activities against pure
> >> emulated IOMMU (especially ARM SMMU).
> >
> > I am testing this on ARM64 and see below continuous error prints:
> >
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> > virtio_iommu_translate sid=8 is not known!!
> >
> >
> > Also in guest I do not see device-tree node with virtio-iommu.
> do you mean the virtio-mmio with #iommu-cells property?
>
> This one is created statically by virt machine. I would be surprised if it were
> not there. Are you using the virt = virt2.10 machine. Machines before do not
> support its instantiation.
>
> Please can you add a printf in hw/arm/virt.c create_virtio_mmio() at the
> moment when this node is created. Also you can add a printf in
> bind_virtio_iommu_device() to make sure the binding with the PCI host
> bridge is added on machine init done.
>
> Also worth to check, CONFIG_VIRTIO_IOMMU=y on guest side.

It works on my side. The driver config was disabled and also I was using guest kernel which was not have deferred-probing. Now after fixing it works on my side
I placed some prints to see dma-map are mapping regions in virtio-iommu, it uses emulated iommu.

I will continue to add VFIO support now on this and more testing !!

Thanks
-Bharat

>
> Thanks
>
> Eric
>
> > I am using qemu-tree you mentioned below and iommu-driver patches
> published by Jean-P.
> > Qemu command line have additional ""-device virtio-iommu-device". What
> I am missing ?
>
>
> >
> > Thanks
> > -Bharat
> >
> >>
> >> Best Regards
> >>
> >> Eric
> >>
> >> This series can be found at:
> >> https://github.com/eauger/qemu/tree/virtio-iommu-rfcv2
> >>
> >> 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
> >>
> >> History:
> >> 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                      | 623
> ++++++++++++++++++++++++++
> >>  include/hw/arm/virt.h                         |   5 +
> >>  include/hw/virtio/virtio-iommu.h              |  60 +++
> >>  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, 957 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
> >

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Eric Auger-2
Hi Bharat,

On 09/06/2017 13:30, Bharat Bhushan wrote:

> Hi Eric,
>
>> -----Original Message-----
>> From: Auger Eric [mailto:[hidden email]]
>> Sent: Friday, June 09, 2017 12:14 PM
>> To: Bharat Bhushan <[hidden email]>;
>> [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]
>> Cc: [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]
>> Subject: Re: [RFC v2 0/8] VIRTIO-IOMMU device
>>
>> Hi Bharat,
>>
>> On 09/06/2017 08:16, Bharat Bhushan wrote:
>>> Hi Eric,
>>>
>>>> -----Original Message-----
>>>> From: Eric Auger [mailto:[hidden email]]
>>>> Sent: Wednesday, June 07, 2017 9:31 PM
>>>> To: [hidden email]; [hidden email];
>>>> [hidden email]; [hidden email];
>> [hidden email];
>>>> [hidden email]; [hidden email]; jean-
>>>> [hidden email]
>>>> Cc: [hidden email]; [hidden email];
>> [hidden email];
>>>> [hidden email]; [hidden email];
>>>> [hidden email]; [hidden email]; [hidden email]; Bharat
>> Bhushan
>>>> <[hidden email]>
>>>> Subject: [RFC v2 0/8] VIRTIO-IOMMU device
>>>>
>>>> This series implements the virtio-iommu device. This is a proof of
>>>> concept based on the virtio-iommu specification written by Jean-Philippe
>> Brucker [1].
>>>> This was tested with a guest using the virtio-iommu driver [2] and
>>>> exposed with a virtio-net-pci using dma ops.
>>>>
>>>> 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.
>>>>
>>>> This should allow to start some benchmarking activities against pure
>>>> emulated IOMMU (especially ARM SMMU).
>>>
>>> I am testing this on ARM64 and see below continuous error prints:
>>>
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>>
>>>
>>> Also in guest I do not see device-tree node with virtio-iommu.
>> do you mean the virtio-mmio with #iommu-cells property?
>>
>> This one is created statically by virt machine. I would be surprised if it were
>> not there. Are you using the virt = virt2.10 machine. Machines before do not
>> support its instantiation.
>>
>> Please can you add a printf in hw/arm/virt.c create_virtio_mmio() at the
>> moment when this node is created. Also you can add a printf in
>> bind_virtio_iommu_device() to make sure the binding with the PCI host
>> bridge is added on machine init done.
>>
>> Also worth to check, CONFIG_VIRTIO_IOMMU=y on guest side.
>
> It works on my side.
Great.

 The driver config was disabled and also I was using guest kernel which
was not have deferred-probing.
Yes I did not mention in my cover letter the guest I have been using is
based on Jean-Philippe's branch, featuring deferred IOMMU probing. I I
have not tried yet with an upstream guest.
 Now after fixing it works on my side
> I placed some prints to see dma-map are mapping regions in virtio-iommu, it uses emulated iommu.
>
> I will continue to add VFIO support now on this and more testing !!

OK. I will do the VFIO integration first on the vsmmuv3 device as I
already prepared the VFIO replay and hopefully we will sync ;-)

Thanks

Eric

>
> Thanks
> -Bharat
>
>>
>> Thanks
>>
>> Eric
>>
>>> I am using qemu-tree you mentioned below and iommu-driver patches
>> published by Jean-P.
>>> Qemu command line have additional ""-device virtio-iommu-device". What
>> I am missing ?
>>
>>
>>>
>>> Thanks
>>> -Bharat
>>>
>>>>
>>>> Best Regards
>>>>
>>>> Eric
>>>>
>>>> This series can be found at:
>>>> https://github.com/eauger/qemu/tree/virtio-iommu-rfcv2
>>>>
>>>> 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
>>>>
>>>> History:
>>>> 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                      | 623
>> ++++++++++++++++++++++++++
>>>>  include/hw/arm/virt.h                         |   5 +
>>>>  include/hw/virtio/virtio-iommu.h              |  60 +++
>>>>  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, 957 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
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Eric Auger-2
In reply to this post by Bharat Bhushan-2
Hi,

On 09/06/2017 13:30, Bharat Bhushan wrote:

> Hi Eric,
>
>> -----Original Message-----
>> From: Auger Eric [mailto:[hidden email]]
>> Sent: Friday, June 09, 2017 12:14 PM
>> To: Bharat Bhushan <[hidden email]>;
>> [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]
>> Cc: [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]
>> Subject: Re: [RFC v2 0/8] VIRTIO-IOMMU device
>>
>> Hi Bharat,
>>
>> On 09/06/2017 08:16, Bharat Bhushan wrote:
>>> Hi Eric,
>>>
>>>> -----Original Message-----
>>>> From: Eric Auger [mailto:[hidden email]]
>>>> Sent: Wednesday, June 07, 2017 9:31 PM
>>>> To: [hidden email]; [hidden email];
>>>> [hidden email]; [hidden email];
>> [hidden email];
>>>> [hidden email]; [hidden email]; jean-
>>>> [hidden email]
>>>> Cc: [hidden email]; [hidden email];
>> [hidden email];
>>>> [hidden email]; [hidden email];
>>>> [hidden email]; [hidden email]; [hidden email]; Bharat
>> Bhushan
>>>> <[hidden email]>
>>>> Subject: [RFC v2 0/8] VIRTIO-IOMMU device
>>>>
>>>> This series implements the virtio-iommu device. This is a proof of
>>>> concept based on the virtio-iommu specification written by Jean-Philippe
>> Brucker [1].
>>>> This was tested with a guest using the virtio-iommu driver [2] and
>>>> exposed with a virtio-net-pci using dma ops.
>>>>
>>>> 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.

For those who may play with the device, this was tested with a
virtio-net-pci device using the following command:

-device
virtio-net-pci,netdev=tap0,mac=<MAC>,iommu_platform,disable-modern=off,disable-legacy=on
\

I tried to run the guest using a virtio-blk-pci device using
-device
virtio-blk-pci,scsi=off,drive=<>,iommu_platform=off,disable-modern=off,disable-legacy=on,werror=stop,rerror=stop
\

and the guest does *not* boot whereas it does without any iommu.

However I am not sure the issue is related to the actual virtual iommu
device as I have the exact same issue with vsmmuv3 emulated device (This
was originally reported by Tomasz). So the issue may come from the
infrastructure around. To be further investigated ...

Thanks

Eric

>>>>
>>>> This should allow to start some benchmarking activities against pure
>>>> emulated IOMMU (especially ARM SMMU).
>>>
>>> I am testing this on ARM64 and see below continuous error prints:
>>>
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>> virtio_iommu_translate sid=8 is not known!!
>>>
>>>
>>> Also in guest I do not see device-tree node with virtio-iommu.
>> do you mean the virtio-mmio with #iommu-cells property?
>>
>> This one is created statically by virt machine. I would be surprised if it were
>> not there. Are you using the virt = virt2.10 machine. Machines before do not
>> support its instantiation.
>>
>> Please can you add a printf in hw/arm/virt.c create_virtio_mmio() at the
>> moment when this node is created. Also you can add a printf in
>> bind_virtio_iommu_device() to make sure the binding with the PCI host
>> bridge is added on machine init done.
>>
>> Also worth to check, CONFIG_VIRTIO_IOMMU=y on guest side.
>
> It works on my side. The driver config was disabled and also I was using guest kernel which was not have deferred-probing. Now after fixing it works on my side
> I placed some prints to see dma-map are mapping regions in virtio-iommu, it uses emulated iommu.
>
> I will continue to add VFIO support now on this and more testing !!
>
> Thanks
> -Bharat
>
>>
>> Thanks
>>
>> Eric
>>
>>> I am using qemu-tree you mentioned below and iommu-driver patches
>> published by Jean-P.
>>> Qemu command line have additional ""-device virtio-iommu-device". What
>> I am missing ?
>>
>>
>>>
>>> Thanks
>>> -Bharat
>>>
>>>>
>>>> Best Regards
>>>>
>>>> Eric
>>>>
>>>> This series can be found at:
>>>> https://github.com/eauger/qemu/tree/virtio-iommu-rfcv2
>>>>
>>>> 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
>>>>
>>>> History:
>>>> 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                      | 623
>> ++++++++++++++++++++++++++
>>>>  include/hw/arm/virt.h                         |   5 +
>>>>  include/hw/virtio/virtio-iommu.h              |  60 +++
>>>>  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, 957 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
>>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 5/8] virtio_iommu: Add the iommu regions

Bharat Bhushan-2
In reply to this post by Eric Auger-2
Hi Eric,

> -----Original Message-----
> From: Eric Auger [mailto:[hidden email]]
> Sent: Wednesday, June 07, 2017 9:31 PM
> To: [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; jean-
> [hidden email]
> Cc: [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; Bharat Bhushan
> <[hidden email]>
> Subject: [RFC v2 5/8] virtio_iommu: Add the iommu regions
>
> 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 <[hidden email]>
> ---
>  hw/virtio/trace-events   |  1 +
>  hw/virtio/virtio-iommu.c | 97
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 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 ea1caa7..902c779 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,59 @@
>  /* Max size */
>  #define VIOMMU_DEFAULT_QUEUE_SIZE 256
>
> +static inline uint16_t smmu_get_sid(IOMMUDevice *dev)

This should be virtio-iommu not smmu ?

Thanks
-Bharat

> +{
> +    return  ((pci_bus_num(dev->bus) & 0xff) << 8) | 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) {
> +        sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
> +
> +        sdev->viommu = s;
> +        sdev->bus = bus;
> +        sdev->devfn = devfn;
> +
> +        memory_region_init_iommu(&sdev->iommu_mr, OBJECT(s),
> +                                 &s->iommu_ops, TYPE_VIRTIO_IOMMU,
> +                                 UINT64_MAX);
> +        address_space_init(&sdev->as, &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 +262,26 @@ static void
> virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>      }
>  }
>
> +static IOMMUTLBEntry virtio_iommu_translate(MemoryRegion *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 = smmu_get_sid(sdev);
> +
> +    trace_virtio_iommu_translate(mr->name, sid, addr, flag);
> +    return entry;
> +}
> +
>  static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t
> *config_data)
>  {
>      VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
> @@ -253,6 +327,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);
> @@ -266,6 +355,14 @@ static void virtio_iommu_device_realize(DeviceState
> *dev, Error **errp)
>
>      s->config.page_sizes = ~((1ULL << 12) - 1);
>      s->config.input_range.end = -1UL;
> +
> +    s->iommu_ops.translate = virtio_iommu_translate;
> +    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)
> --
> 2.5.5


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Bharat Bhushan-2
In reply to this post by Eric Auger-2
Hi Eric,

I started added replay in virtio-iommu and came across how MSI interrupts with work with VFIO.
I understand that on intel this works differently but vsmmu will have same requirement.
kvm-msi-irq-route are added using the msi-address to be translated by viommu and not the final translated address.
While currently the irqfd framework does not know about emulated iommus (virtio-iommu, vsmmuv3/vintel-iommu).
So in my view we have following options:
- Programming with translated address when setting up kvm-msi-irq-route
- Route the interrupts via QEMU, which is bad from performance
- vhost-virtio-iommu may solve the problem in long term

Is there any other better option I am missing?

Thanks
-Bharat

> -----Original Message-----
> From: Auger Eric [mailto:[hidden email]]
> Sent: Friday, June 09, 2017 5:24 PM
> To: Bharat Bhushan <[hidden email]>;
> [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]
> Cc: [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]; [hidden email];
> [hidden email]; [hidden email]
> Subject: Re: [Qemu-devel] [RFC v2 0/8] VIRTIO-IOMMU device
>
> Hi Bharat,
>
> On 09/06/2017 13:30, Bharat Bhushan wrote:
> > Hi Eric,
> >
> >> -----Original Message-----
> >> From: Auger Eric [mailto:[hidden email]]
> >> Sent: Friday, June 09, 2017 12:14 PM
> >> To: Bharat Bhushan <[hidden email]>;
> >> [hidden email]; [hidden email];
> >> [hidden email]; [hidden email]; [hidden email];
> >> [hidden email]; [hidden email]
> >> Cc: [hidden email]; [hidden email];
> [hidden email];
> >> [hidden email]; [hidden email];
> >> [hidden email]; [hidden email]; [hidden email]
> >> Subject: Re: [RFC v2 0/8] VIRTIO-IOMMU device
> >>
> >> Hi Bharat,
> >>
> >> On 09/06/2017 08:16, Bharat Bhushan wrote:
> >>> Hi Eric,
> >>>
> >>>> -----Original Message-----
> >>>> From: Eric Auger [mailto:[hidden email]]
> >>>> Sent: Wednesday, June 07, 2017 9:31 PM
> >>>> To: [hidden email]; [hidden email];
> >>>> [hidden email]; [hidden email];
> >> [hidden email];
> >>>> [hidden email]; [hidden email]; jean-
> >>>> [hidden email]
> >>>> Cc: [hidden email]; [hidden email];
> >> [hidden email];
> >>>> [hidden email]; [hidden email];
> >>>> [hidden email]; [hidden email]; [hidden email]; Bharat
> >> Bhushan
> >>>> <[hidden email]>
> >>>> Subject: [RFC v2 0/8] VIRTIO-IOMMU device
> >>>>
> >>>> This series implements the virtio-iommu device. This is a proof of
> >>>> concept based on the virtio-iommu specification written by
> >>>> Jean-Philippe
> >> Brucker [1].
> >>>> This was tested with a guest using the virtio-iommu driver [2] and
> >>>> exposed with a virtio-net-pci using dma ops.
> >>>>
> >>>> 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.
> >>>>
> >>>> This should allow to start some benchmarking activities against
> >>>> pure emulated IOMMU (especially ARM SMMU).
> >>>
> >>> I am testing this on ARM64 and see below continuous error prints:
> >>>
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>> virtio_iommu_translate sid=8 is not known!!
> >>>
> >>>
> >>> Also in guest I do not see device-tree node with virtio-iommu.
> >> do you mean the virtio-mmio with #iommu-cells property?
> >>
> >> This one is created statically by virt machine. I would be surprised
> >> if it were not there. Are you using the virt = virt2.10 machine.
> >> Machines before do not support its instantiation.
> >>
> >> Please can you add a printf in hw/arm/virt.c create_virtio_mmio() at
> >> the moment when this node is created. Also you can add a printf in
> >> bind_virtio_iommu_device() to make sure the binding with the PCI host
> >> bridge is added on machine init done.
> >>
> >> Also worth to check, CONFIG_VIRTIO_IOMMU=y on guest side.
> >
> > It works on my side.
> Great.
>
>  The driver config was disabled and also I was using guest kernel which was
> not have deferred-probing.
> Yes I did not mention in my cover letter the guest I have been using is based
> on Jean-Philippe's branch, featuring deferred IOMMU probing. I I have not
> tried yet with an upstream guest.
>  Now after fixing it works on my side
> > I placed some prints to see dma-map are mapping regions in virtio-iommu,
> it uses emulated iommu.
> >
> > I will continue to add VFIO support now on this and more testing !!
>
> OK. I will do the VFIO integration first on the vsmmuv3 device as I already
> prepared the VFIO replay and hopefully we will sync ;-)
>
> Thanks
>
> Eric
> >
> > Thanks
> > -Bharat
> >
> >>
> >> Thanks
> >>
> >> Eric
> >>
> >>> I am using qemu-tree you mentioned below and iommu-driver patches
> >> published by Jean-P.
> >>> Qemu command line have additional ""-device virtio-iommu-device".
> >>> What
> >> I am missing ?
> >>
> >>
> >>>
> >>> Thanks
> >>> -Bharat
> >>>
> >>>>
> >>>> Best Regards
> >>>>
> >>>> Eric
> >>>>
> >>>> This series can be found at:
> >>>> https://github.com/eauger/qemu/tree/virtio-iommu-rfcv2
> >>>>
> >>>> 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
> >>>>
> >>>> History:
> >>>> 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                      | 623
> >> ++++++++++++++++++++++++++
> >>>>  include/hw/arm/virt.h                         |   5 +
> >>>>  include/hw/virtio/virtio-iommu.h              |  60 +++
> >>>>  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, 957 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
> >>>
> >

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 0/8] VIRTIO-IOMMU device

Jean-Philippe Brucker
On 19/06/17 08:54, Bharat Bhushan wrote:

> Hi Eric,
>
> I started added replay in virtio-iommu and came across how MSI interrupts with work with VFIO.
> I understand that on intel this works differently but vsmmu will have same requirement.
> kvm-msi-irq-route are added using the msi-address to be translated by viommu and not the final translated address.
> While currently the irqfd framework does not know about emulated iommus (virtio-iommu, vsmmuv3/vintel-iommu).
> So in my view we have following options:
> - Programming with translated address when setting up kvm-msi-irq-route
> - Route the interrupts via QEMU, which is bad from performance
> - vhost-virtio-iommu may solve the problem in long term
>
> Is there any other better option I am missing?

Since we're on the topic of MSIs... I'm currently trying to figure out how
we'll handle MSIs in the nested translation mode, where the guest manages
S1 page tables and the host doesn't know about GVA->GPA translation.

I'm also wondering about the benefits of having SW-mapped MSIs in the
guest. It seems unavoidable for vSMMU since that's what a physical system
would do. But in a paravirtualized solution there doesn't seem to be any
compelling reason for having the guest map MSI doorbells. These addresses
are never accessed directly, they are only used for setting up IRQ routing
(at least on kvmtool). So here's what I'd like to have. Note that I
haven't investigated the feasibility in Qemu yet, I don't know how it
deals with MSIs.

(1) Guest uses the guest-physical MSI doorbell when setting up MSIs. For
ARM with GICv3 this would be GITS_TRANSLATER, for x86 it would be the
fixed MSI doorbell. This way the host wouldn't need to inspect IOMMU
mappings when handling writes to PCI MSI-X tables.

(2) In nested mode (with VFIO) on ARM, the pSMMU will still translate MSIs
via S1+S2. Therefore the host needs to map MSIs at stage-1, and I'd like
to use the (currently unused) TTB1 tables in that case. In addition, using
TTB1 would be useful for SVM, when endpoints write MSIs with PASIDs and we
don't want to map them in user address space.

This means that the host needs to use different doorbell addresses in
nested mode, since it would be unable to map at S1 the same IOVA as S2
(TTB1 manages negative addresses - 0xffff............, which are not
representable as GPAs.) It also requires to use 32-bit page tables for
endpoints that are not capable of using 64-bit MSI addresses.


Now (2) is entirely handled in the host kernel, so it's more a Linux
question. But does (1) seem acceptable for virtio-iommu in Qemu?

Thanks,
Jean

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 3/8] virtio_iommu: add skeleton

Jean-Philippe Brucker
In reply to this post by Bharat Bhushan-2
On 06/08/2017 12:09 PM, Bharat Bhushan wrote:

>> From: Eric Auger [mailto:[hidden email]]
>> Sent: Wednesday, June 07, 2017 9:31 PM
>> To: [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; jean-
>> [hidden email]
>> Cc: [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; [hidden email];
>> [hidden email]; [hidden email]; Bharat Bhushan
>> <[hidden email]>
>> Subject: [RFC v2 3/8] virtio_iommu: add skeleton
>>
>> This patchs adds the skeleton for the virtio-iommu device.
>>
>> Signed-off-by: Eric Auger <[hidden email]>
>> +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 = ~((1ULL << 12) - 1);
>
> This is hardcoded to 4K, Should this be aligned to Host-page size ?

I wonder if we should introduce per-address-space page sizes, to cater
for emulated and VFIO devices being managed by the same IOMMU.

For an emulated device, it seems that the page granularity can be
arbitrary, so maybe TARGET_PAGE_MASK would be more convenient. But for
VFIO, the page granularity is a property of the physical IOMMU.

In kvmtool I instantiate two virtio-iommus for vfio and virtio devices,
so the page size issue hasn't come up, but here things won't work if the
page granularity advertised in config.page_sizes is smaller than the
pIOMMU page size.

Adding address space properties is tricky because they change when
attaching devices, and I wanted to avoid this complication. In nested
mode I have to add one AS state, where the AS is active and properties
are freezed (attaching an incompatible device is then rejected). Maybe
we need to do the same for map/unmap.

A simpler solution (for me, that is), would be to put the greatest page
granularity of all VFIO devices into page_sizes, but it doesn't take
hotplug into account.

Thanks,
Jean

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [RFC v2 6/8] virtio-iommu: Implement the translation and commands

Jean-Philippe Brucker
In reply to this post by Eric Auger-2
On 07/06/17 17:01, Eric Auger wrote:

> This patch adds the actual implementation for the translation routine
> and the virtio-iommu commands.
>
> Signed-off-by: Eric Auger <[hidden email]>
>
> ---[...]
>  static int virtio_iommu_attach(VirtIOIOMMU *s,
>                                 struct virtio_iommu_req_attach *req)
> @@ -95,10 +135,34 @@ 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, reserved);
>  
> -    return VIRTIO_IOMMU_S_UNSUPP;
> +    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
> +    if (dev) {
> +        return -1;

I guess you could return S_INVAL here. However, if the device is already
attached to AS0, it should be detached and attached to AS1. The Linux
driver relies on this behavior when moving a device from kernel to user
address space and back.

Thanks,
Jean

Loading...