[Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

This patch series integrates VFIO/VHOST with virtio-iommu.

This version is mainly about rebasing on v4 version on
virtio-iommu device framework from Eric Augur and
addresing review comments.

This patch series allows PCI pass-through using virtio-iommu.
  
This series is based on:
 - virtio-iommu kernel driver by Jean-Philippe Brucker
    [1] [RFC] virtio-iommu version 0.4
    git://linux-arm.org/virtio-iommu.git branch viommu/v0.4

 - virtio-iommu device emulation by Eric Augur.
   [RFC v4 00/16] VIRTIO-IOMMU device
   https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4

Changes are available at : https://github.com/bharaty/qemu.git virtio-iommu-vfio-integration-v4

v3->v4:
 - Rebase to v4 version from Eric
 - Fixes from Eric with DPDK in VM
 - Logical division in multiple patches

v2->v3:
 - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
   Which is based on top of v2.10-rc0 that
 - Fixed issue with two PCI devices
 - Addressed review comments

v1->v2:
  - Added trace events
  - removed vSMMU3 link in patch description

Bharat Bhushan (5):
  target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route
  virtio-iommu: Add iommu notifier for map/unmap
  virtio-iommu: Call iommu notifier for attach/detach
  virtio-iommu: add iommu replay
  virtio-iommu: add iommu notifier memory-region

 hw/virtio/trace-events           |   5 ++
 hw/virtio/virtio-iommu.c         | 181 ++++++++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-iommu.h |   6 ++
 target/arm/kvm.c                 |  27 ++++++
 target/arm/trace-events          |   3 +
 5 files changed, 219 insertions(+), 3 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v4 1/5] target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route

[Bharat Bhushan]

Translate msi address if device is behind virtio-iommu.
This logic is similar to vSMMUv3/Intel iommu emulation.

This RFC patch does not handle the case where both vsmmuv3 and
virtio-iommu are available.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
---
v3->v4
 - No changes

 target/arm/kvm.c        | 27 +++++++++++++++++++++++++++
 target/arm/trace-events |  3 +++
 2 files changed, 30 insertions(+)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 211a7bf..895a630 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -21,7 +21,11 @@
 #include "kvm_arm.h"
 #include "cpu.h"
 #include "internals.h"
+#include "trace.h"
 #include "hw/arm/arm.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/msi.h"
+#include "hw/virtio/virtio-iommu.h"
 #include "exec/memattrs.h"
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
@@ -666,6 +670,29 @@ int kvm_arm_vgic_probe(void)
 int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
                              uint64_t address, uint32_t data, PCIDevice *dev)
 {
+    AddressSpace *as = pci_device_iommu_address_space(dev);
+    IOMMUTLBEntry entry;
+    IOMMUDevice *sdev;
+    IOMMUMemoryRegionClass *imrc;
+
+    if (as == &address_space_memory) {
+        return 0;
+    }
+
+    /* MSI doorbell address is translated by an IOMMU */
+    sdev = container_of(as, IOMMUDevice, as);
+
+    imrc = memory_region_get_iommu_class_nocheck(&sdev->iommu_mr);
+
+    entry = imrc->translate(&sdev->iommu_mr, address, IOMMU_WO);
+
+    route->u.msi.address_lo = entry.translated_addr;
+    route->u.msi.address_hi = entry.translated_addr >> 32;
+
+    trace_kvm_arm_fixup_msi_route(address, sdev->devfn,
+                                  sdev->iommu_mr.parent_obj.name,
+                                  entry.translated_addr);
+
     return 0;
 }
 
diff --git a/target/arm/trace-events b/target/arm/trace-events
index 9e37131..8b3c220 100644
--- a/target/arm/trace-events
+++ b/target/arm/trace-events
@@ -8,3 +8,6 @@ arm_gt_tval_write(int timer, uint64_t value) "gt_tval_write: timer %d value 0x%"
 arm_gt_ctl_write(int timer, uint64_t value) "gt_ctl_write: timer %d value 0x%" PRIx64
 arm_gt_imask_toggle(int timer, int irqstate) "gt_ctl_write: timer %d IMASK toggle, new irqstate %d"
 arm_gt_cntvoff_write(uint64_t value) "gt_cntvoff_write: value 0x%" PRIx64
+
+# target/arm/kvm.c
+kvm_arm_fixup_msi_route(uint64_t iova, uint32_t devid, const char *name, uint64_t gpa) "MSI addr = 0x%"PRIx64" is translated for devfn=%d through %s into 0x%"PRIx64
-- 
1.9.3

[Qemu-devel] [PATCH v4 2/5] virtio-iommu: Add iommu notifier for map/unmap

[Bharat Bhushan]

This patch extends VIRTIO_IOMMU_T_MAP/UNMAP request to
notify registered iommu-notifier.
This is needed for VFIO support,

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v3->v4:
 Follwoig fixes by Eric
 - Calling virtio_iommu_notify_map() for all device in AS
 - virtio_iommu_notify_unmap() moved to a function,
   This is needed as per changes in base framework (v4)

 hw/virtio/trace-events           |  2 ++
 hw/virtio/virtio-iommu.c         | 69 ++++++++++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-iommu.h |  6 ++++
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 2793604..251b595 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -49,3 +49,5 @@ virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid)
 virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t addr, uint64_t size, uint32_t flags, size_t filled) "dev= %d, subtype=%d addr=0x%"PRIx64" size=0x%"PRIx64" flags=%d filled=0x%lx"
 virtio_iommu_fill_none_property(uint32_t devid) "devid=%d"
 virtio_iommu_set_page_size_mask(const char *iommu_mr, uint64_t mask) "mr=%s page_size_mask=0x%"PRIx64
+virtio_iommu_notify_map(const char *name, hwaddr iova, hwaddr paddr, hwaddr map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64""
+virtio_iommu_notify_unmap(const char *name, hwaddr iova, hwaddr map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64""
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1873b9a..085e972 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -95,6 +95,38 @@ static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
     }
 }
 
+static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr iova,
+                                    hwaddr paddr, hwaddr size)
+{
+    IOMMUTLBEntry entry;
+
+    entry.target_as = &address_space_memory;
+    entry.addr_mask = size - 1;
+
+    entry.iova = iova;
+    trace_virtio_iommu_notify_map(mr->parent_obj.name, iova, paddr, size);
+    entry.perm = IOMMU_RW;
+    entry.translated_addr = paddr;
+
+    memory_region_notify_iommu(mr, entry);
+}
+
+static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
+                                      hwaddr size)
+{
+    IOMMUTLBEntry entry;
+
+    entry.target_as = &address_space_memory;
+    entry.addr_mask = size - 1;
+
+    entry.iova = iova;
+    trace_virtio_iommu_notify_unmap(mr->parent_obj.name, iova, size);
+    entry.perm = IOMMU_NONE;
+    entry.translated_addr = 0;
+
+    memory_region_notify_iommu(mr, entry);
+}
+
 static void virtio_iommu_detach_dev_from_as(viommu_dev *dev)
 {
     QLIST_REMOVE(dev, next);
@@ -291,6 +323,8 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
     viommu_as *as;
     viommu_interval *interval;
     viommu_mapping *mapping;
+    VirtioIOMMUNotifierNode *node;
+    viommu_dev *dev;
 
     interval = g_malloc0(sizeof(*interval));
 
@@ -318,9 +352,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
 
     g_tree_insert(as->mappings, interval, mapping);
 
+    /* All devices in an address-space share mapping */
+    QLIST_FOREACH(node, &s->notifiers_list, next) {
+        QLIST_FOREACH(dev, &as->device_list, next) {
+            if (dev->id == node->iommu_dev->devfn) {
+                virtio_iommu_notify_map(&node->iommu_dev->iommu_mr,
+                                        virt_addr, phys_addr, size);
+            }
+        }
+    }
+
     return VIRTIO_IOMMU_S_OK;
 }
 
+static void virtio_iommu_remove_mapping(VirtIOIOMMU *s, viommu_as *as,
+                                        viommu_interval *interval)
+{
+    VirtioIOMMUNotifierNode *node;
+    viommu_dev *dev;
+
+    g_tree_remove(as->mappings, (gpointer)(interval));
+    QLIST_FOREACH(node, &s->notifiers_list, next) {
+        QLIST_FOREACH(dev, &as->device_list, next) {
+            if (dev->id == node->iommu_dev->devfn) {
+                virtio_iommu_notify_unmap(&node->iommu_dev->iommu_mr,
+                                          interval->low,
+                                          interval->high - interval->low + 1);
+            }
+        }
+    }
+}
+
 static int virtio_iommu_unmap(VirtIOIOMMU *s,
                               struct virtio_iommu_req_unmap *req)
 {
@@ -352,18 +414,18 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
         current.high = high;
 
         if (low == interval.low && size >= mapping->size) {
-            g_tree_remove(as->mappings, (gpointer)(&current));
+            virtio_iommu_remove_mapping(s, as, &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));
+            virtio_iommu_remove_mapping(s, as, &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));
+            virtio_iommu_remove_mapping(s, as, &current);
         } else {
             break;
         }
@@ -806,6 +868,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
 
