On 4/29/22 09:07, Tian, Kevin wrote:
>> From: Joao Martins <joao.m.martins@xxxxxxxxxx>
>> Sent: Friday, April 29, 2022 5:09 AM
>>
>> +static int __set_dirty_tracking_range_locked(struct iommu_domain
>> *domain,
>
> suppose anything using iommu_domain as the first argument should
> be put in the iommu layer. Here it's more reasonable to use iopt
> as the first argument or simply merge with the next function.
>
OK
>> + struct io_pagetable *iopt,
>> + bool enable)
>> +{
>> + const struct iommu_domain_ops *ops = domain->ops;
>> + struct iommu_iotlb_gather gather;
>> + struct iopt_area *area;
>> + int ret = -EOPNOTSUPP;
>> + unsigned long iova;
>> + size_t size;
>> +
>> + iommu_iotlb_gather_init(&gather);
>> +
>> + for (area = iopt_area_iter_first(iopt, 0, ULONG_MAX); area;
>> + area = iopt_area_iter_next(area, 0, ULONG_MAX)) {
>
> how is this different from leaving iommu driver to walk the page table
> and the poke the modifier bit for all present PTEs?
It isn't. Moving towards a single op makes this simpler for iommu core API.
> As commented in last
> patch this may allow removing the range op completely.
>
Yes.
>> + iova = iopt_area_iova(area);
>> + size = iopt_area_last_iova(area) - iova;
>> +
>> + if (ops->set_dirty_tracking_range) {
>> + ret = ops->set_dirty_tracking_range(domain, iova,
>> + size, &gather,
>> + enable);
>> + if (ret < 0)
>> + break;
>> + }
>> + }
>> +
>> + iommu_iotlb_sync(domain, &gather);
>> +
>> + return ret;
>> +}
>> +
>> +static int iommu_set_dirty_tracking(struct iommu_domain *domain,
>> + struct io_pagetable *iopt, bool enable)
>
> similarly rename to __iopt_set_dirty_tracking() and use iopt as the
> leading argument.
>
/me nods
