Re: [PATCH v4] iommu/arm-smmu: Use pm_runtime in fault handlers

[Will Deacon]

On Thu, Aug 06, 2026 at 03:52:41PM +0530, Prakash Gupta wrote:
> Commit d4a44f0750bb ("iommu/arm-smmu: Invoke pm_runtime across the driver")
> enabled pm_runtime for the arm-smmu device. On systems where the SMMU
> sits in a power domain, all register accesses must be done while the
> device is runtime active to avoid unclocked register reads and
> potential NoC errors.
> 
> So far, this has not been an issue for most SMMU clients because
> stall-on-fault is enabled by default. While a translation fault is
> being handled, the SMMU stalls further translations for that context
> bank, so the fault handler would not race with a powered-down SMMU.
> 
> Adreno SMMU now disables stall-on-fault in the presence of fault
> storms to avoid saturating SMMU resources and hanging the GMU. With
> stall-on-fault disabled, the SMMU can generate faults while its power
> domain may no longer be enabled, which makes unclocked accesses to
> fault-status registers in the SMMU fault handlers possible.
> 
> Guard the context and global fault handlers with
> arm_smmu_rpm_get_if_active() and arm_smmu_rpm_put() so that all SMMU
> fault register accesses are done with the SMMU powered. If the SMMU is
> not runtime active, the fault can be safely ignored as
> arm_smmu_device_reset() clears fault registers on resume.
> 
> Additionally, disable fault reporting in arm_smmu_runtime_suspend()
> before powering down. pm_runtime_get_if_active() returns 0 during
> RPM_SUSPENDING, so without this, level-triggered fault interrupts would
> cause an interrupt storm while the device is being suspended.
> arm_smmu_device_reset() re-enables fault reporting on resume.

Separate patch?

> Also call synchronize_irq() for each context IRQ after masking fault
> reporting but before clk_bulk_disable(). This closes a race where a
> fault handler that passed arm_smmu_rpm_get_if_active() before the IRQ
> was masked could still be executing MMIO accesses after the clocks are
> cut.

Separate patch?

> Fixes: b13044092c1e ("drm/msm: Temporarily disable stall-on-fault after a page fault")
> Co-developed-by: Pratyush Brahma <pratyush.brahma@xxxxxxxxxxxxxxxx>
> Signed-off-by: Pratyush Brahma <pratyush.brahma@xxxxxxxxxxxxxxxx>
> Signed-off-by: Prakash Gupta <prakash.gupta@xxxxxxxxxxxxxxxx>
> Reviewed-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> ---
> Changes in v4:
> - Add synchronize_irq() loop in arm_smmu_runtime_suspend() after masking
>   fault reporting but before clk_bulk_disable(), closing a race where an
>   in-flight fault handler could access MMIO registers after clocks are cut
> - Link to v3: https://patch.msgid.link/20260630-smmu-rpm-v3-1-f69874a580fa@xxxxxxxxxxxxxxxx
> 
> Changes in v3:
> - Add arm_smmu_rpm_get_if_active() wrapper that returns 1 when pm_runtime
>   is disabled, ensuring fault handlers work on non-pm_runtime systems
> - Disable fault reporting in arm_smmu_runtime_suspend() before powering
>   down to prevent interrupt storms during RPM_SUSPENDING state
> - Use pm_runtime_put_autosuspend() in arm_smmu_rpm_put() instead of
>   private __pm_runtime_put_autosuspend()
> - Link to v2: https://patch.msgid.link/20260313-smmu-rpm-v2-1-8c2236b402b0@xxxxxxxxxxxxxxxx
> 
> Changes in v2:
> - Switched from arm_smmu_rpm_get()/arm_smmu_rpm_put() wrappers to
>   pm_runtime_get_if_active()/pm_runtime_put_autosuspend() APIs
> - Added support for smmu->impl->global_fault callback in global fault handler
> - Remove threaded irq context fault restriction to allow modifying stall
>   mode for adreno smmu
> - Link to v1: https://patch.msgid.link/20260127-smmu-rpm-v1-1-2ef2f4c85305@xxxxxxxxxxxxxxxx
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c | 101 +++++++++++++++++++++++++---------
>  1 file changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 0bd21d206eb3..2b692a5293a2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -79,11 +79,16 @@ static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
>  
>  static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
>  {
> -	if (pm_runtime_enabled(smmu->dev)) {
> -		pm_runtime_mark_last_busy(smmu->dev);
> -		__pm_runtime_put_autosuspend(smmu->dev);
> +	if (pm_runtime_enabled(smmu->dev))
> +		pm_runtime_put_autosuspend(smmu->dev);
> +}
>  
> -	}
> +static inline int arm_smmu_rpm_get_if_active(struct arm_smmu_device *smmu)
> +{
> +	if (!pm_runtime_enabled(smmu->dev))
> +		return 1;
> +
> +	return pm_runtime_get_if_active(smmu->dev);
>  }

Can this race with pm_runtime_disable(), perhaps via
arm_smmu_device_shutdown()? If so, then we could return -EINVAL, which
doesn't look like it's handled correctly by the caller here:

>  static void arm_smmu_rpm_use_autosuspend(struct arm_smmu_device *smmu)
> @@ -462,10 +467,20 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
>  	int idx = smmu_domain->cfg.cbndx;
>  	int ret;
>  
> +	if (!arm_smmu_rpm_get_if_active(smmu))
> +		return IRQ_NONE;

^^^

Is this assuming that the irq is edge-triggered? Is that guaranteed?

Will