Re: [PATCH v4 6/8] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops

[Pranjal Shrivastava <praan@google.com>]

On Wed, Dec 24, 2025 at 01:09:32PM +0530, Ashish Mhetre wrote:
> 
> 
> On 11/18/2025 12:44 AM, Pranjal Shrivastava wrote:
> > Implement pm_runtime and system sleep ops for arm-smmu-v3.
> > 
> > The suspend callback configures the SMMU to abort new transactions,
> > disables the main translation unit and then drains the command queue
> > to ensure completion of any in-flight commands.
> > 
> > The resume callback restores the MSI configuration and performs a full
> > device reset via `arm_smmu_device_reset` to bring the SMMU back to an
> > operational state. The MSIs are cached during the msi_write and are
> > restored during the resume operation by using the helper.
> > 
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 109 ++++++++++++++++++++
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |   3 +
> >   2 files changed, 112 insertions(+)
> > 
> 
> Hi Pranjal, Nic,
> 
> I tested these patches on Tegra264 with CMDQV enabled and 16 vcmdqs
> assigned to guest. I found an issue in resume path of CMDQV.
> In tegra241_cmdqv_hw_reset() function, the PROD and CONS pointers for
> VCMDQs are being set to 0 on resume. Because of this, commands from
> VCMDQs we not being consumed post resume and I got CMD_SYNC timeouts.
> We need to restore the prod and cons indices for VCMDQs on resume.
> This is similar to what is done for SMMU's physical CMDQ.

Hi Ashish,

Thanks for testing this with Tegra, I'll include this patch for CMDQV.

> Also, current implementation uses pm_runtime_force_suspend/resume()
> which requires runtime PM to be enabled, but runtime PM is only enabled
> when dev->pm_domain exists. I had to use dedicated system sleep callbacks
> that directly call arm_smmu_runtime_suspend/resume(), bypassing the runtime
> PM dependency to get suspend/resume working on Tegra264.
> 

Thanks for pointing this out, I'll revert to the approach I had in v1:
https://lore.kernel.org/all/20250319004254.2547950-4-praan@google.com/

> I got it working by making following changes on top of your series and
> validated that after resume all commands are being consumed and SMMU clients
> are working fine:
> 
> 
> From 8ead50a7e2fbdfa30fbe2c927c47a440b447c864 Mon Sep 17 00:00:00 2001
> From: Ashish Mhetre<amhetre@nvidia.com>
> Date: Tue, 23 Dec 2025 08:42:31 +0000
> Subject: [PATCH] iommu/tegra241-cmdqv: Restore PROD and CONS after resume
> X-NVConfidentiality: public
> 
> PROD and CONS indices for vcmdqs are getting set to 0 after resume.
> Because of this the vcmdq is not consuming commands after resume.
> Fix this by restoring PROD and CONS indices after resume from
> saved pointers.
> 
> Signed-off-by: Ashish Mhetre<amhetre@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> index 378104cd395e..00ec684fe3a4 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> @@ -483,6 +483,8 @@ static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)
> 
>         /* Configure and enable VCMDQ */
>         writeq_relaxed(vcmdq->cmdq.q.q_base, REG_VCMDQ_PAGE1(vcmdq, BASE));
> +       writel_relaxed(vcmdq->cmdq.q.llq.prod, REG_VCMDQ_PAGE0(vcmdq, PROD));
> +       writel_relaxed(vcmdq->cmdq.q.llq.cons, REG_VCMDQ_PAGE0(vcmdq, CONS));
> 
>         ret = vcmdq_write_config(vcmdq, VCMDQ_EN);
>         if (ret) {
> --
> 2.25.1
> 
> From 51a11812bb40d341e11233129a01ce248957bd25 Mon Sep 17 00:00:00 2001
> From: Ashish Mhetre <amhetre@nvidia.com>
> Date: Tue, 23 Dec 2025 09:30:04 +0000
> Subject: [PATCH] iommu/arm-smmu-v3: Fix system suspend/resume when runtime
> PM
>  is not enabled
> X-NVConfidentiality: public The current implementation uses
> pm_runtime_force_suspend() and
> pm_runtime_force_resume() as system sleep callbacks. These generic PM
> helpers are designed to bridge runtime PM with system sleep by forcing
> the device through the runtime PM path.
> However, these helpers only work correctly when runtime PM is enabled
> for the device. In arm_smmu_device_probe(), runtime PM is conditionally
> enabled:
>     if (dev->pm_domain) {
>         pm_runtime_set_active(dev);
>         pm_runtime_enable(dev);
>     }
> On platforms where the SMMU does not have an associated power domain,
> pm_runtime_enable() is never called. As a result, when the system
> enters suspend, pm_runtime_force_suspend() effectively becomes a
> no-op, it does not invoke arm_smmu_runtime_suspend(), leaving the
> SMMU hardware in an undefined state during system sleep. Fix this by
> introducing dedicated system sleep callbacks
> (arm_smmu_pm_suspend/resume) that directly invoke the existing runtime
> suspend/resume functions. This ensures the SMMU is properly suspended
> and resumed during system sleep, regardless of whether runtime PM is
> enabled.
> The pm_runtime_suspended() check ensures we don't double-suspend the
> device if it was already suspended via runtime PM during normal
> operation. Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 20 +++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index b1c9c733ed8d..4707adc63d86 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -5180,9 +5180,35 @@ static int __maybe_unused
> arm_smmu_runtime_resume(struct device *dev)
>         return ret;
>  }
> +static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
> +{
> +       if (pm_runtime_suspended(dev))
> +               return 0;
> +
> +       return arm_smmu_runtime_resume(dev);
> +}
> +
> +static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
> +{
> +       if (pm_runtime_suspended(dev))
> +               return 0;
> +
> +       return arm_smmu_runtime_suspend(dev);
> +}
> +
>  static const struct dev_pm_ops arm_smmu_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> -                               pm_runtime_force_resume)
> +       SET_SYSTEM_SLEEP_PM_OPS(arm_smmu_pm_suspend,
> +                               arm_smmu_pm_resume)
>         SET_RUNTIME_PM_OPS(arm_smmu_runtime_suspend,
>                            arm_smmu_runtime_resume, NULL)
>  };
> --
> 2.25.1
> 
> 
> 
> 
> Please see if these changes make sense and squash to the series if they do.
> 

I'll include the CMDQV patch from you as is in my next version and 
revert to use the SLEEP_PM_OPS as in my v1. It'd be nice to have the
`Tested-by` from you for the next version too!

Thanks!
Praan