Re: [PATCH v5 24/27] iommu/arm-smmu-v3: Bring back SVA BTM support

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Mar 20, 2024 at 01:07:01AM +0800, Michael Shavit wrote:
> On Tue, Mar 5, 2024 at 7:44 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > BTM support is a feature where the CPU TLB invalidation can be forwarded
> > to the IOMMU and also invalidate the IOTLB. For this to work the CPU and
> > IOMMU ASID must be the same.
> >
> > Retain the prior SVA design here of keeping the ASID allocator for the
> > IOMMU private to SMMU and force SVA domains to set an ASID that matches
> > the CPU ASID.
> >
> > This requires changing the ASID assigned to a S1 domain if it happens to
> > be overlapping with the required CPU ASID. We hold on to the CPU ASID so
> > long as the SVA iommu_domain exists, so SVA domain conflict is not
> > possible.
> >
> > With the asid per-smmu we no longer have a problem that two per-smmu
> > iommu_domain's would need to share a CPU ASID entry in the IOMMU's xarray.
> >
> > Use the same ASID move algorithm for the S1 domains as before with some
> > streamlining around how the xarray is being used. Do not synchronize the
> > ASID's if BTM mode is not supported. Just leave BTM features off
> > everywhere.
> >
> > Audit all the places that touch cd->asid and think carefully about how the
> > locking works with the change to the cd->asid by the move algorithm. Use
> > xarray internal locking during xa_alloc() instead of double locking. Add a
> > note that concurrent S1 invalidation doesn't fully work.
> >
> > Note that this is all still dead code, ARM_SMMU_FEAT_BTM is never set.
> 
> Clearly a lot of work/thought has gone into this patch, but I have to
> wonder if this complexity is worth carrying forward for a feature that
> was never enabled... Is moving this to part 3 or dropping it all
> together an option?

Yes, we can do any of those. If we drop it here then it will be moved
to this series:

https://lore.kernel.org/linux-iommu/20240208151837.35068-1-shameerali.kolothum.thodi@huawei.com/

Which is the final bit of work to actually enable the feature. We are
getting quite close, it needs some of Nicolin's iommufd enablement
work to cross the finish line, I think.

I agree dead code like this should not be in the kernel..

Will? Please advise

Jason