[PATCH v1 00/20] iommu: Introduce and roll out test_dev domain op

[PATCH v1 00/20] iommu: Introduce and roll out test_dev domain op

[Nicolin Chen]

Add a new test_dev domain op for drivers to run a compatibility test prior
to the actual attachment at the driver level. Any incompatible attachment
will be rejected early, allowing the iommu core to postpone any concurrent
attachment during a device reset state.

This is a preparatory series for new iommu_dev_reset APIs:
https://lore.kernel.org/all/cover.1756682135.git.nicolinc@nvidia.com/

The change is a must to keep attach uAPI behavior consistent.

This is on github:
https://github.com/nicolinc/iommufd/tree/iommu_test_dev-v1

Nicolin Chen (20):
  iommu: Lock group->mutex in iommu_deferred_attach()
  iommu: Introduce a test_dev domain op and an internal helper
  iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev
  iommu/intel: Implement test_dev callbacks to domain ops
  iommu/amd: Implement test_dev callbacks to domain ops
  iommu/arm-smmu: Implement arm_smmu_test_dev
  iommu/qcom_iommu: Implement test_dev callbacks to domain ops
  iommu/riscv: Implement riscv_iommu_test_paging_domain
  iommu/mkt_iommu: Implement mtk_iommu_test_device
  iommu/apple-dart: Implement test_dev callbacks to domain ops
  iommu/ipmmu-vmsa: Implement ipmmu_domain_test_device
  iommu/sun50i-iommu: Implement sun50i_iommu_domain_test_device
  iommu/rockchip-iommu: Implement rk_iommu_identity_test_dev
  iommu/msm_iommu: Implement msm_iommu_domain_test_dev
  iommu/fsl_pamu_domain: Implement fsl_pamu_domain_test_device
  iommu/omap-iommu: Implement omap_iommu_domain_test_dev
  iommu/s390-iommu: Implement s390_iommu_domain_test_device
  iommufd/selftest: Implement mock_domain_nop_test
  iommu/virtio-iommu: Implement viommu_domain_test_dev
  iommu/tegra-smmu: Implement tegra_smmu_domain_test_dev

 drivers/iommu/amd/amd_iommu.h                 |   3 -
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   2 +
 include/linux/iommu.h                         |  17 ++-
 drivers/iommu/amd/iommu.c                     |  27 ++--
 drivers/iommu/amd/pasid.c                     |  29 ++--
 drivers/iommu/apple-dart.c                    |  50 +++++--
 .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     |   6 +-
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |   4 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 113 ++++++++++------
 drivers/iommu/arm/arm-smmu/arm-smmu.c         | 116 +++++++++-------
 drivers/iommu/arm/arm-smmu/qcom_iommu.c       |  46 +++++--
 drivers/iommu/fsl_pamu_domain.c               |  50 +++++--
 drivers/iommu/intel/iommu.c                   |  66 ++++++----
 drivers/iommu/intel/nested.c                  |  29 ++--
 drivers/iommu/intel/svm.c                     |  11 +-
 drivers/iommu/iommu.c                         | 124 +++++++++++++-----
 drivers/iommu/iommufd/selftest.c              |  45 +++++--
 drivers/iommu/ipmmu-vmsa.c                    |  38 ++++--
 drivers/iommu/msm_iommu.c                     |  31 ++++-
 drivers/iommu/mtk_iommu.c                     |  17 ++-
 drivers/iommu/omap-iommu.c                    |  41 +++---
 drivers/iommu/riscv/iommu.c                   |  16 ++-
 drivers/iommu/rockchip-iommu.c                |  18 ++-
 drivers/iommu/s390-iommu.c                    |  22 +++-
 drivers/iommu/sun50i-iommu.c                  |  16 ++-
 drivers/iommu/tegra-smmu.c                    |  23 ++--
 drivers/iommu/virtio-iommu.c                  |  16 ++-
 27 files changed, 671 insertions(+), 305 deletions(-)

-- 
2.43.0

[PATCH v1 01/20] iommu: Lock group->mutex in iommu_deferred_attach()

[Nicolin Chen]

The iommu_deferred_attach() function invokes __iommu_attach_device() while
not holding the group->mutex, like other __iommu_attach_device() callers.

Though there is no pratical bug being triggered so far, it would be better
to apply the same locking to this __iommu_attach_device(), since the IOMMU
drivers nowaday are more aware of the group->mutex -- some of them use the
iommu_group_mutex_assert() function that could be potentially in the path
of an attach_dev callback function invoked by the __iommu_attach_device().

The iommu_deferred_attach() will soon need to invoke a new domain op that
must be locked with __iommu_attach_device under group->mutex.

So, grab the mutex to guard __iommu_attach_device() like other callers.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommu.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2ca990dfbb884..170e522b5bda4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2185,10 +2185,17 @@ EXPORT_SYMBOL_GPL(iommu_attach_device);
 
 int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
 {
-	if (dev->iommu && dev->iommu->attach_deferred)
-		return __iommu_attach_device(domain, dev, NULL);
+	/*
+	 * This is called on the dma mapping fast path so avoid locking. This is
+	 * racy, but we have an expectation that the driver will setup its DMAs
+	 * inside probe while being single threaded to avoid racing.
+	 */
+	if (!dev->iommu || !dev->iommu->attach_deferred)
+		return 0;
 
-	return 0;
+	guard(mutex)(&dev->iommu_group->mutex);
+
+	return __iommu_attach_device(domain, dev, NULL);
 }
 
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
-- 
2.43.0

[PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

Add a new test_dev domain op for driver to test the compatibility between
a domain and a device at the driver level, before calling into the actual
attachment/replacement of a domain. Support pasid for set_dev_pasid call.

Move existing core-level compatibility tests to a helper function. Invoke
it prior to:
 * __iommu_attach_device() or its wrapper __iommu_device_set_domain()
 * __iommu_set_group_pasid()

And keep them within the group->mutex, so drivers can simply move all the
sanity and compatibility tests from their attach_dev callbacks to the new
test_dev callbacks without concerning about a race condition.

This may be a public API someday for VFIO/IOMMUFD to run a list of attach
tests without doing any actual attachment, which may result in a list of
failed tests. So encourage drivers to avoid printks to prevent kernel log
spam.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 include/linux/iommu.h |  17 +++++--
 drivers/iommu/iommu.c | 111 ++++++++++++++++++++++++++++++------------
 2 files changed, 93 insertions(+), 35 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 801b2bd9e8d49..2ec99502dc29c 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -714,7 +714,12 @@ struct iommu_ops {
 
 /**
  * struct iommu_domain_ops - domain specific operations
- * @attach_dev: attach an iommu domain to a device
+ * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
+ *            A driver-level callback of this op should do a thorough sanity, to
+ *            make sure a device is compatible with the domain. So the following
+ *            @attach_dev and @set_dev_pasid functions would likely succeed with
+ *            only one exception due to a temporary failure like out of memory.
+ *            It's suggested to avoid the kernel prints in this op.
  *  Return:
  * * 0		- success
  * * EINVAL	- can indicate that device and domain are incompatible due to
@@ -722,11 +727,15 @@ struct iommu_ops {
  *		  driver shouldn't log an error, since it is legitimate for a
  *		  caller to test reuse of existing domains. Otherwise, it may
  *		  still represent some other fundamental problem
- * * ENOMEM	- out of memory
- * * ENOSPC	- non-ENOMEM type of resource allocation failures
  * * EBUSY	- device is attached to a domain and cannot be changed
  * * ENODEV	- device specific errors, not able to be attached
  * * <others>	- treated as ENODEV by the caller. Use is discouraged
+ * @attach_dev: attach an iommu domain to a device
+ *  Return:
+ * * 0		- success
+ * * ENOMEM	- out of memory
+ * * ENOSPC	- non-ENOMEM type of resource allocation failures
+ * * <others>	- Use is discouraged
  * @set_dev_pasid: set or replace an iommu domain to a pasid of device. The pasid of
  *                 the device should be left in the old config in error case.
  * @map_pages: map a physically contiguous set of pages of the same size to
@@ -751,6 +760,8 @@ struct iommu_ops {
  * @free: Release the domain after use.
  */
 struct iommu_domain_ops {
+	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
+			ioasid_t pasid, struct iommu_domain *old);
 	int (*attach_dev)(struct iommu_domain *domain, struct device *dev,
 			  struct iommu_domain *old);
 	int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 170e522b5bda4..95f0e2898b6b5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -99,6 +99,9 @@ static int bus_iommu_probe(const struct bus_type *bus);
 static int iommu_bus_notifier(struct notifier_block *nb,
 			      unsigned long action, void *data);
 static void iommu_release_device(struct device *dev);
+static int __iommu_domain_test_device(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old);
 static int __iommu_attach_device(struct iommu_domain *domain,
 				 struct device *dev, struct iommu_domain *old);
 static int __iommu_attach_group(struct iommu_domain *domain,
@@ -642,6 +645,10 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 	if (group->default_domain)
 		iommu_create_device_direct_mappings(group->default_domain, dev);
 	if (group->domain) {
+		ret = __iommu_domain_test_device(group->domain, dev,
+						 IOMMU_NO_PASID, NULL);
+		if (ret)
+			goto err_remove_gdev;
 		ret = __iommu_device_set_domain(group, dev, group->domain, NULL,
 						0);
 		if (ret)
@@ -2185,6 +2192,8 @@ EXPORT_SYMBOL_GPL(iommu_attach_device);
 
 int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
 {
+	int ret;
+
 	/*
 	 * This is called on the dma mapping fast path so avoid locking. This is
 	 * racy, but we have an expectation that the driver will setup its DMAs
@@ -2195,6 +2204,10 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
 
 	guard(mutex)(&dev->iommu_group->mutex);
 
+	ret = __iommu_domain_test_device(domain, dev, IOMMU_NO_PASID, NULL);
+	if (ret)
+		return ret;
+
 	return __iommu_attach_device(domain, dev, NULL);
 }
 
@@ -2262,6 +2275,53 @@ static bool domain_iommu_ops_compatible(const struct iommu_ops *ops,
 	return false;
 }
 
+/*
+ * Test if a future attach for a domain to the device will always fail. This may
+ * indicate that the device and the domain are incompatible in some way. Actual
+ * attach may still fail for some temporary failure like out of memory.
+ *
+ * If pasid != IOMMU_NO_PASID, it is meant to test a future set_dev_pasid call.
+ */
+static int __iommu_domain_test_device(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
+{
+	const struct iommu_ops *ops = dev_iommu_ops(dev);
+	struct iommu_group *group = dev->iommu_group;
+
+	lockdep_assert_held(&group->mutex);
+
+	if (!domain_iommu_ops_compatible(ops, domain))
+		return -EINVAL;
+
+	if (pasid != IOMMU_NO_PASID) {
+		struct group_device *gdev;
+
+		if (!domain->ops->set_dev_pasid || !ops->blocked_domain ||
+		    !ops->blocked_domain->ops->set_dev_pasid)
+			return -EOPNOTSUPP;
+		/*
+		 * Skip PASID validation for devices without PASID support
+		 * (max_pasids = 0). These devices cannot issue transactions
+		 * with PASID, so they don't affect group's PASID usage.
+		 */
+		for_each_group_device(group, gdev) {
+			if (gdev->dev->iommu->max_pasids > 0 &&
+			    pasid >= gdev->dev->iommu->max_pasids)
+				return -EINVAL;
+		}
+	}
+
+	/*
+	 * Domains that don't implement a test_dev callback function must have a
+	 * simple domain attach behavior. The sanity above should be enough.
+	 */
+	if (!domain->ops->test_dev)
+		return 0;
+
+	return domain->ops->test_dev(domain, dev, pasid, old);
+}
+
 static int __iommu_attach_group(struct iommu_domain *domain,
 				struct iommu_group *group)
 {
@@ -2272,8 +2332,7 @@ static int __iommu_attach_group(struct iommu_domain *domain,
 		return -EBUSY;
 
 	dev = iommu_group_first_dev(group);
-	if (!dev_has_iommu(dev) ||
-	    !domain_iommu_ops_compatible(dev_iommu_ops(dev), domain))
+	if (!dev_has_iommu(dev))
 		return -EINVAL;
 
 	return __iommu_group_set_domain(group, domain);
@@ -2381,6 +2440,13 @@ static int __iommu_group_set_domain_internal(struct iommu_group *group,
 	if (WARN_ON(!new_domain))
 		return -EINVAL;
 
+	for_each_group_device(group, gdev) {
+		ret = __iommu_domain_test_device(new_domain, gdev->dev,
+						 IOMMU_NO_PASID, group->domain);
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * Changing the domain is done by calling attach_dev() on the new
 	 * domain. This switch does not have to be atomic and DMA can be
@@ -3479,38 +3545,19 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
 {
 	/* Caller must be a probed driver on dev */
 	struct iommu_group *group = dev->iommu_group;
-	struct group_device *device;
-	const struct iommu_ops *ops;
 	void *entry;
 	int ret;
 
 	if (!group)
 		return -ENODEV;
-
-	ops = dev_iommu_ops(dev);
-
-	if (!domain->ops->set_dev_pasid ||
-	    !ops->blocked_domain ||
-	    !ops->blocked_domain->ops->set_dev_pasid)
-		return -EOPNOTSUPP;
-
-	if (!domain_iommu_ops_compatible(ops, domain) ||
-	    pasid == IOMMU_NO_PASID)
+	if (pasid == IOMMU_NO_PASID)
 		return -EINVAL;
 
 	mutex_lock(&group->mutex);
-	for_each_group_device(group, device) {
-		/*
-		 * Skip PASID validation for devices without PASID support
-		 * (max_pasids = 0). These devices cannot issue transactions
-		 * with PASID, so they don't affect group's PASID usage.
-		 */
-		if ((device->dev->iommu->max_pasids > 0) &&
-		    (pasid >= device->dev->iommu->max_pasids)) {
-			ret = -EINVAL;
-			goto out_unlock;
-		}
-	}
+
+	ret = __iommu_domain_test_device(domain, dev, pasid, NULL);
+	if (ret)
+		goto out_unlock;
 
 	entry = iommu_make_pasid_array_entry(domain, handle);
 
@@ -3573,12 +3620,7 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
 
 	if (!group)
 		return -ENODEV;
-
-	if (!domain->ops->set_dev_pasid)
-		return -EOPNOTSUPP;
-
-	if (!domain_iommu_ops_compatible(dev_iommu_ops(dev), domain) ||
-	    pasid == IOMMU_NO_PASID || !handle)
+	if (pasid == IOMMU_NO_PASID || !handle)
 		return -EINVAL;
 
 	mutex_lock(&group->mutex);
@@ -3615,6 +3657,11 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
 	ret = 0;
 
 	if (curr_domain != domain) {
+		ret = __iommu_domain_test_device(domain, dev, pasid,
+						 curr_domain);
+		if (ret)
+			goto out_unlock;
+
 		ret = __iommu_set_group_pasid(domain, group,
 					      pasid, curr_domain);
 		if (ret)
-- 
2.43.0

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Niklas Schnelle]

On Sun, 2025-10-12 at 17:04 -0700, Nicolin Chen wrote:
> Add a new test_dev domain op for driver to test the compatibility between
> a domain and a device at the driver level, before calling into the actual
> attachment/replacement of a domain. Support pasid for set_dev_pasid call.
> 
> Move existing core-level compatibility tests to a helper function. Invoke
> it prior to:
>  * __iommu_attach_device() or its wrapper __iommu_device_set_domain()
>  * __iommu_set_group_pasid()

Should this list also include iommu_deferred_attach()? The code does
include it.

> 
> And keep them within the group->mutex, so drivers can simply move all the
> sanity and compatibility tests from their attach_dev callbacks to the new
> test_dev callbacks without concerning about a race condition.
> 
> This may be a public API someday for VFIO/IOMMUFD to run a list of attach
> tests without doing any actual attachment, which may result in a list of
> failed tests. So encourage drivers to avoid printks to prevent kernel log
> spam.
> 
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  include/linux/iommu.h |  17 +++++--
>  drivers/iommu/iommu.c | 111 ++++++++++++++++++++++++++++++------------
>  2 files changed, 93 insertions(+), 35 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 801b2bd9e8d49..2ec99502dc29c 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -714,7 +714,12 @@ struct iommu_ops {
>  
>  /**
>   * struct iommu_domain_ops - domain specific operations
> - * @attach_dev: attach an iommu domain to a device
> + * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
> + *            A driver-level callback of this op should do a thorough sanity, to

You're missing the word "check" above.

> + *            make sure a device is compatible with the domain. So the following
> + *            @attach_dev and @set_dev_pasid functions would likely succeed with
> + *            only one exception due to a temporary failure like out of memory.

Nit: "… only one exception …" / "… like out of memory …" this sounds a
bit odd to me because on the one hand it's one exception but then also
a group (temporary failures).

Maybe better:
"… would likely succeed with only the exception of temporary failures
like out of memory."?

> + *            It's suggested to avoid the kernel prints in this op.
>   *  Return:
>   * * 0		- success
>   * * EINVAL	- can indicate that device and domain are incompatible due to
> @@ -722,11 +727,15 @@ struct iommu_ops {
>   *		  driver shouldn't log an error, since it is legitimate for a
>   *		  caller to test reuse of existing domains. Otherwise, it may
>   *		  still represent some other fundamental problem
> - * * ENOMEM	- out of memory
> - * * ENOSPC	- non-ENOMEM type of resource allocation failures
>   * * EBUSY	- device is attached to a domain and cannot be changed
>   * * ENODEV	- device specific errors, not able to be attached
>   * * <others>	- treated as ENODEV by the caller. Use is discouraged
> + * @attach_dev: attach an iommu domain to a device
> + *  Return:
> + * * 0		- success
> + * * ENOMEM	- out of memory
> + * * ENOSPC	- non-ENOMEM type of resource allocation failures
> + * * <others>	- Use is discouraged
>   * @set_dev_pasid: set or replace an iommu domain to a pasid of device. The pasid of
>   *                 the device should be left in the old config in error case.
>   * @map_pages: map a physically contiguous set of pages of the same size to
> @@ -751,6 +760,8 @@ struct iommu_ops {
>   * @free: Release the domain after use.
>   */
>  struct iommu_domain_ops {
> +	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
> +			ioasid_t pasid, struct iommu_domain *old);
>  	int (*attach_dev)(struct iommu_domain *domain, struct device *dev,
>  			  struct iommu_domain *old);
>  	int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
> 
--- snip ---
> @@ -3615,6 +3657,11 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
>  	ret = 0;
>  
>  	if (curr_domain != domain) {
> +		ret = __iommu_domain_test_device(domain, dev, pasid,
> +						 curr_domain);
> +		if (ret)
> +			goto out_unlock;
> +
>  		ret = __iommu_set_group_pasid(domain, group,
>  					      pasid, curr_domain);
>  		if (ret)

Apart from the comment and commit description nits mentioned above this
looks good to me.

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

Hi Niklas,

On Mon, Oct 13, 2025 at 11:53:55AM +0200, Niklas Schnelle wrote:
> On Sun, 2025-10-12 at 17:04 -0700, Nicolin Chen wrote:
> > Add a new test_dev domain op for driver to test the compatibility between
> > a domain and a device at the driver level, before calling into the actual
> > attachment/replacement of a domain. Support pasid for set_dev_pasid call.
> > 
> > Move existing core-level compatibility tests to a helper function. Invoke
> > it prior to:
> >  * __iommu_attach_device() or its wrapper __iommu_device_set_domain()
> >  * __iommu_set_group_pasid()
> 
> Should this list also include iommu_deferred_attach()? The code does
> include it.

iommu_deferred_attach() invokes __iommu_attach_device(), so it is
already included in the list :)

> >  /**
> >   * struct iommu_domain_ops - domain specific operations
> > - * @attach_dev: attach an iommu domain to a device
> > + * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
> > + *            A driver-level callback of this op should do a thorough sanity, to
> 
> You're missing the word "check" above.

Ack.

> > + *            make sure a device is compatible with the domain. So the following
> > + *            @attach_dev and @set_dev_pasid functions would likely succeed with
> > + *            only one exception due to a temporary failure like out of memory.
> 
> Nit: "… only one exception …" / "… like out of memory …" this sounds a
> bit odd to me because on the one hand it's one exception but then also
> a group (temporary failures).
> 
> Maybe better:
> "… would likely succeed with only the exception of temporary failures
> like out of memory."?

Sure. I can do that. Fixing both parts, it would be:

 * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
 *            A driver callback of this op should do a thorough sanity check, to
 *            make sure a device is compatible with the domain, so the following
 *            @attach_dev and @set_dev_pasid functions would likely succeed with
 *            only the exception of temporary failures like out of memory.

> --- snip ---
> > @@ -3615,6 +3657,11 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
> >  	ret = 0;
> >  
> >  	if (curr_domain != domain) {
> > +		ret = __iommu_domain_test_device(domain, dev, pasid,
> > +						 curr_domain);
> > +		if (ret)
> > +			goto out_unlock;
> > +
> >  		ret = __iommu_set_group_pasid(domain, group,
> >  					      pasid, curr_domain);
> >  		if (ret)
> 
> Apart from the comment and commit description nits mentioned above this
> looks good to me.
> 
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

Thanks for the review!
Nicolin

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Niklas Schnelle]

On Mon, 2025-10-13 at 10:22 -0700, Nicolin Chen wrote:
> Hi Niklas,
> 
> On Mon, Oct 13, 2025 at 11:53:55AM +0200, Niklas Schnelle wrote:
> > On Sun, 2025-10-12 at 17:04 -0700, Nicolin Chen wrote:
> > > Add a new test_dev domain op for driver to test the compatibility between
> > > a domain and a device at the driver level, before calling into the actual
> > > attachment/replacement of a domain. Support pasid for set_dev_pasid call.
> > > 
> > > Move existing core-level compatibility tests to a helper function. Invoke
> > > it prior to:
> > >  * __iommu_attach_device() or its wrapper __iommu_device_set_domain()
> > >  * __iommu_set_group_pasid()
> > 
> > Should this list also include iommu_deferred_attach()? The code does
> > include it.
> 
> iommu_deferred_attach() invokes __iommu_attach_device(), so it is
> already included in the list :)

Ok makes sense, though it does list __iommu_device_set_domain()
separately. Either way is fine for me.

> 
> > >  /**
> > >   * struct iommu_domain_ops - domain specific operations
> > > - * @attach_dev: attach an iommu domain to a device
> > > + * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
> > > + *            A driver-level callback of this op should do a thorough sanity, to
> > 
> > You're missing the word "check" above.
> 
> Ack.
> 
> > > + *            make sure a device is compatible with the domain. So the following
> > > + *            @attach_dev and @set_dev_pasid functions would likely succeed with
> > > + *            only one exception due to a temporary failure like out of memory.
> > 
> > Nit: "… only one exception …" / "… like out of memory …" this sounds a
> > bit odd to me because on the one hand it's one exception but then also
> > a group (temporary failures).
> > 
> > Maybe better:
> > "… would likely succeed with only the exception of temporary failures
> > like out of memory."?
> 
> Sure. I can do that. Fixing both parts, it would be:
> 
>  * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
>  *            A driver callback of this op should do a thorough sanity check, to
>  *            make sure a device is compatible with the domain, so the following
>  *            @attach_dev and @set_dev_pasid functions would likely succeed with
>  *            only the exception of temporary failures like out of memory.
> 

Sounds good, thanks!

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Jason Gunthorpe]

On Sun, Oct 12, 2025 at 05:04:59PM -0700, Nicolin Chen wrote:

> And keep them within the group->mutex, so drivers can simply move all the
> sanity and compatibility tests from their attach_dev callbacks to the new
> test_dev callbacks without concerning about a race condition.

I'm not sure about this.. For the problem we are trying to solve this
would be racy as the test would be done and the group mutex
unlocked. Then later it will be re-tested and attached.

>  /**
>   * struct iommu_domain_ops - domain specific operations
> - * @attach_dev: attach an iommu domain to a device
> + * @test_dev: Test compatibility prior to an @attach_dev or @set_dev_pasid call.
> + *            A driver-level callback of this op should do a thorough sanity, to
> + *            make sure a device is compatible with the domain. So the following
> + *            @attach_dev and @set_dev_pasid functions would likely succeed with
> + *            only one exception due to a temporary failure like out of memory.
> + *            It's suggested to avoid the kernel prints in this op.

This is a general rule, even normal attach should be following it or
iommufd will be spamming the log.

>   *  Return:
>   * * 0		- success
>   * * EINVAL	- can indicate that device and domain are incompatible due to
> @@ -722,11 +727,15 @@ struct iommu_ops {
>   *		  driver shouldn't log an error, since it is legitimate for a
>   *		  caller to test reuse of existing domains. Otherwise, it may
>   *		  still represent some other fundamental problem
> - * * ENOMEM	- out of memory
> - * * ENOSPC	- non-ENOMEM type of resource allocation failures
>   * * EBUSY	- device is attached to a domain and cannot be changed
>   * * ENODEV	- device specific errors, not able to be attached
>   * * <others>	- treated as ENODEV by the caller. Use is discouraged
> + * @attach_dev: attach an iommu domain to a device
> + *  Return:
> + * * 0		- success
> + * * ENOMEM	- out of memory
> + * * ENOSPC	- non-ENOMEM type of resource allocation failures
> + * * <others>	- Use is discouraged
>   * @set_dev_pasid: set or replace an iommu domain to a pasid of device. The pasid of
>   *                 the device should be left in the old config in error case.
>   * @map_pages: map a physically contiguous set of pages of the same size to
> @@ -751,6 +760,8 @@ struct iommu_ops {
>   * @free: Release the domain after use.
>   */
>  struct iommu_domain_ops {
> +	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
> +			ioasid_t pasid, struct iommu_domain *old);

Because of the starting remark I'm skeptical that old should be
included here.

The rest looks OK

Jason

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

On Mon, Oct 20, 2025 at 01:27:36PM -0300, Jason Gunthorpe wrote:
> On Sun, Oct 12, 2025 at 05:04:59PM -0700, Nicolin Chen wrote:
> 
> > And keep them within the group->mutex, so drivers can simply move all the
> > sanity and compatibility tests from their attach_dev callbacks to the new
> > test_dev callbacks without concerning about a race condition.
> 
> I'm not sure about this.. For the problem we are trying to solve this
> would be racy as the test would be done and the group mutex
> unlocked. Then later it will be re-tested and attached.

Oh right, we'll have to retest in iommu_dev_reset_done(). I missed
that.

> > @@ -751,6 +760,8 @@ struct iommu_ops {
> >   * @free: Release the domain after use.
> >   */
> >  struct iommu_domain_ops {
> > +	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
> > +			ioasid_t pasid, struct iommu_domain *old);
> 
> Because of the starting remark I'm skeptical that old should be
> included here.

Hmm, the followings functions sanitizes "old":
 - qcom_iommu_identity_attach() drivers/iommu/arm/arm-smmu/qcom_iommu.c
 - iommu_sva_set_dev_pasid() in drivers/iommu/amd/pasid.c

Thanks
Nicolin

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Jason Gunthorpe]

On Mon, Oct 20, 2025 at 11:51:49AM -0700, Nicolin Chen wrote:
> On Mon, Oct 20, 2025 at 01:27:36PM -0300, Jason Gunthorpe wrote:
> > On Sun, Oct 12, 2025 at 05:04:59PM -0700, Nicolin Chen wrote:
> > 
> > > And keep them within the group->mutex, so drivers can simply move all the
> > > sanity and compatibility tests from their attach_dev callbacks to the new
> > > test_dev callbacks without concerning about a race condition.
> > 
> > I'm not sure about this.. For the problem we are trying to solve this
> > would be racy as the test would be done and the group mutex
> > unlocked. Then later it will be re-tested and attached.
> 
> Oh right, we'll have to retest in iommu_dev_reset_done(). I missed
> that.
> 
> > > @@ -751,6 +760,8 @@ struct iommu_ops {
> > >   * @free: Release the domain after use.
> > >   */
> > >  struct iommu_domain_ops {
> > > +	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
> > > +			ioasid_t pasid, struct iommu_domain *old);
> > 
> > Because of the starting remark I'm skeptical that old should be
> > included here.
> 
> Hmm, the followings functions sanitizes "old":
>  - qcom_iommu_identity_attach() drivers/iommu/arm/arm-smmu/qcom_iommu.c

That shouldn't be copied over to test??

        if (domain == identity_domain || !domain)
                return 0;

That is just optimizing away the attach if it has nothing to do

        qcom_domain = to_qcom_iommu_domain(domain);
        if (WARN_ON(!qcom_domain->iommu))
                return -EINVAL;

That can't never happen

>  - iommu_sva_set_dev_pasid() in drivers/iommu/amd/pasid.c

Its broken, you are not required by API to detach a domain before
setting a new one. Keep it in attach, hope someone fixes this driver
someday.

Jason

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

On Mon, Oct 27, 2025 at 08:23:10PM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 20, 2025 at 11:51:49AM -0700, Nicolin Chen wrote:
> > On Mon, Oct 20, 2025 at 01:27:36PM -0300, Jason Gunthorpe wrote:
> > > On Sun, Oct 12, 2025 at 05:04:59PM -0700, Nicolin Chen wrote:
> > > > @@ -751,6 +760,8 @@ struct iommu_ops {
> > > >   * @free: Release the domain after use.
> > > >   */
> > > >  struct iommu_domain_ops {
> > > > +	int (*test_dev)(struct iommu_domain *domain, struct device *dev,
> > > > +			ioasid_t pasid, struct iommu_domain *old);
> > > 
> > > Because of the starting remark I'm skeptical that old should be
> > > included here.
> > 
> > Hmm, the followings functions sanitizes "old":
> >  - qcom_iommu_identity_attach() drivers/iommu/arm/arm-smmu/qcom_iommu.c
> 
> That shouldn't be copied over to test??
> 
>         if (domain == identity_domain || !domain)
>                 return 0;
> 
> That is just optimizing away the attach if it has nothing to do
> 
>         qcom_domain = to_qcom_iommu_domain(domain);
>         if (WARN_ON(!qcom_domain->iommu))
>                 return -EINVAL;
> 
> That can't never happen
> 
> >  - iommu_sva_set_dev_pasid() in drivers/iommu/amd/pasid.c
> 
> Its broken, you are not required by API to detach a domain before
> setting a new one. Keep it in attach, hope someone fixes this driver
> someday.

OK. I am leaving them alone. And no more old passed to test_dev.

Thanks
Nicolin

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

On Sun, Oct 12, 2025 at 05:04:59PM -0700, Nicolin Chen wrote:
> @@ -3479,38 +3545,19 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
...
> -	for_each_group_device(group, device) {
> -		/*
> -		 * Skip PASID validation for devices without PASID support
> -		 * (max_pasids = 0). These devices cannot issue transactions
> -		 * with PASID, so they don't affect group's PASID usage.
> -		 */
> -		if ((device->dev->iommu->max_pasids > 0) &&
> -		    (pasid >= device->dev->iommu->max_pasids)) {
> -			ret = -EINVAL;
> -			goto out_unlock;
> -		}
> -	}
> +
> +	ret = __iommu_domain_test_device(domain, dev, pasid, NULL);

I realized that here it needs to be under for_each_group_device,
as its following __iommu_set_group_pasid() calls attach_dev() on
every group_device. So, the new code should run a test_dev() on
every group_device to keep the consistency.

> @@ -3615,6 +3657,11 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
>  	ret = 0;
>  
>  	if (curr_domain != domain) {
> +		ret = __iommu_domain_test_device(domain, dev, pasid,
> +						 curr_domain);
> +		if (ret)
> +			goto out_unlock;
> +
>  		ret = __iommu_set_group_pasid(domain, group,
>  					      pasid, curr_domain);

Needs the same fix above.

Nicolin

RE: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Tian, Kevin]

> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Monday, October 13, 2025 8:05 AM
> @@ -714,7 +714,12 @@ struct iommu_ops {
> 
>  /**
>   * struct iommu_domain_ops - domain specific operations
> - * @attach_dev: attach an iommu domain to a device
> + * @test_dev: Test compatibility prior to an @attach_dev or
> @set_dev_pasid call.
> + *            A driver-level callback of this op should do a thorough sanity, to
> + *            make sure a device is compatible with the domain. So the following
> + *            @attach_dev and @set_dev_pasid functions would likely succeed
> with
> + *            only one exception due to a temporary failure like out of memory.
> + *            It's suggested to avoid the kernel prints in this op.
>   *  Return:
>   * * 0		- success
>   * * EINVAL	- can indicate that device and domain are incompatible due
> to
> @@ -722,11 +727,15 @@ struct iommu_ops {
>   *		  driver shouldn't log an error, since it is legitimate for a
>   *		  caller to test reuse of existing domains. Otherwise, it may
>   *		  still represent some other fundamental problem
> - * * ENOMEM	- out of memory
> - * * ENOSPC	- non-ENOMEM type of resource allocation failures
>   * * EBUSY	- device is attached to a domain and cannot be changed
>   * * ENODEV	- device specific errors, not able to be attached
>   * * <others>	- treated as ENODEV by the caller. Use is discouraged
> + * @attach_dev: attach an iommu domain to a device
> + *  Return:
> + * * 0		- success
> + * * ENOMEM	- out of memory
> + * * ENOSPC	- non-ENOMEM type of resource allocation failures
> + * * <others>	- Use is discouraged

It might need more work to meet this requirement. e.g. after patch4
I could still spot other errors easily in the attach path:

intel_iommu_attach_device()
  iopf_for_domain_set()
    intel_iommu_enable_iopf():

        if (!info->pri_enabled)
                return -ENODEV;

intel_iommu_attach_device()
  dmar_domain_attach_device()
    domain_attach_iommu():
      
       curr = xa_cmpxchg(&domain->iommu_array, iommu->seq_id,
                          NULL, info, GFP_KERNEL);
        if (curr) {
                ret = xa_err(curr) ? : -EBUSY;
                goto err_clear;
        }

intel_iommu_attach_device()
  dmar_domain_attach_device()
    domain_setup_first_level()
      __domain_setup_first_level()
        intel_pasid_setup_first_level():

        pte = intel_pasid_get_entry(dev, pasid);
        if (!pte) {
                spin_unlock(&iommu->lock);
                return -ENODEV;
        }

        if (pasid_pte_is_present(pte)) {
                spin_unlock(&iommu->lock);
                return -EBUSY;
        }

On the other hand, how do we communicate whatever errors returned
by attach_dev in the reset_done path back to userspace? As noted above
resource allocation failures could still occur in attach_dev, but userspace
may think the requested attach in middle of a reset has succeeded as
long as it passes the test_dev check.

Does it work better to block the attaching process upon ongoing reset
and wake it up later upon reset_done to resume attach?

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

On Thu, Oct 30, 2025 at 08:47:18AM +0000, Tian, Kevin wrote:
> It might need more work to meet this requirement. e.g. after patch4
> I could still spot other errors easily in the attach path:
> 
> intel_iommu_attach_device()
>   iopf_for_domain_set()
>     intel_iommu_enable_iopf():
> 
>         if (!info->pri_enabled)
>                 return -ENODEV;

Yea, I missed that.

> intel_iommu_attach_device()
>   dmar_domain_attach_device()
>     domain_attach_iommu():
>       
>        curr = xa_cmpxchg(&domain->iommu_array, iommu->seq_id,
>                           NULL, info, GFP_KERNEL);
>         if (curr) {
>                 ret = xa_err(curr) ? : -EBUSY;
>                 goto err_clear;
>         }

There is actually an xa_load() in this function:

	curr = xa_load(&domain->iommu_array, iommu->seq_id);
	if (curr) {
		curr->refcnt++;
		kfree(info);
		return 0;
	}

	[...]

	info->refcnt	= 1;
	info->did	= num;
	info->iommu	= iommu;
	curr = xa_cmpxchg(&domain->iommu_array, iommu->seq_id,
			  NULL, info, GFP_KERNEL);
	if (curr) {
		ret = xa_err(curr) ? : -EBUSY;
		goto err_clear;
	}

It seems that this xa_cmpxchg could be just xa_store()?

> intel_iommu_attach_device()
>   dmar_domain_attach_device()
>     domain_setup_first_level()
>       __domain_setup_first_level()
>         intel_pasid_setup_first_level():

Yea. There are a few others in the track also..

>         pte = intel_pasid_get_entry(dev, pasid);
>         if (!pte) {
>                 spin_unlock(&iommu->lock);
>                 return -ENODEV;
>         }
> 
>         if (pasid_pte_is_present(pte)) {
>                 spin_unlock(&iommu->lock);
>                 return -EBUSY;
>         }

Hmm, this is fenced by iommu->lock and can race with !attach_dev
callbacks. It might be difficult to shift these to test_dev..

> On the other hand, how do we communicate whatever errors returned
> by attach_dev in the reset_done path back to userspace? As noted above
> resource allocation failures could still occur in attach_dev, but userspace
> may think the requested attach in middle of a reset has succeeded as
> long as it passes the test_dev check.

That's a legit point. Jason pointed out that we would end up with
some inconsistency between driver and core as well, at the SMMUv3
patch. So, this test_dev doesn't seemingly solve our problem very
well..

> Does it work better to block the attaching process upon ongoing reset
> and wake it up later upon reset_done to resume attach?

Yea, I think returning -EBUSY would be the simplest solution like
we did in the previous version.

But the concern is that VF might not be aware of a PF reset, so it
can still race an attachment, which would be -EBUSY as well. Then,
if its driver doesn't retry/defer the attach, this might break it?

FWIW, I am thinking of another design based on Jason's remarks:
https://lore.kernel.org/linux-iommu/aQBopHFub8wyQh5C@Asurada-Nvidia/

So, instead of core initiating the round trip between the blocking
domain and group->domain, it forwards dev_reset_prepare/done to the
driver where it does a low-level attachment that wouldn't fail:
  For SMMUv3, it's an STE update.
  For intel_iommu, it seems to be the context table update?

Then, any concurrent would be allowed to carry on to go through all
the compatibility/sanity checks as usual, but it would bypass the
final step: STE or context table update.

Thanks
Nicolin

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Jason Gunthorpe]

On Thu, Oct 30, 2025 at 12:43:59PM -0700, Nicolin Chen wrote:

> FWIW, I am thinking of another design based on Jason's remarks:
> https://lore.kernel.org/linux-iommu/aQBopHFub8wyQh5C@Asurada-Nvidia/
> 
> So, instead of core initiating the round trip between the blocking
> domain and group->domain, it forwards dev_reset_prepare/done to the
> driver where it does a low-level attachment that wouldn't fail:
>   For SMMUv3, it's an STE update.
>   For intel_iommu, it seems to be the context table update?

Kevin, how bad do you think the UAPI issue is if we ignore it?

Jason

RE: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, November 4, 2025 2:54 AM
> 
> On Thu, Oct 30, 2025 at 12:43:59PM -0700, Nicolin Chen wrote:
> 
> > FWIW, I am thinking of another design based on Jason's remarks:
> > https://lore.kernel.org/linux-iommu/aQBopHFub8wyQh5C@Asurada-
> Nvidia/
> >
> > So, instead of core initiating the round trip between the blocking
> > domain and group->domain, it forwards dev_reset_prepare/done to the
> > driver where it does a low-level attachment that wouldn't fail:
> >   For SMMUv3, it's an STE update.
> >   For intel_iommu, it seems to be the context table update?
> 
> Kevin, how bad do you think the UAPI issue is if we ignore it?
> 

yeah probably better to leave it. I didn't see a clean way and the
value didn't justify the complexity.

Regarding to PF reset, it's a devastating operation while the vf user
is operating the vf w/o any awareness. there must be certain
coordination in userspace. otherwise nobody can recover the
registers. Comparing to that, solving the domain attach problem
is less important...

Re: [PATCH v1 02/20] iommu: Introduce a test_dev domain op and an internal helper

[Nicolin Chen]

On Wed, Nov 05, 2025 at 06:57:31AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Tuesday, November 4, 2025 2:54 AM
> > 
> > On Thu, Oct 30, 2025 at 12:43:59PM -0700, Nicolin Chen wrote:
> > 
> > > FWIW, I am thinking of another design based on Jason's remarks:
> > > https://lore.kernel.org/linux-iommu/aQBopHFub8wyQh5C@Asurada-
> > Nvidia/
> > >
> > > So, instead of core initiating the round trip between the blocking
> > > domain and group->domain, it forwards dev_reset_prepare/done to the
> > > driver where it does a low-level attachment that wouldn't fail:
> > >   For SMMUv3, it's an STE update.
> > >   For intel_iommu, it seems to be the context table update?
> > 
> > Kevin, how bad do you think the UAPI issue is if we ignore it?
> > 
> 
> yeah probably better to leave it. I didn't see a clean way and the
> value didn't justify the complexity.
> 
> Regarding to PF reset, it's a devastating operation while the vf user
> is operating the vf w/o any awareness. there must be certain
> coordination in userspace. otherwise nobody can recover the
> registers. Comparing to that, solving the domain attach problem
> is less important...

If I capture these correctly, we should go with a -EBUSY version:
 - Reject concurrent attachments during a device reset
 - Skip reset for devices having sibling group devices
 - Allow PF to stop IOMMU, ignoring VFs
?

That sounds pretty much like this v4:
https://lore.kernel.org/linux-iommu/0f6021b500c74db33af8118210dd7a2b2fd31b3c.1756682135.git.nicolinc@nvidia.com/
by dropping the SRIOV concern.

Thanks
Nicolin

[PATCH v1 03/20] iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callbacks to this
new test_dev callback function. The IOMMU core makes sure an attach_dev()
must be invoked after a successful test_dev callback.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   2 +
 .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     |   6 +-
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |   4 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 113 +++++++++++-------
 4 files changed, 74 insertions(+), 51 deletions(-)

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 ae23aacc38402..acb1dbc592cf0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -963,6 +963,8 @@ void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
 			     struct arm_smmu_cd *cdptr,
 			     const struct arm_smmu_cd *target);
 
+int arm_smmu_domain_test_dev(struct iommu_domain *domain, struct device *dev,
+			     ioasid_t pasid, struct iommu_domain *old_domain);
 int arm_smmu_set_pasid(struct arm_smmu_master *master,
 		       struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
 		       struct arm_smmu_cd *cd, struct iommu_domain *old);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
index 313201a616991..a253f9c8bb290 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
@@ -152,11 +152,6 @@ static int arm_smmu_attach_dev_nested(struct iommu_domain *domain,
 	struct arm_smmu_ste ste;
 	int ret;
 
-	if (nested_domain->vsmmu->smmu != master->smmu)
-		return -EINVAL;
-	if (arm_smmu_ssids_in_use(&master->cd_table))
-		return -EBUSY;
-
 	mutex_lock(&arm_smmu_asid_lock);
 	/*
 	 * The VM has to control the actual ATS state at the PCI device because
@@ -187,6 +182,7 @@ static void arm_smmu_domain_nested_free(struct iommu_domain *domain)
 }
 
 static const struct iommu_domain_ops arm_smmu_nested_ops = {
+	.test_dev = arm_smmu_domain_test_dev,
 	.attach_dev = arm_smmu_attach_dev_nested,
 	.free = arm_smmu_domain_nested_free,
 };
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 59a480974d80f..610d9e826c07e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -276,9 +276,6 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
 	struct arm_smmu_cd target;
 	int ret;
 
-	if (!(master->smmu->features & ARM_SMMU_FEAT_SVA))
-		return -EOPNOTSUPP;
-
 	/* Prevent arm_smmu_mm_release from being called while we are attaching */
 	if (!mmget_not_zero(domain->mm))
 		return -EINVAL;
@@ -319,6 +316,7 @@ static void arm_smmu_sva_domain_free(struct iommu_domain *domain)
 }
 
 static const struct iommu_domain_ops arm_smmu_sva_domain_ops = {
+	.test_dev		= arm_smmu_domain_test_dev,
 	.set_dev_pasid		= arm_smmu_sva_set_dev_pasid,
 	.free			= arm_smmu_sva_domain_free
 };
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 a33fbd12a0dd9..3448e55bbcdbb 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2765,9 +2765,6 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
 
 	iommu_group_mutex_assert(master->dev);
 
-	if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
-		return -EOPNOTSUPP;
-
 	/*
 	 * Drivers for devices supporting PRI or stall require iopf others have
 	 * device-specific fault handlers and don't need IOPF, so this is not a
@@ -2776,10 +2773,6 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
 	if (!master->stall_enabled)
 		return 0;
 
-	/* We're not keeping track of SIDs in fault events */
-	if (master->num_streams != 1)
-		return -EOPNOTSUPP;
-
 	if (master->iopf_refcount) {
 		master->iopf_refcount++;
 		master_domain->using_iopf = true;
@@ -2937,14 +2930,6 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
 		 * one of them.
 		 */
 		spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-		if (smmu_domain->enforce_cache_coherency &&
-		    !arm_smmu_master_canwbs(master)) {
-			spin_unlock_irqrestore(&smmu_domain->devices_lock,
-					       flags);
-			ret = -EINVAL;
-			goto err_iopf;
-		}
-
 		if (state->ats_enabled)
 			atomic_inc(&smmu_domain->nr_ats_masters);
 		list_add(&master_domain->devices_elm, &smmu_domain->devices);
@@ -2962,8 +2947,6 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
 	}
 	return 0;
 
-err_iopf:
-	arm_smmu_disable_iopf(master, master_domain);
 err_free_master_domain:
 	kfree(master_domain);
 err_free_vmaster:
@@ -3002,13 +2985,79 @@ void arm_smmu_attach_commit(struct arm_smmu_attach_state *state)
 	master->ats_enabled = state->ats_enabled;
 }
 
+int arm_smmu_domain_test_dev(struct iommu_domain *domain, struct device *dev,
+			     ioasid_t pasid, struct iommu_domain *old_domain)
+{
+	struct arm_smmu_domain *device_domain = to_smmu_domain_devices(domain);
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+
+	if (!dev_iommu_fwspec_get(dev))
+		return -ENOENT;
+
+	switch (domain->type) {
+	case IOMMU_DOMAIN_NESTED: {
+		struct arm_smmu_nested_domain *nested_domain =
+			to_smmu_nested_domain(domain);
+
+		if (WARN_ON(pasid != IOMMU_NO_PASID))
+			return -EOPNOTSUPP;
+		if (nested_domain->vsmmu->smmu != master->smmu)
+			return -EINVAL;
+		if (arm_smmu_ssids_in_use(&master->cd_table))
+			return -EBUSY;
+		break;
+	}
+	case IOMMU_DOMAIN_SVA:
+		if (!(master->smmu->features & ARM_SMMU_FEAT_SVA))
+			return -EOPNOTSUPP;
+		break;
+	default: {
+		struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+
+		if (smmu_domain->smmu != master->smmu)
+			return -EINVAL;
+		if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2 &&
+		    arm_smmu_ssids_in_use(&master->cd_table))
+			return -EBUSY;
+		if (pasid != IOMMU_NO_PASID) {
+			struct iommu_domain *sid_domain =
+				iommu_get_domain_for_dev(master->dev);
+
+			if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1)
+				return -EINVAL;
+			if (!master->cd_table.in_ste &&
+			    sid_domain->type != IOMMU_DOMAIN_IDENTITY &&
+			    sid_domain->type != IOMMU_DOMAIN_BLOCKED)
+				return -EINVAL;
+		}
+		break;
+	}
+	}
+
+	if (domain->iopf_handler) {
+		if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
+			return -EOPNOTSUPP;
+		/* We're not keeping track of SIDs in fault events */
+		if (master->stall_enabled && master->num_streams != 1)
+			return -EOPNOTSUPP;
+	}
+
+	if (device_domain) {
+		scoped_guard(spinlock_irqsave, &device_domain->devices_lock) {
+			if (device_domain->enforce_cache_coherency &&
+			    !arm_smmu_master_canwbs(master))
+				return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
 			       struct iommu_domain *old_domain)
 {
 	int ret = 0;
 	struct arm_smmu_ste target;
-	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-	struct arm_smmu_device *smmu;
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_attach_state state = {
 		.old_domain = old_domain,
@@ -3017,21 +3066,13 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
 	struct arm_smmu_master *master;
 	struct arm_smmu_cd *cdptr;
 
-	if (!fwspec)
-		return -ENOENT;
-
 	state.master = master = dev_iommu_priv_get(dev);
-	smmu = master->smmu;
-
-	if (smmu_domain->smmu != smmu)
-		return -EINVAL;
 
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
 		cdptr = arm_smmu_alloc_cd_ptr(master, IOMMU_NO_PASID);
 		if (!cdptr)
 			return -ENOMEM;
-	} else if (arm_smmu_ssids_in_use(&master->cd_table))
-		return -EBUSY;
+	}
 
 	/*
 	 * Prevent arm_smmu_share_asid() from trying to change the ASID
@@ -3078,15 +3119,8 @@ static int arm_smmu_s1_set_dev_pasid(struct iommu_domain *domain,
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_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;
 
-	if (smmu_domain->smmu != smmu)
-		return -EINVAL;
-
-	if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1)
-		return -EINVAL;
-
 	/*
 	 * We can read cd.asid outside the lock because arm_smmu_set_pasid()
 	 * will fix it
@@ -3136,14 +3170,6 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
 
 	/* The core code validates pasid */
 
-	if (smmu_domain->smmu != master->smmu)
-		return -EINVAL;
-
-	if (!master->cd_table.in_ste &&
-	    sid_domain->type != IOMMU_DOMAIN_IDENTITY &&
-	    sid_domain->type != IOMMU_DOMAIN_BLOCKED)
-		return -EINVAL;
-
 	cdptr = arm_smmu_alloc_cd_ptr(master, pasid);
 	if (!cdptr)
 		return -ENOMEM;
@@ -3695,6 +3721,7 @@ static const struct iommu_ops arm_smmu_ops = {
 	.user_pasid_table	= 1,
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev		= arm_smmu_domain_test_dev,
 		.attach_dev		= arm_smmu_attach_dev,
 		.enforce_cache_coherency = arm_smmu_enforce_cache_coherency,
 		.set_dev_pasid		= arm_smmu_s1_set_dev_pasid,
-- 
2.43.0

Re: [PATCH v1 03/20] iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev

[Jason Gunthorpe]

On Sun, Oct 12, 2025 at 05:05:00PM -0700, Nicolin Chen wrote:
> 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 a33fbd12a0dd9..3448e55bbcdbb 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2765,9 +2765,6 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
>  
>  	iommu_group_mutex_assert(master->dev);
>  
> -	if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
> -		return -EOPNOTSUPP;

Stuff like this is also optimizing the codegen, it shouldn't be
removed.

> +int arm_smmu_domain_test_dev(struct iommu_domain *domain, struct device *dev,
> +			     ioasid_t pasid, struct iommu_domain *old_domain)
> +{
> +	struct arm_smmu_domain *device_domain = to_smmu_domain_devices(domain);
> +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> +
> +	if (!dev_iommu_fwspec_get(dev))
> +		return -ENOENT;
> +
> +	switch (domain->type) {
> +	case IOMMU_DOMAIN_NESTED: {
> +		struct arm_smmu_nested_domain *nested_domain =
> +			to_smmu_nested_domain(domain);
> +
> +		if (WARN_ON(pasid != IOMMU_NO_PASID))
> +			return -EOPNOTSUPP;
> +		if (nested_domain->vsmmu->smmu != master->smmu)
> +			return -EINVAL;
> +		if (arm_smmu_ssids_in_use(&master->cd_table))
> +			return -EBUSY;

This gives me alot of pause.. Here we are detecting if a S1 PASID is
installed for a S2 attach, but to your purpose this can be made
inconsistent by userspace during a FLR..

I don't see any reasonable way to mitigate this??

Which makes me wonder if we should just try to solve the simple
obvious things like direct, permanent incompatability and still have
some kind of recovery code to leave things in blocking if they fail to
attach

Jason

Re: [PATCH v1 03/20] iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev

[Nicolin Chen]

On Mon, Oct 20, 2025 at 01:32:31PM -0300, Jason Gunthorpe wrote:
> On Sun, Oct 12, 2025 at 05:05:00PM -0700, Nicolin Chen wrote:
> > 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 a33fbd12a0dd9..3448e55bbcdbb 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2765,9 +2765,6 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
> >  
> >  	iommu_group_mutex_assert(master->dev);
> >  
> > -	if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
> > -		return -EOPNOTSUPP;
> 
> Stuff like this is also optimizing the codegen, it shouldn't be
> removed.

Okay. I assume we should just copy it to test_dev() then.

> > +int arm_smmu_domain_test_dev(struct iommu_domain *domain, struct device *dev,
> > +			     ioasid_t pasid, struct iommu_domain *old_domain)
> > +{
> > +	struct arm_smmu_domain *device_domain = to_smmu_domain_devices(domain);
> > +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > +
> > +	if (!dev_iommu_fwspec_get(dev))
> > +		return -ENOENT;
> > +
> > +	switch (domain->type) {
> > +	case IOMMU_DOMAIN_NESTED: {
> > +		struct arm_smmu_nested_domain *nested_domain =
> > +			to_smmu_nested_domain(domain);
> > +
> > +		if (WARN_ON(pasid != IOMMU_NO_PASID))
> > +			return -EOPNOTSUPP;
> > +		if (nested_domain->vsmmu->smmu != master->smmu)
> > +			return -EINVAL;
> > +		if (arm_smmu_ssids_in_use(&master->cd_table))
> > +			return -EBUSY;
> 
> This gives me alot of pause.. Here we are detecting if a S1 PASID is
> installed for a S2 attach, but to your purpose this can be made
> inconsistent by userspace during a FLR..

Ah right, the used_ssids could mismatch with the group->domain!

> I don't see any reasonable way to mitigate this??

Right. It can't simply go through a regular attach_dev call since
driver wouldn't expect any inconsistency in the core.

Driver would have to be aware of the reset state, and make a copy
of the old domain's CD/STE to use for a test_dev() during a reset.

> Which makes me wonder if we should just try to solve the simple
> obvious things like direct, permanent incompatability and still have
> some kind of recovery code to leave things in blocking if they fail to
> attach

I don't quite get this. Mind elaborating?

Thanks
Nicolin

Re: [PATCH v1 03/20] iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev

[Jason Gunthorpe]

On Mon, Oct 20, 2025 at 01:08:16PM -0700, Nicolin Chen wrote:

> > I don't see any reasonable way to mitigate this??
> 
> Right. It can't simply go through a regular attach_dev call since
> driver wouldn't expect any inconsistency in the core.
> 
> Driver would have to be aware of the reset state, and make a copy
> of the old domain's CD/STE to use for a test_dev() during a reset.

It seems convoluted :\

IDK if you want to do that some kind of "attach but really don't" flag
so all the tracking was kept, just the STE was forced to blocking.

Then since all the fake attaches are tracked and validated switching
to a real ste shouldn't fail.

For instance SMMU could continue to build the CD table and act like
the CD is active, but the STE wouldn't point to it.

Basically, this approach doesn't seem to solve all the problems, it
reduces them sure, but there are still enough gaps.

So I think we either have to live with them and call it a user bug to
change the domain improperly during FLR.. If it happens dump the
translation into blocking and move on..

Or we have to do some kind of fake attach tracking which doesn't sound
appealing..

Jason

Re: [PATCH v1 03/20] iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev

[Nicolin Chen]

On Mon, Oct 27, 2025 at 08:26:40PM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 20, 2025 at 01:08:16PM -0700, Nicolin Chen wrote:
> 
> > > I don't see any reasonable way to mitigate this??
> > 
> > Right. It can't simply go through a regular attach_dev call since
> > driver wouldn't expect any inconsistency in the core.
> > 
> > Driver would have to be aware of the reset state, and make a copy
> > of the old domain's CD/STE to use for a test_dev() during a reset.
> 
> It seems convoluted :\

Yea. Both core and driver would end up with complexities Orz.

> IDK if you want to do that some kind of "attach but really don't" flag
> so all the tracking was kept, just the STE was forced to blocking.
> 
> Then since all the fake attaches are tracked and validated switching
> to a real ste shouldn't fail.
> 
> For instance SMMU could continue to build the CD table and act like
> the CD is active, but the STE wouldn't point to it.
> 
> Basically, this approach doesn't seem to solve all the problems, it
> reduces them sure, but there are still enough gaps.

Yea, this works for reset because the physical STE is on an ABORT
state, so SW can concurrently mutate the CD as much as it wants.
Yet, with a translating STE, CD table can't be mutated I suppose?

So this "attach but really don't" flag would be used by the reset
case exclusively. It feels like the surgical in-driver approach
that we thought about in the beginning v.s. the core-managed one
that we wished for to fix all drivers.

By doing so, I think the core could simply forward the two reset
callbacks to the driver where it does an STE-level suspend/resume
and raises a reset flag so that any concurrent attach_dev would
bypass the last STE updates until the reset_done is finished.

This, however, will require driver to implement those callbacks
individually, if it cares to fix the vulnerability..

> So I think we either have to live with them and call it a user bug to
> change the domain improperly during FLR.. If it happens dump the
> translation into blocking and move on..

But, we previously discussed that it'd not be a user bug for VFs,
unless they are all getting reset by the PCI core?

Otherwise, returning -EBUSY rejecting concurrent attach would be
the easiest way.

Thanks
Nicolin

[PATCH v1 04/20] iommu/intel: Implement test_dev callbacks to domain ops

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callbacks to the
new test_dev callback functions. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Following the test_dev guideline, replace dev_err with dev_dbg.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/intel/iommu.c  | 66 +++++++++++++++++++++---------------
 drivers/iommu/intel/nested.c | 29 +++++++++++-----
 drivers/iommu/intel/svm.c    | 11 +++---
 3 files changed, 67 insertions(+), 39 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index f0396591cd9bb..10d422bd463a2 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3537,6 +3537,26 @@ int paging_domain_compatible(struct iommu_domain *domain, struct device *dev)
 	return 0;
 }
 
+static int intel_iommu_test_device(struct iommu_domain *domain,
+				   struct device *dev, ioasid_t pasid,
+				   struct iommu_domain *old)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct intel_iommu *iommu = info->iommu;
+
+	if (pasid != IOMMU_NO_PASID) {
+		if (WARN_ON_ONCE(!(domain->type & __IOMMU_DOMAIN_PAGING)))
+			return -EINVAL;
+		if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
+			return -EOPNOTSUPP;
+		if (domain->dirty_ops)
+			return -EINVAL;
+		if (context_copied(iommu, info->bus, info->devfn))
+			return -EBUSY;
+	}
+	return paging_domain_compatible(domain, dev);
+}
+
 static int intel_iommu_attach_device(struct iommu_domain *domain,
 				     struct device *dev,
 				     struct iommu_domain *old)
@@ -3545,10 +3565,6 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 
 	device_block_translation(dev);
 
-	ret = paging_domain_compatible(domain, dev);
-	if (ret)
-		return ret;
-
 	ret = iopf_for_domain_set(domain, dev);
 	if (ret)
 		return ret;
@@ -4151,22 +4167,6 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
 	struct dev_pasid_info *dev_pasid;
 	int ret;
 
-	if (WARN_ON_ONCE(!(domain->type & __IOMMU_DOMAIN_PAGING)))
-		return -EINVAL;
-
-	if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
-		return -EOPNOTSUPP;
-
-	if (domain->dirty_ops)
-		return -EINVAL;
-
-	if (context_copied(iommu, info->bus, info->devfn))
-		return -EBUSY;
-
-	ret = paging_domain_compatible(domain, dev);
-	if (ret)
-		return ret;
-
 	dev_pasid = domain_add_dev_pasid(domain, dev, pasid);
 	if (IS_ERR(dev_pasid))
 		return PTR_ERR(dev_pasid);
@@ -4178,12 +4178,9 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
 	if (intel_domain_is_fs_paging(dmar_domain))
 		ret = domain_setup_first_level(iommu, dmar_domain,
 					       dev, pasid, old);
-	else if (intel_domain_is_ss_paging(dmar_domain))
+	else /* paging_domain_compatible() made sure it's ss_paging */
 		ret = domain_setup_second_level(iommu, dmar_domain,
 						dev, pasid, old);
-	else if (WARN_ON(true))
-		ret = -EINVAL;
-
 	if (ret)
 		goto out_unwind_iopf;
 
@@ -4403,6 +4400,21 @@ static int device_setup_pass_through(struct device *dev)
 				      context_setup_pass_through_cb, dev);
 }
 
+static int identity_domain_test_dev(struct iommu_domain *domain,
+				    struct device *dev, ioasid_t pasid,
+				    struct iommu_domain *old)
+{
+	if (pasid != IOMMU_NO_PASID) {
+		struct device_domain_info *info = dev_iommu_priv_get(dev);
+		struct intel_iommu *iommu = info->iommu;
+
+		if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
+			return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static int identity_domain_attach_dev(struct iommu_domain *domain,
 				      struct device *dev,
 				      struct iommu_domain *old)
@@ -4440,9 +4452,6 @@ static int identity_domain_set_dev_pasid(struct iommu_domain *domain,
 	struct intel_iommu *iommu = info->iommu;
 	int ret;
 
-	if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
-		return -EOPNOTSUPP;
-
 	ret = iopf_for_domain_replace(domain, old, dev);
 	if (ret)
 		return ret;
@@ -4460,12 +4469,14 @@ static int identity_domain_set_dev_pasid(struct iommu_domain *domain,
 static struct iommu_domain identity_domain = {
 	.type = IOMMU_DOMAIN_IDENTITY,
 	.ops = &(const struct iommu_domain_ops) {
+		.test_dev	= identity_domain_test_dev,
 		.attach_dev	= identity_domain_attach_dev,
 		.set_dev_pasid	= identity_domain_set_dev_pasid,
 	},
 };
 
 const struct iommu_domain_ops intel_fs_paging_domain_ops = {
+	.test_dev = intel_iommu_test_device,
 	.attach_dev = intel_iommu_attach_device,
 	.set_dev_pasid = intel_iommu_set_dev_pasid,
 	.map_pages = intel_iommu_map_pages,
@@ -4479,6 +4490,7 @@ const struct iommu_domain_ops intel_fs_paging_domain_ops = {
 };
 
 const struct iommu_domain_ops intel_ss_paging_domain_ops = {
+	.test_dev = intel_iommu_test_device,
 	.attach_dev = intel_iommu_attach_device,
 	.set_dev_pasid = intel_iommu_set_dev_pasid,
 	.map_pages = intel_iommu_map_pages,
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index 760d7aa2ade84..708b8e653d5cd 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -18,19 +18,17 @@
 #include "iommu.h"
 #include "pasid.h"
 
-static int intel_nested_attach_dev(struct iommu_domain *domain,
-				   struct device *dev, struct iommu_domain *old)
+static int intel_nested_test_dev(struct iommu_domain *domain,
+				 struct device *dev, ioasid_t pasid,
+				 struct iommu_domain *old)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 	struct intel_iommu *iommu = info->iommu;
-	unsigned long flags;
-	int ret = 0;
-
-	device_block_translation(dev);
+	int ret;
 
 	if (iommu->agaw < dmar_domain->s2_domain->agaw) {
-		dev_err_ratelimited(dev, "Adjusted guest address width not compatible\n");
+		dev_dbg_ratelimited(dev, "Adjusted guest address width not compatible\n");
 		return -ENODEV;
 	}
 
@@ -41,10 +39,24 @@ static int intel_nested_attach_dev(struct iommu_domain *domain,
 	 */
 	ret = paging_domain_compatible(&dmar_domain->s2_domain->domain, dev);
 	if (ret) {
-		dev_err_ratelimited(dev, "s2 domain is not compatible\n");
+		dev_dbg_ratelimited(dev, "s2 domain is not compatible\n");
 		return ret;
 	}
 
+	return 0;
+}
+
+static int intel_nested_attach_dev(struct iommu_domain *domain,
+				   struct device *dev, struct iommu_domain *old)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct intel_iommu *iommu = info->iommu;
+	unsigned long flags;
+	int ret = 0;
+
+	device_block_translation(dev);
+
 	ret = domain_attach_iommu(dmar_domain, iommu);
 	if (ret) {
 		dev_err_ratelimited(dev, "Failed to attach domain to iommu\n");
@@ -192,6 +204,7 @@ static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
 }
 
 static const struct iommu_domain_ops intel_nested_domain_ops = {
+	.test_dev		= intel_nested_test_dev,
 	.attach_dev		= intel_nested_attach_dev,
 	.set_dev_pasid		= intel_nested_set_dev_pasid,
 	.free			= intel_nested_domain_free,
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index e147f71f91b72..5901caa4ceebc 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -145,6 +145,12 @@ static int intel_iommu_sva_supported(struct device *dev)
 	return 0;
 }
 
+static int intel_svm_test_dev(struct iommu_domain *domain, struct device *dev,
+			      ioasid_t pasid, struct iommu_domain *old)
+{
+	return intel_iommu_sva_supported(dev);
+}
+
 static int intel_svm_set_dev_pasid(struct iommu_domain *domain,
 				   struct device *dev, ioasid_t pasid,
 				   struct iommu_domain *old)
@@ -156,10 +162,6 @@ static int intel_svm_set_dev_pasid(struct iommu_domain *domain,
 	unsigned long sflags;
 	int ret = 0;
 
-	ret = intel_iommu_sva_supported(dev);
-	if (ret)
-		return ret;
-
 	dev_pasid = domain_add_dev_pasid(domain, dev, pasid);
 	if (IS_ERR(dev_pasid))
 		return PTR_ERR(dev_pasid);
@@ -195,6 +197,7 @@ static void intel_svm_domain_free(struct iommu_domain *domain)
 }
 
 static const struct iommu_domain_ops intel_svm_domain_ops = {
+	.test_dev		= intel_svm_test_dev,
 	.set_dev_pasid		= intel_svm_set_dev_pasid,
 	.free			= intel_svm_domain_free
 };
-- 
2.43.0

[PATCH v1 05/20] iommu/amd: Implement test_dev callbacks to domain ops

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callbacks to the
new test_dev callback functions. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Correct the errno upon malloc failure. Also, drop the function prototype
of iommu_sva_set_dev_pasid() from the header and make it static, as only
pasid.c uses it.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/amd/amd_iommu.h |  3 ---
 drivers/iommu/amd/iommu.c     | 27 +++++++++++++++++++--------
 drivers/iommu/amd/pasid.c     | 29 +++++++++++++++++++----------
 3 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 9b4b589a54b57..f99fa4da34996 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -52,9 +52,6 @@ struct protection_domain *protection_domain_alloc(void);
 struct iommu_domain *amd_iommu_domain_alloc_sva(struct device *dev,
 						struct mm_struct *mm);
 void amd_iommu_domain_free(struct iommu_domain *dom);
-int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
-			    struct device *dev, ioasid_t pasid,
-			    struct iommu_domain *old);
 void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
 				struct iommu_domain *domain);
 
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index e16ad510c8c8a..dc0406427dcf8 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -70,6 +70,8 @@ int amd_iommu_max_glx_val = -1;
  */
 DEFINE_IDA(pdom_ids);
 
+static int amd_iommu_test_device(struct iommu_domain *dom, struct device *dev,
+				 ioasid_t pasid, struct iommu_domain *old);
 static int amd_iommu_attach_device(struct iommu_domain *dom, struct device *dev,
 				   struct iommu_domain *old);
 
@@ -2670,6 +2672,7 @@ static struct iommu_domain blocked_domain = {
 static struct protection_domain identity_domain;
 
 static const struct iommu_domain_ops identity_domain_ops = {
+	.test_dev = amd_iommu_test_device,
 	.attach_dev = amd_iommu_attach_device,
 };
 
@@ -2686,12 +2689,26 @@ void amd_iommu_init_identity_domain(void)
 	protection_domain_init(&identity_domain);
 }
 
+static int amd_iommu_test_device(struct iommu_domain *dom, struct device *dev,
+				 ioasid_t pasid, struct iommu_domain *old)
+{
+	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
+
+	/*
+	 * Restrict to devices with compatible IOMMU hardware support
+	 * when enforcement of dirty tracking is enabled.
+	 */
+	if (dom->dirty_ops && !amd_iommu_hd_support(iommu))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int amd_iommu_attach_device(struct iommu_domain *dom, struct device *dev,
 				   struct iommu_domain *old)
 {
 	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
 	struct protection_domain *domain = to_pdomain(dom);
-	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
 	int ret;
 
 	/*
@@ -2703,13 +2720,6 @@ static int amd_iommu_attach_device(struct iommu_domain *dom, struct device *dev,
 
 	dev_data->defer_attach = false;
 
-	/*
-	 * Restrict to devices with compatible IOMMU hardware support
-	 * when enforcement of dirty tracking is enabled.
-	 */
-	if (dom->dirty_ops && !amd_iommu_hd_support(iommu))
-		return -EINVAL;
-
 	if (dev_data->domain)
 		detach_device(dev);
 
@@ -3047,6 +3057,7 @@ const struct iommu_ops amd_iommu_ops = {
 	.def_domain_type = amd_iommu_def_domain_type,
 	.page_response = amd_iommu_page_response,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= amd_iommu_test_device,
 		.attach_dev	= amd_iommu_attach_device,
 		.map_pages	= amd_iommu_map_pages,
 		.unmap_pages	= amd_iommu_unmap_pages,
diff --git a/drivers/iommu/amd/pasid.c b/drivers/iommu/amd/pasid.c
index 77c8e9a91cbca..474494a66dd40 100644
--- a/drivers/iommu/amd/pasid.c
+++ b/drivers/iommu/amd/pasid.c
@@ -99,31 +99,39 @@ static const struct mmu_notifier_ops sva_mn = {
 	.release = sva_mn_release,
 };
 
-int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
-			    struct device *dev, ioasid_t pasid,
-			    struct iommu_domain *old)
+static int iommu_sva_test_dev(struct iommu_domain *domain, struct device *dev,
+			      ioasid_t pasid, struct iommu_domain *old)
 {
-	struct pdom_dev_data *pdom_dev_data;
-	struct protection_domain *sva_pdom = to_pdomain(domain);
 	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
-	unsigned long flags;
-	int ret = -EINVAL;
 
 	if (old)
 		return -EOPNOTSUPP;
 
 	/* PASID zero is used for requests from the I/O device without PASID */
 	if (!is_pasid_valid(dev_data, pasid))
-		return ret;
+		return -EINVAL;
 
 	/* Make sure PASID is enabled */
 	if (!is_pasid_enabled(dev_data))
-		return ret;
+		return -EINVAL;
+
+	return 0;
+}
+
+static int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
+				   struct device *dev, ioasid_t pasid,
+				   struct iommu_domain *old)
+{
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+	struct protection_domain *sva_pdom = to_pdomain(domain);
+	struct pdom_dev_data *pdom_dev_data;
+	unsigned long flags;
+	int ret;
 
 	/* Add PASID to protection domain pasid list */
 	pdom_dev_data = kzalloc(sizeof(*pdom_dev_data), GFP_KERNEL);
 	if (pdom_dev_data == NULL)
-		return ret;
+		return -ENOMEM;
 
 	pdom_dev_data->pasid = pasid;
 	pdom_dev_data->dev_data = dev_data;
@@ -175,6 +183,7 @@ static void iommu_sva_domain_free(struct iommu_domain *domain)
 }
 
 static const struct iommu_domain_ops amd_sva_domain_ops = {
+	.test_dev      = iommu_sva_test_dev,
 	.set_dev_pasid = iommu_sva_set_dev_pasid,
 	.free	       = iommu_sva_domain_free
 };
-- 
2.43.0

[PATCH v1 06/20] iommu/arm-smmu: Implement arm_smmu_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu.c | 116 ++++++++++++++++----------
 1 file changed, 71 insertions(+), 45 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 5e690cf85ec96..5752eecc1d434 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -672,6 +672,37 @@ static int arm_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
 	return __arm_smmu_alloc_bitmap(smmu->context_map, start, smmu->num_context_banks);
 }
 
+static enum arm_smmu_context_fmt
+arm_smmu_get_context_fmt(struct arm_smmu_domain *smmu_domain)
+{
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	enum arm_smmu_context_fmt fmt = cfg->fmt;
+
+	/*
+	 * Choosing a suitable context format is even more fiddly. Until we
+	 * grow some way for the caller to express a preference, and/or move
+	 * the decision into the io-pgtable code where it arguably belongs,
+	 * just aim for the closest thing to the rest of the system, and hope
+	 * that the hardware isn't esoteric enough that we can't assume AArch64
+	 * support to be a superset of AArch32 support...
+	 */
+	if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_L)
+		fmt = ARM_SMMU_CTX_FMT_AARCH32_L;
+	if (IS_ENABLED(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) &&
+	    !IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_ARM_LPAE) &&
+	    (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S) &&
+	    (smmu_domain->stage == ARM_SMMU_DOMAIN_S1))
+		fmt = ARM_SMMU_CTX_FMT_AARCH32_S;
+	if ((IS_ENABLED(CONFIG_64BIT) || cfg->fmt == ARM_SMMU_CTX_FMT_NONE) &&
+	    (smmu->features & (ARM_SMMU_FEAT_FMT_AARCH64_64K |
+			       ARM_SMMU_FEAT_FMT_AARCH64_16K |
+			       ARM_SMMU_FEAT_FMT_AARCH64_4K)))
+		fmt = ARM_SMMU_CTX_FMT_AARCH64;
+
+	return fmt;
+}
+
 static int arm_smmu_init_domain_context(struct arm_smmu_domain *smmu_domain,
 					struct arm_smmu_device *smmu,
 					struct device *dev)
@@ -712,31 +743,8 @@ static int arm_smmu_init_domain_context(struct arm_smmu_domain *smmu_domain,
 	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
 		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
 
-	/*
-	 * Choosing a suitable context format is even more fiddly. Until we
-	 * grow some way for the caller to express a preference, and/or move
-	 * the decision into the io-pgtable code where it arguably belongs,
-	 * just aim for the closest thing to the rest of the system, and hope
-	 * that the hardware isn't esoteric enough that we can't assume AArch64
-	 * support to be a superset of AArch32 support...
-	 */
-	if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_L)
-		cfg->fmt = ARM_SMMU_CTX_FMT_AARCH32_L;
-	if (IS_ENABLED(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) &&
-	    !IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_ARM_LPAE) &&
-	    (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S) &&
-	    (smmu_domain->stage == ARM_SMMU_DOMAIN_S1))
-		cfg->fmt = ARM_SMMU_CTX_FMT_AARCH32_S;
-	if ((IS_ENABLED(CONFIG_64BIT) || cfg->fmt == ARM_SMMU_CTX_FMT_NONE) &&
-	    (smmu->features & (ARM_SMMU_FEAT_FMT_AARCH64_64K |
-			       ARM_SMMU_FEAT_FMT_AARCH64_16K |
-			       ARM_SMMU_FEAT_FMT_AARCH64_4K)))
-		cfg->fmt = ARM_SMMU_CTX_FMT_AARCH64;
-
-	if (cfg->fmt == ARM_SMMU_CTX_FMT_NONE) {
-		ret = -EINVAL;
-		goto out_unlock;
-	}
+	cfg->fmt = arm_smmu_get_context_fmt(smmu_domain);
+	WARN_ON(cfg->fmt == ARM_SMMU_CTX_FMT_NONE);
 
 	switch (smmu_domain->stage) {
 	case ARM_SMMU_DOMAIN_S1:
@@ -1165,14 +1173,11 @@ static void arm_smmu_master_install_s2crs(struct arm_smmu_master_cfg *cfg,
 	}
 }
 
-static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
-			       struct iommu_domain *old)
+static int arm_smmu_test_dev(struct iommu_domain *domain, struct device *dev,
+			     ioasid_t pasid, struct iommu_domain *old)
 {
-	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
-	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-	struct arm_smmu_master_cfg *cfg;
-	struct arm_smmu_device *smmu;
-	int ret;
+	struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
+	struct arm_smmu_domain *smmu_domain;
 
 	/*
 	 * FIXME: The arch/arm DMA API code tries to attach devices to its own
@@ -1181,11 +1186,40 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
 	 * domains, just say no (but more politely than by dereferencing NULL).
 	 * This should be at least a WARN_ON once that's sorted.
 	 */
-	cfg = dev_iommu_priv_get(dev);
 	if (!cfg)
 		return -ENODEV;
 
-	smmu = cfg->smmu;
+	if (domain == arm_smmu_ops.identity_domain ||
+	    domain == arm_smmu_ops.blocked_domain)
+		return 0;
+
+	smmu_domain = to_smmu_domain(domain);
+	scoped_guard(mutex, &smmu_domain->init_mutex) {
+		/* arm_smmu_init_domain_context() will initialize it */
+		if (!smmu_domain->smmu)
+			return 0;
+		/*
+		 * Sanity check the domain. We don't support domains across
+		 * different SMMUs.
+		 */
+		if (smmu_domain->smmu != cfg->smmu)
+			return -EINVAL;
+		if (arm_smmu_get_context_fmt(smmu_domain) ==
+		    ARM_SMMU_CTX_FMT_NONE)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
+			       struct iommu_domain *old)
+{
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct arm_smmu_device *smmu = cfg->smmu;
+	int ret;
 
 	ret = arm_smmu_rpm_get(smmu);
 	if (ret < 0)
@@ -1196,15 +1230,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
 	if (ret < 0)
 		goto rpm_put;
 
-	/*
-	 * Sanity check the domain. We don't support domains across
-	 * different SMMUs.
-	 */
-	if (smmu_domain->smmu != smmu) {
-		ret = -EINVAL;
-		goto rpm_put;
-	}
-
 	/* Looks ok, so add the device to the domain */
 	arm_smmu_master_install_s2crs(cfg, S2CR_TYPE_TRANS,
 				      smmu_domain->cfg.cbndx, fwspec);
@@ -1221,8 +1246,6 @@ static int arm_smmu_attach_dev_type(struct device *dev,
 	struct arm_smmu_device *smmu;
 	int ret;
 
-	if (!cfg)
-		return -ENODEV;
 	smmu = cfg->smmu;
 
 	ret = arm_smmu_rpm_get(smmu);
@@ -1242,6 +1265,7 @@ static int arm_smmu_attach_dev_identity(struct iommu_domain *domain,
 }
 
 static const struct iommu_domain_ops arm_smmu_identity_ops = {
+	.test_dev = arm_smmu_test_dev,
 	.attach_dev = arm_smmu_attach_dev_identity,
 };
 
@@ -1258,6 +1282,7 @@ static int arm_smmu_attach_dev_blocked(struct iommu_domain *domain,
 }
 
 static const struct iommu_domain_ops arm_smmu_blocked_ops = {
+	.test_dev = arm_smmu_test_dev,
 	.attach_dev = arm_smmu_attach_dev_blocked,
 };
 
@@ -1647,6 +1672,7 @@ static const struct iommu_ops arm_smmu_ops = {
 	.def_domain_type	= arm_smmu_def_domain_type,
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev		= arm_smmu_test_dev,
 		.attach_dev		= arm_smmu_attach_dev,
 		.map_pages		= arm_smmu_map_pages,
 		.unmap_pages		= arm_smmu_unmap_pages,
-- 
2.43.0

[PATCH v1 07/20] iommu/qcom_iommu: Implement test_dev callbacks to domain ops

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callbacks to the
new test_dev callback functions. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Following the test_dev guideline, replace dev_err with dev_dbg.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 46 ++++++++++++++++++-------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 9222a4a48bb33..53b1c279563ba 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -359,18 +359,36 @@ static void qcom_iommu_domain_free(struct iommu_domain *domain)
 	kfree(qcom_domain);
 }
 
-static int qcom_iommu_attach_dev(struct iommu_domain *domain,
-				 struct device *dev, struct iommu_domain *old)
+static int qcom_iommu_domain_test_dev(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
 {
 	struct qcom_iommu_dev *qcom_iommu = dev_iommu_priv_get(dev);
 	struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
-	int ret;
 
 	if (!qcom_iommu) {
-		dev_err(dev, "cannot attach to IOMMU, is it on the same bus?\n");
+		dev_dbg(dev, "cannot attach to IOMMU, is it on the same bus?\n");
 		return -ENXIO;
 	}
 
+	scoped_guard(mutex, &qcom_domain->init_mutex) {
+		/*
+		 * Sanity check the domain. We don't support domains across
+		 * different IOMMUs.
+		 */
+		if (qcom_domain->iommu && qcom_domain->iommu != qcom_iommu)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int qcom_iommu_attach_dev(struct iommu_domain *domain,
+				 struct device *dev, struct iommu_domain *old)
+{
+	struct qcom_iommu_dev *qcom_iommu = dev_iommu_priv_get(dev);
+	int ret;
+
 	/* Ensure that the domain is finalized */
 	pm_runtime_get_sync(qcom_iommu->dev);
 	ret = qcom_iommu_init_domain(domain, qcom_iommu, dev);
@@ -378,13 +396,17 @@ static int qcom_iommu_attach_dev(struct iommu_domain *domain,
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * Sanity check the domain. We don't support domains across
-	 * different IOMMUs.
-	 */
-	if (qcom_domain->iommu != qcom_iommu)
-		return -EINVAL;
+	return 0;
+}
 
+static int qcom_iommu_identity_test(struct iommu_domain *identity_domain,
+				    struct device *dev, ioasid_t pasid,
+				    struct iommu_domain *old)
+{
+	if (old == identity_domain || !old)
+		return 0;
+	if (WARN_ON(!to_qcom_iommu_domain(old)->iommu))
+		return -EINVAL;
 	return 0;
 }
 
@@ -401,8 +423,6 @@ static int qcom_iommu_identity_attach(struct iommu_domain *identity_domain,
 		return 0;
 
 	qcom_domain = to_qcom_iommu_domain(old);
-	if (WARN_ON(!qcom_domain->iommu))
-		return -EINVAL;
 
 	pm_runtime_get_sync(qcom_iommu->dev);
 	for (i = 0; i < fwspec->num_ids; i++) {
@@ -418,6 +438,7 @@ static int qcom_iommu_identity_attach(struct iommu_domain *identity_domain,
 }
 
 static struct iommu_domain_ops qcom_iommu_identity_ops = {
+	.test_dev = qcom_iommu_identity_test,
 	.attach_dev = qcom_iommu_identity_attach,
 };
 
@@ -599,6 +620,7 @@ static const struct iommu_ops qcom_iommu_ops = {
 	.device_group	= generic_device_group,
 	.of_xlate	= qcom_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= qcom_iommu_domain_test_dev,
 		.attach_dev	= qcom_iommu_attach_dev,
 		.map_pages	= qcom_iommu_map,
 		.unmap_pages	= qcom_iommu_unmap,
-- 
2.43.0

[PATCH v1 08/20] iommu/riscv: Implement riscv_iommu_test_paging_domain

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/riscv/iommu.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
index d9429097a2b51..6613ece2c9f41 100644
--- a/drivers/iommu/riscv/iommu.c
+++ b/drivers/iommu/riscv/iommu.c
@@ -1320,6 +1320,18 @@ static bool riscv_iommu_pt_supported(struct riscv_iommu_device *iommu, int pgd_m
 	return false;
 }
 
+static int riscv_iommu_test_paging_domain(struct iommu_domain *iommu_domain,
+					  struct device *dev, ioasid_t pasid,
+					  struct iommu_domain *old)
+{
+	struct riscv_iommu_domain *domain = iommu_domain_to_riscv(iommu_domain);
+	struct riscv_iommu_device *iommu = dev_to_iommu(dev);
+
+	if (!riscv_iommu_pt_supported(iommu, domain->pgd_mode))
+		return -ENODEV;
+	return 0;
+}
+
 static int riscv_iommu_attach_paging_domain(struct iommu_domain *iommu_domain,
 					    struct device *dev,
 					    struct iommu_domain *old)
@@ -1329,9 +1341,6 @@ static int riscv_iommu_attach_paging_domain(struct iommu_domain *iommu_domain,
 	struct riscv_iommu_info *info = dev_iommu_priv_get(dev);
 	u64 fsc, ta;
 
-	if (!riscv_iommu_pt_supported(iommu, domain->pgd_mode))
-		return -ENODEV;
-
 	fsc = FIELD_PREP(RISCV_IOMMU_PC_FSC_MODE, domain->pgd_mode) |
 	      FIELD_PREP(RISCV_IOMMU_PC_FSC_PPN, virt_to_pfn(domain->pgd_root));
 	ta = FIELD_PREP(RISCV_IOMMU_PC_TA_PSCID, domain->pscid) |
@@ -1348,6 +1357,7 @@ static int riscv_iommu_attach_paging_domain(struct iommu_domain *iommu_domain,
 }
 
 static const struct iommu_domain_ops riscv_iommu_paging_domain_ops = {
+	.test_dev = riscv_iommu_test_paging_domain,
 	.attach_dev = riscv_iommu_attach_paging_domain,
 	.free = riscv_iommu_free_paging_domain,
 	.map_pages = riscv_iommu_map_pages,
-- 
2.43.0

[PATCH v1 09/20] iommu/mkt_iommu: Implement mtk_iommu_test_device

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/mtk_iommu.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 9747ef1644138..0cfcd0d08ae64 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -704,6 +704,20 @@ static void mtk_iommu_domain_free(struct iommu_domain *domain)
 	kfree(to_mtk_domain(domain));
 }
 
+static int mtk_iommu_test_device(struct iommu_domain *domain,
+				 struct device *dev, ioasid_t pasid,
+				 struct iommu_domain *old)
+{
+	struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
+	int region_id;
+
+	region_id = mtk_iommu_get_iova_region_id(dev, data->plat_data);
+	if (region_id < 0)
+		return region_id;
+
+	return 0;
+}
+
 static int mtk_iommu_attach_device(struct iommu_domain *domain,
 				   struct device *dev, struct iommu_domain *old)
 {
@@ -716,8 +730,6 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
 	int ret, region_id;
 
 	region_id = mtk_iommu_get_iova_region_id(dev, data->plat_data);
-	if (region_id < 0)
-		return region_id;
 
 	bankid = mtk_iommu_get_bank_id(dev, data->plat_data);
 	mutex_lock(&dom->mutex);
@@ -1019,6 +1031,7 @@ static const struct iommu_ops mtk_iommu_ops = {
 	.get_resv_regions = mtk_iommu_get_resv_regions,
 	.owner		= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= mtk_iommu_test_device,
 		.attach_dev	= mtk_iommu_attach_device,
 		.map_pages	= mtk_iommu_map,
 		.unmap_pages	= mtk_iommu_unmap,
-- 
2.43.0

[PATCH v1 10/20] iommu/apple-dart: Implement test_dev callbacks to domain ops

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callbacks to the
new test_dev callback functions. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Note the apple_dart_finalize_domain() has another caller than attach_dev
so it has to duplicate the pgsize sanity too.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/apple-dart.c | 50 +++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index b5848770ef482..fb63dcb7462a7 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -593,9 +593,6 @@ static int apple_dart_finalize_domain(struct apple_dart_domain *dart_domain,
 	int ret = 0;
 	int i, j;
 
-	if (dart->pgsize > PAGE_SIZE)
-		return -EINVAL;
-
 	mutex_lock(&dart_domain->init_lock);
 
 	if (dart_domain->finalized)
@@ -643,11 +640,6 @@ apple_dart_mod_streams(struct apple_dart_atomic_stream_map *domain_maps,
 {
 	int i, j;
 
-	for (i = 0; i < MAX_DARTS_PER_DEVICE; ++i) {
-		if (domain_maps[i].dart != master_maps[i].dart)
-			return -EINVAL;
-	}
-
 	for (i = 0; i < MAX_DARTS_PER_DEVICE; ++i) {
 		if (!domain_maps[i].dart)
 			break;
@@ -671,6 +663,29 @@ static int apple_dart_domain_add_streams(struct apple_dart_domain *domain,
 				      true);
 }
 
+static int apple_dart_test_dev_paging(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
+{
+	struct apple_dart_domain *dart_domain = to_dart_domain(domain);
+	struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
+	struct apple_dart *dart = cfg->stream_maps[0].dart;
+
+	if (dart->pgsize > PAGE_SIZE)
+		return -EINVAL;
+	if (dart_domain->finalized) {
+		int i;
+
+		for (i = 0; i < MAX_DARTS_PER_DEVICE; ++i) {
+			if (dart_domain->stream_maps[i].dart !=
+			    cfg->stream_maps[i].dart)
+				return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int apple_dart_attach_dev_paging(struct iommu_domain *domain,
 					struct device *dev,
 					struct iommu_domain *old)
@@ -693,6 +708,17 @@ static int apple_dart_attach_dev_paging(struct iommu_domain *domain,
 	return 0;
 }
 
+static int apple_dart_test_dev_identity(struct iommu_domain *domain,
+					struct device *dev, ioasid_t pasid,
+					struct iommu_domain *old)
+{
+	struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
+
+	if (!cfg->supports_bypass)
+		return -EINVAL;
+	return 0;
+}
+
 static int apple_dart_attach_dev_identity(struct iommu_domain *domain,
 					  struct device *dev,
 					  struct iommu_domain *old)
@@ -701,15 +727,13 @@ static int apple_dart_attach_dev_identity(struct iommu_domain *domain,
 	struct apple_dart_stream_map *stream_map;
 	int i;
 
-	if (!cfg->supports_bypass)
-		return -EINVAL;
-
 	for_each_stream_map(i, cfg, stream_map)
 		apple_dart_hw_enable_bypass(stream_map);
 	return 0;
 }
 
 static const struct iommu_domain_ops apple_dart_identity_ops = {
+	.test_dev = apple_dart_test_dev_identity,
 	.attach_dev = apple_dart_attach_dev_identity,
 };
 
@@ -776,8 +800,11 @@ static struct iommu_domain *apple_dart_domain_alloc_paging(struct device *dev)
 
 	if (dev) {
 		struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);
+		struct apple_dart *dart = cfg->stream_maps[0].dart;
 		int ret;
 
+		if (dart->pgsize > PAGE_SIZE)
+			return ERR_PTR(-EINVAL);
 		ret = apple_dart_finalize_domain(dart_domain, cfg);
 		if (ret) {
 			kfree(dart_domain);
@@ -1010,6 +1037,7 @@ static const struct iommu_ops apple_dart_iommu_ops = {
 	.get_resv_regions = apple_dart_get_resv_regions,
 	.owner = THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= apple_dart_test_dev_paging,
 		.attach_dev	= apple_dart_attach_dev_paging,
 		.map_pages	= apple_dart_map_pages,
 		.unmap_pages	= apple_dart_unmap_pages,
-- 
2.43.0

[PATCH v1 11/20] iommu/ipmmu-vmsa: Implement ipmmu_domain_test_device

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/ipmmu-vmsa.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 6667ecc331f01..fdbb26ec6c632 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -589,6 +589,30 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 	kfree(domain);
 }
 
+static int ipmmu_domain_test_device(struct iommu_domain *io_domain,
+				    struct device *dev, ioasid_t pasid,
+				    struct iommu_domain *old)
+{
+	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
+	struct ipmmu_vmsa_device *mmu = to_ipmmu(dev);
+
+	if (!mmu) {
+		dev_dbg(dev, "Cannot attach to IPMMU\n");
+		return -ENXIO;
+	}
+
+	scoped_guard(mutex, &domain->mutex) {
+		/*
+		 * Something is wrong, we can't attach two devices using different
+		 * IOMMUs to the same domain.
+		 */
+		if (domain->mmu && domain->mmu != mmu)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev, struct iommu_domain *old)
 {
@@ -598,11 +622,6 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 	unsigned int i;
 	int ret = 0;
 
-	if (!mmu) {
-		dev_err(dev, "Cannot attach to IPMMU\n");
-		return -ENXIO;
-	}
-
 	mutex_lock(&domain->mutex);
 
 	if (!domain->mmu) {
@@ -616,13 +635,7 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			dev_info(dev, "Using IPMMU context %u\n",
 				 domain->context_id);
 		}
-	} else if (domain->mmu != mmu) {
-		/*
-		 * Something is wrong, we can't attach two devices using
-		 * different IOMMUs to the same domain.
-		 */
-		ret = -EINVAL;
-	} else
+	} else /* domain->mmu == mmu */
 		dev_info(dev, "Reusing IPMMU context %u\n", domain->context_id);
 
 	mutex_unlock(&domain->mutex);
@@ -885,6 +898,7 @@ static const struct iommu_ops ipmmu_ops = {
 			? generic_device_group : generic_single_device_group,
 	.of_xlate = ipmmu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= ipmmu_domain_test_device,
 		.attach_dev	= ipmmu_attach_device,
 		.map_pages	= ipmmu_map,
 		.unmap_pages	= ipmmu_unmap,
-- 
2.43.0

[PATCH v1 12/20] iommu/sun50i-iommu: Implement sun50i_iommu_domain_test_device

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/sun50i-iommu.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index d3b190be18b5a..d7517cfb260d5 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -797,16 +797,21 @@ static struct iommu_domain sun50i_iommu_identity_domain = {
 	.ops = &sun50i_iommu_identity_ops,
 };
 
+static int sun50i_iommu_domain_test_device(struct iommu_domain *domain,
+					   struct device *dev, ioasid_t pasid,
+					   struct iommu_domain *old)
+{
+	if (!sun50i_iommu_from_dev(dev))
+		return -ENODEV;
+	return 0;
+}
+
 static int sun50i_iommu_attach_device(struct iommu_domain *domain,
 				      struct device *dev,
 				      struct iommu_domain *old)
 {
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
-	struct sun50i_iommu *iommu;
-
-	iommu = sun50i_iommu_from_dev(dev);
-	if (!iommu)
-		return -ENODEV;
+	struct sun50i_iommu *iommu = sun50i_iommu_from_dev(dev);
 
 	dev_dbg(dev, "Attaching to IOMMU domain\n");
 
@@ -851,6 +856,7 @@ static const struct iommu_ops sun50i_iommu_ops = {
 	.of_xlate	= sun50i_iommu_of_xlate,
 	.probe_device	= sun50i_iommu_probe_device,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= sun50i_iommu_domain_test_device,
 		.attach_dev	= sun50i_iommu_attach_device,
 		.flush_iotlb_all = sun50i_iommu_flush_iotlb_all,
 		.iotlb_sync_map = sun50i_iommu_iotlb_sync_map,
-- 
2.43.0

[PATCH v1 13/20] iommu/rockchip-iommu: Implement rk_iommu_identity_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/rockchip-iommu.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 85f3667e797c3..89cb65f76fe52 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -959,20 +959,25 @@ static int rk_iommu_enable(struct rk_iommu *iommu)
 	return ret;
 }
 
+static int rk_iommu_identity_test_dev(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
+{
+	/* Allow 'virtual devices' (eg drm) to detach from domain */
+	if (!rk_iommu_from_dev(dev))
+		return -ENODEV;
+	return 0;
+}
+
 static int rk_iommu_identity_attach(struct iommu_domain *identity_domain,
 				    struct device *dev,
 				    struct iommu_domain *old)
 {
-	struct rk_iommu *iommu;
+	struct rk_iommu *iommu = rk_iommu_from_dev(dev);
 	struct rk_iommu_domain *rk_domain;
 	unsigned long flags;
 	int ret;
 
-	/* Allow 'virtual devices' (eg drm) to detach from domain */
-	iommu = rk_iommu_from_dev(dev);
-	if (!iommu)
-		return -ENODEV;
-
 	rk_domain = to_rk_domain(iommu->domain);
 
 	dev_dbg(dev, "Detaching from iommu domain\n");
@@ -997,6 +1002,7 @@ static int rk_iommu_identity_attach(struct iommu_domain *identity_domain,
 }
 
 static struct iommu_domain_ops rk_identity_ops = {
+	.test_dev = rk_iommu_identity_test_dev,
 	.attach_dev = rk_iommu_identity_attach,
 };
 
-- 
2.43.0

[PATCH v1 14/20] iommu/msm_iommu: Implement msm_iommu_domain_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Following the test_dev guideline, replace -EEXIST with -EBUSY.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/msm_iommu.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 819add75a6652..b7b21c97d1bd7 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -391,6 +391,31 @@ static struct iommu_device *msm_iommu_probe_device(struct device *dev)
 	return &iommu->iommu;
 }
 
+static int msm_iommu_domain_test_dev(struct iommu_domain *domain,
+				     struct device *dev, ioasid_t pasid,
+				     struct iommu_domain *old)
+{
+	struct msm_iommu_dev *iommu;
+
+	guard(spinlock_irqsave)(&msm_iommu_lock);
+
+	list_for_each_entry(iommu, &qcom_iommu_devices, dev_node) {
+		struct msm_iommu_ctx_dev *master = list_first_entry(
+			&iommu->ctx_list, struct msm_iommu_ctx_dev, list);
+
+		if (master->of_node != dev->of_node)
+			continue;
+		list_for_each_entry(master, &iommu->ctx_list, list) {
+			if (master->num) {
+				dev_dbg(dev, "domain already attached");
+				return -EBUSY;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static int msm_iommu_attach_dev(struct iommu_domain *domain, struct device *dev,
 				struct iommu_domain *old)
 {
@@ -414,11 +439,6 @@ static int msm_iommu_attach_dev(struct iommu_domain *domain, struct device *dev,
 				goto fail;
 
 			list_for_each_entry(master, &iommu->ctx_list, list) {
-				if (master->num) {
-					dev_err(dev, "domain already attached");
-					ret = -EEXIST;
-					goto fail;
-				}
 				master->num =
 					msm_iommu_alloc_ctx(iommu->context_map,
 							    0, iommu->ncb);
@@ -695,6 +715,7 @@ static struct iommu_ops msm_iommu_ops = {
 	.device_group = generic_device_group,
 	.of_xlate = qcom_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= msm_iommu_domain_test_dev,
 		.attach_dev	= msm_iommu_attach_dev,
 		.map_pages	= msm_iommu_map,
 		.unmap_pages	= msm_iommu_unmap,
-- 
2.43.0

[PATCH v1 15/20] iommu/fsl_pamu_domain: Implement fsl_pamu_domain_test_device

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/fsl_pamu_domain.c | 50 +++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 9664ef9840d2c..fbdaa74936394 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -237,6 +237,43 @@ static int update_domain_stash(struct fsl_dma_domain *dma_domain, u32 val)
 	return ret;
 }
 
+static int fsl_pamu_domain_test_device(struct iommu_domain *domain,
+				       struct device *dev, ioasid_t pasid,
+				       struct iommu_domain *old)
+{
+	struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
+	const u32 *liodn;
+	int len, i;
+
+	/* Use LIODN of the PCI controller while attaching a PCI device. */
+	if (dev_is_pci(dev)) {
+		/*
+		 * make dev point to pci controller device so we can get the
+		 * LIODN programmed by u-boot.
+		 */
+		dev = pci_bus_to_host(to_pci_dev(dev)->bus)->parent;
+	}
+
+	liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
+	if (!liodn) {
+		pr_debug("missing fsl,liodn property at %pOF\n", dev->of_node);
+		return -ENODEV;
+	}
+
+	guard(spin_lock_irqsave)(&dma_domain->domain_lock);
+
+	for (i = 0; i < len / sizeof(u32); i++) {
+		/* Ensure that LIODN value is valid */
+		if (liodn[i] < PAACE_NUMBER_ENTRIES)
+			continue;
+		pr_debug("Invalid liodn %d, attach device failed for %pOF\n",
+			 liodn[i], dev->of_node);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int fsl_pamu_attach_device(struct iommu_domain *domain,
 				  struct device *dev, struct iommu_domain *old)
 {
@@ -263,21 +300,9 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain,
 	}
 
 	liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
-	if (!liodn) {
-		pr_debug("missing fsl,liodn property at %pOF\n", dev->of_node);
-		return -ENODEV;
-	}
 
 	spin_lock_irqsave(&dma_domain->domain_lock, flags);
 	for (i = 0; i < len / sizeof(u32); i++) {
-		/* Ensure that LIODN value is valid */
-		if (liodn[i] >= PAACE_NUMBER_ENTRIES) {
-			pr_debug("Invalid liodn %d, attach device failed for %pOF\n",
-				 liodn[i], dev->of_node);
-			ret = -ENODEV;
-			break;
-		}
-
 		attach_device(dma_domain, liodn[i], dev);
 		ret = pamu_set_liodn(dma_domain, dev, liodn[i]);
 		if (ret)
@@ -434,6 +459,7 @@ static const struct iommu_ops fsl_pamu_ops = {
 	.probe_device	= fsl_pamu_probe_device,
 	.device_group   = fsl_pamu_device_group,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= fsl_pamu_domain_test_device,
 		.attach_dev	= fsl_pamu_attach_device,
 		.iova_to_phys	= fsl_pamu_iova_to_phys,
 		.free		= fsl_pamu_domain_free,
-- 
2.43.0

Re: [PATCH v1 15/20] iommu/fsl_pamu_domain: Implement fsl_pamu_domain_test_device

[Nicolin Chen]

On Sun, Oct 12, 2025 at 05:05:12PM -0700, Nicolin Chen wrote:
> +static int fsl_pamu_domain_test_device(struct iommu_domain *domain,
> +				       struct device *dev, ioasid_t pasid,
> +				       struct iommu_domain *old)
> +{
...
> +	liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
> +	if (!liodn) {
> +		pr_debug("missing fsl,liodn property at %pOF\n", dev->of_node);
> +		return -ENODEV;
> +	}
> +
> +	guard(spin_lock_irqsave)(&dma_domain->domain_lock);

Sorry. This should be:
	guard(spinlock_irqsave)(&dma_domain->domain_lock);

I have fixed this locally, and did the full build tests on all
impacted arches.

I will send a v2 next week while collecting review comments here.

Thanks
Nicolin

[PATCH v1 16/20] iommu/omap-iommu: Implement omap_iommu_domain_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Following the test_dev guideline, replace dev_err with dev_dbg.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/omap-iommu.c | 41 ++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 9f0057ccea573..26a7803e4144f 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1392,9 +1392,6 @@ static int omap_iommu_attach_init(struct device *dev,
 	int i;
 
 	odomain->num_iommus = omap_iommu_count(dev);
-	if (!odomain->num_iommus)
-		return -ENODEV;
-
 	odomain->iommus = kcalloc(odomain->num_iommus, sizeof(*iommu),
 				  GFP_ATOMIC);
 	if (!odomain->iommus)
@@ -1431,6 +1428,31 @@ static void omap_iommu_detach_fini(struct omap_iommu_domain *odomain)
 	odomain->iommus = NULL;
 }
 
+static int omap_iommu_domain_test_dev(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
+{
+	struct omap_iommu_arch_data *arch_data = dev_iommu_priv_get(dev);
+	struct omap_iommu_domain *omap_domain = to_omap_domain(domain);
+
+	if (!arch_data || !arch_data->iommu_dev) {
+		dev_dbg(dev, "device doesn't have an associated iommu\n");
+		return -ENODEV;
+	}
+
+	scoped_guard(spinlock, &omap_domain->lock) {
+		/* only a single client device can be attached to a domain */
+		if (omap_domain->dev) {
+			dev_dbg(dev, "iommu domain is already attached\n");
+			return -EINVAL;
+		}
+		if (!omap_iommu_count(dev))
+			return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int omap_iommu_attach_dev(struct iommu_domain *domain,
 				 struct device *dev, struct iommu_domain *old)
 {
@@ -1441,20 +1463,8 @@ static int omap_iommu_attach_dev(struct iommu_domain *domain,
 	int ret = 0;
 	int i;
 
-	if (!arch_data || !arch_data->iommu_dev) {
-		dev_err(dev, "device doesn't have an associated iommu\n");
-		return -ENODEV;
-	}
-
 	spin_lock(&omap_domain->lock);
 
-	/* only a single client device can be attached to a domain */
-	if (omap_domain->dev) {
-		dev_err(dev, "iommu domain is already attached\n");
-		ret = -EINVAL;
-		goto out;
-	}
-
 	ret = omap_iommu_attach_init(dev, omap_domain);
 	if (ret) {
 		dev_err(dev, "failed to allocate required iommu data %d\n",
@@ -1724,6 +1734,7 @@ static const struct iommu_ops omap_iommu_ops = {
 	.device_group	= generic_single_device_group,
 	.of_xlate	= omap_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= omap_iommu_domain_test_dev,
 		.attach_dev	= omap_iommu_attach_dev,
 		.map_pages	= omap_iommu_map,
 		.unmap_pages	= omap_iommu_unmap,
-- 
2.43.0

[PATCH v1 17/20] iommu/s390-iommu: Implement s390_iommu_domain_test_device

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/s390-iommu.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 366e47978ac07..3c6141a4a1faf 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -694,6 +694,20 @@ static int blocking_domain_attach_device(struct iommu_domain *domain,
 	return 0;
 }
 
+static int s390_iommu_domain_test_device(struct iommu_domain *domain,
+					 struct device *dev, ioasid_t pasid,
+					 struct iommu_domain *old)
+{
+	struct zpci_dev *zdev = to_zpci_dev(dev);
+
+	if (!zdev)
+		return -ENODEV;
+	if (WARN_ON(domain->geometry.aperture_start > zdev->end_dma ||
+		    domain->geometry.aperture_end < zdev->start_dma))
+		return -EINVAL;
+	return 0;
+}
+
 static int s390_iommu_attach_device(struct iommu_domain *domain,
 				    struct device *dev,
 				    struct iommu_domain *old)
@@ -704,13 +718,6 @@ static int s390_iommu_attach_device(struct iommu_domain *domain,
 	u8 status;
 	int cc;
 
-	if (!zdev)
-		return -ENODEV;
-
-	if (WARN_ON(domain->geometry.aperture_start > zdev->end_dma ||
-		domain->geometry.aperture_end < zdev->start_dma))
-		return -EINVAL;
-
 	blocking_domain_attach_device(&blocking_domain, dev);
 
 	/* If we fail now DMA remains blocked via blocking domain */
@@ -1177,6 +1184,7 @@ static struct iommu_domain blocking_domain = {
 	.device_group = generic_device_group, \
 	.get_resv_regions = s390_iommu_get_resv_regions, \
 	.default_domain_ops = &(const struct iommu_domain_ops) { \
+		.test_dev	= s390_iommu_domain_test_device, \
 		.attach_dev	= s390_iommu_attach_device, \
 		.map_pages	= s390_iommu_map_pages, \
 		.unmap_pages	= s390_iommu_unmap_pages, \
-- 
2.43.0

Re: [PATCH v1 17/20] iommu/s390-iommu: Implement s390_iommu_domain_test_device

[Niklas Schnelle]

On Sun, 2025-10-12 at 17:05 -0700, Nicolin Chen wrote:
> Move sanity and compatibility tests from the attach_dev callback to the
> new test_dev callback function. The IOMMU core makes sure an attach_dev
> call must be invoked after a successful test_dev call.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/s390-iommu.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index 366e47978ac07..3c6141a4a1faf 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -694,6 +694,20 @@ static int blocking_domain_attach_device(struct iommu_domain *domain,
>  	return 0;
>  }
>  
> +static int s390_iommu_domain_test_device(struct iommu_domain *domain,
> +					 struct device *dev, ioasid_t pasid,
> +					 struct iommu_domain *old)
> +{
> +	struct zpci_dev *zdev = to_zpci_dev(dev);
> +
> +	if (!zdev)
> +		return -ENODEV;
> +	if (WARN_ON(domain->geometry.aperture_start > zdev->end_dma ||
> +		    domain->geometry.aperture_end < zdev->start_dma))
> +		return -EINVAL;
> +	return 0;
> +}
> +
>  static int s390_iommu_attach_device(struct iommu_domain *domain,
>  				    struct device *dev,
>  				    struct iommu_domain *old)
> @@ -704,13 +718,6 @@ static int s390_iommu_attach_device(struct iommu_domain *domain,
>  	u8 status;
>  	int cc;
>  
> -	if (!zdev)
> -		return -ENODEV;
> -
> -	if (WARN_ON(domain->geometry.aperture_start > zdev->end_dma ||
> -		domain->geometry.aperture_end < zdev->start_dma))
> -		return -EINVAL;
> -
>  	blocking_domain_attach_device(&blocking_domain, dev);
>  
>  	/* If we fail now DMA remains blocked via blocking domain */
> @@ -1177,6 +1184,7 @@ static struct iommu_domain blocking_domain = {
>  	.device_group = generic_device_group, \
>  	.get_resv_regions = s390_iommu_get_resv_regions, \
>  	.default_domain_ops = &(const struct iommu_domain_ops) { \
> +		.test_dev	= s390_iommu_domain_test_device, \
>  		.attach_dev	= s390_iommu_attach_device, \
>  		.map_pages	= s390_iommu_map_pages, \
>  		.unmap_pages	= s390_iommu_unmap_pages, \

Looks good to me, thanks!

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

[PATCH v1 18/20] iommufd/selftest: Implement mock_domain_nop_test

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommufd/selftest.c | 45 +++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 5661d2da2b679..f9c871a280b03 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -58,6 +58,7 @@ enum {
 	MOCK_PFN_HUGE_IOVA = _MOCK_PFN_START << 2,
 };
 
+static struct iopf_queue *mock_iommu_iopf_queue;
 static int mock_dev_enable_iopf(struct device *dev, struct iommu_domain *domain);
 static void mock_dev_disable_iopf(struct device *dev, struct iommu_domain *domain);
 
@@ -215,6 +216,37 @@ static inline struct selftest_obj *to_selftest_obj(struct iommufd_object *obj)
 	return container_of(obj, struct selftest_obj, obj);
 }
 
+static int mock_domain_nop_test(struct iommu_domain *domain, struct device *dev,
+				ioasid_t pasid, struct iommu_domain *old)
+{
+	struct mock_dev *mdev = to_mock_dev(dev);
+
+	if (domain->dirty_ops && (mdev->flags & MOCK_FLAGS_DEVICE_NO_DIRTY))
+		return -EINVAL;
+
+	iommu_group_mutex_assert(dev);
+	if (domain->type == IOMMU_DOMAIN_NESTED) {
+		struct mock_viommu *viommu =
+			to_mock_nested(domain)->mock_viommu;
+		unsigned long vdev_id = 0;
+		int rc;
+
+		if (viommu) {
+			rc = iommufd_viommu_get_vdev_id(&viommu->core, dev,
+							&vdev_id);
+			if (rc)
+				return rc;
+		}
+	}
+
+	if (domain->iopf_handler) {
+		if (!mock_iommu_iopf_queue)
+			return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int mock_domain_nop_attach(struct iommu_domain *domain,
 				  struct device *dev, struct iommu_domain *old)
 {
@@ -223,16 +255,13 @@ static int mock_domain_nop_attach(struct iommu_domain *domain,
 	unsigned long vdev_id = 0;
 	int rc;
 
-	if (domain->dirty_ops && (mdev->flags & MOCK_FLAGS_DEVICE_NO_DIRTY))
-		return -EINVAL;
-
 	iommu_group_mutex_assert(dev);
 	if (domain->type == IOMMU_DOMAIN_NESTED) {
 		new_viommu = to_mock_nested(domain)->mock_viommu;
 		if (new_viommu) {
 			rc = iommufd_viommu_get_vdev_id(&new_viommu->core, dev,
 							&vdev_id);
-			if (rc)
+			if (WARN_ON(rc))
 				return rc;
 		}
 	}
@@ -296,6 +325,7 @@ static int mock_domain_set_dev_pasid_nop(struct iommu_domain *domain,
 }
 
 static const struct iommu_domain_ops mock_blocking_ops = {
+	.test_dev = mock_domain_nop_test,
 	.attach_dev = mock_domain_nop_attach,
 	.set_dev_pasid = mock_domain_set_dev_pasid_nop
 };
@@ -630,8 +660,6 @@ static bool mock_domain_capable(struct device *dev, enum iommu_cap cap)
 	return false;
 }
 
-static struct iopf_queue *mock_iommu_iopf_queue;
-
 static struct mock_iommu_device {
 	struct iommu_device iommu_dev;
 	struct completion complete;
@@ -658,9 +686,6 @@ static int mock_dev_enable_iopf(struct device *dev, struct iommu_domain *domain)
 	if (!domain || !domain->iopf_handler)
 		return 0;
 
-	if (!mock_iommu_iopf_queue)
-		return -ENODEV;
-
 	if (mdev->iopf_refcount) {
 		mdev->iopf_refcount++;
 		return 0;
@@ -958,6 +983,7 @@ static const struct iommu_ops mock_ops = {
 	.default_domain_ops =
 		&(struct iommu_domain_ops){
 			.free = mock_domain_free,
+			.test_dev = mock_domain_nop_test,
 			.attach_dev = mock_domain_nop_attach,
 			.map_pages = mock_domain_map_pages,
 			.unmap_pages = mock_domain_unmap_pages,
@@ -1021,6 +1047,7 @@ mock_domain_cache_invalidate_user(struct iommu_domain *domain,
 
 static struct iommu_domain_ops domain_nested_ops = {
 	.free = mock_domain_free_nested,
+	.test_dev = mock_domain_nop_test,
 	.attach_dev = mock_domain_nop_attach,
 	.cache_invalidate_user = mock_domain_cache_invalidate_user,
 	.set_dev_pasid = mock_domain_set_dev_pasid_nop,
-- 
2.43.0

[PATCH v1 19/20] iommu/virtio-iommu: Implement viommu_domain_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/virtio-iommu.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index d314fa5cd8476..766dded8fc8a6 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -730,6 +730,18 @@ static struct iommu_domain *viommu_domain_alloc_identity(struct device *dev)
 	return domain;
 }
 
+static int viommu_domain_test_dev(struct iommu_domain *domain,
+				  struct device *dev, ioasid_t pasid,
+				  struct iommu_domain *old)
+{
+	struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
+	struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+	if (vdomain->viommu != vdev->viommu)
+		return -EINVAL;
+	return 0;
+}
+
 static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev,
 			     struct iommu_domain *old)
 {
@@ -738,9 +750,6 @@ static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev,
 	struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
 	struct viommu_domain *vdomain = to_viommu_domain(domain);
 
-	if (vdomain->viommu != vdev->viommu)
-		return -EINVAL;
-
 	/*
 	 * In the virtio-iommu device, when attaching the endpoint to a new
 	 * domain, it is detached from the old one and, if as a result the
@@ -1099,6 +1108,7 @@ static const struct iommu_ops viommu_ops = {
 	.of_xlate		= viommu_of_xlate,
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev		= viommu_domain_test_dev,
 		.attach_dev		= viommu_attach_dev,
 		.map_pages		= viommu_map_pages,
 		.unmap_pages		= viommu_unmap_pages,
-- 
2.43.0

[PATCH v1 20/20] iommu/tegra-smmu: Implement tegra_smmu_domain_test_dev

[Nicolin Chen]

Move sanity and compatibility tests from the attach_dev callback to the
new test_dev callback function. The IOMMU core makes sure an attach_dev
call must be invoked after a successful test_dev call.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 336e0a3ff41fb..cfbe67678a426 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -489,6 +489,18 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu,
 	mutex_unlock(&smmu->lock);
 }
 
+static int tegra_smmu_domain_test_dev(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+	if (!fwspec || !fwspec->num_ids)
+		return -ENOENT;
+
+	return 0;
+}
+
 static int tegra_smmu_attach_dev(struct iommu_domain *domain,
 				 struct device *dev, struct iommu_domain *old)
 {
@@ -498,9 +510,6 @@ static int tegra_smmu_attach_dev(struct iommu_domain *domain,
 	unsigned int index;
 	int err;
 
-	if (!fwspec)
-		return -ENOENT;
-
 	for (index = 0; index < fwspec->num_ids; index++) {
 		err = tegra_smmu_as_prepare(smmu, as);
 		if (err)
@@ -509,9 +518,6 @@ static int tegra_smmu_attach_dev(struct iommu_domain *domain,
 		tegra_smmu_enable(smmu, fwspec->ids[index], as->id);
 	}
 
-	if (index == 0)
-		return -ENODEV;
-
 	return 0;
 
 disable:
@@ -532,9 +538,6 @@ static int tegra_smmu_identity_attach(struct iommu_domain *identity_domain,
 	struct tegra_smmu *smmu;
 	unsigned int index;
 
-	if (!fwspec)
-		return -ENODEV;
-
 	if (old == identity_domain || !old)
 		return 0;
 
@@ -548,6 +551,7 @@ static int tegra_smmu_identity_attach(struct iommu_domain *identity_domain,
 }
 
 static struct iommu_domain_ops tegra_smmu_identity_ops = {
+	.test_dev = tegra_smmu_domain_test_dev,
 	.attach_dev = tegra_smmu_identity_attach,
 };
 
@@ -1005,6 +1009,7 @@ static const struct iommu_ops tegra_smmu_ops = {
 	.device_group = tegra_smmu_device_group,
 	.of_xlate = tegra_smmu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.test_dev	= tegra_smmu_domain_test_dev,
 		.attach_dev	= tegra_smmu_attach_dev,
 		.map_pages	= tegra_smmu_map,
 		.unmap_pages	= tegra_smmu_unmap,
-- 
2.43.0

Re: [PATCH v1 00/20] iommu: Introduce and roll out test_dev domain op

[Jason Gunthorpe]

On Sun, Oct 12, 2025 at 05:04:57PM -0700, Nicolin Chen wrote:
> Add a new test_dev domain op for drivers to run a compatibility test prior
> to the actual attachment at the driver level. Any incompatible attachment
> will be rejected early, allowing the iommu core to postpone any concurrent
> attachment during a device reset state.

I had to go back and find the original email from kevin to understand
this..

 
This is a preparatory series for new iommu_dev_reset APIs:
https://lore.kernel.org/all/cover.1756682135.git.nicolinc@nvidia.com/

That series parks the domain attachment at BLOCKED during a device
reset. To keep the uAPI this also required that any change in domain
during this reset sequence is just recorded and kept in the background
until the reset is finished.

This creates a weird hole where userspace could propose to attach to a
domain that is incompatible with the device during FLR, have that
attach queued and then ultimately have the domain attach fail when the
FLR concludes.

This can be mitigated by splitting out the compatability test from the
attach and having the core code check the compatability before
accepting a queued domain attach.

It was felt that the subtle uAPI change warrants this rework.

Jason