| Message ID | 20230313204158.1495067-5-jacob.jun.pan@linux.intel.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v6,1/7] iommu/vt-d: Remove virtual command interface | expand |
On 3/14/23 4:41 AM, Jacob Pan wrote: > From: Jason Gunthorpe<jgg@nvidia.com> > > Instead SVA drivers can use a simple global IDA to allocate PASIDs for > each mm_struct. > > Future work would be to allow drivers using the SVA APIs to reserve global > PASIDs from this IDA for their internal use, eg with the DMA API PASID > support. > > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > Signed-off-by: Jacob Pan<jacob.jun.pan@linux.intel.com> > --- > v5: > - Put removing iommu_sva_find() to a separate patch (Kevin) > - Make pasid allocation range to be inclusive (Tina) > - Simplified return code handling (Baolu) > v4: > - Keep GFP_ATOMIC flag for PASID allocation, will changed to > GFP_KERNEL in a separate patch. > --- > drivers/iommu/iommu-sva.c | 43 ++++++++++++++------------------------- > drivers/iommu/iommu-sva.h | 2 -- > 2 files changed, 15 insertions(+), 30 deletions(-) > > diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c > index 4f357ef14f04..d4640731727a 100644 > --- a/drivers/iommu/iommu-sva.c > +++ b/drivers/iommu/iommu-sva.c > @@ -9,47 +9,34 @@ > #include "iommu-sva.h" > > static DEFINE_MUTEX(iommu_sva_lock); > -static DECLARE_IOASID_SET(iommu_sva_pasid); > +static DEFINE_IDA(iommu_global_pasid_ida); > > -/** > - * iommu_sva_alloc_pasid - Allocate a PASID for the mm > - * @mm: the mm > - * @min: minimum PASID value (inclusive) > - * @max: maximum PASID value (inclusive) > - * > - * 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 will be available for the lifetime of the mm. > - * > - * Returns 0 on success and < 0 on error. > - */ > -int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) > +/* Allocate a PASID for the mm within range (inclusive) */ > +static 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) > + if (!pasid_valid(min) || !pasid_valid(max) || > + min == 0 || max < min) I still think the last line change is unnecessary. Otherwise, you probably will get below CHECK warning: CHECK: Alignment should match open parenthesis #259: FILE: drivers/iommu/iommu-sva.c:20: + if (!pasid_valid(min) || !pasid_valid(max) || + min == 0 || max < min) Besides that, Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> Best regards, baolu
Hi Baolu, On Tue, 14 Mar 2023 10:16:41 +0800, Baolu Lu <baolu.lu@linux.intel.com> wrote: > On 3/14/23 4:41 AM, Jacob Pan wrote: > > From: Jason Gunthorpe<jgg@nvidia.com> > > > > Instead SVA drivers can use a simple global IDA to allocate PASIDs for > > each mm_struct. > > > > Future work would be to allow drivers using the SVA APIs to reserve > > global PASIDs from this IDA for their internal use, eg with the DMA API > > PASID support. > > > > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > > Signed-off-by: Jacob Pan<jacob.jun.pan@linux.intel.com> > > --- > > v5: > > - Put removing iommu_sva_find() to a separate patch (Kevin) > > - Make pasid allocation range to be inclusive (Tina) > > - Simplified return code handling (Baolu) > > v4: > > - Keep GFP_ATOMIC flag for PASID allocation, will changed to > > GFP_KERNEL in a separate patch. > > --- > > drivers/iommu/iommu-sva.c | 43 ++++++++++++++------------------------- > > drivers/iommu/iommu-sva.h | 2 -- > > 2 files changed, 15 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c > > index 4f357ef14f04..d4640731727a 100644 > > --- a/drivers/iommu/iommu-sva.c > > +++ b/drivers/iommu/iommu-sva.c > > @@ -9,47 +9,34 @@ > > #include "iommu-sva.h" > > > > static DEFINE_MUTEX(iommu_sva_lock); > > -static DECLARE_IOASID_SET(iommu_sva_pasid); > > +static DEFINE_IDA(iommu_global_pasid_ida); > > > > -/** > > - * iommu_sva_alloc_pasid - Allocate a PASID for the mm > > - * @mm: the mm > > - * @min: minimum PASID value (inclusive) > > - * @max: maximum PASID value (inclusive) > > - * > > - * 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 will be available for the lifetime of > > the mm. > > - * > > - * Returns 0 on success and < 0 on error. > > - */ > > -int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t > > max) +/* Allocate a PASID for the mm within range (inclusive) */ > > +static 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) > > + if (!pasid_valid(min) || !pasid_valid(max) || > > + min == 0 || max < min) > > I still think the last line change is unnecessary. Otherwise, you > probably will get below CHECK warning: > Got it :) I misunderstood, I thought you meant should not change to pasid_valid() will fix > CHECK: Alignment should match open parenthesis > #259: FILE: drivers/iommu/iommu-sva.c:20: > + if (!pasid_valid(min) || !pasid_valid(max) || > + min == 0 || max < min) > > Besides that, > > Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> > > Best regards, > baolu > Thanks, Jacob
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c index 4f357ef14f04..d4640731727a 100644 --- a/drivers/iommu/iommu-sva.c +++ b/drivers/iommu/iommu-sva.c @@ -9,47 +9,34 @@ #include "iommu-sva.h" static DEFINE_MUTEX(iommu_sva_lock); -static DECLARE_IOASID_SET(iommu_sva_pasid); +static DEFINE_IDA(iommu_global_pasid_ida); -/** - * iommu_sva_alloc_pasid - Allocate a PASID for the mm - * @mm: the mm - * @min: minimum PASID value (inclusive) - * @max: maximum PASID value (inclusive) - * - * 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 will be available for the lifetime of the mm. - * - * Returns 0 on success and < 0 on error. - */ -int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) +/* Allocate a PASID for the mm within range (inclusive) */ +static 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) + if (!pasid_valid(min) || !pasid_valid(max) || + min == 0 || max < min) return -EINVAL; mutex_lock(&iommu_sva_lock); /* Is a PASID already associated with this mm? */ if (pasid_valid(mm->pasid)) { - if (mm->pasid < min || mm->pasid >= max) + if (mm->pasid < min || mm->pasid > max) ret = -EOVERFLOW; goto out; } - pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm); - if (!pasid_valid(pasid)) - ret = -ENOMEM; - else - mm->pasid = pasid; + ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_ATOMIC); + if (ret < 0) + goto out; + mm->pasid = ret; + ret = 0; out: mutex_unlock(&iommu_sva_lock); return ret; } -EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid); /** * iommu_sva_bind_device() - Bind a process address space to a device @@ -221,8 +208,8 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data) void mm_pasid_drop(struct mm_struct *mm) { - if (pasid_valid(mm->pasid)) { - ioasid_free(mm->pasid); - mm->pasid = INVALID_IOASID; - } + if (likely(!pasid_valid(mm->pasid))) + return; + + ida_free(&iommu_global_pasid_ida, mm->pasid); } diff --git a/drivers/iommu/iommu-sva.h b/drivers/iommu/iommu-sva.h index 102eae1817a2..c22d0174ad61 100644 --- a/drivers/iommu/iommu-sva.h +++ b/drivers/iommu/iommu-sva.h @@ -8,8 +8,6 @@ #include <linux/ioasid.h> #include <linux/mm_types.h> -int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max); - /* I/O Page fault */ struct device; struct iommu_fault;