On 4/29/22 09:12, Tian, Kevin wrote:
>> From: Joao Martins <joao.m.martins@xxxxxxxxxx>
>> Sent: Friday, April 29, 2022 5:09 AM
> [...]
>> +
>> +static int iommu_read_and_clear_dirty(struct iommu_domain *domain,
>> + struct iommufd_dirty_data *bitmap)
>
> In a glance this function and all previous helpers doesn't rely on any
> iommufd objects except that the new structures are named as
> iommufd_xxx.
>
> I wonder whether moving all of them to the iommu layer would make
> more sense here.
>
I suppose, instinctively, I was trying to make this tie to iommufd only,
to avoid getting it called in cases we don't except when made as a generic
exported kernel facility.
(note: iommufd can be built as a module).
>> +{
>> + const struct iommu_domain_ops *ops = domain->ops;
>> + struct iommu_iotlb_gather gather;
>> + struct iommufd_dirty_iter iter;
>> + int ret = 0;
>> +
>> + if (!ops || !ops->read_and_clear_dirty)
>> + return -EOPNOTSUPP;
>> +
>> + iommu_dirty_bitmap_init(&iter.dirty, bitmap->iova,
>> + __ffs(bitmap->page_size), &gather);
>> + ret = iommufd_dirty_iter_init(&iter, bitmap);
>> + if (ret)
>> + return -ENOMEM;
>> +
>> + for (; iommufd_dirty_iter_done(&iter);
>> + iommufd_dirty_iter_advance(&iter)) {
>> + ret = iommufd_dirty_iter_get(&iter);
>> + if (ret)
>> + break;
>> +
>> + ret = ops->read_and_clear_dirty(domain,
>> + iommufd_dirty_iova(&iter),
>> + iommufd_dirty_iova_length(&iter), &iter.dirty);
>> +
>> + iommufd_dirty_iter_put(&iter);
>> +
>> + if (ret)
>> + break;
>> + }
>> +
>> + iommu_iotlb_sync(domain, &gather);
>> + iommufd_dirty_iter_free(&iter);
>> +
>> + return ret;
>> +}
>> +
