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

2020-10-22 Thread Jean-Philippe Brucker 
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

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

2020-10-16 Thread Auger Eric 
Hi Jean,

On 10/8/20 7:15 PM, Jean-Philippe Brucker wrote:
> From: Bharat Bhushan 
> 
> Implement the replay callback to setup all mappings for a new memory
> region.
> 
> Signed-off-by: Bharat Bhushan 
> Signed-off-by: Jean-Philippe Brucker 
> ---
> 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(>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(>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

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

2020-10-08 Thread Jean-Philippe Brucker 
From: Bharat Bhushan 

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

Signed-off-by: Bharat Bhushan 
Signed-off-by: Jean-Philippe Brucker 
---
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);
+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(>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(>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
-- 
2.28.0