+    QLIST_INIT(&s->notifiers_list);
     virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
                 sizeof(struct virtio_iommu_config));
 
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index f9c988f..7e04184 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -46,6 +46,11 @@ typedef struct IOMMUPciBus {
     IOMMUDevice  *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
 } IOMMUPciBus;
 
+typedef struct VirtioIOMMUNotifierNode {
+    IOMMUDevice *iommu_dev;
+    QLIST_ENTRY(VirtioIOMMUNotifierNode) next;
+} VirtioIOMMUNotifierNode;
+
 typedef struct VirtIOIOMMU {
     VirtIODevice parent_obj;
     VirtQueue *vq;
@@ -56,6 +61,7 @@ typedef struct VirtIOIOMMU {
     GTree *address_spaces;
     QemuMutex mutex;
     GTree *devices;
+    QLIST_HEAD(, VirtioIOMMUNotifierNode) notifiers_list;
 } VirtIOIOMMU;
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH v4 3/5] virtio-iommu: Call iommu notifier for attach/detach

[Bharat Bhushan]

iommu-notifier are called when a device is attached
or detached to as address-space.
This is needed for VFIO.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v3->v4:
 Follwoig fixes by Eric
 - Return "false" from virtio_iommu_mapping_unmap/map()
 - Calling virtio_iommu_notify_unmap/map() for all device in as

 hw/virtio/virtio-iommu.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 085e972..ff91bce 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -127,8 +127,42 @@ static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
     memory_region_notify_iommu(mr, entry);
 }
 
+static gboolean virtio_iommu_mapping_unmap(gpointer key, gpointer value,
+                                           gpointer data)
+{
+    viommu_mapping *mapping = (viommu_mapping *) value;
+    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
+
+    virtio_iommu_notify_unmap(mr, mapping->virt_addr, mapping->size);
+
+    return false;
+}
+
+static gboolean virtio_iommu_mapping_map(gpointer key, gpointer value,
+                                         gpointer data)
+{
+    viommu_mapping *mapping = (viommu_mapping *) value;
+    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
+
+    virtio_iommu_notify_map(mr, mapping->virt_addr, mapping->phys_addr,
+                            mapping->size);
+
+    return false;
+}
+
 static void virtio_iommu_detach_dev_from_as(viommu_dev *dev)
 {
+    VirtioIOMMUNotifierNode *node;
+    VirtIOIOMMU *s = dev->viommu;
+    viommu_as *as = dev->as;
+
+    QLIST_FOREACH(node, &s->notifiers_list, next) {
+        if (dev->id == node->iommu_dev->devfn) {
+            g_tree_foreach(as->mappings, virtio_iommu_mapping_unmap,
+                           &node->iommu_dev->iommu_mr);
+        }
+    }
+
     QLIST_REMOVE(dev, next);
     dev->as = NULL;
 }
@@ -260,6 +294,7 @@ 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);
+    VirtioIOMMUNotifierNode *node;
     viommu_as *as;
     viommu_dev *dev;
 
@@ -284,6 +319,14 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
     dev->as = as;
     g_tree_ref(as->mappings);
 
+    /* replay existing address space mappings on the associated mr */
+    QLIST_FOREACH(node, &s->notifiers_list, next) {
+        if (devid == node->iommu_dev->devfn) {
+            g_tree_foreach(as->mappings, virtio_iommu_mapping_map,
+                           &node->iommu_dev->iommu_mr);
+        }
+    }
+
     return VIRTIO_IOMMU_S_OK;
 }
 
-- 
1.9.3

[Qemu-devel] [PATCH v4 4/5] virtio-iommu: add iommu replay

[Bharat Bhushan]

Default replay does not work with virtio-iommu,
so this patch provide virtio-iommu replay functionality.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
---
v3->v4:
 - Replay functionality moved in separate patch
 - No other changes

 hw/virtio/trace-events   |  1 +
 hw/virtio/virtio-iommu.c | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 251b595..840d54f 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -51,3 +51,4 @@ virtio_iommu_fill_none_property(uint32_t devid) "devid=%d"
 virtio_iommu_set_page_size_mask(const char *iommu_mr, uint64_t mask) "mr=%s page_size_mask=0x%"PRIx64
 virtio_iommu_notify_map(const char *name, hwaddr iova, hwaddr paddr, hwaddr map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64""
 virtio_iommu_notify_unmap(const char *name, hwaddr iova, hwaddr map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64""
+virtio_iommu_remap(hwaddr iova, hwaddr pa, hwaddr size) "iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64""
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index ff91bce..d4d34cf 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -906,6 +906,43 @@ static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
     return (ua > ub) - (ua < ub);
 }
 
+static gboolean virtio_iommu_remap(gpointer key, gpointer value, gpointer data)
+{
+    viommu_mapping *mapping = (viommu_mapping *) value;
+    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
+
+    trace_virtio_iommu_remap(mapping->virt_addr, mapping->phys_addr,
+                             mapping->size);
+    /* unmap previous entry and map again */
+    virtio_iommu_notify_unmap(mr, mapping->virt_addr, mapping->size);
+
+    virtio_iommu_notify_map(mr, mapping->virt_addr, mapping->phys_addr,
+                            mapping->size);
+    return false;
+}
+
+static void virtio_iommu_replay(IOMMUMemoryRegion *mr, IOMMUNotifier *n)
+{
+    IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+    VirtIOIOMMU *s = sdev->viommu;
+    uint32_t sid;
+    viommu_dev *dev;
+
+    sid = virtio_iommu_get_sid(sdev);
+
+    qemu_mutex_lock(&s->mutex);
+
+    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(sid));
+    if (!dev || !dev->as) {
+        goto unlock;
+    }
+
+    g_tree_foreach(dev->as->mappings, virtio_iommu_remap, mr);
+
+unlock:
+    qemu_mutex_unlock(&s->mutex);
+}
+
 static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -1003,6 +1040,7 @@ static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
 
     imrc->translate = virtio_iommu_translate;
     imrc->set_page_size_mask = virtio_iommu_set_page_size_mask;
