public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
@ 2026-01-21 14:12 Danilo Krummrich
  2026-01-21 14:46 ` Greg KH
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-01-21 14:12 UTC (permalink / raw)
  To: gregkh, rafael, broonie, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov
  Cc: driver-core, linux-kernel, linux-arm-kernel, iommu, linux-arm-msm,
	Danilo Krummrich

Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
qcom_smmu_impl_init") intended to also probe the TBU driver when
CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
platform_driver_register() call into qcom_smmu_impl_init() which is
called from arm_smmu_device_probe().

However, it neither makes sense to register drivers from probe()
callbacks of other drivers, nor does the driver core allow registering
drivers with a device lock already being held.

The latter was revealed by commit dc23806a7c47 ("driver core: enforce
device_lock for driver_match_device()") leading to a deadlock condition
described in [1].

Additionally, it was noted by Robin that the current approach is
potentially racy with async probe [2].

Hence, fix this by registering the qcom_smmu_tbu_driver from
module_init(). Unfortunately, due to the vendoring of the driver, this
requires an indirection through arm-smmu-impl.c.

Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 14 +++++++++++++
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 +++++++++----
 drivers/iommu/arm/arm-smmu/arm-smmu.c      | 24 +++++++++++++++++++++-
 drivers/iommu/arm/arm-smmu/arm-smmu.h      |  5 +++++
 4 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index db9b9a8e139c..4565a58bb213 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -228,3 +228,17 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 
 	return smmu;
 }
+
+int __init arm_smmu_impl_module_init(void)
+{
+	if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
+		return qcom_smmu_module_init();
+
+	return 0;
+}
+
+void __exit arm_smmu_impl_module_exit(void)
+{
+	if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
+		qcom_smmu_module_exit();
+}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 573085349df3..22906d2c9a2d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -774,10 +774,6 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
 {
 	const struct device_node *np = smmu->dev->of_node;
 	const struct of_device_id *match;
-	static u8 tbu_registered;
-
-	if (!tbu_registered++)
-		platform_driver_register(&qcom_smmu_tbu_driver);
 
 #ifdef CONFIG_ACPI
 	if (np == NULL) {
@@ -802,3 +798,13 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
 
 	return smmu;
 }
+
+int __init qcom_smmu_module_init(void)
+{
+	return platform_driver_register(&qcom_smmu_tbu_driver);
+}
+
+void __exit qcom_smmu_module_exit(void)
+{
+	platform_driver_unregister(&qcom_smmu_tbu_driver);
+}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 5e690cf85ec9..1e218fbea35a 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -2365,7 +2365,29 @@ static struct platform_driver arm_smmu_driver = {
 	.remove = arm_smmu_device_remove,
 	.shutdown = arm_smmu_device_shutdown,
 };
-module_platform_driver(arm_smmu_driver);
+
+static int __init arm_smmu_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&arm_smmu_driver);
+	if (ret)
+		return ret;
+
+	ret = arm_smmu_impl_module_init();
+	if (ret)
+		platform_driver_unregister(&arm_smmu_driver);
+
+	return ret;
+}
+module_init(arm_smmu_init);
+
+static void __exit arm_smmu_exit(void)
+{
+	arm_smmu_impl_module_exit();
+	platform_driver_unregister(&arm_smmu_driver);
+}
+module_exit(arm_smmu_exit);
 
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will@kernel.org>");
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 2dbf3243b5ad..26d2e33cd328 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -540,6 +540,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
 struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu);
 struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
 
+int __init arm_smmu_impl_module_init(void);
+void __exit arm_smmu_impl_module_exit(void);
+int __init qcom_smmu_module_init(void);
+void __exit qcom_smmu_module_exit(void);
+
 void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx);
 int arm_mmu500_reset(struct arm_smmu_device *smmu);
 

base-commit: dc23806a7c47ec5f1293aba407fb69519f976ee0
-- 
2.52.0


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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
@ 2026-01-21 14:46 ` Greg KH
  2026-01-21 15:11 ` Robin Murphy
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2026-01-21 14:46 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: rafael, broonie, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On Wed, Jan 21, 2026 at 03:12:01PM +0100, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
> 
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
  2026-01-21 14:46 ` Greg KH
