RE: [PATCH v5 5/6] iommu: Support mm PASID 1:n with sva domains

[Zhang, Tina]

Hi Jason,

> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Saturday, October 7, 2023 2:06 AM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>
> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>;
> Michael Shavit <mshavit@xxxxxxxxxx>; Vasant Hegde
> <vasant.hegde@xxxxxxx>; iommu@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 5/6] iommu: Support mm PASID 1:n with sva domains
> 
> On Mon, Sep 25, 2023 at 10:38:12AM +0800, Tina Zhang wrote:
> > Each mm bound to devices gets a PASID and corresponding sva domains
> > allocated in iommu_sva_bind_device(), which are referenced by
> iommu_mm
> > field of the mm. The PASID is released in __mmdrop(), while a sva
> > domain is released when no one is using it (the reference count is
> > decremented in iommu_sva_unbind_device()). However, although sva
> > domains and their PASID are separate objects such that their own life
> > cycles could be handled independently, an enqcmd use case may require
> > releasing the PASID in releasing the mm (i.e., once a PASID is
> > allocated for a mm, it will be permanently used by the mm and won't be
> > released until the end of mm) and only allows to drop the PASID after
> > the sva domains are released. To this end, mmgrab() is called in
> > iommu_sva_domain_alloc() to increment the mm reference count and
> > mmdrop() is invoked in
> > iommu_domain_free() to decrement the mm reference count.
> >
> > Since the required info of PASID and sva domains is kept in struct
> > iommu_mm_data of a mm, use mm->iommu_mm field instead of the old
> pasid
> > field in mm struct. The sva domain list is protected by iommu_sva_lock.
> >
> > Besides, this patch removes mm_pasid_init(), as with the introduced
> > iommu_mm structure, initializing mm pasid in mm_init() is unnecessary.
> >
> > Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > Reviewed-by: Vasant Hegde <vasant.hegde@xxxxxxx>
> > Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
> > ---
> >
> > Change in v5:
> > - Use smp_store_release() & READ_ONCE() in storing and loading mm's
> >   pasid value.
> >
> > Change in v4:
> > - Rebase to v6.6-rc1.
> >
> >  drivers/iommu/iommu-sva.c | 40 +++++++++++++++++++++++++++-----------
> -
> >  include/linux/iommu.h     | 18 +++++++++++-------
> >  kernel/fork.c             |  1 -
> >  3 files changed, 39 insertions(+), 20 deletions(-)
> 
> I was wondering what Nicolin's issue was, didn't see anything.
> 
> But I think you should incorporate this into the patch.
> 
> And there is a straightforward way to move the global lock into the
> iommu_mm_data that we can explore later.
OK. Let's try it. Hope this could solve the issue reported by Nicolin.

Regards,
-Tina