[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y+tW3kxFeOMcELww@myrica>
Date: Tue, 14 Feb 2023 09:39:42 +0000
From: Jean-Philippe Brucker <jean-philippe@...aro.org>
To: Jacob Pan <jacob.jun.pan@...ux.intel.com>
Cc: LKML <linux-kernel@...r.kernel.org>, iommu@...ts.linux.dev,
Jason Gunthorpe <jgg@...dia.com>,
Lu Baolu <baolu.lu@...ux.intel.com>,
Joerg Roedel <joro@...tes.org>,
Jean-Philippe Brucker <jean-philippe@...aro.com>,
Robin Murphy <robin.murphy@....com>,
Will Deacon <will@...nel.org>,
David Woodhouse <dwmw2@...radead.org>,
Raj Ashok <ashok.raj@...el.com>,
"Tian, Kevin" <kevin.tian@...el.com>, Yi Liu <yi.l.liu@...el.com>
Subject: Re: [PATCH v2 2/3] iommu/sva: Stop using ioasid_set for SVA
On Mon, Feb 13, 2023 at 10:44:13PM -0800, Jacob Pan wrote:
> From: Jason Gunthorpe <jgg@...dia.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@...dia.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> ---
> v2: minor fixes
> - let idxd driver use iommu.h for ioasid_t
> - return 0 after mm_pasid_set()
> ---
> drivers/dma/idxd/idxd.h | 1 +
> drivers/iommu/iommu-sva.c | 53 +++++++++++----------------------------
> drivers/iommu/iommu-sva.h | 3 ---
> include/linux/sched/mm.h | 8 +-----
> 4 files changed, 17 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
> index 7ced8d283d98..417e602a46b6 100644
> --- a/drivers/dma/idxd/idxd.h
> +++ b/drivers/dma/idxd/idxd.h
> @@ -13,6 +13,7 @@
> #include <linux/ioasid.h>
> #include <linux/bitmap.h>
> #include <linux/perf_event.h>
> +#include <linux/iommu.h>
Unrelated
> #include <uapi/linux/idxd.h>
> #include "registers.h"
>
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index 24bf9b2b58aa..8fb6f1867af3 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -9,24 +9,11 @@
> #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)
> +static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
> {
> - int ret = 0;
> - ioasid_t pasid;
> + int ret;
>
> if (min == INVALID_IOASID || max == INVALID_IOASID ||
> min == 0 || max < min)
> @@ -37,39 +24,29 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
> if (pasid_valid(mm->pasid)) {
> if (mm->pasid < min || mm->pasid >= max)
> ret = -EOVERFLOW;
> + else
> + ret = 0;
> goto out;
> }
>
> - pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
> - if (!pasid_valid(pasid))
> - ret = -ENOMEM;
> - else
> - mm_pasid_set(mm, pasid);
> + ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_KERNEL);
> + if (ret < min)
Just check ret < 0
> + goto out;
> + mm_pasid_set(mm, ret);
> + ret = 0;
> out:
> mutex_unlock(&iommu_sva_lock);
> return ret;
> }
> -EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
>
> -/* ioasid_find getter() requires a void * argument */
> -static bool __mmget_not_zero(void *mm)
> +void mm_pasid_drop(struct mm_struct *mm)
> {
> - return mmget_not_zero(mm);
> -}
> + pr_alert("%s %d", __func__, mm->pasid);
Debug print
Thanks,
Jean
> + if (likely(!pasid_valid(mm->pasid)))
> + return;
>
> -/**
> - * 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);
> + ida_free(&iommu_global_pasid_ida, mm->pasid);
> }
> -EXPORT_SYMBOL_GPL(iommu_sva_find);
>
> /**
> * iommu_sva_bind_device() - Bind a process address space to a device
> diff --git a/drivers/iommu/iommu-sva.h b/drivers/iommu/iommu-sva.h
> index 7215a761b962..c22d0174ad61 100644
> --- a/drivers/iommu/iommu-sva.h
> +++ b/drivers/iommu/iommu-sva.h
> @@ -8,9 +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);
> -struct mm_struct *iommu_sva_find(ioasid_t pasid);
> -
> /* I/O Page fault */
> struct device;
> struct iommu_fault;
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 2a243616f222..cdc138086230 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -463,13 +463,7 @@ static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid)
> mm->pasid = pasid;
> }
>
> -static inline void mm_pasid_drop(struct mm_struct *mm)
> -{
> - if (pasid_valid(mm->pasid)) {
> - ioasid_free(mm->pasid);
> - mm->pasid = INVALID_IOASID;
> - }
> -}
> +void mm_pasid_drop(struct mm_struct *mm);
> #else
> static inline void mm_pasid_init(struct mm_struct *mm) {}
> static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid) {}
> --
> 2.25.1
>
Powered by blists - more mailing lists