[PATCH] iommu: arm-smmu-qcom: Skip fault-info reads when suspended

[PATCH] iommu: arm-smmu-qcom: Skip fault-info reads when suspended

[Bibek Kumar Patro]

qcom_adreno_smmu_get_fault_info() accesses SMMU registers without
holding a runtime PM reference. A fault is raised while the SMMU is
active, but the GPU may drop its power vote before the threaded fault
handler reaches the callback, allowing the SMMU to runtime suspend.

Accessing the SMMU registers after suspend has started is unsafe and
may cause subsequent register accesses during runtime resume to fail
with a NoC error and an asynchronous SError.

Use pm_runtime_get_if_active() to keep the SMMU active while collecting
the fault information, and skip the register reads if suspend has
already started.

Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
---
qcom_adreno_smmu_get_fault_info() accesses SMMU registers without
holding a runtime PM reference. A fault is raised while the SMMU is
active, but the GPU may drop its power vote before the threaded fault
handler reaches the callback, allowing the SMMU to runtime suspend.

The failure sequence:

  GPU context fault
          |
          v
  qcom_adreno_smmu_get_fault_info()
          |
          | GPU drops its power vote
          | SMMU starts runtime suspend
          v
  SMMU register access without an active RPM reference
          |
          v
  later Adreno PRR private-interface operation
          |
          v
  pm_runtime_resume_and_get()
          |
          v
  arm_smmu_runtime_resume()
          |
          v
  arm_smmu_device_reset()
          |
          v
  SMMU register access
          |
          v
  NoC error / asynchronous SError

Use pm_runtime_get_if_active() to keep the SMMU active while collecting
the fault information. If suspend has already started, skip the register
reads instead of accessing the SMMU while it is being powered down.
---
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 795e8ec7e8fa..b1787a4ba0d3 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -129,6 +129,18 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
 	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
+	/*
+	 * A fault can only fire while the SMMU is active (GPU is issuing
+	 * transactions). Use get_if_active so that if by a rare race the
+	 * device has already suspended, we skip the register reads rather
+	 * than triggering a cold resume which would call device_reset and
+	 * destroy the fault state we are trying to capture.
+	 */
+	if (pm_runtime_get_if_active(smmu->dev) <= 0) {
+		dev_warn(smmu->dev, "get_fault_info: device not active, skipping\n");
+		return;
+	}
+
 	info->fsr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSR);
 	info->fsynr0 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR0);
 	info->fsynr1 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR1);
@@ -136,6 +148,8 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
 	info->cbfrsynra = arm_smmu_gr1_read(smmu, ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
 	info->ttbr0 = arm_smmu_cb_readq(smmu, cfg->cbndx, ARM_SMMU_CB_TTBR0);
 	info->contextidr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_CONTEXTIDR);
+
+	pm_runtime_put_autosuspend(smmu->dev);
 }
 
 static void qcom_adreno_smmu_set_stall(const void *cookie, bool enabled)

---
base-commit: f61bb3920fa8cb44b84d32e0fa9fbaab4eb00c80
change-id: 20260912-priv_call_runtime_handlers-7f3547544040

Best regards,
--  
Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>

Re: [PATCH] iommu: arm-smmu-qcom: Skip fault-info reads when suspended

[Robin Murphy]

On 11/09/2026 11:25 pm, Bibek Kumar Patro wrote:
> qcom_adreno_smmu_get_fault_info() accesses SMMU registers without
> holding a runtime PM reference. A fault is raised while the SMMU is
> active, but the GPU may drop its power vote before the threaded fault
> handler reaches the callback, allowing the SMMU to runtime suspend.
> 
> Accessing the SMMU registers after suspend has started is unsafe and
> may cause subsequent register accesses during runtime resume to fail
> with a NoC error and an asynchronous SError.

But hang on, if I'm following the code correctly, the callchain would be:

