On Fri, Apr 29, 2022 at 08:07:14AM +0000, 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.
>
> > + 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? As commented in last
> patch this may allow removing the range op completely.
Yea, I'm not super keen on the two ops either, especially since they
are so wildly different.
I would expect that set_dirty_tracking turns on tracking for the
entire iommu domain, for all present and future maps
While set_dirty_tracking_range - I guess it only does the range, so if
we make a new map then the new range will be untracked? But that is
now racy, we have to map and then call set_dirty_tracking_range
It seems better for the iommu driver to deal with this and ARM should
atomically make the new maps dirty tracking..
> > +int iopt_set_dirty_tracking(struct io_pagetable *iopt,
> > + struct iommu_domain *domain, bool enable)
> > +{
> > + struct iommu_domain *dom;
> > + unsigned long index;
> > + int ret = -EOPNOTSUPP;
Returns EOPNOTSUPP if the xarray is empty?
Jason