@ 2026-01-21 15:11 ` Robin Murphy
  2026-01-21 15:54 ` Bjorn Andersson
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2026-01-21 15:11 UTC (permalink / raw)
  To: Danilo Krummrich, gregkh, rafael, broonie, will, joro,
	robin.clark, hanguidong02, quic_c_gdjako, dmitry.baryshkov
  Cc: driver-core, linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On 21/01/2026 2:12 pm, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].

On third look, the current lack of unregister, when it can end up in a 
module which (theoretically) supports unloading, is also sketchy :)

> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
 >
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Looks OK to me, thanks Danilo!

Acked-by: Robin Murphy <robin.murphy@arm.com>

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
  2026-01-21 14:46 ` Greg KH
  2026-01-21 15:11 ` Robin Murphy
@ 2026-01-21 15:54 ` Bjorn Andersson
  2026-01-21 16:02 ` Konrad Dybcio
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2026-01-21 15:54 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, broonie, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On Wed, Jan 21, 2026 at 03:12:01PM +0100, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
> 
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Thank you Danilo.

Tested-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 14 +++++++++++++
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 +++++++++----
>  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 24 +++++++++++++++++++++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  5 +++++
>  4 files changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index db9b9a8e139c..4565a58bb213 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -228,3 +228,17 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>  
>  	return smmu;
>  }
> +
> +int __init arm_smmu_impl_module_init(void)
> +{
> +	if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
> +		return qcom_smmu_module_init();
> +
> +	return 0;
> +}
> +
> +void __exit arm_smmu_impl_module_exit(void)
> +{
> +	if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
> +		qcom_smmu_module_exit();
> +}
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 573085349df3..22906d2c9a2d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -774,10 +774,6 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
>  {
>  	const struct device_node *np = smmu->dev->of_node;
>  	const struct of_device_id *match;
> -	static u8 tbu_registered;
> -
> -	if (!tbu_registered++)
> -		platform_driver_register(&qcom_smmu_tbu_driver);
>  
>  #ifdef CONFIG_ACPI
>  	if (np == NULL) {
> @@ -802,3 +798,13 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
>  
>  	return smmu;
>  }
> +
> +int __init qcom_smmu_module_init(void)
> +{
> +	return platform_driver_register(&qcom_smmu_tbu_driver);
> +}
> +
> +void __exit qcom_smmu_module_exit(void)
> +{
> +	platform_driver_unregister(&qcom_smmu_tbu_driver);
> +}
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 5e690cf85ec9..1e218fbea35a 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -2365,7 +2365,29 @@ static struct platform_driver arm_smmu_driver = {
>  	.remove = arm_smmu_device_remove,
>  	.shutdown = arm_smmu_device_shutdown,
>  };
> -module_platform_driver(arm_smmu_driver);
> +
> +static int __init arm_smmu_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&arm_smmu_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = arm_smmu_impl_module_init();
> +	if (ret)
> +		platform_driver_unregister(&arm_smmu_driver);
> +
> +	return ret;
> +}
> +module_init(arm_smmu_init);
> +
> +static void __exit arm_smmu_exit(void)
> +{
> +	arm_smmu_impl_module_exit();
> +	platform_driver_unregister(&arm_smmu_driver);
> +}
> +module_exit(arm_smmu_exit);
>  
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>  MODULE_AUTHOR("Will Deacon <will@kernel.org>");
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 2dbf3243b5ad..26d2e33cd328 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -540,6 +540,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
>  struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu);
>  struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
>  
> +int __init arm_smmu_impl_module_init(void);
> +void __exit arm_smmu_impl_module_exit(void);
> +int __init qcom_smmu_module_init(void);
> +void __exit qcom_smmu_module_exit(void);
> +
>  void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx);
>  int arm_mmu500_reset(struct arm_smmu_device *smmu);
>  
> 
> base-commit: dc23806a7c47ec5f1293aba407fb69519f976ee0
> -- 
> 2.52.0
> 
> 

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-01-21 15:54 ` Bjorn Andersson
@ 2026-01-21 16:02 ` Konrad Dybcio
  2026-01-21 16:05 ` Mark Brown
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2026-01-21 16:02 UTC (permalink / raw)
  To: Danilo Krummrich, gregkh, rafael, broonie, will, robin.murphy,
	joro, robin.clark, hanguidong02, quic_c_gdjako, dmitry.baryshkov
  Cc: driver-core, linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On 1/21/26 3:12 PM, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
> 
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Acked-by: Konrad Dybcio <konradybcio@kernel.org>

Konrad

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
                   ` (3 preceding siblings ...)
  2026-01-21 16:02 ` Konrad Dybcio
@ 2026-01-21 16:05 ` Mark Brown
  2026-01-22  3:03 ` Wang Jiayue
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2026-01-21 16:05 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Wed, Jan 21, 2026 at 03:12:01PM +0100, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().

This fixes the boot lockup on Juno for me:

Tested-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
                   ` (4 preceding siblings ...)
  2026-01-21 16:05 ` Mark Brown
@ 2026-01-22  3:03 ` Wang Jiayue
  2026-01-22  8:18 ` Jörg Rödel
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Wang Jiayue @ 2026-01-22  3:03 UTC (permalink / raw)
  To: dakr
  Cc: broonie, dmitry.baryshkov, driver-core, gregkh, hanguidong02,
	iommu, joro, linux-arm-kernel, linux-arm-msm, linux-kernel,
	quic_c_gdjako, rafael, robin.clark, robin.murphy, will,
	Wang Jiayue

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6226 bytes --]

On Wed, Jan 21, 2026 at 10:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
>
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
>
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
>
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
>
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
>
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")

Glad the backtrace helped identify the root cause.

I applied this patch to the qemu setup where I successfully reproduced the
boot hang earlier, and the deadlock is resolved.

Tested-by: Wang Jiayue <akaieurus@gmail.com>
Reviewed-by: Wang Jiayue <akaieurus@gmail.com>

> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 14 +++++++++++++
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 +++++++++----
>  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 24 +++++++++++++++++++++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  5 +++++
>  4 files changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index db9b9a8e139c..4565a58bb213 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -228,3 +228,17 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>
>         return smmu;
>  }
> +
> +int __init arm_smmu_impl_module_init(void)
> +{
> +       if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
> +               return qcom_smmu_module_init();
> +
> +       return 0;
> +}
> +
> +void __exit arm_smmu_impl_module_exit(void)
> +{
> +       if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
> +               qcom_smmu_module_exit();
> +}
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 573085349df3..22906d2c9a2d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -774,10 +774,6 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
>  {
>         const struct device_node *np = smmu->dev->of_node;
>         const struct of_device_id *match;
> -       static u8 tbu_registered;
> -
> -       if (!tbu_registered++)
> -               platform_driver_register(&qcom_smmu_tbu_driver);
>
>  #ifdef CONFIG_ACPI
>         if (np == NULL) {
> @@ -802,3 +798,13 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
>
>         return smmu;
>  }
> +
> +int __init qcom_smmu_module_init(void)
> +{
> +       return platform_driver_register(&qcom_smmu_tbu_driver);
> +}
> +
> +void __exit qcom_smmu_module_exit(void)
> +{
> +       platform_driver_unregister(&qcom_smmu_tbu_driver);
> +}
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 5e690cf85ec9..1e218fbea35a 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -2365,7 +2365,29 @@ static struct platform_driver arm_smmu_driver = {
>         .remove = arm_smmu_device_remove,
>         .shutdown = arm_smmu_device_shutdown,
>  };
> -module_platform_driver(arm_smmu_driver);
> +
> +static int __init arm_smmu_init(void)
> +{
> +       int ret;
> +
> +       ret = platform_driver_register(&arm_smmu_driver);
> +       if (ret)
> +               return ret;
> +
> +       ret = arm_smmu_impl_module_init();
> +       if (ret)
> +               platform_driver_unregister(&arm_smmu_driver);
> +
> +       return ret;
> +}
> +module_init(arm_smmu_init);
> +
> +static void __exit arm_smmu_exit(void)
> +{
> +       arm_smmu_impl_module_exit();
> +       platform_driver_unregister(&arm_smmu_driver);
> +}
> +module_exit(arm_smmu_exit);
>
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
>  MODULE_AUTHOR("Will Deacon <will@kernel.org>");
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 2dbf3243b5ad..26d2e33cd328 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -540,6 +540,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
>  struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu);
>  struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
>
> +int __init arm_smmu_impl_module_init(void);
> +void __exit arm_smmu_impl_module_exit(void);
> +int __init qcom_smmu_module_init(void);
> +void __exit qcom_smmu_module_exit(void);
> +
>  void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx);
>  int arm_mmu500_reset(struct arm_smmu_device *smmu);
>
>
> base-commit: dc23806a7c47ec5f1293aba407fb69519f976ee0
> --
> 2.52.0
>

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
                   ` (5 preceding siblings ...)
  2026-01-22  3:03 ` Wang Jiayue
@ 2026-01-22  8:18 ` Jörg Rödel
  2026-01-22  8:37   ` Danilo Krummrich
  2026-01-22  9:21 ` Ioana Ciornei
  2026-01-22 14:41 ` Danilo Krummrich
  8 siblings, 1 reply; 13+ messages in thread
From: Jörg Rödel @ 2026-01-22  8:18 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, broonie, will, robin.murphy, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On Wed, Jan 21, 2026 at 03:12:01PM +0100, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
> 
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Acked-by: Joerg Roedel <joerg.roedel@amd.com>

This patch should go via the branch that contains

	dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()").
	
Otherwise we add a Fixes tag pointing to a commit which comes later in the git
history.


-Joerg


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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-22  8:18 ` Jörg Rödel
@ 2026-01-22  8:37   ` Danilo Krummrich
  0 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-01-22  8:37 UTC (permalink / raw)
  To: Jörg Rödel
  Cc: gregkh, rafael, broonie, will, robin.murphy, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On Thu Jan 22, 2026 at 9:18 AM CET, Jörg Rödel wrote:
> Acked-by: Joerg Roedel <joerg.roedel@amd.com>
>
> This patch should go via the branch that contains
>
> 	dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()").

Yes, I will take it through the driver-core tree.

Thanks,
Danilo

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
                   ` (6 preceding siblings ...)
  2026-01-22  8:18 ` Jörg Rödel
@ 2026-01-22  9:21 ` Ioana Ciornei
  2026-01-22 11:52   ` Danilo Krummrich
  2026-01-22 14:41 ` Danilo Krummrich
  8 siblings, 1 reply; 13+ messages in thread