arm_smmu_context_fault
   report_iommu_fault
     msm_gpu_fault_handler
      qcom_adreno_smmu_get_fault_info

and from here we still need to return to arm_smmu_context_fault() to 
clear the FSR, so surely that still has the same problem in itself?

Thanks,
Robin.

> Use pm_runtime_get_if_active() to keep the SMMU active while collecting
> the fault information, and skip the register reads if suspend has
> already started.
> 
> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> ---
> qcom_adreno_smmu_get_fault_info() accesses SMMU registers without
> holding a runtime PM reference. A fault is raised while the SMMU is
> active, but the GPU may drop its power vote before the threaded fault
> handler reaches the callback, allowing the SMMU to runtime suspend.
> 
> The failure sequence:
> 
>    GPU context fault
>            |
>            v
>    qcom_adreno_smmu_get_fault_info()
>            |
>            | GPU drops its power vote
>            | SMMU starts runtime suspend
>            v
>    SMMU register access without an active RPM reference
>            |
>            v
>    later Adreno PRR private-interface operation
>            |
>            v
>    pm_runtime_resume_and_get()
>            |
>            v
>    arm_smmu_runtime_resume()
>            |
>            v
>    arm_smmu_device_reset()
>            |
>            v
>    SMMU register access
>            |
>            v
>    NoC error / asynchronous SError
> 
> Use pm_runtime_get_if_active() to keep the SMMU active while collecting
> the fault information. If suspend has already started, skip the register
> reads instead of accessing the SMMU while it is being powered down.
> ---
>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 795e8ec7e8fa..b1787a4ba0d3 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -129,6 +129,18 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
>   	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>   	struct arm_smmu_device *smmu = smmu_domain->smmu;
>   
> +	/*
> +	 * A fault can only fire while the SMMU is active (GPU is issuing
> +	 * transactions). Use get_if_active so that if by a rare race the
> +	 * device has already suspended, we skip the register reads rather
> +	 * than triggering a cold resume which would call device_reset and
> +	 * destroy the fault state we are trying to capture.
> +	 */
> +	if (pm_runtime_get_if_active(smmu->dev) <= 0) {
> +		dev_warn(smmu->dev, "get_fault_info: device not active, skipping\n");
> +		return;
> +	}
> +
>   	info->fsr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSR);
>   	info->fsynr0 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR0);
>   	info->fsynr1 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR1);
> @@ -136,6 +148,8 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
>   	info->cbfrsynra = arm_smmu_gr1_read(smmu, ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
>   	info->ttbr0 = arm_smmu_cb_readq(smmu, cfg->cbndx, ARM_SMMU_CB_TTBR0);
>   	info->contextidr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_CONTEXTIDR);
> +
> +	pm_runtime_put_autosuspend(smmu->dev);
>   }
>   
>   static void qcom_adreno_smmu_set_stall(const void *cookie, bool enabled)
> 
> ---
> base-commit: f61bb3920fa8cb44b84d32e0fa9fbaab4eb00c80
> change-id: 20260912-priv_call_runtime_handlers-7f3547544040
> 
> Best regards,
> --
> Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> 

Re: [PATCH] iommu: arm-smmu-qcom: Skip fault-info reads when suspended

[Bibek Kumar Patro]

On Sep 14, Robin Murphy wrote:
>On 11/09/2026 11:25 pm, Bibek Kumar Patro wrote:
>>qcom_adreno_smmu_get_fault_info() accesses SMMU registers without
>>holding a runtime PM reference. A fault is raised while the SMMU is
>>active, but the GPU may drop its power vote before the threaded fault
>>handler reaches the callback, allowing the SMMU to runtime suspend.
>>
>>Accessing the SMMU registers after suspend has started is unsafe and
>>may cause subsequent register accesses during runtime resume to fail
>>with a NoC error and an asynchronous SError.
>
>But hang on, if I'm following the code correctly, the callchain would be:
>
>arm_smmu_context_fault
>  report_iommu_fault
>    msm_gpu_fault_handler
>     qcom_adreno_smmu_get_fault_info
>
>and from here we still need to return to arm_smmu_context_fault() to 
>clear the FSR, so surely that still has the same problem in itself?
>

