[PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups
@ 2025-07-24 22:10 Nicolin Chen
  2025-07-24 22:10 ` [PATCH v4 1/2] iommu/arm-smmu-v3: Do not bother impl_ops if IOMMU_VIOMMU_TYPE_ARM_SMMUV3 Nicolin Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Nicolin Chen @ 2025-07-24 22:10 UTC (permalink / raw)
  To: jgg, will
  Cc: joro, robin.murphy, linux-arm-kernel, iommu, linux-kernel,
	linux-tegra, praan

Since the vsmmu and cmdqv patches were accepeted via the iommufd tree,
this is based on Jason's for-next tree.

Per request from Will following the accepted latest vcmdq series to clean
up the impl_ops:

https://lore.kernel.org/linux-iommu/aHE4Y-fbucm-j-yi@willie-the-truck/
"
 It would be nice to avoid adding data members to the ops structure, if
 at all possible. The easiest thing would probably be to add a function
 for getting the vsmmu size and then pushing the two checks against
 'vsmmu_type' down into the impl_ops callbacks so that: 

   1. If the type is IOMMU_VIOMMU_TYPE_ARM_SMMUV3, we don't bother with
      the impl_ops at all in arm_vsmmu_init() and arm_smmu_get_viommu_size()

   2. Otherwise, we pass the type into the impl_ops and they can check it

 Of course, that can be a patch on top of the series as there's no point
 respinning the whole just for this.
"

Add two clean up patches with the first one doing 1 and 2 to prioritize
IOMMU_VIOMMU_TYPE_ARM_SMMUV3 always, and the second one dropping static
vsmmu_type and vsmmu_size data members.

Changelog
v4:
 * Add Reviewed-by from Pranjal
 * Update inline comments
v3:
 https://lore.kernel.org/all/20250721200444.1740461-1-nicolinc@nvidia.com/
 * Add Acked-by from Will
 * Use logical "!=" instead of arithmetic "^"
v2:
 https://lore.kernel.org/all/20250721191236.1739951-1-nicolinc@nvidia.com/
 * Add Acked-by from Will
 * Move get_viommu_size and vsmmu_init validation to arm_smmu_impl_probe()
v1:
 https://lore.kernel.org/all/20250718234822.1734190-1-nicolinc@nvidia.com/

Nicolin Chen (2):
  iommu/arm-smmu-v3: Do not bother impl_ops if
    IOMMU_VIOMMU_TYPE_ARM_SMMUV3
  iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size

 .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     | 20 +++++++++----------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 14 +++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 +--
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 17 ++++++++++++++--
 4 files changed, 39 insertions(+), 15 deletions(-)

base-commit: ab6bc44159d8f0c4ee757e0ce041fa9033e0ead8
-- 
2.43.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 1/2] iommu/arm-smmu-v3: Do not bother impl_ops if IOMMU_VIOMMU_TYPE_ARM_SMMUV3
  2025-07-24 22:10 [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Nicolin Chen
@ 2025-07-24 22:10 ` Nicolin Chen
  2025-07-24 22:10 ` [PATCH v4 2/2] iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size Nicolin Chen
  2025-07-29 22:27 ` [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Jason Gunthorpe
  2 siblings, 0 replies; 4+ messages in thread
From: Nicolin Chen @ 2025-07-24 22:10 UTC (permalink / raw)
  To: jgg, will
  Cc: joro, robin.murphy, linux-arm-kernel, iommu, linux-kernel,
	linux-tegra, praan

When viommu type is IOMMU_VIOMMU_TYPE_ARM_SMMUV3, always return or init the
standard struct arm_vsmmu, instead of going through impl_ops that must have
its own viommu type than the standard IOMMU_VIOMMU_TYPE_ARM_SMMUV3.

Given that arm_vsmmu_init() is called after arm_smmu_get_viommu_size(), any
unsupported viommu->type must be a corruption. And it must be a driver bug
that its vsmmu_size and vsmmu_init ops aren't paired. Warn these two cases.

Suggested-by: Will Deacon <will@kernel.org>
Acked-by: Will Deacon <will@kernel.org>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     | 27 +++++++++++--------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 14 ++++++++++
 2 files changed, 30 insertions(+), 11 deletions(-)

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 d9bea8f1f636d..b963b9b3de542 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
@@ -420,14 +420,13 @@ size_t arm_smmu_get_viommu_size(struct device *dev,
 	    !(smmu->features & ARM_SMMU_FEAT_S2FWB))
 		return 0;
 
-	if (smmu->impl_ops && smmu->impl_ops->vsmmu_size &&
-	    viommu_type == smmu->impl_ops->vsmmu_type)
-		return smmu->impl_ops->vsmmu_size;
+	if (viommu_type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3)
+		return VIOMMU_STRUCT_SIZE(struct arm_vsmmu, core);
 
-	if (viommu_type != IOMMU_VIOMMU_TYPE_ARM_SMMUV3)
+	if (!smmu->impl_ops || !smmu->impl_ops->vsmmu_size ||
+	    viommu_type != smmu->impl_ops->vsmmu_type)
 		return 0;
-
-	return VIOMMU_STRUCT_SIZE(struct arm_vsmmu, core);
+	return smmu->impl_ops->vsmmu_size;
 }
 
 int arm_vsmmu_init(struct iommufd_viommu *viommu,
@@ -447,12 +446,18 @@ int arm_vsmmu_init(struct iommufd_viommu *viommu,
 	/* FIXME Move VMID allocation from the S2 domain allocation to here */
 	vsmmu->vmid = s2_parent->s2_cfg.vmid;
 
-	if (smmu->impl_ops && smmu->impl_ops->vsmmu_init &&
-	    viommu->type == smmu->impl_ops->vsmmu_type)
-		return smmu->impl_ops->vsmmu_init(vsmmu, user_data);
+	if (viommu->type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3) {
+		viommu->ops = &arm_vsmmu_ops;
+		return 0;
+	}
 
-	viommu->ops = &arm_vsmmu_ops;
-	return 0;
+	/*
+	 * Unsupported type should be rejected by arm_smmu_get_viommu_size.
+	 * Seeing one here indicates a kernel bug or some data corruption.
+	 */
+	if (WARN_ON(viommu->type != smmu->impl_ops->vsmmu_type))
+		return -EOPNOTSUPP;
+	return smmu->impl_ops->vsmmu_init(vsmmu, user_data);
 }
 
 int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, u64 *evt)
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 181d07bc1a9d4..9f4ad37058010 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4703,6 +4703,7 @@ static void arm_smmu_impl_remove(void *data)
 static struct arm_smmu_device *arm_smmu_impl_probe(struct arm_smmu_device *smmu)
 {
 	struct arm_smmu_device *new_smmu = ERR_PTR(-ENODEV);
+	const struct arm_smmu_impl_ops *ops;
 	int ret;
 
 	if (smmu->impl_dev && (smmu->options & ARM_SMMU_OPT_TEGRA241_CMDQV))
@@ -4713,11 +4714,24 @@ static struct arm_smmu_device *arm_smmu_impl_probe(struct arm_smmu_device *smmu)
 	if (IS_ERR(new_smmu))
 		return new_smmu;
 
+	ops = new_smmu->impl_ops;
+	if (ops) {
+		/* vsmmu_size and vsmmu_init ops must be paired */
+		if (WARN_ON(!ops->vsmmu_size != !ops->vsmmu_init)) {
+			ret = -EINVAL;
+			goto err_remove;
+		}
+	}
+
 	ret = devm_add_action_or_reset(new_smmu->dev, arm_smmu_impl_remove,
 				       new_smmu);
 	if (ret)
 		return ERR_PTR(ret);
 	return new_smmu;
+
+err_remove:
+	arm_smmu_impl_remove(new_smmu);
+	return ERR_PTR(ret);
 }
 
 static int arm_smmu_device_probe(struct platform_device *pdev)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v4 2/2] iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size
  2025-07-24 22:10 [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Nicolin Chen
  2025-07-24 22:10 ` [PATCH v4 1/2] iommu/arm-smmu-v3: Do not bother impl_ops if IOMMU_VIOMMU_TYPE_ARM_SMMUV3 Nicolin Chen
@ 2025-07-24 22:10 ` Nicolin Chen
  2025-07-29 22:27 ` [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Jason Gunthorpe
  2 siblings, 0 replies; 4+ messages in thread
From: Nicolin Chen @ 2025-07-24 22:10 UTC (permalink / raw)
  To: jgg, will
  Cc: joro, robin.murphy, linux-arm-kernel, iommu, linux-kernel,
	linux-tegra, praan

It's more flexible to have a get_viommu_size op. Replace static vsmmu_size
and vsmmu_type with that.

Suggested-by: Will Deacon <will@kernel.org>
Acked-by: Will Deacon <will@kernel.org>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 11 ++---------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c     |  4 ++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h     |  3 +--
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c  | 17 +++++++++++++++--
 4 files changed, 20 insertions(+), 15 deletions(-)

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 b963b9b3de542..8cd8929bbfdf8 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
@@ -423,10 +423,9 @@ size_t arm_smmu_get_viommu_size(struct device *dev,
 	if (viommu_type == IOMMU_VIOMMU_TYPE_ARM_SMMUV3)
 		return VIOMMU_STRUCT_SIZE(struct arm_vsmmu, core);
 
-	if (!smmu->impl_ops || !smmu->impl_ops->vsmmu_size ||
-	    viommu_type != smmu->impl_ops->vsmmu_type)
+	if (!smmu->impl_ops || !smmu->impl_ops->get_viommu_size)
 		return 0;
-	return smmu->impl_ops->vsmmu_size;
+	return smmu->impl_ops->get_viommu_size(viommu_type);
 }
 
 int arm_vsmmu_init(struct iommufd_viommu *viommu,
@@ -451,12 +450,6 @@ int arm_vsmmu_init(struct iommufd_viommu *viommu,
 		return 0;
 	}
 
-	/*
-	 * Unsupported type should be rejected by arm_smmu_get_viommu_size.
-	 * Seeing one here indicates a kernel bug or some data corruption.
-	 */
-	if (WARN_ON(viommu->type != smmu->impl_ops->vsmmu_type))
-		return -EOPNOTSUPP;
 	return smmu->impl_ops->vsmmu_init(vsmmu, user_data);
 }
 
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 9f4ad37058010..f56113107c8ad 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4716,8 +4716,8 @@ static struct arm_smmu_device *arm_smmu_impl_probe(struct arm_smmu_device *smmu)
 
 	ops = new_smmu->impl_ops;
 	if (ops) {
-		/* vsmmu_size and vsmmu_init ops must be paired */
-		if (WARN_ON(!ops->vsmmu_size != !ops->vsmmu_init)) {
+		/* get_viommu_size and vsmmu_init ops must be paired */
+		if (WARN_ON(!ops->get_viommu_size != !ops->vsmmu_init)) {
 			ret = -EINVAL;
 			goto err_remove;
 		}
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 3fa02c51df9f3..e332f5ba2f8a2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -728,8 +728,7 @@ struct arm_smmu_impl_ops {
 	 */
 	void *(*hw_info)(struct arm_smmu_device *smmu, u32 *length,
 			 enum iommu_hw_info_type *type);
-	const size_t vsmmu_size;
-	const enum iommu_viommu_type vsmmu_type;
+	size_t (*get_viommu_size)(enum iommu_viommu_type viommu_type);
 	int (*vsmmu_init)(struct arm_vsmmu *vsmmu,
 			  const struct iommu_user_data *user_data);
 };
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 4c86eacd36b16..be1aaaf8cd17c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -832,6 +832,13 @@ static void *tegra241_cmdqv_hw_info(struct arm_smmu_device *smmu, u32 *length,
 	return info;
 }
 
+static size_t tegra241_cmdqv_get_vintf_size(enum iommu_viommu_type viommu_type)
+{
+	if (viommu_type != IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV)
+		return 0;
+	return VIOMMU_STRUCT_SIZE(struct tegra241_vintf, vsmmu.core);
+}
+
 static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
 	/* For in-kernel use */
 	.get_secondary_cmdq = tegra241_cmdqv_get_cmdq,
@@ -839,8 +846,7 @@ static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
 	.device_remove = tegra241_cmdqv_remove,
 	/* For user-space use */
 	.hw_info = tegra241_cmdqv_hw_info,
-	.vsmmu_size = VIOMMU_STRUCT_SIZE(struct tegra241_vintf, vsmmu.core),
-	.vsmmu_type = IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
+	.get_viommu_size = tegra241_cmdqv_get_vintf_size,
 	.vsmmu_init = tegra241_cmdqv_init_vintf_user,
 };
 
@@ -1273,6 +1279,13 @@ tegra241_cmdqv_init_vintf_user(struct arm_vsmmu *vsmmu,
 	phys_addr_t page0_base;
 	int ret;
 
+	/*
+	 * Unsupported type should be rejected by tegra241_cmdqv_get_vintf_size.
+	 * Seeing one here indicates a kernel bug or some data corruption.
+	 */
+	if (WARN_ON(vsmmu->core.type != IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV))
+		return -EOPNOTSUPP;
+
 	if (!user_data)
 		return -EINVAL;
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups
  2025-07-24 22:10 [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Nicolin Chen
  2025-07-24 22:10 ` [PATCH v4 1/2] iommu/arm-smmu-v3: Do not bother impl_ops if IOMMU_VIOMMU_TYPE_ARM_SMMUV3 Nicolin Chen
  2025-07-24 22:10 ` [PATCH v4 2/2] iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size Nicolin Chen
@ 2025-07-29 22:27 ` Jason Gunthorpe
  2 siblings, 0 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2025-07-29 22:27 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: will, joro, robin.murphy, linux-arm-kernel, iommu, linux-kernel,
	linux-tegra, praan

On Thu, Jul 24, 2025 at 03:10:00PM -0700, Nicolin Chen wrote:
> Nicolin Chen (2):
>   iommu/arm-smmu-v3: Do not bother impl_ops if
>     IOMMU_VIOMMU_TYPE_ARM_SMMUV3
>   iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size
> 
>  .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     | 20 +++++++++----------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 14 +++++++++++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 +--
>  .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 17 ++++++++++++++--
>  4 files changed, 39 insertions(+), 15 deletions(-)

Applied, thanks

Jason


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-29 22:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-24 22:10 [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Nicolin Chen
2025-07-24 22:10 ` [PATCH v4 1/2] iommu/arm-smmu-v3: Do not bother impl_ops if IOMMU_VIOMMU_TYPE_ARM_SMMUV3 Nicolin Chen
2025-07-24 22:10 ` [PATCH v4 2/2] iommu/arm-smmu-v3: Replace vsmmu_size/type with get_viommu_size Nicolin Chen
2025-07-29 22:27 ` [PATCH v4 0/2] iommu/arm-smmu-v3: Two vsmmu impl_ops cleanups Jason Gunthorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.