mbox series

[v1,0/3] Clean-up arm-smmu-v3-sva.c: remove arm_smmu_bond

Message ID 20230905115013.1572240-1-mshavit@google.com (mailing list archive)
Headers show
Series Clean-up arm-smmu-v3-sva.c: remove arm_smmu_bond | expand

Message

Michael Shavit Sept. 5, 2023, 11:49 a.m. UTC
This small series was originally part of a larger effort to support
set_dev_pasid in arm-smmu-v3.c and a related SVA refactoring. But it can
also stand on its own as an initial and prepatory clean-up.

The crux of this series relies on the observation that SVA won't
allocate multiple SVA domains for the same device and mm pair. There's
therefore no reason for the driver to try to normalize data allocated
for a device/mm pair across set_dev_pasid calls. This simplification
then allows set_dev_pasid to use the SVA iommu_domain to hold
information instead of allocating a "bond" to represent the attachement.
Note that long term, we'll likely want to represent the SVA domain using
the same arm_smmu_domain struct used in arm-smmu-v3. This series serves
as an interim step to make those later refactors easier to reason about.

Note that arm-smmu-v3-sva performs a second level of normalization by
mapping multiple bonds (now SVA domains) attached to devices with the
same SMMU (if those devices have the same RID domain attached) to a
single arm_smmu_mmu_notifier. This is not affected by these patches.


Michael Shavit (3):
  iommu/arm-smmu-v3-sva: Remove unused iommu_sva handle
  iommu/arm-smmu-v3-sva: Remove bond refcount
  iommu/arm-smmu-v3-sva: Remove arm_smmu_bond

 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   | 90 ++++++-------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  1 -
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  2 +-
 3 files changed, 28 insertions(+), 65 deletions(-)


base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef

Comments

Jason Gunthorpe Sept. 5, 2023, 12:35 p.m. UTC | #1
On Tue, Sep 05, 2023 at 07:49:11PM +0800, Michael Shavit wrote:
> 
> This small series was originally part of a larger effort to support
> set_dev_pasid in arm-smmu-v3.c and a related SVA refactoring. But it can
> also stand on its own as an initial and prepatory clean-up.
> 
> The crux of this series relies on the observation that SVA won't
> allocate multiple SVA domains for the same device and mm pair. 

Yes, I think that is true, certainly no-intree user of this stuff
wants to do that.

It is enforced to be true after Tina's series:

https://lore.kernel.org/r/20230905000930.24515-1-tina.zhang@intel.com

Which makes one SVA domain per mm.

> There's therefore no reason for the driver to try to normalize data
> allocated for a device/mm pair across set_dev_pasid calls. 

Indeed, this is where we are trying to get to. This de-duplication
code in every driver is quite horrible.

> Note that arm-smmu-v3-sva performs a second level of normalization by
> mapping multiple bonds (now SVA domains) attached to devices with the
> same SMMU (if those devices have the same RID domain attached) to a
> single arm_smmu_mmu_notifier. This is not affected by these patches.

Ultimately the notifier should be per-iommu_domain as well.

Thanks,
Jason
Michael Shavit Sept. 5, 2023, 1:24 p.m. UTC | #2
On Tue, Sep 5, 2023 at 8:35 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Tue, Sep 05, 2023 at 07:49:11PM +0800, Michael Shavit wrote:
> >
> > Note that arm-smmu-v3-sva performs a second level of normalization by
> > mapping multiple bonds (now SVA domains) attached to devices with the
> > same SMMU (if those devices have the same RID domain attached) to a
> > single arm_smmu_mmu_notifier. This is not affected by these patches.
>
> Ultimately the notifier should be per-iommu_domain as well.

Speaking of, I'm questioning whether the multi-SMMU domain patchseries
and Tina's sva domain sharing are really prerequisites to get rid of
the notifier sharing. Is anyone really depending on or taking
advantage of this? The optimization only kicks in if multiple devices
with the same SMMU, share the same RID iommu domain (although this
would be improved by fixing SVA to not depend on the RID domain) , and
are bound to common MMs.
Jason Gunthorpe Sept. 5, 2023, 1:32 p.m. UTC | #3
On Tue, Sep 05, 2023 at 09:24:22PM +0800, Michael Shavit wrote:
> On Tue, Sep 5, 2023 at 8:35 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Tue, Sep 05, 2023 at 07:49:11PM +0800, Michael Shavit wrote:
> > >
> > > Note that arm-smmu-v3-sva performs a second level of normalization by
> > > mapping multiple bonds (now SVA domains) attached to devices with the
> > > same SMMU (if those devices have the same RID domain attached) to a
> > > single arm_smmu_mmu_notifier. This is not affected by these patches.
> >
> > Ultimately the notifier should be per-iommu_domain as well.
> 
> Speaking of, I'm questioning whether the multi-SMMU domain patchseries
> and Tina's sva domain sharing are really prerequisites to get rid of
> the notifier sharing. Is anyone really depending on or taking
> advantage of this?

Currently I don't see any in-tree user of SVA on ARM except for uacce,
which seems to work with only one device (HiSillicon QM).

So, yes, you can probably make it slightly less efficient for a
time. It will be functionally correct still with multiple registered
notifiers.

Jason
Will Deacon Oct. 12, 2023, 6:06 p.m. UTC | #4
On Tue, 5 Sep 2023 19:49:11 +0800, Michael Shavit wrote:
> This small series was originally part of a larger effort to support
> set_dev_pasid in arm-smmu-v3.c and a related SVA refactoring. But it can
> also stand on its own as an initial and prepatory clean-up.
> 
> The crux of this series relies on the observation that SVA won't
> allocate multiple SVA domains for the same device and mm pair. There's
> therefore no reason for the driver to try to normalize data allocated
> for a device/mm pair across set_dev_pasid calls. This simplification
> then allows set_dev_pasid to use the SVA iommu_domain to hold
> information instead of allocating a "bond" to represent the attachement.
> Note that long term, we'll likely want to represent the SVA domain using
> the same arm_smmu_domain struct used in arm-smmu-v3. This series serves
> as an interim step to make those later refactors easier to reason about.
> 
> [...]

Applied first two patches to will (for-joerg/arm-smmu/updates), thanks!

[1/3] iommu/arm-smmu-v3-sva: Remove unused iommu_sva handle
      https://git.kernel.org/will/c/d912aed14fe4
[2/3] iommu/arm-smmu-v3-sva: Remove bond refcount
      https://git.kernel.org/will/c/37ed36448fcd

Cheers,