Re: [RFC PATCH 0/5] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops

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

On 19/03/2025 12:42 am, Pranjal Shrivastava wrote:
> 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 in some fashion.
> The proposed design tries to make use of this fact for implementing the
> suspend and resume ops.
> 
> 1. Suspend / Resume
> An initial idea for the "suspend" op is to wait till the command queue
> finishes all commands before disabling the SMMU through CR0. In order to
> avoid mis-use / spurious transactions (b/w SMMU disable -> power-down),
> the GBPA register is configured to abort all transactions.

But why disable SMMUEN at all? We can't expect the SMMU to come back in 
the same configuration we left it anyway (*including* GBPA, note), so 
what's the benefit of spending time and effort to potentially change its 
behaviour for a handful of microseconds before it all goes dark anyway?

Draining the command queue shouldn't strictly be necessary it only 
contains invalidations and syncs, since we know all the cached state 
they could refer to is going to be thrown away by the time the SMMU has 
come back anyway. If anything, making sure there are no outstanding IOPF 
handlers and the event queue and PRI queue are clear (as expected) seems 
more important, because the potential to lose hardware tokens or leave a 
stalled transaction across the interconnect would be A Bad Thing.

> The resume operation uses the `arm_smmu_device_reset` function which
> re-initializes the HW using the SW-copies maintained by the driver. For
> example, prod/cons for queues, base addresses for queues & tables. The
> arm_smmu_device_reset also clears the TLBs.
> 
> 2. 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 thought was of 2 approaches to teardown & re-config MSIs:
> 
>   1. Free MSIs on suspend and re-alloc on resume (implemented in series)
>   2. Meddle with the msi_desc and use get_cached_msi_msg to resume MSIs.

3: Just explicitly save/restore the IRQ_CFG{0,1} registers in the PM 
callbacks.
4: Have arm_smmu_write_msi_msg() stash the values, and defer the actual 
IRQ_CFG register writes until arm_smmu_setup_irqs(), alongside IRQ_CTRL.

If the other options aren't sufficiently clean or straightforward, 
another 36 bytes per arm_smmu_device instance isn't the end of the world...

> The first approach (implemented in this series) may be potentially
> slower, whereas the second one could reduce the time taken to resume.
> However, the second one feels hacky as it forces us to cache the msi_msg
> in `arm_smmu_write_msg` or in the core code (`irq_chip_write_msi_msg`)
> as suggested in [1].
>    
> 3. Invoking runtime_pm_get/put
> Given that most of the configuration done by arm-smmu-v3 is stored in
> memory, the initial idea is to focus on areas where the driver accesses
> the hw via exposed ops, like iommmu_ops, iommu_flush_ops, sva_ops etc.
> 
> Instead of wrapping every exposed op with an rpm_get/put, the idea is to
> follow through till a logical common point (lowest common ancestor in
> the call hierarchies) and wrap those with the rpm_get/put calls.

As mentioned already, I think there should be very few, if any, places 
where we actually need an rpm_get() to explicitly resume a suspended 
SMMU. All the initial hardware probing and reset can (and should) be 
done before PM is enabled; IRQ handlers can reasonably assume that any 
call while the SMMU is suspended must be shared or spurious, so don't 
need to look (even more so if we do still clear IRQ_CTRL on suspend); 
any commands issued while suspended are either safe to ignore 
(CFGI/TLBI/PREFETCH) or nonsensical (RESUME/PRI_RESP) - the only one I'm 
not entirely sure about off the top of my head is ATC_INV, which might 
depend on whether the PCI code can guarantee that an endpoint will 
resume with a clean ATC.

I don't see SVA and VFIO needing any special considerations, so it might 
just be the new vIOMMU stuff which may need to hold PM enabled for the 
lifetime of things exposed to userspace.

Thanks,
Robin.

> Future Work
> ===========
> - Elide TLBIs when the SMMU is powered down
> 
> Call for Review
> ================
> Any insights/comments on the proposed changes are appreciated,
> especially in areas related to locking, atomic contexts, early resume
> etc. or any potential optimization.
> 
> Note: The series isn't tested with MSIs as I couldn't find a platform
> that supports MSIs. Any help in testing MSIs is appreciated.
> 
> References
> ===========
> [1]
> https://lore.kernel.org/lkml/20210721013350.17664-1-cuibixuan@huawei.com/T/#m06bf92e2306ba52c106f2d4d24765921d4e9781e
> 
> [2]
> https://lore.kernel.org/all/20180830144541.17740-1-vivek.gautam@codeaurora.org/
> 
> Pranjal Shrivastava (5):
>    iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs
>    iommu/arm-smmu-v3: Add a helper to wait till cmdq drains
>    iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops
>    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     |  22 +-
>   .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  24 ++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 297 ++++++++++++++++--
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   5 +
>   4 files changed, 326 insertions(+), 22 deletions(-)
>