public inbox for iommu@lists.linux-foundation.org
 help / color / mirror / Atom feed
From: Pranjal Shrivastava <praan@google.com>
To: Ashish Mhetre <amhetre@nvidia.com>
Cc: iommu@lists.linux.dev, Will Deacon <will@kernel.org>,
	Joerg Roedel <joro@8bytes.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Mostafa Saleh <smostafa@google.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Daniel Mentz <danielmentz@google.com>,
	Sairaj Kodilkar <sarunkod@amd.com>
Subject: Re: [PATCH v5 00/10] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops
Date: Wed, 11 Feb 2026 13:33:57 +0000	[thread overview]
Message-ID: <aYyFRa4ZtQGlNQ2t@google.com> (raw)
In-Reply-To: <5a69ec64-87ed-4b62-97c1-94ddd4ae1553@nvidia.com>

On Fri, Feb 06, 2026 at 09:18:06PM +0530, Ashish Mhetre wrote:
> 
> 
> On 1/26/2026 8:41 PM, Pranjal Shrivastava wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > As arm-smmu-v3 rapidly finds its way into SoCs designed for hand-held
> > devices, power management capabilities, similar to its predecessors, are
> > crucial for these applications. This series introduces power management
> > support for the arm-smmu-v3 driver.
> > 
> > Design
> > =======
> > The arm-smmu-v3 primarily operates with in-memory data structures
> > through HW registers pointing to these data structures. The proposed design
> > makes use of this fact for implementing suspend and resume ops, centered
> > around a driver-specific biased usage counter.
> > 
> > 1. Biased Usage Counter
> > To safely manage runtime PM state, this series introduces an atomic
> > `nr_cmdq_users` counter. This counter is "biased" by being initialized to
> > 1, representing an "idle but active" state.
> > 
> > The suspend callback waits for this counter to be exactly 1 and then
> > atomically transitions it to 0, signifying a "suspended" state. Any
> > other value indicates that the command queue is in use, and the suspend
> > operation is aborted.
> > 
> > Code paths that submit commands to the hardware (the "fast path") use
> > `atomic_inc_not_zero()` to acquire a reference. This operation only
> > succeeds if the counter is not zero (i.e., not suspended), effectively
> > gating hardware access. The reference is dropped using
> > `atomic_dec_return_release()` after the hardware interaction is
> > complete.
> > 
> > 2. Suspend / Resume Flow
> > The "suspend" op now polls for a short period (100us) for the usage
> > counter to become 1. Once successful, it configures the GBPA register
> > to abort new transactions and disables the SMMU, EVTQ, and PRIQ, leaving
> > only the CMDQ enabled to drain any in-flight commands.
> > 
> > The "resume" operation uses the `arm_smmu_device_reset` function, which
> > re-initializes the HW using the SW-copies maintained by the driver
> > (e.g., prod/cons pointers, queue base addresses) and clears all cached
> > configurations.
> > 
> > 3. Guarding Hardware Access
> > Instead of eliding operations, the driver now ensures the SMMU is active
> > before any hardware access. This is managed by `arm_smmu_rpm_get()` and
> > `arm_smmu_rpm_put()` helpers, which wrap the standard runtime PM calls.
> > These wrappers are now used throughout the driver in interrupt handlers,
> > TLB invalidation paths, and any other function that touches hardware
> > registers. This ensures that operations are implicitly queued until the
> > device resumes.
> > 
> > 4. Interrupt Re-config
> > a. Wired irqs: The series refactors the `arm_smmu_setup_irqs` to be
> > able to enable/disable irqs and install their handlers separately to
> > help with the re-initialization of the interrupts correctly.
> > 
> > b. MSIs: The series relies on caching the `msi_msg` and retrieving it
> > through a newly introduced helper `arm_smmu_resume_msis()` which
> > re-configures the *_IRQ_CFGn registers by writing back the cached msi_msg.
> > 
> > Call for review
> > Any insights/comments on the proposed changes are appreciated,
> > especially in areas related to locking, atomic contexts, and potential
> > optimizations.
> > 
> > Note: The series isn't tested with MSIs and is weakly tested for PCIe
> > clients. The same holds true for tegra241_cmdv changes. Any help in
> > reviewing and testing these parts is much appreciated.
> > 
> > [v5]
> >   - Refactored GERROR handling into a helper function and invoked it during
> >     runtime suspend after disabling the SMMU to capture any late-breaking
> >     gerrors as suggested by Jason.
> >   - Updated `arm_smmu_page_response` to be power-state aware and drop
> >     page faults received while suspended.
> >   - Included a patch from Ashish to correctly restore PROD and CONS
> >     indices for tegra241-cmdqv after a hardware reset.
> >   - Collected Reviewed-bys from Mostafa and Nicolin.
> > 
> > [v4]
> >   - https://lore.kernel.org/all/20251117191433.3360130-1-praan@google.com/
> >   - Dropped the `pm_runtime_get_if_not_suspended()` API in favor of a
> >     simpler, driver-specific biased counter (`nr_cmdq_users`) to manage
> >     runtime PM state.
> >   - Reworked the suspend callback to poll on the biased counter before
> >     disabling the SMMU.
> >   - Addressed comments for the MSI refactor.
> > 
> > [v3]
> >   - https://lore.kernel.org/all/20250616203149.2649118-1-praan@google.com/
> >   - Introduced `pm_runtime_get_if_not_suspended` API to avoid races due
> >     to bouncing RPM states while eliding TLBIs as pointed out by Daniel.
> >   - Addressed Nicolin's comments regarding msi_resume and CMDQV flush
> >   - Addressed Daniel's comments about CMDQ locking and draining
> >   - Addressed issues related to draining the evtq and priq
> >   - Dropped the code to identify and track user-space attachments
> > 
> > [v2]
> >   - https://lore.kernel.org/all/20250418233409.3926715-1-praan@google.com/
> >   - Introduced `arm_smmu_rpm_get_if_active` for eliding TLBIs & CFGIs
> >   - Updated the rpm helper invocation strategy.
> >   - Drained all queues in suspend callback (including tegra241-cmdv)
> >   - Cache and restore msi_msg instead of free-ing realloc-ing on resume
> >   - Added support to identify and track user-space attachments
> >   - Fixed the setup_irqs as per Nicolin & Mostafa's suggestions
> >   - Used force_runtime_suspend/resume instead as per Mostafa's suggestion.
> >   - Added "Reviewed-by" line from Mostafa on an unchanged patch
> > 
> > [v1]
> >   - https://lore.kernel.org/all/20250319004254.2547950-1-praan@google.com/
> > 
> > Ashish Mhetre (1):
> >    iommu/tegra241-cmdqv: Restore PROD and CONS after resume
> > 
> > Pranjal Shrivastava (9):
> >    iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs
> >    iommu/arm-smmu-v3: Add a helper to drain cmd queues
> >    iommu/tegra241-cmdqv: Add a helper to drain VCMDQs
> >    iommu/arm-smmu-v3: Cache and restore MSI config
> >    iommu/arm-smmu-v3: Add a usage counter for cmdq
> >    iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops
> >    iommu/arm-smmu-v3: Handle gerror during suspend
> >    iommu/arm-smmu-v3: Enable pm_runtime and setup devlinks
> >    iommu/arm-smmu-v3: Invoke pm_runtime before hw access
> > 
> >   .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     |  10 +-
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 429 ++++++++++++++++--
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  13 +
> >   .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    |  29 ++
> >   4 files changed, 445 insertions(+), 36 deletions(-)
> > 
> > --
> > 2.52.0.457.g6b5491de43-goog
> > 
> 
> Hi Pranjal,
> 
> I have pulled this series and tested on Tegra264 with CMDQV enabled
> and 16 vcmdqs assigned to guest. I ran multiple iterations of system
> suspend and resume with each iteration following by SMMU tests for
> multiple client devices. Everything worked as expected and tests
> were passing.
> 
> Tested-by: Ashish Mhetre <amhetre@nvidia.com>
> 

