On Wed, Oct 18, 2023 at 09:27:04PM +0100, Joao Martins wrote:
> +int iommufd_check_iova_range(struct iommufd_ioas *ioas,
> + struct iommu_hwpt_get_dirty_iova *bitmap)
> +{
> + unsigned long pgshift, npages;
> + size_t iommu_pgsize;
> + int rc = -EINVAL;
> +
> + pgshift = __ffs(bitmap->page_size);
> + npages = bitmap->length >> pgshift;
npages = bitmap->length / bitmap->page_size;
? (if page_size is a bitmask it is badly named)
> +static int __iommu_read_and_clear_dirty(struct iova_bitmap *bitmap,
> + unsigned long iova, size_t length,
> + void *opaque)
> +{
> + struct iopt_area *area;
> + struct iopt_area_contig_iter iter;
> + struct iova_bitmap_fn_arg *arg = opaque;
> + struct iommu_domain *domain = arg->domain;
> + struct iommu_dirty_bitmap *dirty = arg->dirty;
> + const struct iommu_dirty_ops *ops = domain->dirty_ops;
> + unsigned long last_iova = iova + length - 1;
> + int ret = -EINVAL;
> +
> + iopt_for_each_contig_area(&iter, area, arg->iopt, iova, last_iova) {
> + unsigned long last = min(last_iova, iopt_area_last_iova(area));
> +
> + ret = ops->read_and_clear_dirty(domain, iter.cur_iova,
> + last - iter.cur_iova + 1,
> + 0, dirty);
This seems like a lot of stuff going on with ret..
> + if (ret)
return ret
> + break;
> + }
> +
> + if (!iopt_area_contig_done(&iter))
> + ret = -EINVAL;
return -EINVAL
> +
> + return ret;
return 0;
And remove the -EINVAL. iopt_area_contig_done() captures the case
where the iova range is not fully contained by areas, even the case
where there are no areas.
But otherwise
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Jason