From: Ioana Ciornei @ 2026-01-22  9:21 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, broonie, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On Wed, Jan 21, 2026 at 03:12:01PM +0100, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
> 
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

The boot lockup was visible also on the NXP LX2160ARDB. This patch fixes
it.

Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com> #LX2160ARDB


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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-22  9:21 ` Ioana Ciornei
@ 2026-01-22 11:52   ` Danilo Krummrich
  2026-01-22 12:12     ` Robin Murphy
  0 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-01-22 11:52 UTC (permalink / raw)
  To: Ioana Ciornei
  Cc: gregkh, rafael, broonie, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov, driver-core,
	linux-kernel, linux-arm-kernel, iommu, linux-arm-msm

On Thu Jan 22, 2026 at 10:21 AM CET, Ioana Ciornei wrote:
> The boot lockup was visible also on the NXP LX2160ARDB. This patch fixes
> it.

Interesting, this means that qcom_smmu_impl_init() must be called on this
machine, is this expected?

> Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com> #LX2160ARDB

Thanks for testing!

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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-22 11:52   ` Danilo Krummrich
@ 2026-01-22 12:12     ` Robin Murphy
  0 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2026-01-22 12:12 UTC (permalink / raw)
  To: Danilo Krummrich, Ioana Ciornei
  Cc: gregkh, rafael, broonie, will, joro, robin.clark, hanguidong02,
	quic_c_gdjako, dmitry.baryshkov, driver-core, linux-kernel,
	linux-arm-kernel, iommu, linux-arm-msm

On 2026-01-22 11:52 am, Danilo Krummrich wrote:
> On Thu Jan 22, 2026 at 10:21 AM CET, Ioana Ciornei wrote:
>> The boot lockup was visible also on the NXP LX2160ARDB. This patch fixes
>> it.
> 
> Interesting, this means that qcom_smmu_impl_init() must be called on this
> machine, is this expected?

Yup, same as on the Juno boards and probably others - an Arm SMMU is 
present, which means if ARM_SMMU_QCOM is enabled then 
arm_smmu_impl_init() is going to offer it to ask "hey, is this one of 
yours that you might want to do something with based on your extensive 
and complicated match list that we don't want to maintain in the common 
impl code?", wherein qcom_smmu_impl_init() *should* return without doing 
anything at all as it's not a match...

Cheers,
Robin.

> 
>> Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com> #LX2160ARDB
> 
> Thanks for testing!


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

* Re: [PATCH] iommu/arm-smmu-qcom: do not register driver in probe()
  2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
                   ` (7 preceding siblings ...)
  2026-01-22  9:21 ` Ioana Ciornei
@ 2026-01-22 14:41 ` Danilo Krummrich
  8 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-01-22 14:41 UTC (permalink / raw)
  To: gregkh, rafael, broonie, will, robin.murphy, joro, robin.clark,
	hanguidong02, quic_c_gdjako, dmitry.baryshkov
  Cc: driver-core, linux-kernel, linux-arm-kernel, iommu, linux-arm-msm,
	Danilo Krummrich