Ah, yes this path also should have the same problem for GPU smmu.
But I am carrying [1] in my workspace [2] for testing currently.
So haven't encountered the same yet. We might need to address [1]
also as part of this thread. wdyt?

[1]: https://lore.kernel.org/all/20260313-smmu-rpm-v2-1-8c2236b402b0@oss.qualcomm.com/
[2]: https://github.com/qualcomm-linux/kernel/blame/aff7542c319ede2037eeee316f20c2429062c08a/drivers/iommu/arm/arm-smmu/arm-smmu.c#L500

Thanks & regards,
Bibek

>Thanks,
>Robin.
>
>>Use pm_runtime_get_if_active() to keep the SMMU active while collecting
>>the fault information, and skip the register reads if suspend has
>>already started.
>>
>>Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>---
>>qcom_adreno_smmu_get_fault_info() accesses SMMU registers without
>>holding a runtime PM reference. A fault is raised while the SMMU is
>>active, but the GPU may drop its power vote before the threaded fault
>>handler reaches the callback, allowing the SMMU to runtime suspend.
>>
>>The failure sequence:
>>
>>   GPU context fault
>>           |
>>           v
>>   qcom_adreno_smmu_get_fault_info()
>>           |
>>           | GPU drops its power vote
>>           | SMMU starts runtime suspend
>>           v
>>   SMMU register access without an active RPM reference
>>           |
>>           v
>>   later Adreno PRR private-interface operation
>>           |
>>           v
>>   pm_runtime_resume_and_get()
>>           |
>>           v
>>   arm_smmu_runtime_resume()
>>           |
>>           v
>>   arm_smmu_device_reset()
>>           |
>>           v
>>   SMMU register access
>>           |
>>           v
>>   NoC error / asynchronous SError
>>
>>Use pm_runtime_get_if_active() to keep the SMMU active while collecting
>>the fault information. If suspend has already started, skip the register
>>reads instead of accessing the SMMU while it is being powered down.
>>---
>>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>>diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>index 795e8ec7e8fa..b1787a4ba0d3 100644
>>--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>@@ -129,6 +129,18 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
>>  	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
>>+	/*
>>+	 * A fault can only fire while the SMMU is active (GPU is issuing
>>+	 * transactions). Use get_if_active so that if by a rare race the
>>+	 * device has already suspended, we skip the register reads rather
>>+	 * than triggering a cold resume which would call device_reset and
>>+	 * destroy the fault state we are trying to capture.
>>+	 */
>>+	if (pm_runtime_get_if_active(smmu->dev) <= 0) {
>>+		dev_warn(smmu->dev, "get_fault_info: device not active, skipping\n");
>>+		return;
>>+	}
>>+
>>  	info->fsr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSR);
>>  	info->fsynr0 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR0);
>>  	info->fsynr1 = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_FSYNR1);
>>@@ -136,6 +148,8 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
>>  	info->cbfrsynra = arm_smmu_gr1_read(smmu, ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
>>  	info->ttbr0 = arm_smmu_cb_readq(smmu, cfg->cbndx, ARM_SMMU_CB_TTBR0);
>>  	info->contextidr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_CONTEXTIDR);
>>+
>>+	pm_runtime_put_autosuspend(smmu->dev);
>>  }
>>  static void qcom_adreno_smmu_set_stall(const void *cookie, bool enabled)
>>
>>---
>>base-commit: f61bb3920fa8cb44b84d32e0fa9fbaab4eb00c80
>>change-id: 20260912-priv_call_runtime_handlers-7f3547544040
>>
>>Best regards,
>>--
>>Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>
>