search for: iommu_resv_region

From: Thierry Reding <treding at nvidia.com> Most IOMMU drivers only need to free the memory allocated for each reserved region. Instead of open-coding the loop to do this in each driver, extract the code into a common function that can be used by all these drivers. Changes in v3: - add Reviewed-by from Jean-Philippe Brucker on virtio patch - add Acked-by from Will Deacon on ARM SMMU patch

From: Thierry Reding <treding at nvidia.com> Most IOMMU drivers only need to free the memory allocated for each reserved region. Instead of open-coding the loop to do this in each driver, extract the code into a common function that can be used by all these drivers. Changes in v2: - change subject prefix to "iommu: virtio: " for virtio-iommu.c driver Thierry Thierry Reding (5):

...; @@ -448,6 +454,105 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + u64 addr = le64_to_cpu(mem->addr); > + u64 size = le64_to_cpu(mem->size); > + > + if (len < sizeof(*mem)) > + return -EINVAL; > + > + switch (mem->subtype) { > + case...

...> @@ -448,6 +454,106 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + u64 addr = le64_to_cpu(mem->addr); > + u64 size = le64_to_cpu(mem->size); > + > + if (len < sizeof(*mem)) > + return -EINVAL; > + > + switch (mem->subtype) { > + case...

...> @@ -414,6 +420,101 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + nit: extra void line > + u64 start = le64_to_cpu(mem->start); > + u64 end = le64_to_cpu(mem->end); > + size_t size = end - start + 1; > + > + if (len < sizeof(*mem)) > + return -E...

...+ break; default: return -EINVAL; } @@ -448,6 +454,105 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) return ret; } +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, + struct virtio_iommu_probe_resv_mem *mem, + size_t len) +{ + struct iommu_resv_region *region = NULL; + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + + u64 addr = le64_to_cpu(mem->addr); + u64 size = le64_to_cpu(mem->size); + + if (len < sizeof(*mem)) + return -EINVAL; + + switch (mem->subtype) { + case VIRTIO_IOMMU_RESV_MEM_T_MSI: + region = iommu_al...

...+ break; default: return -EINVAL; } @@ -448,6 +454,106 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) return ret; } +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, + struct virtio_iommu_probe_resv_mem *mem, + size_t len) +{ + struct iommu_resv_region *region = NULL; + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + + u64 addr = le64_to_cpu(mem->addr); + u64 size = le64_to_cpu(mem->size); + + if (len < sizeof(*mem)) + return -EINVAL; + + switch (mem->subtype) { + case VIRTIO_IOMMU_RESV_MEM_T_MSI: + region = iommu_al...

...tail_size; + return len - tail_size; } @@ -414,6 +420,101 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) return ret; } +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, + struct virtio_iommu_probe_resv_mem *mem, + size_t len) +{ + struct iommu_resv_region *region = NULL; + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + + u64 start = le64_to_cpu(mem->start); + u64 end = le64_to_cpu(mem->end); + size_t size = end - start + 1; + + if (len < sizeof(*mem)) + return -EINVAL; + + switch (mem->subtype) { + default: + dev_warn(...

...> @@ -448,6 +454,106 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + u64 addr = le64_to_cpu(mem->addr); > + u64 size = le64_to_cpu(mem->size); > + > + if (len < sizeof(*mem)) > + return -EINVAL; > + > + switch (mem->subtype) { > + case...

...> @@ -448,6 +454,106 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + u64 addr = le64_to_cpu(mem->addr); > + u64 size = le64_to_cpu(mem->size); > + > + if (len < sizeof(*mem)) > + return -EINVAL; > + > + switch (mem->subtype) { > + case...

...tail_size; + return len - tail_size; } @@ -404,6 +410,103 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) return ret; } +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, + struct virtio_iommu_probe_resv_mem *mem, + size_t len) +{ + struct iommu_resv_region *region = NULL; + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + + u64 start = le64_to_cpu(mem->start); + u64 end = le64_to_cpu(mem->end); + size_t size = end - start + 1; + + if (len < sizeof(*mem)) + return -EINVAL; + + switch (mem->subtype) { + default: + if (mem-&...

...@@ static int viommu_replay_mappings(struct viommu_domain *vdomain) return ret; } +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, + struct virtio_iommu_probe_resv_mem *mem, + size_t len) +{ + size_t size; + u64 start64, end64; + phys_addr_t start, end; + struct iommu_resv_region *region = NULL; + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + + start = start64 = le64_to_cpu(mem->start); + end = end64 = le64_to_cpu(mem->end); + size = end64 - start64 + 1; + + /* Catch any overflow, including the unlikely end64 - start64 + 1 = 0 */ + if (start != start...

...-- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2710,15 +2710,6 @@ static void arm_smmu_get_resv_regions(struct device *dev, iommu_dma_get_resv_regions(dev, head); } -static void arm_smmu_put_resv_regions(struct device *dev, - struct list_head *head) -{ - struct iommu_resv_region *entry, *next; - - list_for_each_entry_safe(entry, next, head, list) - kfree(entry); -} - static struct iommu_ops arm_smmu_ops = { .capable = arm_smmu_capable, .domain_alloc = arm_smmu_domain_alloc, @@ -2736,7 +2727,7 @@ static struct iommu_ops arm_smmu_ops = { .domain_set_attr = arm_smmu...

...> @@ -414,6 +420,101 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + u64 start = le64_to_cpu(mem->start); > + u64 end = le64_to_cpu(mem->end); > + size_t size = end - start + 1; > + > + if (len < sizeof(*mem)) > + return -EINVAL; > + > +...

...> @@ -404,6 +410,103 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain) > return ret; > } > > +static int viommu_add_resv_mem(struct viommu_endpoint *vdev, > + struct virtio_iommu_probe_resv_mem *mem, > + size_t len) > +{ > + struct iommu_resv_region *region = NULL; > + unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + u64 start = le64_to_cpu(mem->start); > + u64 end = le64_to_cpu(mem->end); > + size_t size = end - start + 1; > + > + if (len < sizeof(*mem)) > + return -EINVAL; > + > +...

...allocated for each reserved region will be > + * freed. If an IOMMU driver allocates additional resources per region, it is > + * going to have to implement a custom callback. > + */ > +void iommu_put_resv_regions_simple(struct device *dev, struct list_head *list) > +{ > + struct iommu_resv_region *entry, *next; > + > + list_for_each_entry_safe(entry, next, list, list) > + kfree(entry); > +} > +EXPORT_SYMBOL(iommu_put_resv_regions_simple); Can you call this directly from iommu_put_resv_regions() if the function pointer in ops is NULL? That would save having to plumb the defa...

...in->nr_endpoints) > + return; As for patch 1, a NULL check in viommu_sync_req() would allow dropping this one Thanks, Jean > + viommu_sync_req(vdomain->viommu); > +} > + > static void viommu_get_resv_regions(struct device *dev, struct list_head *head) > { > struct iommu_resv_region *entry, *new_entry, *msi = NULL; > @@ -1049,6 +1058,8 @@ static bool viommu_capable(struct device *dev, enum iommu_cap cap) > switch (cap) { > case IOMMU_CAP_CACHE_COHERENCY: > return true; > + case IOMMU_CAP_DEFERRED_FLUSH: > + return true; > default: > return...

Implement the virtio-iommu driver following version 0.5 of the specification [1]. Previous version of this code was sent back in April [2], implementing the first public RFC. Since then there has been lots of progress and discussion on the specification side, and I think the driver is in a good shape now. The reason patches 1-3 are only RFC is that I'm waiting on feedback from the Virtio TC

Implement the virtio-iommu driver following version 0.5 of the specification [1]. Previous version of this code was sent back in April [2], implementing the first public RFC. Since then there has been lots of progress and discussion on the specification side, and I think the driver is in a good shape now. The reason patches 1-3 are only RFC is that I'm waiting on feedback from the Virtio TC

...IOMMU is set up. > > > > > > > > + viommu_sync_req(vdomain->viommu); > > > > +} > > > > + > > > > static void viommu_get_resv_regions(struct device *dev, struct list_head *head) > > > > { > > > > struct iommu_resv_region *entry, *new_entry, *msi = NULL; > > > > @@ -1049,6 +1058,8 @@ static bool viommu_capable(struct device *dev, enum iommu_cap cap) > > > > switch (cap) { > > > > case IOMMU_CAP_CACHE_COHERENCY: > > > > return true; > > > > + case...