Re: [PATCH v2] iommu: Allow device driver to use its own PASID space for SVA

[Baolu Lu]

On 5/20/26 23:07, Joonwon Kang wrote:
> For SVA, the IOMMU core always allocates PASID from the global PASID
> space. The use of this global PASID space comes from the limitation of
> the ENQCMD instruction in Intel CPUs that it fetches its PASID operand
> from IA32_PASID, which is per-process; when a process wants to
> communicate with multiple devices with the ENQCMD instruction, it cannot
> change its PASID for each device without the kernel's intervention. Also
> note that ARM introduced a similar instruction, which is ST64BV0.
>
> Due to this nature, SVA with ARM SMMU v3 has been found not working in
> our environment when other modules/devices compete for PASID. The
> environment looks as follows:
>
> - The device is not a PCIe device.
> - The device is to use SVA.
> - The supported SSID/PASID space is very small for the device; only 1 to
>    3 SSIDs are supported.
>
> With this setup, when other modules have allocated all the PASIDs that
> our device is expected to use from the global PASID space via APIs like
> iommu_alloc_global_pasid() or iommu_sva_bind_device(), SVA binding to
> our device fails due to the lack of available PASIDs.
>
> This commit resolves the issue by allowing device driver to maintain its
> own PASID space and assign a PASID from that for the process-device bond
> via a new API called `iommu_sva_bind_device_pasid(dev, mm, pasid)`. Doing
> that, however, will disallow the process to execute the ENQCMD-like
> instructions at EL0. It is because the process cannot change its PASID in
> IA32_PASID(or ACCDATA_EL1 on ARM) for each device without the kernel's
> intervention. For this reason, calling `iommu_sva_bind_device()` and then
> `iommu_sva_bind_device_pasid()` for the same process will not be allowed
> and vice versa.
>
> Currently, there is a limitation that a process simultaneously doing SVA
> with multiple devices with different PASIDs is not supported. So, calling
> `iommu_sva_bind_device_pasid()` multiple times for the same process with
> different devices will not be allowed for now while that for
> `iommu_sva_bind_device()` will be.
>
> Another limitation is that a process cannot do `iommu_sva_bind_device()`
> if it has ever done `iommu_sva_bind_device_pasid()` even though it has
> been unbound after use.
>
> Suggested-by: Jason Gunthorpe<jgg@xxxxxxxx>
> Suggested-by: Kevin Tian<kevin.tian@xxxxxxxxx>
> Signed-off-by: Joonwon Kang<joonwonkang@xxxxxxxxxx>
> ---
> v2: Reuse iommu_mm->pasid after SVA bound by iommu_sva_bind_device_pasid()
>      is unbound.
> v1: Initial version.
>
>   arch/x86/kernel/traps.c   |   9 +--
>   drivers/iommu/iommu-sva.c | 151 +++++++++++++++++++++++++++++---------
>   include/linux/iommu.h     |  14 +++-
>   3 files changed, 134 insertions(+), 40 deletions(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 0ca3912ecb7f..0131c8e5fb10 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -857,13 +857,12 @@ static bool try_fixup_enqcmd_gp(void)
>   		return false;
>   
>   	/*
> -	 * If the mm has not been allocated a
> -	 * PASID, the #GP can not be fixed up.
> +	 * If the mm has not been allocated a PASID or ENQCMD has been
> +	 * disallowed, the #GP can not be fixed up.
>   	 */
> -	if (!mm_valid_pasid(current->mm))
> -		return false;
> -
>   	pasid = mm_get_enqcmd_pasid(current->mm);
> +	if (pasid == IOMMU_PASID_INVALID)
> +		return false;
>   
>   	/*
>   	 * Did this thread already have its PASID activated?
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index bc7c7232a43e..a83333651ad0 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -10,6 +10,9 @@
>   
>   #include "iommu-priv.h"
>   
> +/* Whether pasid is to be allocated from the global PASID space */
> +#define IOMMU_PASID_GLOBAL_ANY IOMMU_NO_PASID
> +
>   static DEFINE_MUTEX(iommu_sva_lock);
>   static bool iommu_sva_present;
>   static LIST_HEAD(iommu_sva_mms);
> @@ -17,10 +20,11 @@ static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>   						   struct mm_struct *mm);
>   
>   /* Allocate a PASID for the mm within range (inclusive) */
> -static struct iommu_mm_data *iommu_alloc_mm_data(struct mm_struct *mm, struct device *dev)
> +static struct iommu_mm_data *iommu_alloc_mm_data(struct mm_struct *mm,
> +						 struct device *dev,
> +						 ioasid_t pasid)
>   {
>   	struct iommu_mm_data *iommu_mm;
> -	ioasid_t pasid;
>   
>   	lockdep_assert_held(&iommu_sva_lock);
>   
> @@ -30,8 +34,27 @@ static struct iommu_mm_data *iommu_alloc_mm_data(struct mm_struct *mm, struct de
>   	iommu_mm = mm->iommu_mm;
>   	/* Is a PASID already associated with this mm? */
>   	if (iommu_mm) {
> +		if ((pasid == IOMMU_PASID_GLOBAL_ANY && !iommu_mm->pasid_global) ||
> +		    (pasid != IOMMU_PASID_GLOBAL_ANY && iommu_mm->pasid_global))
> +			return ERR_PTR(-EBUSY);
> +
> +		if (!iommu_mm->pasid_global) {
> +			if (list_empty(&iommu_mm->sva_domains))
> +				iommu_mm->pasid = pasid;
> +
> +			if (pasid != iommu_mm->pasid) {
> +				/*
> +				 * Currently, a process simultaneously doing
> +				 * SVA with multiple devices with different
> +				 * PASIDs is not supported.
> +				 */

I am a bit confused by the change in this helper and the comments above.

Currently, when an mm is bound to a device, it uses a PASID allocated
from the global pool. That implies that all devices access the
application's address space with the same PASID. Now we want to extend
this by allowing the device driver to manage the PASID for SVA, which
should mean different devices might use different PASIDs to access the
application's address space. But this does not seem to match the logic
in this helper.

Perhaps I overlooked something?

> +				return ERR_PTR(-ENOSPC);
> +			}
> +		}
> +
>   		if (iommu_mm->pasid >= dev->iommu->max_pasids)
>   			return ERR_PTR(-EOVERFLOW);
> +
>   		return iommu_mm;
>   	}

Thanks,
baolu