[PATCH 0/3] iommu/arm-smmuv3: Update domain_alloc_paging_flags()

[PATCH 0/3] iommu/arm-smmuv3: Update domain_alloc_paging_flags()

[Jason Gunthorpe]

The last cycle saw the removal of the NULL device during domain allocation
and the introduction of domain_alloc_paging_flags(). Update smmuv3 to this
new design.

Jason Gunthorpe (3):
  iommu/arm-smmu-v3: Remove arm_smmu_domain_finalise() during attach
  iommu/arm-smmu-v3: Make domain_alloc_paging_flags() directly determine
    the S1/S2
  iommu/arm-smmu-v3: Remove domain_alloc_paging()

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 88 +++++++--------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 -
 2 files changed, 28 insertions(+), 61 deletions(-)


base-commit: 121ea6330bec79ce3376959196360ca7114c7ee7
-- 
2.43.0

[PATCH 1/3] iommu/arm-smmu-v3: Remove arm_smmu_domain_finalise() during attach

[Jason Gunthorpe]

Domains are now always finalized during allocation because the core code
no longer permits a NULL dev argument to domain_alloc_paging/_flags().

Remove the late finalize during attach that supported domains that were
not fully initialized.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 37 +++++----------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 -
 2 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 7605b264fbf983..e62c8d0f47a903 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2353,7 +2353,6 @@ struct arm_smmu_domain *arm_smmu_domain_alloc(void)
 	if (!smmu_domain)
 		return ERR_PTR(-ENOMEM);
 
-	mutex_init(&smmu_domain->init_mutex);
 	INIT_LIST_HEAD(&smmu_domain->devices);
 	spin_lock_init(&smmu_domain->devices_lock);
 