Thanks a lot for helping test this, Ashish, especially on Tegra CMDQV!

Regards,
Praan

      reply	other threads:[~2026-02-11 13:34 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-26 15:11 [PATCH v5 00/10] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 01/10] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 02/10] iommu/arm-smmu-v3: Add a helper to drain cmd queues Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 03/10] iommu/tegra241-cmdqv: Add a helper to drain VCMDQs Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 04/10] iommu/tegra241-cmdqv: Restore PROD and CONS after resume Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 05/10] iommu/arm-smmu-v3: Cache and restore MSI config Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 06/10] iommu/arm-smmu-v3: Add a usage counter for cmdq Pranjal Shrivastava
2026-03-08 21:23   ` Daniel Mentz
2026-03-09 19:46     ` Pranjal Shrivastava
2026-03-09 21:13       ` Pranjal Shrivastava
2026-03-09 22:56         ` Daniel Mentz
2026-03-10 16:46           ` Pranjal Shrivastava
2026-03-09 22:41       ` Daniel Mentz
2026-03-10 16:54         ` Pranjal Shrivastava
2026-03-11  4:43           ` Daniel Mentz
2026-03-11 13:53             ` Pranjal Shrivastava
2026-03-11 22:56               ` Daniel Mentz
2026-03-16 15:36                 ` Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 07/10] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops Pranjal Shrivastava
2026-03-10  1:45   ` Daniel Mentz
2026-03-10 16:58     ` Pranjal Shrivastava
2026-03-10 21:16       ` Daniel Mentz
2026-03-10 21:40         ` Pranjal Shrivastava
2026-03-11  0:12           ` Daniel Mentz
2026-03-11  5:31             ` Pranjal Shrivastava
2026-03-11 17:26               ` Daniel Mentz
2026-03-16 15:05                 ` Pranjal Shrivastava
2026-03-12 19:20   ` Daniel Mentz
2026-01-26 15:11 ` [PATCH v5 08/10] iommu/arm-smmu-v3: Handle gerror during suspend Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 09/10] iommu/arm-smmu-v3: Enable pm_runtime and setup devlinks Pranjal Shrivastava
2026-01-26 15:11 ` [PATCH v5 10/10] iommu/arm-smmu-v3: Invoke pm_runtime before hw access Pranjal Shrivastava
2026-02-06 15:48 ` [PATCH v5 00/10] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Ashish Mhetre
2026-02-11 13:33   ` Pranjal Shrivastava [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aYyFRa4ZtQGlNQ2t@google.com \
    --to=praan@google.com \
    --cc=amhetre@nvidia.com \
    --cc=danielmentz@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@ziepe.ca \
    --cc=joro@8bytes.org \
    --cc=nicolinc@nvidia.com \
    --cc=robin.murphy@arm.com \
    --cc=sarunkod@amd.com \
    --cc=smostafa@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox