Hi Jean,
On 8/17/20 7:15 PM, Jean-Philippe Brucker wrote:
> Let IOMMU drivers allocate a single PASID per mm. Store the mm in the
> IOASID set to allow refcounting and searching mm by PASID, when handling
> an I/O page fault.
>
> Reviewed-by: Lu Baolu
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/Kconfig | 5 +++
> drivers/iommu/Makefile| 1 +
> drivers/iommu/iommu-sva-lib.h | 15 +++
> drivers/iommu/iommu-sva-lib.c | 85 +++
> 4 files changed, 106 insertions(+)
> create mode 100644 drivers/iommu/iommu-sva-lib.h
> create mode 100644 drivers/iommu/iommu-sva-lib.c
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index bef5d75e306b..fb1787377eb6 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -103,6 +103,11 @@ config IOMMU_DMA
> select IRQ_MSI_IOMMU
> select NEED_SG_DMA_LENGTH
>
> +# Shared Virtual Addressing library
> +config IOMMU_SVA_LIB
> + bool
> + select IOASID
> +
> config FSL_PAMU
> bool "Freescale IOMMU support"
> depends on PCI
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 11f1771104f3..61bd30cd8369 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
> obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
> obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
> obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> +obj-$(CONFIG_IOMMU_SVA_LIB) += iommu-sva-lib.o
> diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
> new file mode 100644
> index ..b40990aef3fd
> --- /dev/null
> +++ b/drivers/iommu/iommu-sva-lib.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * SVA library for IOMMU drivers
> + */
> +#ifndef _IOMMU_SVA_LIB_H
> +#define _IOMMU_SVA_LIB_H
> +
> +#include
> +#include
> +
> +int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max);
> +void iommu_sva_free_pasid(struct mm_struct *mm);
> +struct mm_struct *iommu_sva_find(ioasid_t pasid);
> +
> +#endif /* _IOMMU_SVA_LIB_H */
> diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c
> new file mode 100644
> index ..db7e6c104d6b
> --- /dev/null
> +++ b/drivers/iommu/iommu-sva-lib.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Helpers for IOMMU drivers implementing SVA
> + */
> +#include
> +#include
> +
> +#include "iommu-sva-lib.h"
> +
> +static DEFINE_MUTEX(iommu_sva_lock);
> +static DECLARE_IOASID_SET(iommu_sva_pasid);
> +
> +/**
> + * 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 must be released with iommu_sva_free_pasid().
> + *
> + * Returns 0 on success and < 0 on error.
> + */
> +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.
> + return -EINVAL;
> +
> + mutex_lock(_sva_lock);
> + if (mm->pasid) {
> + if (mm->pasid >= min && mm->pasid <= max)
> + ioasid_get(mm->pasid);
> + else
> + ret = -EOVERFLOW;
> + } else {
> + pasid = ioasid_alloc(_sva_pasid, min, max, mm);
> + if (pasid == INVALID_IOASID)
> + ret = -ENOMEM;
> + else
> + mm->pasid = pasid;
> + }
> + mutex_unlock(_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(_sva_lock);
> + if (ioasid_put(mm->pasid))
> + mm->pasid = 0;
ditto: 0 versus INVALID_IOASID
> + mutex_unlock(_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?
> +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