@@ -2362,7 +2361,9 @@ struct arm_smmu_domain *arm_smmu_domain_alloc(void)
 
 static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
 {
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
 	struct arm_smmu_domain *smmu_domain;
+	int ret;
 
 	/*
 	 * Allocate the domain and initialise some of its data structures.
@@ -2373,15 +2374,10 @@ static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
 	if (IS_ERR(smmu_domain))
 		return ERR_CAST(smmu_domain);
 
-	if (dev) {
-		struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-		int ret;
-
-		ret = arm_smmu_domain_finalise(smmu_domain, master->smmu, 0);
-		if (ret) {
-			kfree(smmu_domain);
-			return ERR_PTR(ret);
-		}
+	ret = arm_smmu_domain_finalise(smmu_domain, master->smmu, 0);
+	if (ret) {
+		kfree(smmu_domain);
+		return ERR_PTR(ret);
 	}
 	return &smmu_domain->domain;
 }
@@ -2858,15 +2854,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	state.master = master = dev_iommu_priv_get(dev);
 	smmu = master->smmu;
 
-	mutex_lock(&smmu_domain->init_mutex);
-
-	if (!smmu_domain->smmu) {
-		ret = arm_smmu_domain_finalise(smmu_domain, smmu, 0);
-	} else if (smmu_domain->smmu != smmu)
-		ret = -EINVAL;
-
-	mutex_unlock(&smmu_domain->init_mutex);
-	if (ret)
+	if (smmu_domain->smmu != smmu)
 		return ret;
 
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
@@ -2923,16 +2911,9 @@ static int arm_smmu_s1_set_dev_pasid(struct iommu_domain *domain,
 	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
 	struct arm_smmu_device *smmu = master->smmu;
 	struct arm_smmu_cd target_cd;
-	int ret = 0;
 
-	mutex_lock(&smmu_domain->init_mutex);
-	if (!smmu_domain->smmu)
-		ret = arm_smmu_domain_finalise(smmu_domain, smmu, 0);
-	else if (smmu_domain->smmu != smmu)
-		ret = -EINVAL;
-	mutex_unlock(&smmu_domain->init_mutex);
-	if (ret)
-		return ret;
+	if (smmu_domain->smmu != smmu)
+		return -EINVAL;
 
 	if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1)
 		return -EINVAL;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 0107d3f333a1cc..e121b30e612f64 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -813,7 +813,6 @@ enum arm_smmu_domain_stage {
 
 struct arm_smmu_domain {
 	struct arm_smmu_device		*smmu;
-	struct mutex			init_mutex; /* Protects smmu pointer */
 
 	struct io_pgtable_ops		*pgtbl_ops;
 	atomic_t			nr_ats_masters;
-- 
2.43.0

[PATCH 2/3] iommu/arm-smmu-v3: Make domain_alloc_paging_flags() directly determine the S1/S2

[Jason Gunthorpe]

The selection of S1/S2 is a bit indirect today, make
domain_alloc_paging_flags() directly decode the flags and select the
correct S1/S2 type.

Directly reject flag combinations the HW doesn't support when processing
the flags.

Fix missing rejection of some flag combinations that are not supported
today (ie NEST_PARENT | DIRTY_TRACKING) by using a switch statement to
list out exactly the combinations that are currently supported.

Move the determination of the stage out of arm_smmu_domain_finalise() and
into both callers. As today the default stage is S1 if supported in HW.

This makes arm_smmu_domain_alloc_paging_flags() self contained and no
longer calling arm_smmu_domain_alloc_paging().

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 42 +++++++++++++++------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index e62c8d0f47a903..00105ccadc8b2b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2374,6 +2374,11 @@ static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
 	if (IS_ERR(smmu_domain))
 		return ERR_CAST(smmu_domain);
 
+	if (master->smmu->features & ARM_SMMU_FEAT_TRANS_S1)
+		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
+	else
+		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
+
 	ret = arm_smmu_domain_finalise(smmu_domain, master->smmu, 0);
 	if (ret) {
 		kfree(smmu_domain);
@@ -2447,12 +2452,6 @@ static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
 				 struct arm_smmu_domain *smmu_domain);
 	bool enable_dirty = flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
 
-	/* Restrict the stage to what we can actually support */
-	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
-		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
-	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
-		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
-
 	pgtbl_cfg = (struct io_pgtable_cfg) {
 		.pgsize_bitmap	= smmu->pgsize_bitmap,
 		.coherent_walk	= smmu->features & ARM_SMMU_FEAT_COHERENCY,
@@ -3124,6 +3123,7 @@ arm_smmu_domain_alloc_paging_flags(struct device *dev, u32 flags,
 				   const struct iommu_user_data *user_data)
 {
 	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	struct arm_smmu_device *smmu = master->smmu;
 	const u32 PAGING_FLAGS = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
 				 IOMMU_HWPT_ALLOC_PASID |
 				 IOMMU_HWPT_ALLOC_NEST_PARENT;
@@ -3135,25 +3135,43 @@ arm_smmu_domain_alloc_paging_flags(struct device *dev, u32 flags,
 	if (user_data)
 		return ERR_PTR(-EOPNOTSUPP);
 
-	if (flags & IOMMU_HWPT_ALLOC_PASID)
-		return arm_smmu_domain_alloc_paging(dev);
-
 	smmu_domain = arm_smmu_domain_alloc();
 	if (IS_ERR(smmu_domain))
 		return ERR_CAST(smmu_domain);
 
-	if (flags & IOMMU_HWPT_ALLOC_NEST_PARENT) {
-		if (!(master->smmu->features & ARM_SMMU_FEAT_NESTING)) {
+	switch (flags) {
+	case 0:
+		/* Prefer S1 if available */
+		if (smmu->features & ARM_SMMU_FEAT_TRANS_S1)
+			smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
+		else
+			smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
+		break;
+	case IOMMU_HWPT_ALLOC_NEST_PARENT:
+		if (!(smmu->features & ARM_SMMU_FEAT_NESTING)) {
 			ret = -EOPNOTSUPP;
 			goto err_free;
 		}
 		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
 		smmu_domain->nest_parent = true;
+		break;
+	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING:
+	case IOMMU_HWPT_ALLOC_DIRTY_TRACKING | IOMMU_HWPT_ALLOC_PASID:
+	case IOMMU_HWPT_ALLOC_PASID:
+		if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1)) {
+			ret = -EOPNOTSUPP;
+			goto err_free;
+		}
+		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		goto err_free;
 	}
 
 	smmu_domain->domain.type = IOMMU_DOMAIN_UNMANAGED;
 	smmu_domain->domain.ops = arm_smmu_ops.default_domain_ops;
-	ret = arm_smmu_domain_finalise(smmu_domain, master->smmu, flags);
+	ret = arm_smmu_domain_finalise(smmu_domain, smmu, flags);
 	if (ret)
 		goto err_free;
 	return &smmu_domain->domain;
-- 
2.43.0

[PATCH 3/3] iommu/arm-smmu-v3: Remove domain_alloc_paging()

[Jason Gunthorpe]

arm_smmu_domain_alloc_paging_flags() with a flags = 0 now does the same
thing as arm_smmu_domain_alloc_paging(), remove
arm_smmu_domain_alloc_paging().

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 31 ---------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 00105ccadc8b2b..f285b98f982c14 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -83,8 +83,6 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ 0, NULL},
 };
 
-static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
-				    struct arm_smmu_device *smmu, u32 flags);
 static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
 
 static void parse_driver_options(struct arm_smmu_device *smmu)
@@ -2359,34 +2357,6 @@ struct arm_smmu_domain *arm_smmu_domain_alloc(void)
 	return smmu_domain;
 }
 
