RE: [PATCH v2 14/18] iommu/arm-smmu-v3: Support domains with shared CDs

[Zhang, Tina]

Hi,

> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, June 7, 2023 8:00 PM
> To: Michael Shavit <mshavit@xxxxxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>; Robin Murphy <robin.murphy@xxxxxxx>;
> Joerg Roedel <joro@xxxxxxxxxx>; jean-philippe@xxxxxxxxxx;
> nicolinc@xxxxxxxxxx; baolu.lu@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 14/18] iommu/arm-smmu-v3: Support domains with
> shared CDs
> 
> On Wed, Jun 07, 2023 at 12:06:07AM +0530, Michael Shavit wrote:
> > > What we definately shouldn't do is try to have different SVA
> > > iommu_domain's pointing at the same ASID. That is again making SVA
> > > special, which we are trying to get away from :)
> >
> > Fwiw, this change is preserving the status-quo in that regard;
> > arm-smmu-v3-sva.c is already doing this. But yes, I agree that
> > resolving the limitation is a better long term solution... and
> > something I can try to look at further.
> 
> I suppose we also don't really have a entirely clear picture what allocating
> multiple SVA domains should even do in the iommu driver.
> 
> The driver would like to share the ASID, but things are much cleaner for
> everything if the driver model has ASID 1:1 with the iommu_domain.
> 
> It suggests we are missing some core code in iommu_sva_bind_device() to try
> to re-use existing SVA iommu_domains. This would certainly be better than
> trying to teach every driver how to share and refcount its ASID concept...
> 
> Today we have this super hacky iommu_get_domain_for_dev_pasid() thing
> that allows SVA domain reuse for a single device.
> 
> Possibly what we should do is conver the u32 pasid in the mm_struct to a
> struct iommu_mm_data * and put alot more stuff in there. eg a linked list of
> all SVA domains.
If we are going to have 1:1 between SVA domain and pasid, why we need a linked list of all SVA domains? Would a SVA domain pointer be enough?

I've got a patch-set which takes this suggestion to add an iommu_mm_data struct field to mm_struct. I'll send it out for review soon. The motivation of that patch-set is to let the invalidate_range() callback use the SVA domain referenced by mm->iommu_mm_data->sva_domain to do per-iommu IOTLB invalidation.

Regards,
-Tina

> 
> > Splitting this part into a follow-up patch series would definitely be
> > easier and helpful if you're all ok with it :) .
> 
> I think splitting it into a series to re-organize the way ste/cd stuff works is a
> nice contained topic.
> 
> Adjusting the way the ASID works with SVA is another good topic
> 
> And so on
> 
> Jason