[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 at 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 at oss.qualcomm.com>
>>
>