Re: [PATCH v5 14/27] iommu/arm-smmu-v3: Make changing domains be hitless for ATS

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Mar 21, 2024 at 10:53:20PM +0800, Michael Shavit wrote:
> On Thu, Mar 21, 2024 at 9:28 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Thu, Mar 21, 2024 at 08:26:43PM +0800, Michael Shavit wrote:
> > > Overall I think the patch works, but it took me a while to really
> > > digest the big picture. Just to make sure I fully understand:
> > >
> > > We're trying to satisfy the following invariants for correctness:
> > > 1. Devices cannot get translations from a domain that was detached
> > > after arm_smmu_attach_dev() returns.
> >
> > Yes, regardless of ATS this is the API requirement.
> >
> > > 2. Devices cannot get cached translations from a domain after
> > > arm_smmu_atc_inv_domain() returns, regardless of whether the domain is
> > > simultaneously being attached/detached from a device.
> >
> > There is no simultaneously here since the group lock is held by the
> > core code.
> 
> I meant a call to arm_smmu_attach_dev concurrent with
> arm_smmu_atc_inv_domain (through tlb_flush_all).

Oh, yes that is the concurrency that drives alot of this organization

> > Yes, we can't just disable ATS due to the PASIDs. A SVA PASID might be
> > present and disabling ATS to change the RID domain would completely
> > wreck it. Today this scenario is prevented by sva_enable, which is
> > removed in following patches.
> 
> What do you mean by wrecking? We might slow it down to a crawl but we
> wouldn't be corrupting or destroying anything by disabling ATS while
> SVA is running would we?
> Oh does disabling ATS abort all transactions rather than making it transparent?

As you deduced, without ATS a non-present page will return an abort
back to the device instead of a non-present ATS reply which should
then trigger PRI.

IOW the SVA domain can no longer handle page faults and is effectively
non-functioning if ATS is disabled.
 
Jason