+    imrc->replay = virtio_iommu_replay;
 }
 
 static const TypeInfo virtio_iommu_info = {
-- 
1.9.3

[Qemu-devel] [PATCH v4 5/5] virtio-iommu: add iommu notifier memory-region

[Bharat Bhushan]

Finally add notify_flag_changed() to for memory-region
access flag iommu flag change notifier

Finally add the memory notifier

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
---
v3->v4:
 - notify_flag_changed functionality moved in separate patch
 - No other changes

 hw/virtio/trace-events   |  2 ++
 hw/virtio/virtio-iommu.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 840d54f..a9de0d4 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -52,3 +52,5 @@ virtio_iommu_set_page_size_mask(const char *iommu_mr, uint64_t mask) "mr=%s page
 virtio_iommu_notify_map(const char *name, hwaddr iova, hwaddr paddr, hwaddr map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64""
 virtio_iommu_notify_unmap(const char *name, hwaddr iova, hwaddr map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64""
 virtio_iommu_remap(hwaddr iova, hwaddr pa, hwaddr size) "iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64""
+virtio_iommu_notify_flag_add(const char *iommu) "Add virtio-iommu notifier node for memory region %s"
+virtio_iommu_notify_flag_del(const char *iommu) "Del virtio-iommu notifier node for memory region %s"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index d4d34cf..a9b0d72 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -764,6 +764,36 @@ push:
     }
 }
 
+static void virtio_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu_mr,
+                                             IOMMUNotifierFlag old,
+                                             IOMMUNotifierFlag new)
+{
+    IOMMUDevice *sdev = container_of(iommu_mr, IOMMUDevice, iommu_mr);
+    VirtIOIOMMU *s = sdev->viommu;
+    VirtioIOMMUNotifierNode *node = NULL;
+    VirtioIOMMUNotifierNode *next_node = NULL;
+
+    if (old == IOMMU_NOTIFIER_NONE) {
+        trace_virtio_iommu_notify_flag_add(iommu_mr->parent_obj.name);
+        node = g_malloc0(sizeof(*node));
+        node->iommu_dev = sdev;
+        QLIST_INSERT_HEAD(&s->notifiers_list, node, next);
+        return;
+    }
+
+    /* update notifier node with new flags */
+    QLIST_FOREACH_SAFE(node, &s->notifiers_list, next, next_node) {
+        if (node->iommu_dev == sdev) {
+            if (new == IOMMU_NOTIFIER_NONE) {
+                trace_virtio_iommu_notify_flag_del(iommu_mr->parent_obj.name);
+                QLIST_REMOVE(node, next);
+                g_free(node);
+            }
+            return;
+        }
+    }
+}
+
 static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
                                             IOMMUAccessFlags flag)
 {
@@ -1041,6 +1071,7 @@ static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
     imrc->translate = virtio_iommu_translate;
     imrc->set_page_size_mask = virtio_iommu_set_page_size_mask;
     imrc->replay = virtio_iommu_replay;
+    imrc->notify_flag_changed = virtio_iommu_notify_flag_changed;
 }
 
 static const TypeInfo virtio_iommu_info = {
-- 
1.9.3

Re: [Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

Hi Peter,

While vfio with virtio-iommu I observed one issue,  When virtio-iommu device exists but guest kernel does not have virtio-iommu driver (not enabled in Config) then IOMMU faults are reported on host.

This is because no mapping is created in IOMMU, not even default guest-physical to real-physical. While looking at vfio_listener_region_add(), it does not create initial mapping in IOMMU and relies on guest to create mapping. Is this something known or I am missing something?

Thanks
-Bharat

> -----Original Message-----
> From: Bharat Bhushan [mailto:Bharat.Bhushan@nxp.com]
> Sent: Wednesday, September 27, 2017 12:03 PM
> To: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org
> Cc: wei@redhat.com; kevin.tian@intel.com; marc.zyngier@arm.com;
> tn@semihalf.com; will.deacon@arm.com; drjones@redhat.com;
> robin.murphy@arm.com; christoffer.dall@linaro.org;
> bharatb.yadav@gmail.com; Bharat Bhushan <bharat.bhushan@nxp.com>
> Subject: [PATCH v4 0/5] virtio-iommu: VFIO integration
> 
> This patch series integrates VFIO/VHOST with virtio-iommu.
> 
> This version is mainly about rebasing on v4 version on virtio-iommu device
> framework from Eric Augur and addresing review comments.
> 
> This patch series allows PCI pass-through using virtio-iommu.
> 
> This series is based on:
>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>     [1] [RFC] virtio-iommu version 0.4
>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
>  - virtio-iommu device emulation by Eric Augur.
>    [RFC v4 00/16] VIRTIO-IOMMU device
>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> 
> Changes are available at : https://github.com/bharaty/qemu.git virtio-
> iommu-vfio-integration-v4
> 
> v3->v4:
>  - Rebase to v4 version from Eric
>  - Fixes from Eric with DPDK in VM
>  - Logical division in multiple patches
> 
> v2->v3:
>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>    Which is based on top of v2.10-rc0 that
>  - Fixed issue with two PCI devices
>  - Addressed review comments
> 
> v1->v2:
>   - Added trace events
>   - removed vSMMU3 link in patch description
> 
> Bharat Bhushan (5):
>   target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route
>   virtio-iommu: Add iommu notifier for map/unmap
>   virtio-iommu: Call iommu notifier for attach/detach
>   virtio-iommu: add iommu replay
>   virtio-iommu: add iommu notifier memory-region
> 
>  hw/virtio/trace-events           |   5 ++
>  hw/virtio/virtio-iommu.c         | 181
> ++++++++++++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-iommu.h |   6 ++
>  target/arm/kvm.c                 |  27 ++++++
>  target/arm/trace-events          |   3 +
>  5 files changed, 219 insertions(+), 3 deletions(-)
> 
> --
> 1.9.3

Re: [Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Peter Xu]

On Wed, Sep 27, 2017 at 06:46:18AM +0000, Bharat Bhushan wrote:
> Hi Peter,

Hi, Bharat!

> 
> While vfio with virtio-iommu I observed one issue,  When virtio-iommu device exists but guest kernel does not have virtio-iommu driver (not enabled in Config) then IOMMU faults are reported on host.
> 
> This is because no mapping is created in IOMMU, not even default
guest-physical to real-physical. While looking at vfio_listener_region_add(), it does not create initial mapping in IOMMU and relies on guest to create mapping. Is this something known or I am missing something?

For VT-d, the trick is played using dynamic IOMMU memory region.
Please refer to commit 558e0024a428 ("intel_iommu: allow dynamic
switch of IOMMU region") for more information.

The whole idea is that, the IOMMU region will not be enabled only if
the guest enables that explicitly for the device.  Otherwise (for your
case, when guest driver is not loaded at all), the IOMMU region is by
default off, then the default GPA region will be used to build up the
mapping (just like when we don't have vIOMMU at all).  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

Hi Peter,

> -----Original Message-----
> From: Peter Xu [mailto:peterx@redhat.com]
> Sent: Wednesday, September 27, 2017 12:32 PM
> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org; wei@redhat.com;
> kevin.tian@intel.com; marc.zyngier@arm.com; tn@semihalf.com;
> will.deacon@arm.com; drjones@redhat.com; robin.murphy@arm.com;
> christoffer.dall@linaro.org; bharatb.yadav@gmail.com
> Subject: Re: [PATCH v4 0/5] virtio-iommu: VFIO integration
> 
> On Wed, Sep 27, 2017 at 06:46:18AM +0000, Bharat Bhushan wrote:
> > Hi Peter,
> 
> Hi, Bharat!
> 
> >
> > While vfio with virtio-iommu I observed one issue,  When virtio-iommu
> device exists but guest kernel does not have virtio-iommu driver (not
> enabled in Config) then IOMMU faults are reported on host.
> >
> > This is because no mapping is created in IOMMU, not even default
> guest-physical to real-physical. While looking at vfio_listener_region_add(), it
> does not create initial mapping in IOMMU and relies on guest to create
> mapping. Is this something known or I am missing something?
> 
> For VT-d, the trick is played using dynamic IOMMU memory region.
> Please refer to commit 558e0024a428 ("intel_iommu: allow dynamic switch of
> IOMMU region") for more information.
> 
> The whole idea is that, the IOMMU region will not be enabled only if the
> guest enables that explicitly for the device.  Otherwise (for your case, when
> guest driver is not loaded at all), the IOMMU region is by default off, then
> the default GPA region will be used to build up the mapping (just like when
> we don't have vIOMMU at all).  Thanks,

Thanks, I will analyze and see how we can use for virtio-iommu.

Regards
-Bharat

> 
> --
> Peter Xu

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Linu Cherian]

Hi,

On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> This patch series integrates VFIO/VHOST with virtio-iommu.
> 
> This version is mainly about rebasing on v4 version on
> virtio-iommu device framework from Eric Augur and
> addresing review comments.
> 
> This patch series allows PCI pass-through using virtio-iommu.
>   
> This series is based on:
>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>     [1] [RFC] virtio-iommu version 0.4
>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
>  - virtio-iommu device emulation by Eric Augur.
>    [RFC v4 00/16] VIRTIO-IOMMU device
>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> 
> Changes are available at : https://github.com/bharaty/qemu.git virtio-iommu-vfio-integration-v4
> 

# With the above sources, was trying to test the vfio-pci device assigned to 
  guest using Qemu.
# Both guest and host kernels are configured with 4k as page size.
# releavant qemu command snippet,
  -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
  -net none -device vfio-pci,host=xxx


On guest booting, observed mutliple messages as below,

qemu-system-aarch64: iommu has granularity incompatible with target AS

# On adding necessary prints, 0x5000 is len, 0x4fff is address mask
  and the code expects the address mask to be 0xfff.

if (len & iotlb->addr_mask) {
        error_report

# vfio_dma_map is failing due to this error.

Any pointers ?


> v3->v4:
>  - Rebase to v4 version from Eric
>  - Fixes from Eric with DPDK in VM
>  - Logical division in multiple patches
> 
> v2->v3:
>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>    Which is based on top of v2.10-rc0 that
>  - Fixed issue with two PCI devices
>  - Addressed review comments
> 
> v1->v2:
>   - Added trace events
>   - removed vSMMU3 link in patch description
> 
> Bharat Bhushan (5):
>   target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route
>   virtio-iommu: Add iommu notifier for map/unmap
>   virtio-iommu: Call iommu notifier for attach/detach
>   virtio-iommu: add iommu replay
>   virtio-iommu: add iommu notifier memory-region
> 
>  hw/virtio/trace-events           |   5 ++
>  hw/virtio/virtio-iommu.c         | 181 ++++++++++++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-iommu.h |   6 ++
>  target/arm/kvm.c                 |  27 ++++++
>  target/arm/trace-events          |   3 +
>  5 files changed, 219 insertions(+), 3 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

Hi,

> -----Original Message-----
> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> Sent: Wednesday, September 27, 2017 1:11 PM
> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> bharatb.yadav@gmail.com
> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> 
> Hi,
> 
> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> > This patch series integrates VFIO/VHOST with virtio-iommu.
> >
> > This version is mainly about rebasing on v4 version on virtio-iommu
> > device framework from Eric Augur and addresing review comments.
> >
> > This patch series allows PCI pass-through using virtio-iommu.
> >
> > This series is based on:
> >  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >     [1] [RFC] virtio-iommu version 0.4
> >     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> >
> >  - virtio-iommu device emulation by Eric Augur.
> >    [RFC v4 00/16] VIRTIO-IOMMU device
> >    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> >
> > Changes are available at : https://github.com/bharaty/qemu.git
> > virtio-iommu-vfio-integration-v4
> >
> 
> # With the above sources, was trying to test the vfio-pci device assigned to
>   guest using Qemu.
> # Both guest and host kernels are configured with 4k as page size.
> # releavant qemu command snippet,
>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
>   -net none -device vfio-pci,host=xxx
> 
> 
> On guest booting, observed mutliple messages as below,
> 
> qemu-system-aarch64: iommu has granularity incompatible with target AS
> 
> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
>   and the code expects the address mask to be 0xfff.

I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 

Thanks
-Bharat

> 
> if (len & iotlb->addr_mask) {
>         error_report
> 
> # vfio_dma_map is failing due to this error.
> 
> Any pointers ?
> 
> 
> > v3->v4:
> >  - Rebase to v4 version from Eric
> >  - Fixes from Eric with DPDK in VM
> >  - Logical division in multiple patches
> >
> > v2->v3:
> >  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >    Which is based on top of v2.10-rc0 that
> >  - Fixed issue with two PCI devices
> >  - Addressed review comments
> >
> > v1->v2:
> >   - Added trace events
> >   - removed vSMMU3 link in patch description
> >
> > Bharat Bhushan (5):
> >   target/arm/kvm: Translate the MSI doorbell in
> kvm_arch_fixup_msi_route
> >   virtio-iommu: Add iommu notifier for map/unmap
> >   virtio-iommu: Call iommu notifier for attach/detach
> >   virtio-iommu: add iommu replay
> >   virtio-iommu: add iommu notifier memory-region
> >
> >  hw/virtio/trace-events           |   5 ++
> >  hw/virtio/virtio-iommu.c         | 181
> ++++++++++++++++++++++++++++++++++++++-
> >  include/hw/virtio/virtio-iommu.h |   6 ++
> >  target/arm/kvm.c                 |  27 ++++++
> >  target/arm/trace-events          |   3 +
> >  5 files changed, 219 insertions(+), 3 deletions(-)
> >
> > --
> > 1.9.3
> >
> >
> 
> --
> Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,

On 27/09/2017 10:30, Bharat Bhushan wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>> Sent: Wednesday, September 27, 2017 1:11 PM
>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>> bharatb.yadav@gmail.com
>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>
>> Hi,
>>
>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>
>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>> device framework from Eric Augur and addresing review comments.
>>>
>>> This patch series allows PCI pass-through using virtio-iommu.
>>>
>>> This series is based on:
>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>     [1] [RFC] virtio-iommu version 0.4
>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4

Just to make sure, do you use the v0.4 virtio-iommu driver from above
branch?

Thanks

Eric
>>>
>>>  - virtio-iommu device emulation by Eric Augur.
>>>    [RFC v4 00/16] VIRTIO-IOMMU device
>>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
>>>
>>> Changes are available at : https://github.com/bharaty/qemu.git
>>> virtio-iommu-vfio-integration-v4
>>>
>>
>> # With the above sources, was trying to test the vfio-pci device assigned to
>>   guest using Qemu.
>> # Both guest and host kernels are configured with 4k as page size.
>> # releavant qemu command snippet,
>>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
>>   -net none -device vfio-pci,host=xxx
>>
>>
>> On guest booting, observed mutliple messages as below,
>>
>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>
>> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
>>   and the code expects the address mask to be 0xfff.
> 
> I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
> 
> Thanks
> -Bharat
> 
>>
>> if (len & iotlb->addr_mask) {
>>         error_report
>>
>> # vfio_dma_map is failing due to this error.
>>
>> Any pointers ?
>>
>>
>>> v3->v4:
>>>  - Rebase to v4 version from Eric
>>>  - Fixes from Eric with DPDK in VM
>>>  - Logical division in multiple patches
>>>
>>> v2->v3:
>>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>>>    Which is based on top of v2.10-rc0 that
>>>  - Fixed issue with two PCI devices
>>>  - Addressed review comments
>>>
>>> v1->v2:
>>>   - Added trace events
>>>   - removed vSMMU3 link in patch description
>>>
>>> Bharat Bhushan (5):
>>>   target/arm/kvm: Translate the MSI doorbell in
>> kvm_arch_fixup_msi_route
>>>   virtio-iommu: Add iommu notifier for map/unmap
>>>   virtio-iommu: Call iommu notifier for attach/detach
>>>   virtio-iommu: add iommu replay
>>>   virtio-iommu: add iommu notifier memory-region
>>>
>>>  hw/virtio/trace-events           |   5 ++
>>>  hw/virtio/virtio-iommu.c         | 181
>> ++++++++++++++++++++++++++++++++++++++-
>>>  include/hw/virtio/virtio-iommu.h |   6 ++
>>>  target/arm/kvm.c                 |  27 ++++++
>>>  target/arm/trace-events          |   3 +
>>>  5 files changed, 219 insertions(+), 3 deletions(-)
>>>
>>> --
>>> 1.9.3
>>>
>>>
>>
>> --
>> Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Linu Cherian]

On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
> Hi Linu,
> 
> On 27/09/2017 10:30, Bharat Bhushan wrote:
> > Hi,
> > 
> >> -----Original Message-----
> >> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> >> Sent: Wednesday, September 27, 2017 1:11 PM
> >> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> >> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> >> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> >> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> >> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> >> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> >> bharatb.yadav@gmail.com
> >> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> >>
> >> Hi,
> >>
> >> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> >>> This patch series integrates VFIO/VHOST with virtio-iommu.
> >>>
> >>> This version is mainly about rebasing on v4 version on virtio-iommu
> >>> device framework from Eric Augur and addresing review comments.
> >>>
> >>> This patch series allows PCI pass-through using virtio-iommu.
> >>>
> >>> This series is based on:
> >>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >>>     [1] [RFC] virtio-iommu version 0.4
> >>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
> Just to make sure, do you use the v0.4 virtio-iommu driver from above
> branch?

Yes, Eric i have that.

>From guest kernel,
  0.000000] Linux version 4.13.0-rc1-gd1949df
.
.
] virtio_iommu virtio0: aperture: 0x0-0xffffffffffffffff
[    0.927886] virtio_iommu virtio0: page mask: 0x40201000
[    0.931682] virtio_iommu virtio0: probe successful


Guest kernel source is on this commit,

commit d1949dfbf5c0d181b290625c28c5359284686e3a
Author: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Date:   Mon Jul 17 19:01:07 2017 +0100

    iommu/virtio-iommu: add MSI window probe
    
    Using the probe request, extract RESV_MEM information. When we encounter a
    MSI doorbell region, set it up as a IOMMU_RESV_MSI region. This will tell
    other subsystems that there is no need to map the MSI doorbell in the
    virtio-iommu, because MSIs bypass it.
    
    Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

with CONFIG_VIRTIO_IOMMU=y in config.

> 
> Thanks
> 
> Eric
> >>>
> >>>  - virtio-iommu device emulation by Eric Augur.
> >>>    [RFC v4 00/16] VIRTIO-IOMMU device
> >>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> >>>
> >>> Changes are available at : https://github.com/bharaty/qemu.git
> >>> virtio-iommu-vfio-integration-v4
> >>>
> >>
> >> # With the above sources, was trying to test the vfio-pci device assigned to
> >>   guest using Qemu.
> >> # Both guest and host kernels are configured with 4k as page size.
> >> # releavant qemu command snippet,
> >>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
> >>   -net none -device vfio-pci,host=xxx
> >>
> >>
> >> On guest booting, observed mutliple messages as below,
> >>
> >> qemu-system-aarch64: iommu has granularity incompatible with target AS
> >>
> >> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
> >>   and the code expects the address mask to be 0xfff.
> > 
> > I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
> > 
> > Thanks
> > -Bharat
> > 
> >>
> >> if (len & iotlb->addr_mask) {
> >>         error_report
> >>
> >> # vfio_dma_map is failing due to this error.
> >>
> >> Any pointers ?
> >>
> >>
> >>> v3->v4:
> >>>  - Rebase to v4 version from Eric
> >>>  - Fixes from Eric with DPDK in VM
> >>>  - Logical division in multiple patches
> >>>
> >>> v2->v3:
> >>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >>>    Which is based on top of v2.10-rc0 that
> >>>  - Fixed issue with two PCI devices
> >>>  - Addressed review comments
> >>>
> >>> v1->v2:
> >>>   - Added trace events
> >>>   - removed vSMMU3 link in patch description
> >>>
> >>> Bharat Bhushan (5):
> >>>   target/arm/kvm: Translate the MSI doorbell in
> >> kvm_arch_fixup_msi_route
> >>>   virtio-iommu: Add iommu notifier for map/unmap
> >>>   virtio-iommu: Call iommu notifier for attach/detach
> >>>   virtio-iommu: add iommu replay
> >>>   virtio-iommu: add iommu notifier memory-region
> >>>
> >>>  hw/virtio/trace-events           |   5 ++
> >>>  hw/virtio/virtio-iommu.c         | 181
> >> ++++++++++++++++++++++++++++++++++++++-
> >>>  include/hw/virtio/virtio-iommu.h |   6 ++
> >>>  target/arm/kvm.c                 |  27 ++++++
> >>>  target/arm/trace-events          |   3 +
> >>>  5 files changed, 219 insertions(+), 3 deletions(-)
> >>>
> >>> --
> >>> 1.9.3
> >>>
> >>>
> >>
> >> --
> >> Linu cherian

-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Linu Cherian]

On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
> Hi Linu,
> 
> On 27/09/2017 10:30, Bharat Bhushan wrote:
> > Hi,
> > 
> >> -----Original Message-----
> >> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> >> Sent: Wednesday, September 27, 2017 1:11 PM
> >> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> >> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> >> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> >> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> >> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> >> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> >> bharatb.yadav@gmail.com
> >> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> >>
> >> Hi,
> >>
> >> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> >>> This patch series integrates VFIO/VHOST with virtio-iommu.
> >>>
> >>> This version is mainly about rebasing on v4 version on virtio-iommu
> >>> device framework from Eric Augur and addresing review comments.
> >>>
> >>> This patch series allows PCI pass-through using virtio-iommu.
> >>>
> >>> This series is based on:
> >>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >>>     [1] [RFC] virtio-iommu version 0.4
> >>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
> Just to make sure, do you use the v0.4 virtio-iommu driver from above
> branch?
> 
> Thanks

I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
Hope you are referring to the same.


> 
> Eric
> >>>
> >>>  - virtio-iommu device emulation by Eric Augur.
> >>>    [RFC v4 00/16] VIRTIO-IOMMU device
> >>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> >>>
> >>> Changes are available at : https://github.com/bharaty/qemu.git
> >>> virtio-iommu-vfio-integration-v4
> >>>
> >>
> >> # With the above sources, was trying to test the vfio-pci device assigned to
> >>   guest using Qemu.
> >> # Both guest and host kernels are configured with 4k as page size.
> >> # releavant qemu command snippet,
> >>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
> >>   -net none -device vfio-pci,host=xxx
> >>
> >>
> >> On guest booting, observed mutliple messages as below,
> >>
> >> qemu-system-aarch64: iommu has granularity incompatible with target AS
> >>
> >> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
> >>   and the code expects the address mask to be 0xfff.
> > 
> > I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
> > 
> > Thanks
> > -Bharat
> > 
> >>
> >> if (len & iotlb->addr_mask) {
> >>         error_report
> >>
> >> # vfio_dma_map is failing due to this error.
> >>
> >> Any pointers ?
> >>
> >>
> >>> v3->v4:
> >>>  - Rebase to v4 version from Eric
> >>>  - Fixes from Eric with DPDK in VM
> >>>  - Logical division in multiple patches
> >>>
> >>> v2->v3:
> >>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >>>    Which is based on top of v2.10-rc0 that
> >>>  - Fixed issue with two PCI devices
> >>>  - Addressed review comments
> >>>
> >>> v1->v2:
> >>>   - Added trace events
> >>>   - removed vSMMU3 link in patch description
> >>>
> >>> Bharat Bhushan (5):
> >>>   target/arm/kvm: Translate the MSI doorbell in
> >> kvm_arch_fixup_msi_route
> >>>   virtio-iommu: Add iommu notifier for map/unmap
> >>>   virtio-iommu: Call iommu notifier for attach/detach
> >>>   virtio-iommu: add iommu replay
> >>>   virtio-iommu: add iommu notifier memory-region
> >>>
> >>>  hw/virtio/trace-events           |   5 ++
> >>>  hw/virtio/virtio-iommu.c         | 181
> >> ++++++++++++++++++++++++++++++++++++++-
> >>>  include/hw/virtio/virtio-iommu.h |   6 ++
> >>>  target/arm/kvm.c                 |  27 ++++++
> >>>  target/arm/trace-events          |   3 +
> >>>  5 files changed, 219 insertions(+), 3 deletions(-)
> >>>
> >>> --
> >>> 1.9.3
> >>>
> >>>
> >>
> >> --
> >> Linu cherian

-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,

On 27/09/2017 11:21, Linu Cherian wrote:
> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>> Hi Linu,
>>
>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>> bharatb.yadav@gmail.com
>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>
>>>> Hi,
>>>>
>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>
>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>> device framework from Eric Augur and addresing review comments.
>>>>>
>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>
>>>>> This series is based on:
>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>
>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>> branch?
>>
>> Thanks
> 
> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
> Hope you are referring to the same.

Yes that's the right one. I will also investigate on my side this afternoon.

Thanks

Eric
> 
> 
>>
>> Eric
>>>>>
>>>>>  - virtio-iommu device emulation by Eric Augur.
>>>>>    [RFC v4 00/16] VIRTIO-IOMMU device
>>>>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
>>>>>
>>>>> Changes are available at : https://github.com/bharaty/qemu.git
>>>>> virtio-iommu-vfio-integration-v4
>>>>>
>>>>
>>>> # With the above sources, was trying to test the vfio-pci device assigned to
>>>>   guest using Qemu.
>>>> # Both guest and host kernels are configured with 4k as page size.
>>>> # releavant qemu command snippet,
>>>>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
>>>>   -net none -device vfio-pci,host=xxx
>>>>
>>>>
>>>> On guest booting, observed mutliple messages as below,
>>>>
>>>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>>>
>>>> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
>>>>   and the code expects the address mask to be 0xfff.
>>>
>>> I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
>>>
>>> Thanks
>>> -Bharat
>>>
>>>>
>>>> if (len & iotlb->addr_mask) {
>>>>         error_report
>>>>
>>>> # vfio_dma_map is failing due to this error.
>>>>
>>>> Any pointers ?
>>>>
>>>>
>>>>> v3->v4:
>>>>>  - Rebase to v4 version from Eric
>>>>>  - Fixes from Eric with DPDK in VM
>>>>>  - Logical division in multiple patches
>>>>>
>>>>> v2->v3:
>>>>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>>>>>    Which is based on top of v2.10-rc0 that
>>>>>  - Fixed issue with two PCI devices
>>>>>  - Addressed review comments
>>>>>
>>>>> v1->v2:
>>>>>   - Added trace events
>>>>>   - removed vSMMU3 link in patch description
>>>>>
>>>>> Bharat Bhushan (5):
>>>>>   target/arm/kvm: Translate the MSI doorbell in
>>>> kvm_arch_fixup_msi_route
>>>>>   virtio-iommu: Add iommu notifier for map/unmap
>>>>>   virtio-iommu: Call iommu notifier for attach/detach
>>>>>   virtio-iommu: add iommu replay
>>>>>   virtio-iommu: add iommu notifier memory-region
>>>>>
>>>>>  hw/virtio/trace-events           |   5 ++
>>>>>  hw/virtio/virtio-iommu.c         | 181
>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>  include/hw/virtio/virtio-iommu.h |   6 ++
>>>>>  target/arm/kvm.c                 |  27 ++++++
>>>>>  target/arm/trace-events          |   3 +
>>>>>  5 files changed, 219 insertions(+), 3 deletions(-)
>>>>>
>>>>> --
>>>>> 1.9.3
>>>>>
>>>>>
>>>>
>>>> --
>>>> Linu cherian
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Linu Cherian]

Hi Eric,


On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
> Hi Linu,
> 
> On 27/09/2017 11:21, Linu Cherian wrote:
> > On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
> >> Hi Linu,
> >>
> >> On 27/09/2017 10:30, Bharat Bhushan wrote:
> >>> Hi,
> >>>
> >>>> -----Original Message-----
> >>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> >>>> Sent: Wednesday, September 27, 2017 1:11 PM
> >>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> >>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> >>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> >>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> >>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> >>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> >>>> bharatb.yadav@gmail.com
> >>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> >>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
> >>>>>
> >>>>> This version is mainly about rebasing on v4 version on virtio-iommu
> >>>>> device framework from Eric Augur and addresing review comments.
> >>>>>
> >>>>> This patch series allows PCI pass-through using virtio-iommu.
> >>>>>
> >>>>> This series is based on:
> >>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >>>>>     [1] [RFC] virtio-iommu version 0.4
> >>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> >>
> >> Just to make sure, do you use the v0.4 virtio-iommu driver from above
> >> branch?
> >>
> >> Thanks
> > 
> > I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
> > Hope you are referring to the same.
> 
> Yes that's the right one. I will also investigate on my side this afternoon.
> 
> Thanks
> 
> Eric

With the below workaround, atleast ping works for me.

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 249964a..2904617 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
        .attach_dev             = viommu_attach_dev,
        .map                    = viommu_map,
        .unmap                  = viommu_unmap,
-       .map_sg                 = viommu_map_sg,
+       .map_sg                 = default_iommu_map_sg,
        .iova_to_phys           = viommu_iova_to_phys,
        .add_device             = viommu_add_device,
        .remove_device          = viommu_remove_device,


Looks like the qemu backend doesnt have support to handle the map requests from 
virtio_iommu_map_sg, since it  merges multiple map requests into one with 
mapsize larger than page size(for eg. 0x5000). 

Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
the map size to be a power of 2.

 if (len & iotlb->addr_mask) {
        error_report("iommu has granularity incompatible with target AS");
        return false;
    }

Just trying to understand how this is not hitting in your case. 
 
-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,
On 04/10/2017 13:49, Linu Cherian wrote:
> Hi Eric,
> 
> 
> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>> Hi Linu,
>>
>> On 27/09/2017 11:21, Linu Cherian wrote:
>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>> Hi Linu,
>>>>
>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>> Hi,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>> bharatb.yadav@gmail.com
>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>
>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>
>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>
>>>>>>> This series is based on:
>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>
>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>> branch?
>>>>
>>>> Thanks
>>>
>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>> Hope you are referring to the same.
>>
>> Yes that's the right one. I will also investigate on my side this afternoon.
>>
>> Thanks
>>
>> Eric
> 
> With the below workaround, atleast ping works for me.
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 249964a..2904617 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
>         .attach_dev             = viommu_attach_dev,
>         .map                    = viommu_map,
>         .unmap                  = viommu_unmap,
> -       .map_sg                 = viommu_map_sg,
> +       .map_sg                 = default_iommu_map_sg,
>         .iova_to_phys           = viommu_iova_to_phys,
>         .add_device             = viommu_add_device,
>         .remove_device          = viommu_remove_device,
> 
> 
> Looks like the qemu backend doesnt have support to handle the map requests from 
> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
> mapsize larger than page size(for eg. 0x5000). 
On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
request for each sg element. The map size matches the sg element size.
Then each request is sent separately in _viommu_send_reqs_sync. I don't
see any concatenation. Looks Jean has a plan to check if it can
concatenate anything (/* TODO: merge physically-contiguous mappings if
any */) but this is not implemented yet.

However you should be allowed to map 1 sg element of 5 pages and then
notify the host about this event I think. Still looking at the code...

I still can't reproduce the issue at the moment. What kind of device are
you assigning?

Thanks

Eric
> 
> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
> the map size to be a power of 2.
> 
>  if (len & iotlb->addr_mask) {
>         error_report("iommu has granularity incompatible with target AS");
>         return false;
>     }
> 
> Just trying to understand how this is not hitting in your case. 
>  
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,
On 05/10/2017 12:46, Auger Eric wrote:
> Hi Linu,
> On 04/10/2017 13:49, Linu Cherian wrote:
>> Hi Eric,
>>
>>
>> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>>> Hi Linu,
>>>
>>> On 27/09/2017 11:21, Linu Cherian wrote:
>>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>>> Hi Linu,
>>>>>
>>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>>> Hi,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>>> bharatb.yadav@gmail.com
>>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>>
>>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>>
>>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>>
>>>>>>>> This series is based on:
>>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>>
>>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>>> branch?
>>>>>
>>>>> Thanks
>>>>
>>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>>> Hope you are referring to the same.
>>>
>>> Yes that's the right one. I will also investigate on my side this afternoon.
>>>
>>> Thanks
>>>
>>> Eric
>>
>> With the below workaround, atleast ping works for me.
>>
>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>> index 249964a..2904617 100644
>> --- a/drivers/iommu/virtio-iommu.c
>> +++ b/drivers/iommu/virtio-iommu.c
>>         .attach_dev             = viommu_attach_dev,
>>         .map                    = viommu_map,
>>         .unmap                  = viommu_unmap,
>> -       .map_sg                 = viommu_map_sg,
>> +       .map_sg                 = default_iommu_map_sg,
>>         .iova_to_phys           = viommu_iova_to_phys,
>>         .add_device             = viommu_add_device,
>>         .remove_device          = viommu_remove_device,
>>
>>
>> Looks like the qemu backend doesnt have support to handle the map requests from 
>> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
>> mapsize larger than page size(for eg. 0x5000). 
> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
> request for each sg element. The map size matches the sg element size.
> Then each request is sent separately in _viommu_send_reqs_sync. I don't
> see any concatenation. Looks Jean has a plan to check if it can
> concatenate anything (/* TODO: merge physically-contiguous mappings if
> any */) but this is not implemented yet.

Hopefully I was just able to reproduce your issue with an igb device. I
keep on debugging...

vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
qemu-system-aarch64: iommu has granularity incompatible with target AS


Thanks

Eric
> 
> However you should be allowed to map 1 sg element of 5 pages and then
> notify the host about this event I think. Still looking at the code...
> 
> I still can't reproduce the issue at the moment. What kind of device are
> you assigning?
> 
> Thanks
> 
> Eric
>>
>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
>> the map size to be a power of 2.
>>
>>  if (len & iotlb->addr_mask) {
>>         error_report("iommu has granularity incompatible with target AS");
>>         return false;
>>     }
>>
>> Just trying to understand how this is not hitting in your case. 
>>  
>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,

On 05/10/2017 13:54, Auger Eric wrote:
> Hi Linu,
> On 05/10/2017 12:46, Auger Eric wrote:
>> Hi Linu,
>> On 04/10/2017 13:49, Linu Cherian wrote:
>>> Hi Eric,
>>>
>>>
>>> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>>>> Hi Linu,
>>>>
>>>> On 27/09/2017 11:21, Linu Cherian wrote:
>>>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>>>> Hi Linu,
>>>>>>
>>>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>>>> bharatb.yadav@gmail.com
>>>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>>>
>>>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>>>
>>>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>>>
>>>>>>>>> This series is based on:
>>>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>>>
>>>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>>>> branch?
>>>>>>
>>>>>> Thanks
>>>>>
>>>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>>>> Hope you are referring to the same.
>>>>
>>>> Yes that's the right one. I will also investigate on my side this afternoon.
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>
>>> With the below workaround, atleast ping works for me.
>>>
>>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>>> index 249964a..2904617 100644
>>> --- a/drivers/iommu/virtio-iommu.c
>>> +++ b/drivers/iommu/virtio-iommu.c
>>>         .attach_dev             = viommu_attach_dev,
>>>         .map                    = viommu_map,
>>>         .unmap                  = viommu_unmap,
>>> -       .map_sg                 = viommu_map_sg,
>>> +       .map_sg                 = default_iommu_map_sg,
>>>         .iova_to_phys           = viommu_iova_to_phys,
>>>         .add_device             = viommu_add_device,
>>>         .remove_device          = viommu_remove_device,
>>>
>>>
>>> Looks like the qemu backend doesnt have support to handle the map requests from 
>>> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
>>> mapsize larger than page size(for eg. 0x5000). 
>> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
>> request for each sg element. The map size matches the sg element size.
>> Then each request is sent separately in _viommu_send_reqs_sync. I don't
>> see any concatenation. Looks Jean has a plan to check if it can
>> concatenate anything (/* TODO: merge physically-contiguous mappings if
>> any */) but this is not implemented yet.
> 
> Hopefully I was just able to reproduce your issue with an igb device. I
> keep on debugging...
> 
> vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
> qemu-system-aarch64: iommu has granularity incompatible with target AS
> 
> 
> Thanks
> 
> Eric
>>
>> However you should be allowed to map 1 sg element of 5 pages and then
>> notify the host about this event I think. Still looking at the code...
>>
>> I still can't reproduce the issue at the moment. What kind of device are
>> you assigning?
>>
>> Thanks
>>
>> Eric
>>>
>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
>>> the map size to be a power of 2.

Actually I missed the most important here ;-)
>>>
>>>  if (len & iotlb->addr_mask) {
This check looks suspiscious to me. In our case the len is not modified
by the previous translation and it fails, I don't see why. It should be
valid to be able to notify 5 granules.

Thanks

Eric
>>>         error_report("iommu has granularity incompatible with target AS");
>>>         return false;
>>>     }
>>>
>>> Just trying to understand how this is not hitting in your case. 
>>>  
>>>
>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,

On 05/10/2017 14:13, Auger Eric wrote:
> Hi Linu,
> 
> On 05/10/2017 13:54, Auger Eric wrote:
>> Hi Linu,
>> On 05/10/2017 12:46, Auger Eric wrote:
>>> Hi Linu,
>>> On 04/10/2017 13:49, Linu Cherian wrote:
>>>> Hi Eric,
>>>>
>>>>
>>>> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>>>>> Hi Linu,
>>>>>
>>>>> On 27/09/2017 11:21, Linu Cherian wrote:
>>>>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>>>>> Hi Linu,
>>>>>>>
>>>>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>>>>> bharatb.yadav@gmail.com
>>>>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>>>>
>>>>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>>>>
>>>>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>>>>
>>>>>>>>>> This series is based on:
>>>>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>>>>
>>>>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>>>>> branch?
>>>>>>>
>>>>>>> Thanks
>>>>>>
>>>>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>>>>> Hope you are referring to the same.
>>>>>
>>>>> Yes that's the right one. I will also investigate on my side this afternoon.
>>>>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>
>>>> With the below workaround, atleast ping works for me.
>>>>
>>>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>>>> index 249964a..2904617 100644
>>>> --- a/drivers/iommu/virtio-iommu.c
>>>> +++ b/drivers/iommu/virtio-iommu.c
>>>>         .attach_dev             = viommu_attach_dev,
>>>>         .map                    = viommu_map,
>>>>         .unmap                  = viommu_unmap,
>>>> -       .map_sg                 = viommu_map_sg,
>>>> +       .map_sg                 = default_iommu_map_sg,
>>>>         .iova_to_phys           = viommu_iova_to_phys,
>>>>         .add_device             = viommu_add_device,
>>>>         .remove_device          = viommu_remove_device,
>>>>
>>>>
>>>> Looks like the qemu backend doesnt have support to handle the map requests from 
>>>> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
>>>> mapsize larger than page size(for eg. 0x5000). 
>>> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
>>> request for each sg element. The map size matches the sg element size.
>>> Then each request is sent separately in _viommu_send_reqs_sync. I don't
>>> see any concatenation. Looks Jean has a plan to check if it can
>>> concatenate anything (/* TODO: merge physically-contiguous mappings if
>>> any */) but this is not implemented yet.
>>
>> Hopefully I was just able to reproduce your issue with an igb device. I
>> keep on debugging...
>>
>> vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>
>>
>> Thanks
>>
>> Eric
>>>
>>> However you should be allowed to map 1 sg element of 5 pages and then
>>> notify the host about this event I think. Still looking at the code...
>>>
>>> I still can't reproduce the issue at the moment. What kind of device are
>>> you assigning?
>>>
>>> Thanks
>>>
>>> Eric
>>>>
>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
>>>> the map size to be a power of 2.
> 
> Actually I missed the most important here ;-)
>>>>
>>>>  if (len & iotlb->addr_mask) {
> This check looks suspiscious to me. In our case the len is not modified
> by the previous translation and it fails, I don't see why. It should be
> valid to be able to notify 5 granules.

So after discussion with Alex, looks the way we notify the host
currently is wrong. we set the addr_mask to the mapping/unmapping size
-1 whereas this should be a page mask instead (granule size or block
size?). So if the guest maps 5 x 4kB pages we should send 5
notifications for each page and not a single one. It is unclear to me if
we can notify with hugepage/block page size mask. Peter may
confirm/infirm this. in vsmmuv3 code I notify by granule or block size.

Bharat, please can you add this to your TODO list?

Linu, thanks a lot for the time you spent debugging this issue.
Curiously on my side, it is really seldom hit but it is ...

Thanks!

Eric
> 
> Thanks
> 
> Eric
>>>>         error_report("iommu has granularity incompatible with target AS");
>>>>         return false;
>>>>     }
>>>>
>>>> Just trying to understand how this is not hitting in your case. 
>>>>  
>>>>
>>>
>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

> >> Thanks
> >>
> >> Eric
> >>>
> >>> However you should be allowed to map 1 sg element of 5 pages and
> >>> then notify the host about this event I think. Still looking at the code...
> >>>
> >>> I still can't reproduce the issue at the moment. What kind of device
> >>> are you assigning?
> >>>
> >>> Thanks
> >>>
> >>> Eric
> >>>>
> >>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
> >>>> expects the map size to be a power of 2.
> >
> > Actually I missed the most important here ;-)
> >>>>
> >>>>  if (len & iotlb->addr_mask) {
> > This check looks suspiscious to me. In our case the len is not
> > modified by the previous translation and it fails, I don't see why. It
> > should be valid to be able to notify 5 granules.
> 
> So after discussion with Alex, looks the way we notify the host currently is
> wrong. we set the addr_mask to the mapping/unmapping size
> -1 whereas this should be a page mask instead (granule size or block size?).
> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
> page and not a single one. It is unclear to me if we can notify with
> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
> code I notify by granule or block size.
> 
> Bharat, please can you add this to your TODO list?
> 
> Linu, thanks a lot for the time you spent debugging this issue.
> Curiously on my side, it is really seldom hit but it is ...

Thanks Linu and Eric, I added this to my todo list.
While I am still not able to reproduce the issue.  I tried with e1000 and now try with ixgbe device. May I know which device can be used to reproduce this issue?

Thanks
-Bharat

> 
> Thanks!
> 
> Eric
> >
> > Thanks
> >
> > Eric
> >>>>         error_report("iommu has granularity incompatible with target AS");
> >>>>         return false;
> >>>>     }
> >>>>
> >>>> Just trying to understand how this is not hitting in your case.
> >>>>
> >>>>
> >>>
> >>
> >

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Bharat,

On 06/10/2017 05:46, Bharat Bhushan wrote:
> 
> 
>>>> Thanks
>>>>
>>>> Eric
>>>>>
>>>>> However you should be allowed to map 1 sg element of 5 pages and
>>>>> then notify the host about this event I think. Still looking at the code...
>>>>>
>>>>> I still can't reproduce the issue at the moment. What kind of device
>>>>> are you assigning?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>>>
>>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
>>>>>> expects the map size to be a power of 2.
>>>
>>> Actually I missed the most important here ;-)
>>>>>>
>>>>>>  if (len & iotlb->addr_mask) {
>>> This check looks suspiscious to me. In our case the len is not
>>> modified by the previous translation and it fails, I don't see why. It
>>> should be valid to be able to notify 5 granules.
>>
>> So after discussion with Alex, looks the way we notify the host currently is
>> wrong. we set the addr_mask to the mapping/unmapping size
>> -1 whereas this should be a page mask instead (granule size or block size?).
>> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
>> page and not a single one. It is unclear to me if we can notify with
>> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
>> code I notify by granule or block size.
>>
>> Bharat, please can you add this to your TODO list?
>>
>> Linu, thanks a lot for the time you spent debugging this issue.
>> Curiously on my side, it is really seldom hit but it is ...
> 
> Thanks Linu and Eric, I added this to my todo list.
> While I am still not able to reproduce the issue.  I tried with e1000 and now try with ixgbe device. May I know which device can be used to reproduce this issue?

On my side I used an i350T2 device (igb) to reproduce the issue.

Thanks

Eric
> 
> Thanks
> -Bharat
> 
>>
>> Thanks!
>>
>> Eric
>>>
>>> Thanks
>>>
>>> Eric
>>>>>>         error_report("iommu has granularity incompatible with target AS");
>>>>>>         return false;
>>>>>>     }
>>>>>>
>>>>>> Just trying to understand how this is not hitting in your case.
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Linu Cherian]

On Fri Oct 06, 2017 at 09:24:20AM +0200, Auger Eric wrote:
> Hi Bharat,
> 
> On 06/10/2017 05:46, Bharat Bhushan wrote:
> > 
> > 
> >>>> Thanks
> >>>>
> >>>> Eric
> >>>>>
> >>>>> However you should be allowed to map 1 sg element of 5 pages and
> >>>>> then notify the host about this event I think. Still looking at the code...
> >>>>>
> >>>>> I still can't reproduce the issue at the moment. What kind of device
> >>>>> are you assigning?
> >>>>>
> >>>>> Thanks
> >>>>>
> >>>>> Eric
> >>>>>>
> >>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
> >>>>>> expects the map size to be a power of 2.
> >>>
> >>> Actually I missed the most important here ;-)
> >>>>>>
> >>>>>>  if (len & iotlb->addr_mask) {
> >>> This check looks suspiscious to me. In our case the len is not
> >>> modified by the previous translation and it fails, I don't see why. It
> >>> should be valid to be able to notify 5 granules.
> >>
> >> So after discussion with Alex, looks the way we notify the host currently is
> >> wrong. we set the addr_mask to the mapping/unmapping size
> >> -1 whereas this should be a page mask instead (granule size or block size?).
> >> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
> >> page and not a single one. It is unclear to me if we can notify with
> >> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
> >> code I notify by granule or block size.
> >>
> >> Bharat, please can you add this to your TODO list?
> >>
> >> Linu, thanks a lot for the time you spent debugging this issue.
> >> Curiously on my side, it is really seldom hit but it is ...
> > 
> > Thanks Linu and Eric, I added this to my todo list.
> > While I am still not able to reproduce the issue.  I tried with e1000 and now try with ixgbe device. May I know which device can be used to reproduce this issue?
> 
> On my side I used an i350T2 device (igb) to reproduce the issue.

Myself used a Thunderx NIC which is an on chip PCI device.


-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

Hi Alex, Eric,

> -----Original Message-----
> From: Qemu-devel [mailto:qemu-devel-
> bounces+bharat.bhushan=nxp.com@nongnu.org] On Behalf Of Bharat
> Bhushan
> Sent: Friday, October 06, 2017 9:16 AM
> To: Auger Eric <eric.auger@redhat.com>; Linu Cherian
> <linuc.decode@gmail.com>
> Cc: peter.maydell@linaro.org; kevin.tian@intel.com; mst@redhat.com;
> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> drjones@redhat.com; qemu-devel@nongnu.org;
> alex.williamson@redhat.com; qemu-arm@nongnu.org;
> linu.cherian@cavium.com; eric.auger.pro@gmail.com;
> robin.murphy@arm.com; christoffer.dall@linaro.org;
> bharatb.yadav@gmail.com
> Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO
> integration
> 
> 
> 
> > >> Thanks
> > >>
> > >> Eric
> > >>>
> > >>> However you should be allowed to map 1 sg element of 5 pages and
> > >>> then notify the host about this event I think. Still looking at the code...
> > >>>
> > >>> I still can't reproduce the issue at the moment. What kind of
> > >>> device are you assigning?
> > >>>
> > >>> Thanks
> > >>>
> > >>> Eric
> > >>>>
> > >>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
> > >>>> expects the map size to be a power of 2.
> > >
> > > Actually I missed the most important here ;-)
> > >>>>
> > >>>>  if (len & iotlb->addr_mask) {
> > > This check looks suspiscious to me. In our case the len is not
> > > modified by the previous translation and it fails, I don't see why.
> > > It should be valid to be able to notify 5 granules.
> >
> > So after discussion with Alex, looks the way we notify the host
> > currently is wrong. we set the addr_mask to the mapping/unmapping size
> > -1 whereas this should be a page mask instead (granule size or block size?).
> > So if the guest maps 5 x 4kB pages we should send 5 notifications for
> > each page and not a single one. It is unclear to me if we can notify
> > with hugepage/block page size mask. Peter may confirm/infirm this. in
> > vsmmuv3 code I notify by granule or block size.

My understanding is that host provides supported page sizes (page_size_mask), and Size of each notification to host should be exactly best fit of supported page-size and/or multiples of supported page-sizes.
So if guest maps 20K size (single request), and supported page size is 4K, so we can still send one 20K size request.
Not sure I get it, why multiples of supported page-size cannot be provided in one notification to host.

Thanks
-Bharat

> >
> > Bharat, please can you add this to your TODO list?
> >
> > Linu, thanks a lot for the time you spent debugging this issue.
> > Curiously on my side, it is really seldom hit but it is ...
> 
> Thanks Linu and Eric, I added this to my todo list.
> While I am still not able to reproduce the issue.  I tried with e1000 and now
> try with ixgbe device. May I know which device can be used to reproduce this
> issue?
> 
> Thanks
> -Bharat
> 
> >
> > Thanks!
> >
> > Eric
> > >
> > > Thanks
> > >
> > > Eric
> > >>>>         error_report("iommu has granularity incompatible with target
> AS");
> > >>>>         return false;
> > >>>>     }
> > >>>>
> > >>>> Just trying to understand how this is not hitting in your case.
> > >>>>
> > >>>>
> > >>>
> > >>
> > >

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Bharat,

On 10/10/2017 08:42, Bharat Bhushan wrote:
> Hi Alex, Eric,
> 
>> -----Original Message-----
>> From: Qemu-devel [mailto:qemu-devel-
>> bounces+bharat.bhushan=nxp.com@nongnu.org] On Behalf Of Bharat
>> Bhushan
>> Sent: Friday, October 06, 2017 9:16 AM
>> To: Auger Eric <eric.auger@redhat.com>; Linu Cherian
>> <linuc.decode@gmail.com>
>> Cc: peter.maydell@linaro.org; kevin.tian@intel.com; mst@redhat.com;
>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>> drjones@redhat.com; qemu-devel@nongnu.org;
>> alex.williamson@redhat.com; qemu-arm@nongnu.org;
>> linu.cherian@cavium.com; eric.auger.pro@gmail.com;
>> robin.murphy@arm.com; christoffer.dall@linaro.org;
>> bharatb.yadav@gmail.com
>> Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO
>> integration
>>
>>
>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>>>
>>>>>> However you should be allowed to map 1 sg element of 5 pages and
>>>>>> then notify the host about this event I think. Still looking at the code...
>>>>>>
>>>>>> I still can't reproduce the issue at the moment. What kind of
>>>>>> device are you assigning?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Eric
>>>>>>>
>>>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
>>>>>>> expects the map size to be a power of 2.
>>>>
>>>> Actually I missed the most important here ;-)
>>>>>>>
>>>>>>>  if (len & iotlb->addr_mask) {
>>>> This check looks suspiscious to me. In our case the len is not
>>>> modified by the previous translation and it fails, I don't see why.
>>>> It should be valid to be able to notify 5 granules.
>>>
>>> So after discussion with Alex, looks the way we notify the host
>>> currently is wrong. we set the addr_mask to the mapping/unmapping size
>>> -1 whereas this should be a page mask instead (granule size or block size?).
>>> So if the guest maps 5 x 4kB pages we should send 5 notifications for
>>> each page and not a single one. It is unclear to me if we can notify
>>> with hugepage/block page size mask. Peter may confirm/infirm this. in
>>> vsmmuv3 code I notify by granule or block size.
> 
> My understanding is that host provides supported page sizes (page_size_mask), and Size of each notification to host should be exactly best fit of supported page-size and/or multiples of supported page-sizes.
> So if guest maps 20K size (single request), and supported page size is 4K, so we can still send one 20K size request.
> Not sure I get it, why multiples of supported page-size cannot be provided in one notification to host.
I think the IOTLB API was originally devised to manage only granule or
block ^2 sizes. We might change this in the future but for the moment,
with respect to this series,  I would simply recommend to stick to the
existing API limitation and concurrently we can work on relaxing this
constraint on another series.

Thanks

Eric
> 
> Thanks
> -Bharat
> 
>>>
>>> Bharat, please can you add this to your TODO list?
>>>
>>> Linu, thanks a lot for the time you spent debugging this issue.
>>> Curiously on my side, it is really seldom hit but it is ...
>>
>> Thanks Linu and Eric, I added this to my todo list.
>> While I am still not able to reproduce the issue.  I tried with e1000 and now
>> try with ixgbe device. May I know which device can be used to reproduce this
>> issue?
>>
>> Thanks
>> -Bharat
>>
>>>
>>> Thanks!
>>>
>>> Eric
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>>>>>         error_report("iommu has granularity incompatible with target
>> AS");
>>>>>>>         return false;
>>>>>>>     }
>>>>>>>
>>>>>>> Just trying to understand how this is not hitting in your case.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Linu Cherian]

