Re: [PATCH v9 02/14] iommu/arm-smmu-v3: Start building a generic PASID layer

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Sep 06, 2024 at 03:08:54PM +0100, Will Deacon wrote:

> - We have a bunch of comments around 'arm_smmu_asid_lock' that refer
>   to arm_smmu_share_asid(), which no longer exists.

So, we can go ahead and put back the BTM support in the pre-existing
slightly racy form. I think we can turn it on for guest support by
operating it only if there is no S2 support in HW which would make it
justified.

Or we could delete the 'arm_smmu_asid_lock' for now.

My idea to fix the race goes through making the domains sharable
across instances because that will change the invalidation in a way
that lets double invalidation happen during the ASID change.

I was planning to wait for that, but it looks like it will be more
time. It is linked to the viommu work which needs to go first.

> - arm_smmu_attach_dev() takes/drops/re-takes the devices_lock indirectly
>   when it calls arm_smmu_attach_prepare() and arm_smmu_attach_commit().

This isn't retaking a lock, the operation being locked ran to
completion, we just need to run two operations..

> - arm_smmu_attach_dev() takes/drops 'arm_smmu_asid_lock' via
>   arm_smmu_domain_finalise()) and then re-takes it before the attach.

finalize won't be called from arm_smmu_attach_dev() very soon, they
are unrelated operations.

> Please note, I'm not saying that there's a bug here, just that it would
> be easier to work with if we had some documentation and lock ordering
> assertions.

There are more assertions than there was now, I think most of the new
code has them already, excluding the group mutex.

Jason