On Wed Jan 21, 2026 at 3:12 PM CET, Danilo Krummrich wrote:
> Commit 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in
> qcom_smmu_impl_init") intended to also probe the TBU driver when
> CONFIG_ARM_SMMU_QCOM_DEBUG is disabled, but also moved the corresponding
> platform_driver_register() call into qcom_smmu_impl_init() which is
> called from arm_smmu_device_probe().
>
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
>
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
>
> Additionally, it was noted by Robin that the current approach is
> potentially racy with async probe [2].
>
> Hence, fix this by registering the qcom_smmu_tbu_driver from
> module_init(). Unfortunately, due to the vendoring of the driver, this
> requires an indirection through arm-smmu-impl.c.
>
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/lkml/7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/0c0d3707-9ea5-44f9-88a1-a65c62e3df8d@arm.com/ [2]
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 0b4eeee2876f ("iommu/arm-smmu-qcom: Register the TBU driver in qcom_smmu_impl_init")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Applied to driver-core-linus, thanks!

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

end of thread, other threads:[~2026-01-22 14:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-21 14:12 [PATCH] iommu/arm-smmu-qcom: do not register driver in probe() Danilo Krummrich
2026-01-21 14:46 ` Greg KH
2026-01-21 15:11 ` Robin Murphy
2026-01-21 15:54 ` Bjorn Andersson
2026-01-21 16:02 ` Konrad Dybcio
2026-01-21 16:05 ` Mark Brown
2026-01-22  3:03 ` Wang Jiayue
2026-01-22  8:18 ` Jörg Rödel
2026-01-22  8:37   ` Danilo Krummrich
2026-01-22  9:21 ` Ioana Ciornei
2026-01-22 11:52   ` Danilo Krummrich
2026-01-22 12:12     ` Robin Murphy
2026-01-22 14:41 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox