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

[Joonwon Kang <joonwonkang@google.com>]

Hi Baolu, thanks for your review.

> 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@ziepe.ca>
> > Suggested-by: Kevin Tian<kevin.tian@intel.com>
> > Signed-off-by: Joonwon Kang<joonwonkang@google.com>
> > ---
> > 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?
> 

I think your understanding is correct. In the long run, the limitations in the
comment and also in the commit message should be removed. I left the work to a
later patch as I am focusing on removing the main blocker first, which is that
a process is blocked by another irrelevant process for doing SVA as described
in the commit message. Currently, SVA for a process with different PASIDs will
only be allowed one after another, not simultaneously, and the current users of
`iommu_sva_bind_device()` should not be affected by this patch.

So, this patch should be enough to fix our current main problem. Can we leave
it to a later patch? or do you think we should remove the limitations now
although there is no requirement yet?

Thanks,
Joonwon Kang

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