-static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
-{
-	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-	struct arm_smmu_domain *smmu_domain;
-	int ret;
-
-	/*
-	 * Allocate the domain and initialise some of its data structures.
-	 * We can't really do anything meaningful until we've added a
-	 * master.
-	 */
-	smmu_domain = arm_smmu_domain_alloc();
-	if (IS_ERR(smmu_domain))
-		return ERR_CAST(smmu_domain);
-
-	if (master->smmu->features & ARM_SMMU_FEAT_TRANS_S1)
-		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
-	else
-		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
-
-	ret = arm_smmu_domain_finalise(smmu_domain, master->smmu, 0);
-	if (ret) {
-		kfree(smmu_domain);
-		return ERR_PTR(ret);
-	}
-	return &smmu_domain->domain;
-}
-
 static void arm_smmu_domain_free_paging(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
@@ -3549,7 +3519,6 @@ static struct iommu_ops arm_smmu_ops = {
 	.blocked_domain		= &arm_smmu_blocked_domain,
 	.capable		= arm_smmu_capable,
 	.hw_info		= arm_smmu_hw_info,
-	.domain_alloc_paging    = arm_smmu_domain_alloc_paging,
 	.domain_alloc_sva       = arm_smmu_sva_domain_alloc,
 	.domain_alloc_paging_flags = arm_smmu_domain_alloc_paging_flags,
 	.probe_device		= arm_smmu_probe_device,
-- 
2.43.0

Re: [PATCH 3/3] iommu/arm-smmu-v3: Remove domain_alloc_paging()

[Will Deacon]

On Thu, Dec 05, 2024 at 11:43:29AM -0400, Jason Gunthorpe wrote:
> arm_smmu_domain_alloc_paging_flags() with a flags = 0 now does the same
> thing as arm_smmu_domain_alloc_paging(), remove
> arm_smmu_domain_alloc_paging().
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 31 ---------------------
>  1 file changed, 31 deletions(-)

Ha! I was reviewing the previous patch and about to reply asking why we
couldn't just implement arm_smmu_domain_alloc_paging() as a wrapper
around arm_smmu_domain_alloc_paging() with flags of 0.

Good thing I read ahead :)

Will

Re: [PATCH 3/3] iommu/arm-smmu-v3: Remove domain_alloc_paging()

[Jason Gunthorpe]

On Mon, Dec 09, 2024 at 11:06:12PM +0000, Will Deacon wrote:
> On Thu, Dec 05, 2024 at 11:43:29AM -0400, Jason Gunthorpe wrote:
> > arm_smmu_domain_alloc_paging_flags() with a flags = 0 now does the same
> > thing as arm_smmu_domain_alloc_paging(), remove
> > arm_smmu_domain_alloc_paging().
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 31 ---------------------
> >  1 file changed, 31 deletions(-)
> 
> Ha! I was reviewing the previous patch and about to reply asking why we
> couldn't just implement arm_smmu_domain_alloc_paging() as a wrapper
> around arm_smmu_domain_alloc_paging() with flags of 0.
> 
> Good thing I read ahead :)

Yes, the main point of the prior patch was to make this patch
possible :)

Jason

Re: [PATCH 0/3] iommu/arm-smmuv3: Update domain_alloc_paging_flags()

[Will Deacon]

On Thu, 05 Dec 2024 11:43:26 -0400, Jason Gunthorpe wrote:
> The last cycle saw the removal of the NULL device during domain allocation
> and the introduction of domain_alloc_paging_flags(). Update smmuv3 to this
> new design.
> 
> Jason Gunthorpe (3):
>   iommu/arm-smmu-v3: Remove arm_smmu_domain_finalise() during attach
>   iommu/arm-smmu-v3: Make domain_alloc_paging_flags() directly determine
>     the S1/S2
>   iommu/arm-smmu-v3: Remove domain_alloc_paging()
> 
> [...]

Applied to arm64 (for-joerg/arm-smmu/updates), thanks!

[1/3] iommu/arm-smmu-v3: Remove arm_smmu_domain_finalise() during attach
      https://git.kernel.org/arm64/c/48e7b8e284e5
[2/3] iommu/arm-smmu-v3: Make domain_alloc_paging_flags() directly determine the S1/S2
      https://git.kernel.org/arm64/c/bb857c5c0150
[3/3] iommu/arm-smmu-v3: Remove domain_alloc_paging()
      https://git.kernel.org/arm64/c/cdfb9840fcc6

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev