On Thu, May 18, 2023 at 09:46:35PM +0100, Joao Martins wrote:
> +int iopt_set_dirty_tracking(struct io_pagetable *iopt,
> + struct iommu_domain *domain, bool enable)
> +{
> + const struct iommu_domain_ops *ops = domain->ops;
> + struct iommu_dirty_bitmap dirty;
> + struct iommu_iotlb_gather gather;
> + struct iopt_area *area;
> + int ret = 0;
> +
> + if (!ops->set_dirty_tracking)
> + return -EOPNOTSUPP;
> +
> + iommu_dirty_bitmap_init(&dirty, NULL, &gather);
> +
> + down_write(&iopt->iova_rwsem);
> + for (area = iopt_area_iter_first(iopt, 0, ULONG_MAX);
> + area && enable;
That's a goofy way to write this.. put this in a function and don't
call it if enable is not set.
Why is this down_write() ?
You can see that this locking already prevents racing dirty read with
domain unmap.
This domain cannot be removed from the iopt eg through
iopt_table_remove_domain() because this is holding the object
reference on the hwpt
The area cannot be unmapped because this is holding the
&iopt->iova_rwsem
There is no other way to call unmap..
You do have to check that area->pages != NULL though
Jason