On Wed Sep 27, 2017 at 08:30:51AM +0000, Bharat Bhushan wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Linu Cherian [mailto:linuc.decode@gmail.com]
> > Sent: Wednesday, September 27, 2017 1:11 PM
> > To: Bharat Bhushan <bharat.bhushan@nxp.com>
> > Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> > peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> > qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> > marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> > drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> > bharatb.yadav@gmail.com
> > Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> > 
> > Hi,
> > 
> > On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> > > This patch series integrates VFIO/VHOST with virtio-iommu.
> > >
> > > This version is mainly about rebasing on v4 version on virtio-iommu
> > > device framework from Eric Augur and addresing review comments.
> > >
> > > This patch series allows PCI pass-through using virtio-iommu.
> > >
> > > This series is based on:
> > >  - virtio-iommu kernel driver by Jean-Philippe Brucker
> > >     [1] [RFC] virtio-iommu version 0.4
> > >     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> > >
> > >  - virtio-iommu device emulation by Eric Augur.
> > >    [RFC v4 00/16] VIRTIO-IOMMU device
> > >    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> > >
> > > Changes are available at : https://github.com/bharaty/qemu.git
> > > virtio-iommu-vfio-integration-v4
> > >
> > 
> > # With the above sources, was trying to test the vfio-pci device assigned to
> >   guest using Qemu.
> > # Both guest and host kernels are configured with 4k as page size.
> > # releavant qemu command snippet,
> >   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
> >   -net none -device vfio-pci,host=xxx
> > 
> > 
> > On guest booting, observed mutliple messages as below,
> > 
> > qemu-system-aarch64: iommu has granularity incompatible with target AS
> > 
> > # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
> >   and the code expects the address mask to be 0xfff.
> 
> I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 

