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

[Zhang, Tina]

> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, July 11, 2023 12:55 AM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>
> Cc: Michael Shavit <mshavit@xxxxxxxxxx>; 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, Jul 05, 2023 at 09:56:50AM +0000, Zhang, Tina wrote:
> 
> > > 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?
> 
> Kevin asked this, we can't assume that a single SVA domain is going to work in
> a multi-iommu-driver system, which we are trying to enable at the core level..
> 
> > 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.
> 
> Huh?
> 
> You are supposed to put the struct mmu_notifier inside the sva_domain
> struct and use container_of().
No. Just want to use iommu_mm to replace the old pasid field.
https://lore.kernel.org/linux-iommu/20230707013441.365583-5-tina.zhang@xxxxxxxxx/

The motivation is mainly to for performance optimization for vIOMMU, though it could also benefit native world.

Currently, in invalidate_range() callback implemented by VT-d driver, due to lacking sva_domain knowledge (i.e., intel_invalidate_range() doesn't know which IOMMUs' IOTLB should be invalidated), intel_invalidate_range() just performs IOMMU IOTLB per device and that leads to superfluous IOTLB invalidations. For example, if there are two devices physically behind an IOMMU and those two devices are used by one user space application, it means in each invalidate_range() callback, two IOTLB and two device-IOTLB invalidations would be performed, instead of one IOTLB plus two device-TLB invalidations (i.e., one IOTLB invalidation is redundant). Things will become worse in a virtualized environment, especially with one vIOMMU exposed to guest, as invoking IOTLB is much more expensive than doing it in native and conceptually there could be more devices behind a vIOMMU that means more superfluous IOTLB invalidations.

So, we want to add sva_domain related info to mm to remove the superfluous IOTLB invalidations.

Thanks,
-Tina

> 
> This is another reason why I'd prefer we de-duplicate SVA domains at the
> core code level as duplicitive notifiers are expensive..
> 
> Please don't add stuff to the mm just for this reason.
> 
> Jason