Re: [PATCH RESEND v9 03/13] iommu/sva: Add PASID helpers

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Tue, Sep 08, 2020 at 09:45:02AM +0200, Auger Eric wrote:
> > +int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
> > +{
> > +	int ret = 0;
> > +	ioasid_t pasid;
> > +
> > +	if (min == INVALID_IOASID || max == INVALID_IOASID ||
> > +	    min == 0 || max < min)
> you may add a comment explaining why min == 0 is forbidden.

Right, I'll add to the function doc "@min must be greater than 0 because
0 indicates an unused mm->pasid."

> > +		return -EINVAL;
> > +
> > +	mutex_lock(&iommu_sva_lock);
> > +	if (mm->pasid) {
> > +		if (mm->pasid >= min && mm->pasid <= max)
> > +			ioasid_get(mm->pasid);
> > +		else
> > +			ret = -EOVERFLOW;
> > +	} else {
> > +		pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
> > +		if (pasid == INVALID_IOASID)
> > +			ret = -ENOMEM;
> > +		else
> > +			mm->pasid = pasid;
> > +	}
> > +	mutex_unlock(&iommu_sva_lock);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
> > +
> > +/**
> > + * iommu_sva_free_pasid - Release the mm's PASID
> > + * @mm: the mm.
> > + *
> > + * Drop one reference to a PASID allocated with iommu_sva_alloc_pasid()
> > + */
> > +void iommu_sva_free_pasid(struct mm_struct *mm)
> > +{
> > +	mutex_lock(&iommu_sva_lock);
> > +	if (ioasid_put(mm->pasid))
> > +		mm->pasid = 0;
> ditto: 0 versus INVALID_IOASID
> > +	mutex_unlock(&iommu_sva_lock);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_sva_free_pasid);
> > +
> > +/* ioasid wants a void * argument */
> shouldn't it be:
> ioasid_find getter() requires a void *arg?

Ok

Thanks,
Jean

> > +static bool __mmget_not_zero(void *mm)
> > +{
> > +	return mmget_not_zero(mm);
> > +}
> > +
> > +/**
> > + * iommu_sva_find() - Find mm associated to the given PASID
> > + * @pasid: Process Address Space ID assigned to the mm
> > + *
> > + * On success a reference to the mm is taken, and must be released with mmput().
> > + *
> > + * Returns the mm corresponding to this PASID, or an error if not found.
> > + */
> > +struct mm_struct *iommu_sva_find(ioasid_t pasid)
> > +{
> > +	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_sva_find);
> > 
> Thanks
> 
> Eric
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu