Re: [PATCH v2 08/22] iommufd: Abstract iopt_pin_pages and iopt_unpin_pages helpers

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Apr 28, 2025 at 03:12:33PM -0700, Nicolin Chen wrote:
> On Mon, Apr 28, 2025 at 08:14:19PM +0000, Pranjal Shrivastava wrote:
> > On Fri, Apr 25, 2025 at 10:58:03PM -0700, Nicolin Chen wrote:
> > > +	iopt_for_each_contig_area(&iter, area, iopt, iova, last_iova) {
> > > +		unsigned long last = min(last_iova, iopt_area_last_iova(area));
> > > +		unsigned long last_index = iopt_area_iova_to_index(area, last);
> > > +		unsigned long index =
> > > +			iopt_area_iova_to_index(area, iter.cur_iova);
> > > +
> > > +		if (area->prevent_access ||
> > 
> > Nit:
> > Shouldn't we return -EBUSY or something if (area->prevent_access == 1) ?
> > IIUC, this just means that an unmap attempt is in progress, hence avoid
> > accessing the area.
> 
> Maybe. But this is what it was. So we need a different patch to
> change that.

Rereading the code. The prevent_access is set by an unmap(), which
means there shouldn't be any pin() and rw() as the caller should
finish unmap() first.

In the newer use case of vCMDQ, it's similar. If VMM is unmapping
the stage-2 mapping, it shouldn't try to allocate a vCMDQ.

-EBUSY makes some sense, but -EINVAL could still stand.

So, I am leaving it as is, since this patch is just about moving
the functions for sharing.

Nicolin