diff mbox series

[v10,05/10] virtio-iommu: Add replay() memory region callback

Message ID 20201008171558.410886-6-jean-philippe@linaro.org
State New
Headers show
Series virtio-iommu: VFIO integration | expand

Commit Message

Jean-Philippe Brucker Oct. 8, 2020, 5:15 p.m. UTC 
From: Bharat Bhushan <bbhushan2@marvell.com>

Implement the replay callback to setup all mappings for a new memory
region.

Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
v10: Homogenize tracepoint arguments
---
 hw/virtio/virtio-iommu.c | 41 ++++++++++++++++++++++++++++++++++++++++
 hw/virtio/trace-events   |  1 +
 2 files changed, 42 insertions(+)

Comments

Eric Auger Oct. 16, 2020, 9:12 a.m. UTC | #1 
Hi Jean,

On 10/8/20 7:15 PM, Jean-Philippe Brucker wrote:
> From: Bharat Bhushan <bbhushan2@marvell.com>

> 

> Implement the replay callback to setup all mappings for a new memory

> region.

> 

> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>

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

> ---

> v10: Homogenize tracepoint arguments

> ---

>  hw/virtio/virtio-iommu.c | 41 ++++++++++++++++++++++++++++++++++++++++

>  hw/virtio/trace-events   |  1 +

>  2 files changed, 42 insertions(+)

> 

> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c

> index 7e6e3cf5200..d2b96846134 100644

> --- a/hw/virtio/virtio-iommu.c

> +++ b/hw/virtio/virtio-iommu.c

> @@ -861,6 +861,46 @@ 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)

> +{

> +    VirtIOIOMMUMapping *mapping = (VirtIOIOMMUMapping *) value;

> +    VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;

> +    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;

> +

> +    trace_virtio_iommu_remap(mr->parent_obj.name, interval->low, interval->high,

> +                             mapping->phys_addr);

> +    virtio_iommu_notify_unmap(mr, interval->low, interval->high);

> +    virtio_iommu_notify_map(mr, interval->low, interval->high,

> +                            mapping->phys_addr);

I don't get the preliminary unmap with the same data. Why isn't the map
sufficient to replay?

The default implementation only notifies for valid entries.
> +    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;

> +    VirtIOIOMMUEndpoint *ep;

> +

> +    sid = virtio_iommu_get_bdf(sdev);

> +

> +    qemu_mutex_lock(&s->mutex);

> +

> +    if (!s->endpoints) {

> +        goto unlock;

> +    }

> +

> +    ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(sid));

> +    if (!ep || !ep->domain) {

> +        goto unlock;

> +    }

> +

> +    g_tree_foreach(ep->domain->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);

> @@ -1091,6 +1131,7 @@ static void virtio_iommu_memory_region_class_init(ObjectClass *klass,

>      IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);

>  

>      imrc->translate = virtio_iommu_translate;

> +    imrc->replay = virtio_iommu_replay;

>  }

>  

>  static const TypeInfo virtio_iommu_info = {

> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events

> index 65a48555c78..16f4729db4b 100644

> --- a/hw/virtio/trace-events

> +++ b/hw/virtio/trace-events

> @@ -108,6 +108,7 @@ virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uin

>  virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start, uint64_t end) "dev= %d, type=%d start=0x%"PRIx64" end=0x%"PRIx64

>  virtio_iommu_notify_map(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64

>  virtio_iommu_notify_unmap(const char *name, uint64_t virt_start, uint64_t virt_end) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64

> +virtio_iommu_remap(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64

>  

>  # virtio-mem.c

>  virtio_mem_send_response(uint16_t type) "type=%" PRIu16

> 

Thanks

Eric
Jean-Philippe Brucker Oct. 22, 2020, 4:42 p.m. UTC | #2 
On Fri, Oct 16, 2020 at 11:12:35AM +0200, Auger Eric wrote:
> > +static gboolean virtio_iommu_remap(gpointer key, gpointer value, gpointer data)

> > +{

> > +    VirtIOIOMMUMapping *mapping = (VirtIOIOMMUMapping *) value;

> > +    VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;

> > +    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;

> > +

> > +    trace_virtio_iommu_remap(mr->parent_obj.name, interval->low, interval->high,

> > +                             mapping->phys_addr);

> > +    virtio_iommu_notify_unmap(mr, interval->low, interval->high);

> > +    virtio_iommu_notify_map(mr, interval->low, interval->high,

> > +                            mapping->phys_addr);

> I don't get the preliminary unmap with the same data. Why isn't the map

> sufficient to replay?

> 

> The default implementation only notifies for valid entries.


Yes it should be enough, I'll remove the unmap

Thanks,
Jean
diff mbox series

Patch

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 7e6e3cf5200..d2b96846134 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -861,6 +861,46 @@  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)
+{
+    VirtIOIOMMUMapping *mapping = (VirtIOIOMMUMapping *) value;
+    VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;
+    IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
+
+    trace_virtio_iommu_remap(mr->parent_obj.name, interval->low, interval->high,
+                             mapping->phys_addr);
+    virtio_iommu_notify_unmap(mr, interval->low, interval->high);
+    virtio_iommu_notify_map(mr, interval->low, interval->high,
+                            mapping->phys_addr);
+    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;
+    VirtIOIOMMUEndpoint *ep;
+
+    sid = virtio_iommu_get_bdf(sdev);
+
+    qemu_mutex_lock(&s->mutex);
+
+    if (!s->endpoints) {
+        goto unlock;
+    }
+
+    ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(sid));
+    if (!ep || !ep->domain) {
+        goto unlock;
+    }
+
+    g_tree_foreach(ep->domain->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);
@@ -1091,6 +1131,7 @@  static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
     IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
 
     imrc->translate = virtio_iommu_translate;
+    imrc->replay = virtio_iommu_replay;
 }
 
 static const TypeInfo virtio_iommu_info = {
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 65a48555c78..16f4729db4b 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -108,6 +108,7 @@  virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uin
 virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start, uint64_t end) "dev= %d, type=%d start=0x%"PRIx64" end=0x%"PRIx64
 virtio_iommu_notify_map(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64
 virtio_iommu_notify_unmap(const char *name, uint64_t virt_start, uint64_t virt_end) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
+virtio_iommu_remap(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64
 
 # virtio-mem.c
 virtio_mem_send_response(uint16_t type) "type=%" PRIu16