Re: [PATCH v3 07/19] iommufd: Dirty tracking data support

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

On 23/09/2023 02:40, Joao Martins wrote:
> On 23/09/2023 02:24, Joao Martins wrote:
>> +int iopt_read_and_clear_dirty_data(struct io_pagetable *iopt,
>> +				   struct iommu_domain *domain,
>> +				   unsigned long flags,
>> +				   struct iommufd_dirty_data *bitmap)
>> +{
>> +	unsigned long last_iova, iova = bitmap->iova;
>> +	unsigned long length = bitmap->length;
>> +	int ret = -EOPNOTSUPP;
>> +
>> +	if ((iova & (iopt->iova_alignment - 1)))
>> +		return -EINVAL;
>> +
>> +	if (check_add_overflow(iova, length - 1, &last_iova))
>> +		return -EOVERFLOW;
>> +
>> +	down_read(&iopt->iova_rwsem);
>> +	ret = iommu_read_and_clear_dirty(domain, flags, bitmap);
>> +	up_read(&iopt->iova_rwsem);
>> +	return ret;
>> +}
> 
> I need to call out that a mistake I made, noticed while submitting. I should be
> walk over iopt_areas here (or in iommu_read_and_clear_dirty()) to check
> area::pages. So this is a comment I have to fix for next version. 

Below is how I fixed it.

Essentially the thinking being that the user passes either an mapped IOVA area
it mapped *or* a subset of a mapped IOVA area. This should also allow the
possibility of having multiple threads read dirties from huge IOVA area splitted
in different chunks (in the case it gets splitted into lowest level).

diff --git a/drivers/iommu/iommufd/io_pagetable.c
b/drivers/iommu/iommufd/io_pagetable.c
index 5a35885aef04..991c57458725 100644
--- a/drivers/iommu/iommufd/io_pagetable.c
+++ b/drivers/iommu/iommufd/io_pagetable.c
@@ -473,7 +473,9 @@ int iopt_read_and_clear_dirty_data(struct io_pagetable *iopt,
 {
        unsigned long last_iova, iova = bitmap->iova;
        unsigned long length = bitmap->length;
-       int ret = -EOPNOTSUPP;
+       struct iopt_area *area;
+       bool found = false;
+       int ret = -EINVAL;

        if ((iova & (iopt->iova_alignment - 1)))
                return -EINVAL;
@@ -482,7 +484,22 @@ int iopt_read_and_clear_dirty_data(struct io_pagetable *iopt,
                return -EOVERFLOW;

        down_read(&iopt->iova_rwsem);
-       ret = iommu_read_and_clear_dirty(domain, flags, bitmap);
+
+       /* Find the portion of IOVA space belonging to area */
+       while ((area = iopt_area_iter_first(iopt, iova, last_iova))) {
+               unsigned long area_last = iopt_area_last_iova(area);
+               unsigned long area_first = iopt_area_iova(area);
+
+               if (!area->pages)
+                       continue;
+
+               found = (iova >= area_first && last_iova <= area_last);
+               if (found)
+                       break;
+       }
+
+       if (found)
+               ret = iommu_read_and_clear_dirty(domain, flags, bitmap);
        up_read(&iopt->iova_rwsem);

        return ret;