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

[Robin Murphy <robin.murphy@arm.com>]

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>
>