Qemu cmd:

qemu-system-aarch64 -machine virt,gic_version=3 -cpu host -enable-kvm -nographic
 -smp 8 -m 1024 -kernel /boot/Image --append "console=ttyAMA0 root=/dev/vda rw" 
-drive if=none,file=/home/ubuntu/fs_images/ubu1604.img,id=hd0,format=raw 
-device virtio-iommu-device -device virtio-blk-device,drive=hd0 -net none -device vfio-pci,host=0002:01:00.3


Qemu log:
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
...





> 
> Thanks
> -Bharat
> 
> > 
> > if (len & iotlb->addr_mask) {
> >         error_report
> > 
> > # vfio_dma_map is failing due to this error.
> > 
> > Any pointers ?
> > 
> > 
> > > v3->v4:
> > >  - Rebase to v4 version from Eric
> > >  - Fixes from Eric with DPDK in VM
> > >  - Logical division in multiple patches
> > >
> > > v2->v3:
> > >  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> > >    Which is based on top of v2.10-rc0 that
> > >  - Fixed issue with two PCI devices
> > >  - Addressed review comments
> > >
> > > v1->v2:
> > >   - Added trace events
> > >   - removed vSMMU3 link in patch description
> > >
> > > Bharat Bhushan (5):
> > >   target/arm/kvm: Translate the MSI doorbell in
> > kvm_arch_fixup_msi_route
> > >   virtio-iommu: Add iommu notifier for map/unmap
> > >   virtio-iommu: Call iommu notifier for attach/detach
> > >   virtio-iommu: add iommu replay
> > >   virtio-iommu: add iommu notifier memory-region
> > >
> > >  hw/virtio/trace-events           |   5 ++
> > >  hw/virtio/virtio-iommu.c         | 181
> > ++++++++++++++++++++++++++++++++++++++-
> > >  include/hw/virtio/virtio-iommu.h |   6 ++
> > >  target/arm/kvm.c                 |  27 ++++++
> > >  target/arm/trace-events          |   3 +
> > >  5 files changed, 219 insertions(+), 3 deletions(-)
> > >
> > > --
> > > 1.9.3
> > >
> > >
> > 
> > --
> > Linu cherian

-- 
Linu cherian