Messages in this thread | | | Date | Mon, 24 Jan 2022 12:33:12 -0800 | From | Fenghua Yu <> | Subject | Re: [PATCH v2 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit |
| |
Hi, Thomas,
On Mon, Jan 24, 2022 at 09:21:24PM +0100, Thomas Gleixner wrote: > On Fri, Dec 17 2021 at 22:01, Fenghua Yu wrote: > > diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c > > index bd41405d34e9..ee2294e02716 100644 > > --- a/drivers/iommu/iommu-sva-lib.c > > +++ b/drivers/iommu/iommu-sva-lib.c > > @@ -18,8 +18,7 @@ static DECLARE_IOASID_SET(iommu_sva_pasid); > > * > > * Try to allocate a PASID for this mm, or take a reference to the existing one > > * provided it fits within the [@min, @max] range. On success the PASID is > > - * available in mm->pasid, and must be released with iommu_sva_free_pasid(). > > - * @min must be greater than 0, because 0 indicates an unused mm->pasid. > > + * available in mm->pasid and will be available for the lifetime of the mm. > > * > > * Returns 0 on success and < 0 on error. > > */ > > @@ -33,38 +32,24 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) > > return -EINVAL; > > > > mutex_lock(&iommu_sva_lock); > > - if (mm->pasid) { > > - if (mm->pasid >= min && mm->pasid <= max) > > - ioasid_get(mm->pasid); > > - else > > + /* Is a PASID already associated with this mm? */ > > + if (pasid_valid(mm->pasid)) { > > + if (mm->pasid < min || mm->pasid >= max) > > ret = -EOVERFLOW; > > - } else { > > - pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm); > > - if (pasid == INVALID_IOASID) > > - ret = -ENOMEM; > > - else > > - mm->pasid = pasid; > > + goto out; > > } > > + > > + pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm); > > + if (!pasid_valid(pasid)) > > + ret = -ENOMEM; > > + else > > + mm_pasid_get(mm, pasid); > > Hrm. This is odd. > > > +/* Associate a PASID with an mm_struct: */ > > +static inline void mm_pasid_get(struct mm_struct *mm, u32 pasid) > > +{ > > + mm->pasid = pasid; > > +} > > This does not get anything. It sets the allocated PASID in the mm. The > refcount on the PASID was already taken by the allocation. So this > should be mm_pasid_set() or mm_pasid_install(), right?
Ok. Will change mm_pasid_get() to mm_pasid_set().
Thank you very much for your review!
-Fenghua
| |