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

[Ashish Mhetre <amhetre@nvidia.com>]

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(+)
>
> 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 d6e75d1646d6..44875c526183 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -26,6 +26,7 @@
>   #include <linux/pci.h>
>   #include <linux/pci-ats.h>
>   #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>   #include <linux/string_choices.h>
>   #include <kunit/visibility.h>
>   #include <uapi/linux/iommufd.h>
> @@ -108,6 +109,33 @@ static const char * const event_class_str[] = {
>   
>   static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
>   
> +/* Runtime PM helpers */
> +__maybe_unused static int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> +{
> +	int ret;
> +
> +	if (pm_runtime_enabled(smmu->dev)) {
> +		ret = pm_runtime_resume_and_get(smmu->dev);
> +		if (ret < 0) {
> +			dev_err(smmu->dev, "Failed to resume device: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +__maybe_unused static void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> +{
> +	int ret;
> +
> +	if (pm_runtime_enabled(smmu->dev)) {
> +		ret = pm_runtime_put_autosuspend(smmu->dev);
> +		if (ret < 0)
> +			dev_err(smmu->dev, "Failed to suspend device: %d\n", ret);
> +	}
> +}
> +
>   static void parse_driver_options(struct arm_smmu_device *smmu)
>   {
>   	int i = 0;
> @@ -5005,6 +5033,86 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev)
>   	arm_smmu_device_disable(smmu);
>   }
>   
> +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
> +{
> +	struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> +	int timeout = ARM_SMMU_SUSPEND_TIMEOUT_US;
> +	u32 enables;
> +	int ret;
> +
> +	/* Try to suspend the device, wait for in-flight submissions */
> +	do {
> +		if (atomic_cmpxchg(&smmu->nr_cmdq_users, 1, 0) == 1)
> +			break;
> +
> +		udelay(1);
> +	} while (--timeout);
> +
> +	if (!timeout) {
> +		dev_warn(smmu->dev, "SMMU in use, aborting suspend\n");
> +		return -EAGAIN;
> +	}
> +
> +	/* Abort all transactions before disable to avoid spurious bypass */
> +	arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0);
> +
> +	/* Disable the SMMU via CR0.EN and all queues except CMDQ */
> +	enables = CR0_CMDQEN;
> +	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0, ARM_SMMU_CR0ACK);
> +	if (ret) {
> +		dev_err(smmu->dev, "Timed-out while disabling smmu\n");
> +		atomic_set(&smmu->nr_cmdq_users, 1);
> +		return ret;
> +	}
> +
> +	/*
> +	 * At this point the SMMU is completely disabled and won't access
> +	 * any translation/config structures, even speculative accesses
> +	 * aren't performed as per the IHI0070 spec (section 6.3.9.6).
> +	 */
> +
> +	/* Wait for the CMDQs to be drained to flush any pending commands */
> +	ret = arm_smmu_drain_queues(smmu);
> +	if (ret)
> +		dev_err(smmu->dev, "Draining queues timed-out..forcing suspend\n");
> +
> +	/* Disable everything */
> +	arm_smmu_device_disable(smmu);
> +	dev_dbg(dev, "Suspended smmu\n");
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
> +{
> +	int ret;
> +	struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "Resuming device\n");
> +
> +	/* Re-configure MSIs */
> +	arm_smmu_resume_msis(smmu);
> +
> +	/*
> +	 * The reset will re-initialize all the base addresses, queues,
> +	 * prod and cons maintained within struct arm_smmu_device as well as
> +	 * re-enable the interrupts.
> +	 */
> +	ret = arm_smmu_device_reset(smmu);
> +
> +	if (ret)
> +		dev_err(dev, "Failed to reset during resume operation: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops arm_smmu_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(arm_smmu_runtime_suspend,
> +			   arm_smmu_runtime_resume, NULL)
> +};
> +

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

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.

>   static const struct of_device_id arm_smmu_of_match[] = {
>   	{ .compatible = "arm,smmu-v3", },
>   	{ },
> @@ -5021,6 +5129,7 @@ static struct platform_driver arm_smmu_driver = {
>   	.driver	= {
>   		.name			= "arm-smmu-v3",
>   		.of_match_table		= arm_smmu_of_match,
> +		.pm                     = &arm_smmu_pm_ops,
>   		.suppress_bind_attrs	= true,
>   	},
>   	.probe	= arm_smmu_device_probe,
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 924580610ce0..eefa5853033c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -503,11 +503,14 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
>   
>   /* High-level queue structures */
>   #define ARM_SMMU_POLL_TIMEOUT_US	1000000 /* 1s! */
> +#define ARM_SMMU_SUSPEND_TIMEOUT_US	100	/* 100us! */
>   #define ARM_SMMU_POLL_SPIN_COUNT	10
>   
>   #define MSI_IOVA_BASE			0x8000000
>   #define MSI_IOVA_LENGTH			0x100000
>   
> +#define RPM_AUTOSUSPEND_DELAY_MS	15
> +
>   enum pri_resp {
>   	PRI_RESP_DENY = 0,
>   	PRI_RESP_FAIL = 1,