* [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops
@ 2026-09-08 17:16 Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 01/15] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs Pranjal Shrivastava
` (14 more replies)
0 siblings, 15 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:16 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
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 software gate embedded in the command queue.
1. CMDQ Gate (CMDQ_PROD_STOP_FLAG)
To safely manage runtime PM without regressing performance on high core
count servers or systems not opting for runtime power management,
this series introduces a CMDQ_PROD_STOP_FLAG (bit 30) in the command
queue's producer index. The flag acts as a Point of Commitment in the
cmpxchg loop of arm_smmu_cmdq_issue_cmdlist(), ensuring no new indices
are reserved once suspension begins.
2. Suspend / Resume Flow
The suspend operation follows a multi-stage quiesce sequence:
a. Stop Traffic: Sets SMMUEN=0 and GBPA=Abort to halt new transactions.
b. Gate CMDQ: Sets the CMDQ_PROD_STOP_FLAG to block new submissions.
c. Command Flush: Waits for any in-flight "owner" threads to commit
their reserved indices to hardware.
d. SW Quiesce: Waits for all concurrent threads to release the shared
cmdq->lock, ensuring no CPUs are left polling CONS register.
e. HW Drain: Polls the CMDQ until all committed commands are consumed.
f. IRQ Quiesce: Disables hardware IRQs and waits for racing handlers to
complete via synchronize_irq()
Entering the suspend sequence implies that the device has no active
clients. In v8, a surgical state-correction is added to runtime_suspend:
if draining fails, the software trackers (cons and owner_prod) are forced
to align with the masked producer index. This voids any stale commands for
the next session and prevents deadlocks or spurious executions upon
resumption.
The resume operation clears the STOP_FLAG & performs a full device
reset via arm_smmu_device_reset(), which re-initializes the HW using the
SW-copies maintained by the driver and clears all cached configurations.
3. Guarding Hardware Access and Elision
The driver ensures the SMMU is active before hardware access via
arm_smmu_rpm_get() and arm_smmu_rpm_put() helpers. To maintain code
clarity, invalidation call-sites no longer perform manual elision checks;
instead, elision is authoritative within the CMDQ submission path.
The renamed helper, arm_smmu_is_active(), is preserved only for
critical diagnostic (ATC), early-drop (Page Response), and gerror
short-circuits. For ATC invalidations, devlinks must guarantee the SMMU
is active if the endpoint is active; a WARN_ON_ONCE() catches
inconsistencies.
4. Implementation-Specific Quiescing (Tegra VCMDQ)
Implementation-specific gating and draining are consolidated into
a unified quiesce_and_drain_queues callback. The tegra241-cmdqv driver
is updated to gate all active local virtual queues (VINTF0) before
draining.
5. Interrupt Re-config
a. Wired irqs: The series refactors arm_smmu_setup_irqs to allow
separate installation of handlers, aiding in correct re-initialization.
b. MSIs: The series caches the msi_msg and restores it during resume
via a new arm_smmu_resume_msis() helper.
c. GERROR: Late-breaking global errors are captured and handled
immediately after SMMU disablement during suspend to ensure no diagnostic
information is lost.
Scalability and Performance
===========================
A key design goal of this series is to ensure that high-performance
systems (typically servers) that do not enable runtime PM are not
penalized. By embedding a stop flag in the command queue's producer
index and designing RPM helpers to perform only read-only checks when
RPM is disabled, command submission on these systems incurs negligible
overhead. Power-managed systems only utilize runtime PM atomics as
necessary, ensuring that scalability is maintained across all hardware
classes.
Sashiko Reviews and Documentation
=================================
The series was run through Sashiko locally. In response to the review
feedback, extensive verbose comments have been added across the series to
thoroughly document hardware/software synchronization invariants, memory
ordering dependencies (Point of Commitment, dma_wmb vs. smp_mb), and
devlink power guarantees. Several edge-case race conditions, unmasking
bugs, and resource leak paths were also addressed.
[v10]
- Picked up common drain helpers from Nicolin's PRI series v3 [1].
- Introduced platform_device_msi_rewrite() in platform/msi core
(suggested by Jason).
- Fixed the stale interrupt issue caused by calling arm_smmu_setup_irqs()
early in probe (as reported by Sashiko in v9) by passing a bool to
arm_smmu_device_reset() to ensure irqs aren't set-up again on resume.
- Fixed a CMDQ gate bypass race where threads waking from
poll_until_not_full() could commit commands after STOP_FLAG was asserted
- Fixed a masking issue in __arm_smmu_cmdq_issue_cmdlist() & tegra241-cmdqv
- Added tegra241_vcmdq_wait_quiescent() to wait for in-flight owner threads
- Fixed VCMDQ drain loop error handling to continue draining remaining
queues rather than abandoning them on timeout (pointed by Sashiko in v9)
- Explicitly zeroed *_IRQ_CFG0 registers during resume as they reset to
unknown values.
- Cleared ARM_SMMU_FEAT_MSI on MSI setup fallback to avoid spurious
MSI rewrites on resume.
- Retitled GERROR patch to "Factor out arm_smmu_handle_gerror()" for clarity
- Handled devlink creation failure in arm_smmu_probe_device()
- Added checks for the retval of arm_smmu_rpm_get() in
arm_smmu_disable_action() & arm_smmu_device_shutdown()
- Clarified rationale for asynchronous put in teardown and shutdown paths
where hardware has already been explicitly disabled.
- Documented safety of eliding ATC invalidations during suspend based on
PCIe link power states and devlink guarantees per Jason's comment.
- Fixed stack timer lifecycle in KUnit tests using timer_setup_on_stack()
and timer_destroy_on_stack(), and resolved mock register endianness.
- Added extensive verbose comments across the patches explaining locking,
synchronization invariants, and devlink power guarantees based on local
Sashiko review.
- Collected Reviewed-by tags from Jason Gunthorpe.
- Rebased onto latest arm/smmu/updates
[1] https://lore.kernel.org/all/cover.1788222485.git.nicolinc@nvidia.com/
[v9]
- https://lore.kernel.org/all/20260728210928.1050849-1-praan@google.com/
- Replaced atomic_fetch_or_relaxed() with atomic_fetch_or() for
correct ordering while asserting the STOP_FLAG
- Moved arm_smmu_drain_queues() after the cmdq->lock wait loop in
runtime_suspend() as per feedback in v8.
- Refactored suspend sequence to use arm_smmu_disable_irqs() and
synchronize_irq() for safe GERROR handling.
- Fixed an off-by-one error in suspend polling timeouts.
- Dropped arm_smmu_cmdq_can_elide() check from inv_range
- Renamed arm_smmu_cmdq_can_elide() to arm_smmu_is_active().
- Removed redundant bitwise masking from owner_prod reads.
- Added comments to explain locking, gating and elision better.
- Added comments to explain secondary cmdq draining and gating.
- Rebased on arm/smmu/updates
[v8]
- https://lore.kernel.org/all/20260601215909.3958732-1-praan@google.com/
- Centralized elision logic: dropped redundant checks from invalidation
call-sites; elision is now authoritative within the CMDQ layer.
- Renamed elision helper to arm_smmu_cmdq_can_elide() and preserved it
only for diagnostic/safety paths (ATC, GERROR, Page Response).
- Consolidated implementation-specific gating and draining into a
unified quiesce_and_drain_queues callback.
- Updated tegra241-cmdqv to gate virtual queues before draining,
addressing non-deterministic timeouts from guest-side submissions.
- Re-ordered probe sequence to enable pm_runtime only at the end,
aligning with SMMUv2 and simplifying error paths.
- Refactored KUnit tests for better, addressed Nicolin's comments & added
a multi-threaded kthread race test.
- Refactored RPM helpers to use early-return patterns for improved clarity.
- Collected R-bs from Nicolin.
[v7]
- https://lore.kernel.org/all/20260527221407.1756491-1-praan@google.com/
- Rebased on the latest arm/smmu/updates branch (which has the removal of
struct arm_smmu_cmdq_ent merged)
- Converted manual cmpxchg loops in suspend/resume to use atomics
- Re-worked to elide invalidations solely based on the CMDQ_PROD_STOP_FLAG
via arm_smmu_can_elide(), dropping any need for pm_runtime_get_if_active
- Added an smp_mb() fence in the reset sequence to ensure that the SMMU
acquires all RAM updates made by newly un-gated threads before SMMUEN=1
- Implemented bitwise masking for the PROD register to prevent software
metadata (STOP_FLAG) bits from being written to physical hardware.
- Introduced a KUnit test suite to verify the CMDQ gating algorithm
[v6]
- https://lore.kernel.org/all/20260414194702.1229094-1-praan@google.com/
- Replaced the atomic nr_cmdq_users counter with CMDQ_PROD_STOP_FLAG
to eliminate atomic overhead on high-core count servers.
- Implemented a 5-step quiesce sequence in runtime_suspend including
pipeline flushes and software completion barriers.
- Introduced arm_smmu_rpm_get_if_active() to elide TLB/CFG/ATC
invalidations when the SMMU is suspended.
- Added WARN_ON_ONCE() in invalidation paths to detect inconsistent
power states for active endpoints.
- Refined batch submission in __arm_smmu_domain_inv_range() to ensure
clean state when dropping batches.
- Refactored GERROR handling for better integration with suspend.
- Added Suggested-by tags for Daniel Mentz.
[v5]
- https://lore.kernel.org/all/20260126151157.3418145-1-praan@google.com/
- 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
Nicolin Chen (2):
iommu/arm-smmu-v3: Add Q_POS() macro
iommu/arm-smmu-v3: Add arm_smmu_drain_queue() helper
Pranjal Shrivastava (12):
iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs
iommu/tegra241-cmdqv: Add a helper to drain VCMDQs
iommu/arm-smmu-v3: Add a helper to drain cmd queues
platform-msi: Introduce platform_device_msi_rewrite()
iommu/arm-smmu-v3: Cache and restore MSI config
iommu/arm-smmu-v3: Factor out arm_smmu_handle_gerror()
iommu/arm-smmu-v3: Add CMDQ_PROD_STOP_FLAG to gate CMDQ submissions
iommu/tegra241-cmdqv: Add a helper to quiesce VCMDQs
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
iommu/arm-smmu-v3: Add KUnit unit tests for Runtime PM
drivers/base/platform-msi.c | 37 +
.../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 20 +-
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 215 ++++++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 721 +++++++++++++++++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 29 +
.../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 128 ++++
include/linux/msi.h | 2 +
7 files changed, 1113 insertions(+), 39 deletions(-)
base-commit: df2908090cda368b01ff43709f51890076c56157
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v10 01/15] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
@ 2026-09-08 17:16 ` Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 02/15] iommu/arm-smmu-v3: Add Q_POS() macro Pranjal Shrivastava
` (13 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:16 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava,
Jason Gunthorpe
Refactor arm_smmu_setup_irqs by splitting it into two parts, one for
registering interrupt handlers and the other one for enabling interrupt
generation in the hardware. This refactor helps in re-initialization of
hardware interrupts as part of a subsequent patch that enables runtime
power management for the arm-smmu-v3 driver.
Additionally, pass a 'resume' flag to arm_smmu_device_reset() to skip
registering IRQ handlers again during resume. On probe, handlers are
registered after queues are configured but before enabling translation.
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 75 +++++++++++++++------
1 file changed, 56 insertions(+), 19 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 5732f3ba0122..270e6617a19a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4756,14 +4756,55 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
}
}
+static void arm_smmu_enable_irqs(struct arm_smmu_device *smmu)
+{
+ int ret;
+ u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
+
+ if (smmu->features & ARM_SMMU_FEAT_PRI)
+ irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
+
+ /*
+ * Enable interrupt generation on the SMMU.
+ * As per the ARM SMMUv3 spec section 6.3.17 (SMMU_IRQ_CTRL):
+ *
+ * Completion of an Update of x_IRQEN from 0 to 1 guarantees that the
+ * MSI configuration in SMMU_x_IRQ_CFG{0,1,2} will be used for all
+ * future MSIs generated from source x. All wired or MSI interrupts
+ * that are triggered from a source relate to occurrences that happened
+ * after the completion of the Update that enabled the source. It is
+ * not permitted to trigger an interrupt that relates to an occurrence
+ * that happened before the source was enabled, even if the source was
+ * previously enabled at the time of the occurrence.
+ */
+ ret = arm_smmu_write_reg_sync(smmu, irqen_flags,
+ ARM_SMMU_IRQ_CTRL, ARM_SMMU_IRQ_CTRLACK);
+ if (ret)
+ dev_warn(smmu->dev, "failed to enable irqs\n");
+}
+
+static int arm_smmu_disable_irqs(struct arm_smmu_device *smmu)
+{
+ return arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
+ ARM_SMMU_IRQ_CTRLACK);
+}
+
static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
{
int ret, irq;
- u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
- /* Disable IRQs first */
- ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
- ARM_SMMU_IRQ_CTRLACK);
+ /*
+ * Disable IRQs first as required by the Arm SMMUv3 spec.
+ * As per the ARM SMMUv3 spec section 6.3.17 (SMMU_IRQ_CTRL):
+ *
+ * An Update of x_IRQEN from 1 to 0 completes when all prior MSIs have
+ * completed. An MSI has completed when it is visible to its
+ * Shareability domain, or when it has aborted, and the abort is
+ * recorded in the appropriate SMMU_(*_)GERROR bit. Completion of this
+ * Update guarantees that no new MSI writes or wired edge events from
+ * source x become visible until the source is re-enabled.
+ */
+ ret = arm_smmu_disable_irqs(smmu);
if (ret) {
dev_err(smmu->dev, "failed to disable irqs\n");
return ret;
@@ -4785,15 +4826,6 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
} else
arm_smmu_setup_unique_irqs(smmu);
- if (smmu->features & ARM_SMMU_FEAT_PRI)
- irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
-
- /* Enable interrupt generation on the SMMU */
- ret = arm_smmu_write_reg_sync(smmu, irqen_flags,
- ARM_SMMU_IRQ_CTRL, ARM_SMMU_IRQ_CTRLACK);
- if (ret)
- dev_warn(smmu->dev, "failed to enable irqs\n");
-
return 0;
}
@@ -4841,7 +4873,7 @@ static void arm_smmu_write_strtab(struct arm_smmu_device *smmu)
writel_relaxed(reg, smmu->base + ARM_SMMU_STRTAB_BASE_CFG);
}
-static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
+static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool resume)
{
int ret;
u32 reg, enables;
@@ -4943,12 +4975,17 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
}
}
- ret = arm_smmu_setup_irqs(smmu);
- if (ret) {
- dev_err(smmu->dev, "failed to setup irqs\n");
- return ret;
+ if (!resume) {
+ ret = arm_smmu_setup_irqs(smmu);
+ if (ret) {
+ dev_err(smmu->dev, "failed to setup irqs\n");
+ return ret;
+ }
}
+ /* Enable interrupt generation on the SMMU */
+ arm_smmu_enable_irqs(smmu);
+
if (is_kdump_kernel())
enables &= ~(CR0_EVTQEN | CR0_PRIQEN);
@@ -5594,7 +5631,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
arm_smmu_rmr_install_bypass_ste(smmu);
/* Reset the device */
- ret = arm_smmu_device_reset(smmu);
+ ret = arm_smmu_device_reset(smmu, false);
if (ret) {
arm_smmu_device_disable(smmu);
return ret;
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 02/15] iommu/arm-smmu-v3: Add Q_POS() macro
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 01/15] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs Pranjal Shrivastava
@ 2026-09-08 17:16 ` Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 03/15] iommu/arm-smmu-v3: Add arm_smmu_drain_queue() helper Pranjal Shrivastava
` (12 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:16 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
From: Nicolin Chen <nicolinc@nvidia.com>
A queue position, the wrap bit combined with the index, is Q_WRP | Q_IDX.
It is a (max_n_shift + 1)-bit value that wraps at twice the queue capacity.
queue_inc_cons(), queue_sync_cons_ovf(), and queue_inc_prod_n() currently
compute such positions by open-coding the two macros at each call site.
Add a Q_POS() macro and switch the open-coded sites to it.
A subsequent change will apply Q_POS() to a position difference, to count
the entries that a queue pointer moved past.
No functional change intended.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 +++++------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 ++
2 files changed, 7 insertions(+), 6 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 270e6617a19a..06b7de2e6e4b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -181,8 +181,8 @@ static void queue_sync_cons_out(struct arm_smmu_queue *q)
static void queue_inc_cons(struct arm_smmu_ll_queue *q)
{
- u32 cons = (Q_WRP(q, q->cons) | Q_IDX(q, q->cons)) + 1;
- q->cons = Q_OVF(q->cons) | Q_WRP(q, cons) | Q_IDX(q, cons);
+ u32 cons = Q_POS(q, q->cons) + 1;
+ q->cons = Q_OVF(q->cons) | Q_POS(q, cons);
}
static void queue_sync_cons_ovf(struct arm_smmu_queue *q)
@@ -192,8 +192,7 @@ static void queue_sync_cons_ovf(struct arm_smmu_queue *q)
if (likely(Q_OVF(llq->prod) == Q_OVF(llq->cons)))
return;
- llq->cons = Q_OVF(llq->prod) | Q_WRP(llq, llq->cons) |
- Q_IDX(llq, llq->cons);
+ llq->cons = Q_OVF(llq->prod) | Q_POS(llq, llq->cons);
queue_sync_cons_out(q);
}
@@ -218,8 +217,8 @@ static int queue_sync_prod_in(struct arm_smmu_queue *q)
static u32 queue_inc_prod_n(struct arm_smmu_ll_queue *q, int n)
{
- u32 prod = (Q_WRP(q, q->prod) | Q_IDX(q, q->prod)) + n;
- return Q_OVF(q->prod) | Q_WRP(q, prod) | Q_IDX(q, prod);
+ u32 prod = Q_POS(q, q->prod) + n;
+ return Q_OVF(q->prod) | Q_POS(q, prod);
}
static void queue_poll_init(struct arm_smmu_device *smmu,
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 dd2fee2f560e..84e618a7fa9e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -187,6 +187,8 @@ struct arm_vsmmu;
#define Q_IDX(llq, p) ((p) & ((1 << (llq)->max_n_shift) - 1))
#define Q_WRP(llq, p) ((p) & (1 << (llq)->max_n_shift))
+/* A position is Q_WRP | Q_IDX, wrapping at twice the queue capacity */
+#define Q_POS(llq, p) (Q_WRP(llq, p) | Q_IDX(llq, p))
#define Q_OVERFLOW_FLAG (1U << 31)
#define Q_OVF(p) ((p) & Q_OVERFLOW_FLAG)
#define Q_ENT(q, p) ((q)->base + \
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 03/15] iommu/arm-smmu-v3: Add arm_smmu_drain_queue() helper
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 01/15] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 02/15] iommu/arm-smmu-v3: Add Q_POS() macro Pranjal Shrivastava
@ 2026-09-08 17:16 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 04/15] iommu/tegra241-cmdqv: Add a helper to drain VCMDQs Pranjal Shrivastava
` (11 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:16 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
From: Nicolin Chen <nicolinc@nvidia.com>
Add a counting-based arm_smmu_drain_queue() helper, to replace queue
specific polling loops. Its until_empty mode serves the suspend and
runtime PM routines that would drain the CMDQ. Any timed-out drain fires
a WARN_ON as well, since reaching the timeout would take some stuck
consumer in any realistic case.
The existing queue_poll() API is not reusable for such a drain: it is the
atomic busy-wait for the command issuing paths, and it assumes a hardware
consumer making progress. A drain caller is sleepable, in contrast, while
the EVTQ/PRIQ consumer is a threaded IRQ handler that needs the CPU: such
a busy wait would starve the handler throughout an entire timeout, whenever
the waiter and the handler shared one CPU on a non-preemptible kernel. So,
this new sleeping helper is marked with a might_sleep() as well, given that
an atomic-context misuse would otherwise hide behind an empty queue.
Note that a drained event is dequeued, but not necessarily handled, since
queue_remove_raw() moves the MMIO CONS before the threaded IRQ handler gets
to push the event onto the IOPF workqueue. A subsequent change will invoke
synchronize_irq() and iopf_queue_flush_dev() to close that gap, and it will
act on the errno of a timed-out drain too.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 81 +++++++++++++++++++++
1 file changed, 81 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 06b7de2e6e4b..84b56849f6dc 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -948,6 +948,87 @@ static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu,
cmds->num, true);
}
+/**
+ * arm_smmu_drain_queue - Drain an SMMU queue
+ * @smmu: the SMMU device
+ * @q: the queue to drain
+ * @until_empty: target selection
+ *
+ * With @until_empty == true (for CMDQ), exit once the queue is observed empty:
+ *
+ * cons0 cons prod
+ * | | |
+ * ---+###################+=====================+=============+--->
+ * |<--------- undrained==0? --------->|
+ *
+ * With @until_empty == false (for EVTQ/PRIQ), exit once "drained" reaches its
+ * target: "pending" (i.e. prod0 - cons0, frozen at the entry time):
+ *
+ * cons0 cons prod0 (prod)
+ * |<---- drained ---->| | |
+ * ---+###################+=====================+=============+--->
+ * |<--------------- pending --------------->|
+ *
+ * Note that a drained entry is dequeued, but not necessarily handled: the
+ * EVTQ/PRIQ callers must follow up with a synchronize_irq() to wait for the
+ * threaded IRQ handler to finish handling the dequeued entries.
+ *
+ * Context: Process context; may sleep.
+ * Return: 0 on success or a negative errno on timeout.
+ */
+static int __maybe_unused arm_smmu_drain_queue(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q,
+ bool until_empty)
+{
+ ktime_t timeout = ktime_add_us(ktime_get(), ARM_SMMU_POLL_TIMEOUT_US);
+ u32 cons, prod, prev, undrained;
+ u32 drained = 0, pending;
+
+ might_sleep();
+
+ cons = readl_relaxed(q->cons_reg);
+ prod = readl_relaxed(q->prod_reg);
+ /* The exit target: the number of entries in the queue at entry */
+ pending = Q_POS(&q->llq, prod - cons);
+
+ while (true) {
+ /* Accumulate the entries consumed since the last poll */
+ prev = cons;
+ cons = readl_relaxed(q->cons_reg);
+ drained += Q_POS(&q->llq, cons - prev);
+
+ prod = readl_relaxed(q->prod_reg);
+ undrained = Q_POS(&q->llq, prod - cons);
+
+ /* Exit on an empty queue, regardless of until_empty */
+ if (!undrained)
+ return 0;
+
+ /* Snapshot mode: exit once the pending entries are drained */
+ if (!until_empty && drained >= pending)
+ return 0;
+
+ /*
+ * A timeout means the consumer might be stuck. In theory, if it
+ * moves 2 * qsize entries or more within a single poll interval
+ * Q_POS() would wrap and undercount drained: that could trigger
+ * a spurious warning too, if the queue was never once observed
+ * empty. Yet, that much consumption in such a short interval is
+ * unrealistic. WARN it only, as a stuck consumer is a real bug.
+ */
+ if (WARN_ON(ktime_compare(ktime_get(), timeout) > 0))
+ break;
+
+ /* The consumer might be a threaded IRQ handler. Yield to it */
+ usleep_range(100, 200);
+ }
+
+ dev_warn_ratelimited(smmu->dev,
+ "queue drain timed out at prod=0x%x cons=0x%x\n",
+ prod, cons);
+ return -ETIMEDOUT;
+}
+
static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
struct iommu_page_response *resp)
{
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 04/15] iommu/tegra241-cmdqv: Add a helper to drain VCMDQs
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (2 preceding siblings ...)
2026-09-08 17:16 ` [PATCH v10 03/15] iommu/arm-smmu-v3: Add arm_smmu_drain_queue() helper Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 05/15] iommu/arm-smmu-v3: Add a helper to drain cmd queues Pranjal Shrivastava
` (10 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
The tegra241-cmdqv driver supports vCMDQs which need to be drained
before suspending the SMMU. The current driver implementation only uses
VINTF0 for vCMDQs owned by the kernel which need to be drained. Add a
helper that drains all the enabled vCMDQs under VINTF0.
Add a drain_queues function ptr to arm_smmu_impl_ops to drain
implementation specified queues, and export arm_smmu_drain_queue().
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 ++
.../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 49 +++++++++++++++++++
3 files changed, 55 insertions(+), 3 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 84b56849f6dc..d809b625f7fb 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -976,9 +976,8 @@ static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu,
* Context: Process context; may sleep.
* Return: 0 on success or a negative errno on timeout.
*/
-static int __maybe_unused arm_smmu_drain_queue(struct arm_smmu_device *smmu,
- struct arm_smmu_queue *q,
- bool until_empty)
+int arm_smmu_drain_queue(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q, bool until_empty)
{
ktime_t timeout = ktime_add_us(ktime_get(), ARM_SMMU_POLL_TIMEOUT_US);
u32 cons, prod, prev, undrained;
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 84e618a7fa9e..deefb17e31eb 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -891,6 +891,7 @@ struct arm_smmu_impl_ops {
size_t (*get_viommu_size)(enum iommu_viommu_type viommu_type);
int (*vsmmu_init)(struct arm_vsmmu *vsmmu,
const struct iommu_user_data *user_data);
+ int (*drain_queues)(struct arm_smmu_device *smmu);
};
/* An SMMUv3 instance */
@@ -1174,6 +1175,9 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
int arm_smmu_cmdq_init(struct arm_smmu_device *smmu,
struct arm_smmu_cmdq *cmdq);
+int arm_smmu_drain_queue(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q, bool until_empty);
+
static inline bool arm_smmu_master_canwbs(struct arm_smmu_master *master)
{
return dev_iommu_fwspec_get(master->dev)->flags &
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 6644075c1431..a394bf0f487f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -447,6 +447,54 @@ tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu,
return &vcmdq->cmdq;
}
+static int tegra241_cmdqv_drain_vintf0_lvcmdqs(struct arm_smmu_device *smmu)
+{
+ struct tegra241_cmdqv *cmdqv =
+ container_of(smmu, struct tegra241_cmdqv, smmu);
+ struct tegra241_vintf *vintf = cmdqv->vintfs[0];
+ int ret = 0;
+ u16 lidx;
+
+ /*
+ * Kernel only uses VINTF0. Return if it's disabled.
+ * Note: Lockless reads of the enabled flags are safe here.
+ * VINTF0 is initialized during probe() (before Runtime PM is
+ * enabled) and de-initialized during remove() (after the driver
+ * core has called pm_runtime_disable()). Thus, this drain helper,
+ * running only in runtime_suspend(), cannot race against VINTF0
+ * creation or destruction.
+ */
+ if (!READ_ONCE(vintf->enabled))
+ return 0;
+
+ for (lidx = 0; lidx < cmdqv->num_lvcmdqs_per_vintf; lidx++) {
+ struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
+ int rc;
+
+ if (!vcmdq || !READ_ONCE(vcmdq->enabled))
+ continue;
+
+ rc = arm_smmu_drain_queue(smmu, &vcmdq->cmdq.q, true);
+ if (rc) {
+ /*
+ * The PM core forces suspend even if a drain times out.
+ * We must record the error but continue draining the
+ * remaining queues to ensure no in-flight commands are
+ * abandoned when the hardware power is cut.
+ */
+ dev_warn_ratelimited(smmu->dev,
+ "vintf0 lvcmdq%u drain timed out\n",
+ lidx);
+ ret = rc;
+ }
+
+ /* Avoid consuming stale commands on resume */
+ vcmdq->cmdq.q.llq.cons = vcmdq->cmdq.q.llq.prod;
+ }
+
+ return ret;
+}
+
/* HW Reset Functions */
/*
@@ -905,6 +953,7 @@ static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
.device_reset = tegra241_cmdqv_hw_reset,
.device_disable = tegra241_cmdqv_hw_disable,
.device_remove = tegra241_cmdqv_remove,
+ .drain_queues = tegra241_cmdqv_drain_vintf0_lvcmdqs,
/* For user-space use */
.hw_info = tegra241_cmdqv_hw_info,
.get_viommu_size = tegra241_cmdqv_get_vintf_size,
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 05/15] iommu/arm-smmu-v3: Add a helper to drain cmd queues
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (3 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 04/15] iommu/tegra241-cmdqv: Add a helper to drain VCMDQs Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 06/15] iommu/tegra241-cmdqv: Restore PROD and CONS after resume Pranjal Shrivastava
` (9 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava,
Jason Gunthorpe
Before we suspend SMMU, we want to ensure that all commands (especially
ATC_INV) have been flushed by the CMDQ, i.e. the CMDQs are empty.
Add arm_smmu_drain_cmdqs() that drains the command queue using the
arm_smmu_drain_queue() helper.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 27 +++++++++++++++++++++
1 file changed, 27 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 d809b625f7fb..dc2fc02b5163 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1028,6 +1028,33 @@ int arm_smmu_drain_queue(struct arm_smmu_device *smmu,
return -ETIMEDOUT;
}
+/*
+ * Lockless, the caller must ensure that no concurrent command submissions
+ * can happen (such as during suspend where clients are already quiesced).
+ * Polling until PROD == CONS ensures all pending commands in the queue have
+ * been consumed by the hardware.
+ */
+static int __maybe_unused arm_smmu_drain_cmdqs(struct arm_smmu_device *smmu)
+{
+ int ret, err;
+
+ /*
+ * We skip cmdq locking here since this helper is only called
+ * from contexts (like suspend) where the caller has already
+ * ensured that new command submissions are fully closed.
+ */
+ ret = arm_smmu_drain_queue(smmu, &smmu->cmdq.q, true);
+
+ /* Drain all implementation-specific queues */
+ if (smmu->impl_ops && smmu->impl_ops->drain_queues) {
+ err = smmu->impl_ops->drain_queues(smmu);
+ if (err)
+ ret = err;
+ }
+
+ return ret;
+}
+
static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
struct iommu_page_response *resp)
{
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 06/15] iommu/tegra241-cmdqv: Restore PROD and CONS after resume
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (4 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 05/15] iommu/arm-smmu-v3: Add a helper to drain cmd queues Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite() Pranjal Shrivastava
` (8 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
From: Ashish Mhetre <amhetre@nvidia.com>
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.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index a394bf0f487f..c7989fcd2c62 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -567,6 +567,16 @@ 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));
+ /*
+ * HW Registers reset to 0 when power-cycled. Restore them from their
+ * SW copies to prevent executing stale/ghost commands after resume.
+ * Note on RPM: Power-cycles can never happen when VCMDQs are assigned
+ * to the Guests since the relevant frameworks (IOMMUFD / VFIO) hold
+ * active PM references preventing suspend while VMs are active.
+ */
+ 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) {
dev_err(vcmdq->cmdqv->dev,
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite()
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (5 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 06/15] iommu/tegra241-cmdqv: Restore PROD and CONS after resume Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 19:40 ` Thomas Gleixner
2026-09-08 17:17 ` [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config Pranjal Shrivastava
` (7 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava,
Jason Gunthorpe
Platform devices using MSIs often lose their MSI configuration registers
during system suspend or power down. Add platform_device_msi_rewrite()
to re-apply the cached MSI configuration for a given IRQ using the
device-provided write_msi_msg callback.
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/base/platform-msi.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/msi.h | 2 ++
2 files changed, 39 insertions(+)
diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 69eed058eb20..022febf2da93 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -94,3 +94,40 @@ void platform_device_msi_free_irqs_all(struct device *dev)
msi_remove_device_irq_domain(dev, MSI_DEFAULT_DOMAIN);
}
EXPORT_SYMBOL_GPL(platform_device_msi_free_irqs_all);
+
+/**
+ * platform_device_msi_rewrite - Rewrite the MSI config for a platform device
+ * @dev: The device for which to rewrite interrupt
+ * @irq: The interrupt number to be rewritten.
+ * @write_msi_msg: Callback to write the interrupt message for @dev
+ *
+ * Rewrites the cached MSI message for a platform device.
+ *
+ * Note: Platform MSI does not automatically cache composed messages. The caller's
+ * @write_msi_msg callback is expected to cache the message (e.g. into desc->msg)
+ * during initial configuration so it can be rewritten on resume.
+ */
+void platform_device_msi_rewrite(struct device *dev, unsigned int irq,
+ irq_write_msi_msg_t write_msi_msg)
+{
+ struct msi_desc *desc;
+ struct msi_msg msg;
+
+ if (!irq || !write_msi_msg)
+ return;
+
+ desc = irq_get_msi_desc(irq);
+ if (!desc) {
+ dev_err(dev, "Failed to get MSI descriptor for irq %u\n", irq);
+ return;
+ }
+
+ __get_cached_msi_msg(desc, &msg);
+ if (!msg.address_hi && !msg.address_lo) {
+ dev_warn(dev, "No cached MSI message found for irq %u\n", irq);
+ return;
+ }
+
+ write_msi_msg(desc, &msg);
+}
+EXPORT_SYMBOL_GPL(platform_device_msi_rewrite);
diff --git a/include/linux/msi.h b/include/linux/msi.h
index a4613de11960..72b0d36160d9 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -673,6 +673,8 @@ struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
int platform_device_msi_init_and_alloc_irqs(struct device *dev, unsigned int nvec,
irq_write_msi_msg_t write_msi_msg);
void platform_device_msi_free_irqs_all(struct device *dev);
+void platform_device_msi_rewrite(struct device *dev, unsigned int irq,
+ irq_write_msi_msg_t write_msi_msg);
bool msi_device_has_isolated_msi(struct device *dev);
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (6 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite() Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 19:56 ` Thomas Gleixner
2026-09-08 17:17 ` [PATCH v10 09/15] iommu/arm-smmu-v3: Factor out arm_smmu_handle_gerror() Pranjal Shrivastava
` (6 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
The SMMU's MSI configuration registers (*_IRQ_CFGn) containing target
address, data and memory attributes lose their state when the SMMU is
powered down. We'll need to cache and restore their contents to ensure
that MSIs work after the system resumes. Address this, by caching the
original `msi_msg` within the `msi_desc` when the config is first written
by `arm_smmu_write_msi_msg`. This primarily includes the target address
and data since the memory attributes are fixed.
Introduce a new helper `arm_smmu_resume_msis` which will later be called
during the driver's resume callback. The helper re-configures the IRQCFG
registers for each relevant interrupt (evtq, gerr, priq) via the new
platform_device_msi_rewrite() helper.
In addition, clear ARM_SMMU_FEAT_MSI if MSI setup fails and the driver
falls back to wired IRQs, preventing spurious MSI resume attempts.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 32 +++++++++++++++++++++
1 file changed, 32 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 dc2fc02b5163..a8166e75db0b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4771,6 +4771,9 @@ static void arm_smmu_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
phys_addr_t *cfg = arm_smmu_msi_cfg[desc->msi_index];
+ /* Cache the msi_msg for resume */
+ desc->msg = *msg;
+
doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
doorbell &= MSI_CFG0_ADDR_MASK;
@@ -4779,6 +4782,31 @@ static void arm_smmu_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
writel_relaxed(ARM_SMMU_MEMATTR_DEVICE_nGnRE, smmu->base + cfg[2]);
}
+static void __maybe_unused arm_smmu_resume_msis(struct arm_smmu_device *smmu)
+{
+ /* Clear the MSI address regs as they reset to unknown value */
+ writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
+ writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
+
+ if (smmu->features & ARM_SMMU_FEAT_PRI)
+ writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
+
+ if (!(smmu->features & ARM_SMMU_FEAT_MSI))
+ return;
+
+ if (!smmu->dev->msi.domain) {
+ dev_err(smmu->dev, "msi_domain absent during resume\n");
+ smmu->features &= ~ARM_SMMU_FEAT_MSI;
+ return;
+ }
+
+ platform_device_msi_rewrite(smmu->dev, smmu->gerr_irq, arm_smmu_write_msi_msg);
+ platform_device_msi_rewrite(smmu->dev, smmu->evtq.q.irq, arm_smmu_write_msi_msg);
+
+ if (smmu->features & ARM_SMMU_FEAT_PRI)
+ platform_device_msi_rewrite(smmu->dev, smmu->priq.q.irq, arm_smmu_write_msi_msg);
+}
+
static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
{
int ret, nvec = ARM_SMMU_MAX_MSIS;
@@ -4798,6 +4826,7 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
if (!dev->msi.domain) {
dev_info(smmu->dev, "msi_domain absent - falling back to wired irqs\n");
+ smmu->features &= ~ARM_SMMU_FEAT_MSI;
return;
}
@@ -4805,6 +4834,7 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
ret = platform_device_msi_init_and_alloc_irqs(dev, nvec, arm_smmu_write_msi_msg);
if (ret) {
dev_warn(dev, "failed to allocate MSIs - falling back to wired irqs\n");
+ smmu->features &= ~ARM_SMMU_FEAT_MSI;
return;
}
@@ -4918,6 +4948,8 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
irq = smmu->combined_irq;
if (irq) {
+ /* MSIs are unsupported for combined irq cases */
+ smmu->features &= ~ARM_SMMU_FEAT_MSI;
/*
* Cavium ThunderX2 implementation doesn't support unique irq
* lines. Use a single irq line for all the SMMUv3 interrupts.
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 09/15] iommu/arm-smmu-v3: Factor out arm_smmu_handle_gerror()
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (7 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 10/15] iommu/arm-smmu-v3: Add CMDQ_PROD_STOP_FLAG to gate CMDQ submissions Pranjal Shrivastava
` (5 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava,
Jason Gunthorpe
The GERROR register's state might be lost when the SMMU is powered down
during runtime suspend, requiring the suspend sequence to handle any
pending errors before the hardware state is lost.
Refactor the gerror handling logic into a helper function. Subsequent
patches will invoke it from the runtime suspend callback after disabling
the SMMU, ensuring that any late-breaking gerrors are logged and ack'ed
before the hardware state is lost.
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 +++++++++--
1 file changed, 9 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 a8166e75db0b..98af7c1fac94 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2406,10 +2406,10 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
static int arm_smmu_device_disable(struct arm_smmu_device *smmu);
-static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
+/* Lockless; must ensure that there are no concurrent callers */
+static irqreturn_t arm_smmu_handle_gerror(struct arm_smmu_device *smmu)
{
u32 gerror, gerrorn, active;
- struct arm_smmu_device *smmu = dev;
gerror = readl_relaxed(smmu->base + ARM_SMMU_GERROR);
gerrorn = readl_relaxed(smmu->base + ARM_SMMU_GERRORN);
@@ -2452,6 +2452,13 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
return IRQ_HANDLED;
}
+static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
+{
+ struct arm_smmu_device *smmu = dev;
+
+ return arm_smmu_handle_gerror(smmu);
+}
+
static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
{
struct arm_smmu_device *smmu = dev;
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 10/15] iommu/arm-smmu-v3: Add CMDQ_PROD_STOP_FLAG to gate CMDQ submissions
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (8 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 09/15] iommu/arm-smmu-v3: Factor out arm_smmu_handle_gerror() Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 11/15] iommu/tegra241-cmdqv: Add a helper to quiesce VCMDQs Pranjal Shrivastava
` (4 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
Introduce a new bit flag, CMDQ_PROD_STOP_FLAG (bit 30), in the command
queue's producer index to safely gate command submissions during device
suspension.
The flag embeds the suspend state directly into the existing global state
The flag is checked in the cmpxchg loop in arm_smmu_cmdq_issue_cmdlist(),
which acts as a Point of Commitment, ensuring that no indices are
reserved or committed once the SMMU begins suspending.
This prevents a situation of "abandoned batches" where indices are
incremented but commands are never written, which would otherwise
lead to timeout during the drain poll.
Update queue_inc_prod_n() to preserve this flag during index
calculations, ensuring that any in-flight commands that successfully
passed the point of commitment can proceed to completion while the
flag remains set.
Suggested-by: Daniel Mentz <danielmentz@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 35 +++++++++++++++++++--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +++
2 files changed, 38 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 98af7c1fac94..65939a9b0619 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -218,7 +218,8 @@ static int queue_sync_prod_in(struct arm_smmu_queue *q)
static u32 queue_inc_prod_n(struct arm_smmu_ll_queue *q, int n)
{
u32 prod = Q_POS(q, q->prod) + n;
- return Q_OVF(q->prod) | Q_POS(q, prod);
+
+ return Q_OVF(q->prod) | Q_STOP(q->prod) | Q_POS(q, prod);
}
static void queue_poll_init(struct arm_smmu_device *smmu,
@@ -726,13 +727,42 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
do {
u64 old;
+ /*
+ * If the SMMU is suspended/suspending, any new CMDs are elided.
+ * This loop is the Point of Commitment. If we haven't cmpxchg'd
+ * our new indices yet, we can safely bail. Once the indices are
+ * committed, we MUST write valid commands to those slots to
+ * avoid indefinite polling in the drain function.
+ */
+ if (Q_STOP(llq.prod)) {
+ local_irq_restore(flags);
+ return 0;
+ }
+
while (!queue_has_space(&llq, n + sync)) {
local_irq_restore(flags);
+
+ /* Avoid waiting for space if the SMMU is suspending */
+ if (Q_STOP(READ_ONCE(cmdq->q.llq.prod)))
+ return 0;
+
if (arm_smmu_cmdq_poll_until_not_full(smmu, cmdq, &llq))
dev_err_ratelimited(smmu->dev, "CMDQ timeout\n");
local_irq_save(flags);
}
+ /*
+ * If we exited the polling loop because the queue finally
+ * has space, but the STOP_FLAG was set during the poll,
+ * we must abort immediately. If we proceed, we will
+ * commit new commands beyond the Point of Commitment while
+ * the SMMU is suspending.
+ */
+ if (Q_STOP(llq.prod)) {
+ local_irq_restore(flags);
+ return 0;
+ }
+
head.cons = llq.cons;
head.prod = queue_inc_prod_n(&llq, n + sync) |
CMDQ_PROD_OWNED_FLAG;
@@ -779,7 +809,8 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
/* b. Stop gathering work by clearing the owned flag */
prod = atomic_fetch_andnot_relaxed(CMDQ_PROD_OWNED_FLAG,
&cmdq->q.llq.atomic.prod);
- prod &= ~CMDQ_PROD_OWNED_FLAG;
+ /* Strip all metadata flags */
+ prod &= CMDQ_PROD_IDX_MASK;
/*
* c. Wait for any gathered work to be written to the queue.
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 deefb17e31eb..794b258550dd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -394,6 +394,11 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
#define CMDQ_ERR_CERROR_ATC_INV_IDX 3
#define CMDQ_PROD_OWNED_FLAG Q_OVERFLOW_FLAG
+#define CMDQ_PROD_STOP_FLAG (1U << 30)
+#define Q_STOP(p) ((p) & CMDQ_PROD_STOP_FLAG)
+
+/* Mask out software-only metadata flags to get the pure queue index/wrap bits */
+#define CMDQ_PROD_IDX_MASK ~(CMDQ_PROD_STOP_FLAG | CMDQ_PROD_OWNED_FLAG)
struct arm_smmu_cmd {
u64 data[CMDQ_ENT_DWORDS];
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 11/15] iommu/tegra241-cmdqv: Add a helper to quiesce VCMDQs
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (9 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 10/15] iommu/arm-smmu-v3: Add CMDQ_PROD_STOP_FLAG to gate CMDQ submissions Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 12/15] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops Pranjal Shrivastava
` (3 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
The tegra241-cmdqv driver supports vCMDQs which need to be quiesced using
the STOP_FLAG. The current driver implementation only uses VINTF0 for
vCMDQs owned by the kernel which need to be stopped. Add a helper that
sets the CMDQ_PROD_STOP_FLAG on these vCMDQs.
Consolidate this logic by renaming the implementation hook to
quiesce_and_drain_queues and ensuring that the tegra241-cmdqv driver
gates all active local virtual queues before starting the drain loop.
Additionally, clear the STOP_FLAG in tegra241_vcmdq_hw_init() as a part
of tegra241_cmdqv_hw_reset().
Suggested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 4 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +-
.../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 77 ++++++++++++++++++-
3 files changed, 76 insertions(+), 7 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 65939a9b0619..7eb939888735 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1077,8 +1077,8 @@ static int __maybe_unused arm_smmu_drain_cmdqs(struct arm_smmu_device *smmu)
ret = arm_smmu_drain_queue(smmu, &smmu->cmdq.q, true);
/* Drain all implementation-specific queues */
- if (smmu->impl_ops && smmu->impl_ops->drain_queues) {
- err = smmu->impl_ops->drain_queues(smmu);
+ if (smmu->impl_ops && smmu->impl_ops->quiesce_and_drain_queues) {
+ err = smmu->impl_ops->quiesce_and_drain_queues(smmu);
if (err)
ret = err;
}
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 794b258550dd..0a841441cc44 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -896,7 +896,7 @@ struct arm_smmu_impl_ops {
size_t (*get_viommu_size)(enum iommu_viommu_type viommu_type);
int (*vsmmu_init)(struct arm_vsmmu *vsmmu,
const struct iommu_user_data *user_data);
- int (*drain_queues)(struct arm_smmu_device *smmu);
+ int (*quiesce_and_drain_queues)(struct arm_smmu_device *smmu);
};
/* An SMMUv3 instance */
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index c7989fcd2c62..61847f2802a3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -447,6 +447,54 @@ tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu,
return &vcmdq->cmdq;
}
+static void tegra241_cmdqv_quiesce_vintf0_lvcmdqs(struct arm_smmu_device *smmu)
+{
+ struct tegra241_cmdqv *cmdqv =
+ container_of(smmu, struct tegra241_cmdqv, smmu);
+ struct tegra241_vintf *vintf = cmdqv->vintfs[0];
+ u16 lidx;
+
+ if (!READ_ONCE(vintf->enabled))
+ return;
+
+ for (lidx = 0; lidx < cmdqv->num_lvcmdqs_per_vintf; lidx++) {
+ struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
+
+ if (!vcmdq || !READ_ONCE(vcmdq->enabled))
+ continue;
+
+ atomic_or(CMDQ_PROD_STOP_FLAG, &vcmdq->cmdq.q.llq.atomic.prod);
+ }
+}
+
+static void tegra241_vcmdq_wait_quiescent(struct arm_smmu_device *smmu,
+ struct tegra241_vcmdq *vcmdq)
+{
+ u32 target = READ_ONCE(vcmdq->cmdq.q.llq.prod) & CMDQ_PROD_IDX_MASK;
+ int timeout = ARM_SMMU_POLL_TIMEOUT_US;
+
+ /* Wait for the last committed owner to reach the hardware */
+ while (atomic_read(&vcmdq->cmdq.owner_prod) != target && timeout) {
+ udelay(1);
+ timeout--;
+ }
+
+ if (!timeout)
+ dev_err(smmu->dev, "vintf0 lvcmdq%u owner wait timeout\n",
+ vcmdq->lidx);
+
+ /* Wait for queue lock to be released */
+ timeout = ARM_SMMU_POLL_TIMEOUT_US;
+ while (atomic_read(&vcmdq->cmdq.lock) != 0 && timeout) {
+ udelay(1);
+ timeout--;
+ }
+
+ if (!timeout)
+ dev_err(smmu->dev, "vintf0 lvcmdq%u lock wait timeout\n",
+ vcmdq->lidx);
+}
+
static int tegra241_cmdqv_drain_vintf0_lvcmdqs(struct arm_smmu_device *smmu)
{
struct tegra241_cmdqv *cmdqv =
@@ -467,6 +515,21 @@ static int tegra241_cmdqv_drain_vintf0_lvcmdqs(struct arm_smmu_device *smmu)
if (!READ_ONCE(vintf->enabled))
return 0;
+ /*
+ * Gate all vCMDQs by setting the STOP_FLAG in a separate,
+ * initial loop to ensure no new commands can be submitted
+ * to any secondary queue while we are waiting to drain them.
+ *
+ * Client devices are suspended at this point due to devlinks,
+ * ensuring no concurrent command submissions race with this
+ * drain sequence.
+ */
+ tegra241_cmdqv_quiesce_vintf0_lvcmdqs(smmu);
+
+ /* Ensure all CPUs observe the STOP_FLAG before draining */
+ smp_mb();
+
+ /* Now that all queues are safely gated, drain them sequentially. */
for (lidx = 0; lidx < cmdqv->num_lvcmdqs_per_vintf; lidx++) {
struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
int rc;
@@ -474,6 +537,9 @@ static int tegra241_cmdqv_drain_vintf0_lvcmdqs(struct arm_smmu_device *smmu)
if (!vcmdq || !READ_ONCE(vcmdq->enabled))
continue;
+ /* Wait for the last committed owner to reach the hardware */
+ tegra241_vcmdq_wait_quiescent(smmu, vcmdq);
+
rc = arm_smmu_drain_queue(smmu, &vcmdq->cmdq.q, true);
if (rc) {
/*
@@ -489,7 +555,7 @@ static int tegra241_cmdqv_drain_vintf0_lvcmdqs(struct arm_smmu_device *smmu)
}
/* Avoid consuming stale commands on resume */
- vcmdq->cmdq.q.llq.cons = vcmdq->cmdq.q.llq.prod;
+ vcmdq->cmdq.q.llq.cons = vcmdq->cmdq.q.llq.prod & CMDQ_PROD_IDX_MASK;
}
return ret;
@@ -566,7 +632,6 @@ 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));
-
/*
* HW Registers reset to 0 when power-cycled. Restore them from their
* SW copies to prevent executing stale/ghost commands after resume.
@@ -574,7 +639,8 @@ static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)
* to the Guests since the relevant frameworks (IOMMUFD / VFIO) hold
* active PM references preventing suspend while VMs are active.
*/
- writel_relaxed(vcmdq->cmdq.q.llq.prod, REG_VCMDQ_PAGE0(vcmdq, PROD));
+ writel_relaxed(vcmdq->cmdq.q.llq.prod & CMDQ_PROD_IDX_MASK,
+ REG_VCMDQ_PAGE0(vcmdq, PROD));
writel_relaxed(vcmdq->cmdq.q.llq.cons, REG_VCMDQ_PAGE0(vcmdq, CONS));
ret = vcmdq_write_config(vcmdq, VCMDQ_EN);
@@ -587,6 +653,9 @@ static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)
return ret;
}
+ /* Clear the CMDQ_PROD_STOP_FLAG */
+ atomic_andnot(CMDQ_PROD_STOP_FLAG, &vcmdq->cmdq.q.llq.atomic.prod);
+
dev_dbg(vcmdq->cmdqv->dev, "%sinited\n", h);
return 0;
}
@@ -963,7 +1032,7 @@ static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
.device_reset = tegra241_cmdqv_hw_reset,
.device_disable = tegra241_cmdqv_hw_disable,
.device_remove = tegra241_cmdqv_remove,
- .drain_queues = tegra241_cmdqv_drain_vintf0_lvcmdqs,
+ .quiesce_and_drain_queues = tegra241_cmdqv_drain_vintf0_lvcmdqs,
/* For user-space use */
.hw_info = tegra241_cmdqv_hw_info,
.get_viommu_size = tegra241_cmdqv_get_vintf_size,
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 12/15] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (10 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 11/15] iommu/tegra241-cmdqv: Add a helper to quiesce VCMDQs Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 13/15] iommu/arm-smmu-v3: Enable pm_runtime and setup devlinks Pranjal Shrivastava
` (2 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
Implement pm_runtime and system sleep ops for arm-smmu-v3.
The suspend callback configures the SMMU to abort transactions, disables
the main translation unit and then drains the command queue. A software
gate (STOP_FLAG) quiesces submissions before power-off. Since devlinks
ensure client devices are suspended before the SMMU, no client DMA can
occur, making EVTQ/PRIQ IRQ synchronization during suspend unnecessary.
Prod indices for EVTQ/PRIQ are synchronized via queue_sync_prod_in() to
retain unread entries across power cycles.
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. The STOP_FLAG
is cleared only after the CMDQ is enabled in hardware.
Standard SET_RUNTIME_PM_OPS() and SET_SYSTEM_SLEEP_PM_OPS() macros are
used to define dev_pm_ops, safely evaluating to NO_OPs when CONFIG_PM is
disabled. The new RPM helpers are marked __maybe_unused to keep
intermediate commits clean until invoked by respective handlers in the
subsequent patches.
Suggested-by: Daniel Mentz <danielmentz@google.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 255 +++++++++++++++++++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 15 ++
2 files changed, 265 insertions(+), 5 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 7eb939888735..1c5d891564e4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -29,6 +29,7 @@
#include <linux/platform_device.h>
#include <linux/sort.h>
#include <linux/string_choices.h>
+#include <linux/pm_runtime.h>
#include <kunit/visibility.h>
#include <uapi/linux/iommufd.h>
@@ -119,6 +120,45 @@ static const char * const event_class_str[] = {
static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
static bool arm_smmu_ats_supported(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))
+ return 0;
+
+ 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 bool arm_smmu_rpm_get_if_active(struct arm_smmu_device *smmu)
+{
+ if (!pm_runtime_enabled(smmu->dev))
+ return true;
+
+ return pm_runtime_get_if_active(smmu->dev) > 0;
+}
+
+__maybe_unused static void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
+{
+ int ret;
+
+ if (!pm_runtime_enabled(smmu->dev))
+ return;
+
+ ret = pm_runtime_put_autosuspend(smmu->dev);
+
+ /* -EAGAIN & -EBUSY aren't failures */
+ if (ret < 0 && ret != -EAGAIN && ret != -EBUSY)
+ dev_err(smmu->dev, "failed to suspend device: %d\n", ret);
+}
+
static void parse_driver_options(struct arm_smmu_device *smmu)
{
int i = 0;
@@ -729,10 +769,66 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
/*
* If the SMMU is suspended/suspending, any new CMDs are elided.
- * This loop is the Point of Commitment. If we haven't cmpxchg'd
- * our new indices yet, we can safely bail. Once the indices are
- * committed, we MUST write valid commands to those slots to
- * avoid indefinite polling in the drain function.
+ *
+ * Note that eliding ATC invalidations (CMDQ_OP_ATC_INV) is safe
+ * because client PCIe endpoints are guaranteed to be suspended
+ * (via device links) before the SMMU is suspended. With the PCIe
+ * links in a low-power state, no new TLPs can be transmitted.
+ * It is strictly the responsibility of the client/endpoint driver
+ * to quiesce DMA and ensure that the ATC state is cleared across
+ * power state transitions.
+ *
+ * This loop acts as the Point of Commitment.
+ * The CMDQ_PROD_STOP_FLAG ensures that no new commands are
+ * committed once the SMMU begins to suspend. The synchronization
+ * relies on the following observability invariants:
+ *
+ * 1. Other CPUs observe the STOP_FLAG only *after* the SMMU is
+ * disabled. This is enforced in arm_smmu_runtime_suspend()
+ * by using a fully ordered atomic_fetch_or() to set the flag,
+ * guaranteeing that SMMUEN=0 (with ABORT set) at the time of
+ * observation which ensures no in-memory structures are
+ * accessed by the SMMU (IHI0070 spec section 6.3.9.6).
+ *
+ * 2. Other CPUs observe the cleared STOP_FLAG before the SMMU
+ * is re-enabled. During resume, arm_smmu_device_reset()
+ * issues CFGI_ALL and TLBI_ALL commands *after* clearing the
+ * STOP_FLAG and before setting SMMUEN=1. The implicit
+ * dma_wmb() executed while submitting these commands ensures
+ * the cleared STOP_FLAG is visible to all other agents.
+ * Thus, any transition from a set STOP_FLAG to SMMUEN=1
+ * involves an invalidate-all operation prior to setting SMMUEN=1.
+ *
+ * Hence, if a CPU observes the STOP_FLAG, it is assured that:
+ * (a) Txns are blocked + No in-memory structures are accessed
+ * (b) If the SMMU is ever re-enabled, an invalidate-all is
+ * performed prior to it being enabled during reset.
+ *
+ * Note: The smp_mb() in arm_smmu_domain_inv_range() orders the
+ * PTE update before the STOP_FLAG read, which ensures that if
+ * CPU1 reads the STOP_FLAG and decides to elide the command,
+ * the PTE update is already globally visible.
+ *
+ * [CPU0] | [CPU1]
+ * arm_smmu_runtime_suspend() { | [PTE update]
+ * SMMUEN = 0; | arm_smmu_domain_inv_range() {
+ * // set STOP_FLAG | smp_mb();
+ * target = atomic_fetch_or(); | arm_smmu_cmdq_issue_cmdlist() {
+ * while (owner != target) | // read STOP_FLAG
+ * // wait for completion | Q_STOP(llq.prod);
+ * arm_smmu_drain_cmdqs(); | // reserve indices
+ * } | cmpxchg(&cmdq->q.llq.atomic.prod);
+ * ... | queue_write();
+ * arm_smmu_device_reset() { | }
+ * // clear STOP_FLAG | }
+ * atomic_andnot(); |
+ * [Invalidate all TLB & CFG] |
+ * SMMUEN = 1; |
+ * } |
+ *
+ * If CPU1 hasn't cmpxchg'd its new indices yet, it observes the STOP_FLAG
+ * and safely bails. Once the indices are committed, CPU1 MUST write valid
+ * commands to those slots to avoid indefinite polling in CPU0's drain path.
*/
if (Q_STOP(llq.prod)) {
local_irq_restore(flags);
@@ -5087,7 +5183,8 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool resume)
/* Command queue */
writeq_relaxed(smmu->cmdq.q.q_base, smmu->base + ARM_SMMU_CMDQ_BASE);
- writel_relaxed(smmu->cmdq.q.llq.prod, smmu->base + ARM_SMMU_CMDQ_PROD);
+ writel_relaxed(smmu->cmdq.q.llq.prod & CMDQ_PROD_IDX_MASK,
+ smmu->base + ARM_SMMU_CMDQ_PROD);
writel_relaxed(smmu->cmdq.q.llq.cons, smmu->base + ARM_SMMU_CMDQ_CONS);
enables = CR0_CMDQEN;
@@ -5098,6 +5195,9 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool resume)
return ret;
}
+ /* Clear the STOP_FLAG to resume CMDQ submissions */
+ atomic_andnot(CMDQ_PROD_STOP_FLAG, &smmu->cmdq.q.llq.atomic.prod);
+
/* Invalidate any cached configuration */
arm_smmu_cmdq_issue_cmd_with_sync(smmu, arm_smmu_make_cmd_cfgi_all());
@@ -5849,6 +5949,150 @@ 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);
+ struct arm_smmu_cmdq *cmdq = &smmu->cmdq;
+ int timeout = ARM_SMMU_SUSPEND_TIMEOUT_US;
+ u32 enables, target;
+ int ret;
+
+ /* 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.
+ *
+ * Note on EVTQ/PRIQ: Due to device links between client devices and
+ * the SMMU, all masters are already runtime suspended and quiescent.
+ * As client DMA is stopped, no new translation faults (EVTQ) or
+ * Page Requests (PRIQ) can be generated, making it safe to disable
+ * these queues without an explicit drain.
+ */
+ enables = CR0_CMDQEN;
+ ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0, ARM_SMMU_CR0ACK);
+ if (ret) {
+ /* GBPA comes into effect when CR0.SMMUEN = 0, no rollback needed */
+ dev_err(smmu->dev, "failed to disable SMMU\n");
+ 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).
+ */
+
+ /*
+ * Mark the primary CMDQ to stop and get the target index before the stop.
+ *
+ * Note that the primary CMDQ's STOP_FLAG acts as a proxy for the SMMU's
+ * global power state. Because all queues are gated synchronously during
+ * suspend, checking the primary queue's flag is sufficient.
+ */
+ target = atomic_fetch_or(CMDQ_PROD_STOP_FLAG, &cmdq->q.llq.atomic.prod);
+ target &= CMDQ_PROD_IDX_MASK;
+
+
+ /* Wait for the last committed owner to reach the hardware */
+ while (atomic_read(&cmdq->owner_prod) != target && timeout) {
+ udelay(1);
+ timeout--;
+ }
+
+ /*
+ * Entering suspend implies no active clients. A timeout here
+ * indicates a fatal CMDQ lockup or hardware stall. We proceed
+ * anyway to prioritize memory safety (avoiding stale TLBs)
+ */
+ if (!timeout)
+ dev_err(smmu->dev, "cmdq owner wait timeout, (check runtime PM + devlinks)\n");
+
+ /* Wait for cmdq->lock == 0 to ensure last CMDQ_CONS_REG is written */
+ timeout = ARM_SMMU_SUSPEND_TIMEOUT_US;
+ while (atomic_read(&cmdq->lock) != 0 && timeout) {
+ udelay(1);
+ timeout--;
+ }
+
+ /* Timing out here implies misconfigured Runtime PM or broken devlinks */
+ if (!timeout)
+ dev_err(smmu->dev, "cmdq lock != 0, forcing suspend. Polling CPUs may fault.\n");
+
+ /* Drain the CMDQs */
+ ret = arm_smmu_drain_cmdqs(smmu);
+ if (ret)
+ dev_warn(smmu->dev, "failed to drain queues, forcing suspend\n");
+
+ /* Disable the SMMU */
+ arm_smmu_device_disable(smmu);
+
+ /* Disable IRQ generation */
+ arm_smmu_disable_irqs(smmu);
+
+ /* Wait for pending gerror handlers */
+ synchronize_irq(smmu->combined_irq ? smmu->combined_irq : smmu->gerr_irq);
+
+ /* Handle any pending gerrors before powering down */
+ arm_smmu_handle_gerror(smmu);
+
+ /* Sync prod pointer for EVTQ and PRIQ to avoid clobbering unread entries on resume */
+ if (queue_sync_prod_in(&smmu->evtq.q) == -EOVERFLOW)
+ dev_warn(smmu->dev, "EVTQ overflow detected during suspend\n");
+
+ if (smmu->features & ARM_SMMU_FEAT_PRI) {
+ if (queue_sync_prod_in(&smmu->priq.q) == -EOVERFLOW)
+ dev_warn(smmu->dev, "PRIQ overflow detected during suspend\n");
+ }
+
+ /* Avoid consuming stale commands on resume if we timed-out */
+ cmdq->q.llq.cons = cmdq->q.llq.prod & CMDQ_PROD_IDX_MASK;
+
+ dev_dbg(dev, "suspended smmu\n");
+
+ return 0;
+}
+
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+ int ret;
+
+ /* Re-configure MSIs */
+ arm_smmu_resume_msis(smmu);
+
+ /* Clears the CMDQ_PROD_STOP_FLAG as well */
+ ret = arm_smmu_device_reset(smmu, true);
+ if (ret)
+ dev_err(dev, "failed to reset during resume operation: %d\n", ret);
+
+ dev_dbg(dev, "resumed smmu\n");
+
+ return ret;
+}
+
+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 int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+{
+ if (pm_runtime_suspended(dev))
+ return 0;
+
+ return arm_smmu_runtime_resume(dev);
+}
+
+static const struct dev_pm_ops arm_smmu_pm_ops = {
+ 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)
+};
+
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v3", },
{ },
@@ -5865,6 +6109,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 0a841441cc44..d0a3d497915c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -663,11 +663,14 @@ arm_smmu_make_cmd_tlbi(enum arm_smmu_cmdq_opcode op, u16 asid, u16 vmid)
/* High-level queue structures */
#define ARM_SMMU_POLL_TIMEOUT_US 1000000 /* 1s! */
+#define ARM_SMMU_SUSPEND_TIMEOUT_US 1000000 /* 1s! */
#define ARM_SMMU_POLL_SPIN_COUNT 10
#define MSI_IOVA_BASE 0x8000000
#define MSI_IOVA_LENGTH 0x100000
+#define RPM_AUTOSUSPEND_DELAY_MS 15
+
struct arm_smmu_ll_queue {
union {
u64 val;
@@ -1234,6 +1237,18 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
bool sync);
bool arm_smmu_erratum_repeat_tlbi_cfgi(void);
+/*
+ * Lockless pre-check to test if the SMMU is actively powered.
+ * Races with concurrent suspend are benign: the cmpxchg loop in
+ * arm_smmu_cmdq_issue_cmdlist() acts as the true commit point.
+ * If we lose the race, that loop observes Q_STOP == 1 and safely
+ * drops the command. If we win, the suspend thread waits for us.
+ */
+static inline bool arm_smmu_is_active(struct arm_smmu_device *smmu)
+{
+ return !Q_STOP(READ_ONCE(smmu->cmdq.q.llq.prod));
+}
+
#ifdef CONFIG_ARM_SMMU_V3_SVA
bool arm_smmu_sva_supported(struct arm_smmu_device *smmu);
void arm_smmu_sva_notifier_synchronize(void);
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 13/15] iommu/arm-smmu-v3: Enable pm_runtime and setup devlinks
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (11 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 12/15] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 14/15] iommu/arm-smmu-v3: Invoke pm_runtime before hw access Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 15/15] iommu/arm-smmu-v3: Add KUnit unit tests for Runtime PM Pranjal Shrivastava
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
Enable PM runtime for SMMUs having a power-domain during smmu probe.
Add a devlink between the clients and SMMU device. The absence of a
power domain effectively disables runtime power management.
Additionally, handle runtime PM teardown in arm_smmu_disable_action()
and skip MMIO accesses during teardown and shutdown if the SMMU is
already runtime suspended, as translation is already disabled and the
hardware is in a low-power state.
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 49 +++++++++++++++++++--
1 file changed, 46 insertions(+), 3 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 1c5d891564e4..0a83d495d8d2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4511,8 +4511,28 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
if (ret)
goto err_disable_pasid;
+ /*
+ * If linking fails on an RPM-enabled SMMU, abort probe to prevent
+ * DMA transactions while the SMMU is autosuspended. Otherwise, warn
+ * and continue since the SMMU is always powered.
+ */
+ if (!device_link_add(dev, smmu->dev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER)) {
+ if (pm_runtime_enabled(smmu->dev)) {
+ dev_err(smmu->dev, "failed to add devlink to %s\n",
+ dev_name(dev));
+ ret = -ENOMEM;
+ goto err_free_cd_tables;
+ }
+
+ dev_warn(smmu->dev, "failed to add devlink to %s\n", dev_name(dev));
+ }
+
return &smmu->iommu;
+err_free_cd_tables:
+ if (arm_smmu_cdtab_allocated(&master->cd_table))
+ arm_smmu_free_cd_tables(master);
err_disable_pasid:
arm_smmu_disable_pasid(master);
arm_smmu_remove_master(master);
@@ -5116,9 +5136,17 @@ static void arm_smmu_disable_action(void *data)
{
struct arm_smmu_device *smmu = data;
- if (smmu->impl_ops && smmu->impl_ops->device_disable)
- smmu->impl_ops->device_disable(smmu);
- arm_smmu_device_disable(smmu);
+ /* If the SMMU is already suspended, nothing to do */
+ if (!pm_runtime_suspended(smmu->dev)) {
+ if (smmu->impl_ops && smmu->impl_ops->device_disable)
+ smmu->impl_ops->device_disable(smmu);
+ arm_smmu_device_disable(smmu);
+ }
+
+ if (pm_runtime_enabled(smmu->dev)) {
+ pm_runtime_dont_use_autosuspend(smmu->dev);
+ pm_runtime_disable(smmu->dev);
+ }
}
static void arm_smmu_write_strtab(struct arm_smmu_device *smmu)
@@ -5931,6 +5959,17 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return ret;
}
+ /*
+ * Safe to enable RPM here as client devices cannot be probed
+ * before this probe function successfully returns.
+ */
+ if (dev->pm_domain) {
+ pm_runtime_set_active(dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, RPM_AUTOSUSPEND_DELAY_MS);
+ pm_runtime_enable(dev);
+ }
+
return 0;
}
@@ -5946,6 +5985,10 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+ /* If the SMMU is already suspended, nothing to do */
+ if (pm_runtime_suspended(&pdev->dev))
+ return;
+
arm_smmu_device_disable(smmu);
}
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 14/15] iommu/arm-smmu-v3: Invoke pm_runtime before hw access
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (12 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 13/15] iommu/arm-smmu-v3: Enable pm_runtime and setup devlinks Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 15/15] iommu/arm-smmu-v3: Add KUnit unit tests for Runtime PM Pranjal Shrivastava
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
Invoke the pm_runtime helpers at all places before accessing the hw.
The idea is to invoke runtime_pm helpers at common points which are used
by exposed ops or interrupt handlers. TLB and CFG invalidations are
elided if the SMMU is suspended by observing the CMDQ_PROD_STOP_FLAG.
In the threaded interrupt handlers (EVTQ/PRIQ), arm_smmu_rpm_get_if_active()
is used to avoid sleeping and deadlocking against synchronize_irq() during
suspend. Because device links guarantee client devices remain suspended
and cannot issue DMA until the SMMU has fully transitioned to RPM_ACTIVE,
no client-generated events can be received or dropped during transient
states like RPM_RESUMING.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
.../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 20 ++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 163 +++++++++++++++++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 +
3 files changed, 175 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
index 25982bdbcbd9..7150b9c497a4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
@@ -15,17 +15,28 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length,
struct iommu_hw_info_arm_smmuv3 *info;
u32 __iomem *base_idr;
unsigned int i;
+ int ret;
+
+ ret = arm_smmu_rpm_get(master->smmu);
+ if (ret < 0)
+ return ERR_PTR(-EIO);
if (*type != IOMMU_HW_INFO_TYPE_DEFAULT &&
*type != IOMMU_HW_INFO_TYPE_ARM_SMMUV3) {
- if (!impl_ops || !impl_ops->hw_info)
- return ERR_PTR(-EOPNOTSUPP);
- return impl_ops->hw_info(master->smmu, length, type);
+ void *ret_ptr = ERR_PTR(-EOPNOTSUPP);
+
+ if (impl_ops && impl_ops->hw_info)
+ ret_ptr = impl_ops->hw_info(master->smmu, length, type);
+
+ arm_smmu_rpm_put(master->smmu);
+ return ret_ptr;
}
info = kzalloc_obj(*info);
- if (!info)
+ if (!info) {
+ arm_smmu_rpm_put(master->smmu);
return ERR_PTR(-ENOMEM);
+ }
base_idr = master->smmu->base + ARM_SMMU_IDR0;
for (i = 0; i <= 5; i++)
@@ -39,6 +50,7 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length,
*length = sizeof(*info);
*type = IOMMU_HW_INFO_TYPE_ARM_SMMUV3;
+ arm_smmu_rpm_put(master->smmu);
return info;
}
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 0a83d495d8d2..8e4ce5eb2228 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -121,7 +121,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
static bool arm_smmu_ats_supported(struct arm_smmu_master *master);
/* Runtime PM helpers */
-__maybe_unused static int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
+int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
{
int ret;
@@ -137,7 +137,7 @@ __maybe_unused static int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
return 0;
}
-__maybe_unused static bool arm_smmu_rpm_get_if_active(struct arm_smmu_device *smmu)
+static bool arm_smmu_rpm_get_if_active(struct arm_smmu_device *smmu)
{
if (!pm_runtime_enabled(smmu->dev))
return true;
@@ -145,7 +145,7 @@ __maybe_unused static bool arm_smmu_rpm_get_if_active(struct arm_smmu_device *sm
return pm_runtime_get_if_active(smmu->dev) > 0;
}
-__maybe_unused static void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
+void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
{
int ret;
@@ -1186,6 +1186,7 @@ static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused
struct iommu_page_response *resp)
{
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+ struct arm_smmu_device *smmu = master->smmu;
u8 resume_resp;
if (WARN_ON(!master->stall_enabled))
@@ -1204,6 +1205,23 @@ static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused
break;
}
+ /*
+ * The SMMU is guaranteed to be active via device_link if any master is
+ * active. Furthermore, on suspend we set GBPA to abort, flushing any
+ * pending stalled transactions.
+ *
+ * Receiving a page fault while suspended or suspending implies a stale
+ * event when the master has already powered down. A lockless pre-check
+ * on STOP_FLAG avoids taking the PM lock if already suspended. If active,
+ * arm_smmu_rpm_get_if_active() atomically acquires a PM reference under
+ * power.lock, avoiding races against concurrent suspend and rejecting
+ * responses during transient states like RPM_SUSPENDING.
+ */
+ if (!arm_smmu_is_active(smmu) || !arm_smmu_rpm_get_if_active(smmu)) {
+ dev_err(smmu->dev, "Ignoring page fault while suspended / suspending\n");
+ return;
+ }
+
arm_smmu_cmdq_issue_cmd(master->smmu,
arm_smmu_make_cmd_resume(master->streams[0].id,
resp->grpid,
@@ -1214,6 +1232,7 @@ static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused
* terminated... at some point in the future. PRI_RESP is fire and
* forget.
*/
+ arm_smmu_rpm_put(smmu);
}
/* Invalidation array manipulation functions */
@@ -1739,7 +1758,6 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master,
smmu, &cmds,
arm_smmu_make_cmd_cfgi_cd(master->streams[i].id, ssid,
leaf));
-
arm_smmu_cmdq_batch_submit(smmu, &cmds);
}
@@ -2050,9 +2068,9 @@ static void arm_smmu_ste_writer_sync_entry(struct arm_smmu_entry_writer *writer)
{
struct arm_smmu_ste_writer *ste_writer =
container_of(writer, struct arm_smmu_ste_writer, writer);
+ struct arm_smmu_device *smmu = writer->master->smmu;
- arm_smmu_cmdq_issue_cmd_with_sync(
- writer->master->smmu,
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu,
arm_smmu_make_cmd_cfgi_ste(ste_writer->sid, true));
}
@@ -2460,6 +2478,40 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);
+ /*
+ * Use a non-sleeping get to avoid a circular dependency deadlock
+ * with arm_smmu_runtime_suspend().
+ *
+ * When using a combined_irq, the suspend thread waits for pending
+ * threaded handlers to complete. If the IRQ thread blocks waiting
+ * for the PM core, it creates a deadlock:
+ *
+ * [Suspend Thread] | [IRQ Thread]
+ * pm_runtime_suspend() |
+ * state = RPM_SUSPENDING; |
+ * | IRQ fires
+ * | arm_smmu_rpm_get()
+ * | sleeps (waiting for suspend)
+ * arm_smmu_runtime_suspend() |
+ * ... |
+ * synchronize_irq() |
+ * sleeps (waiting for IRQ) |
+ *
+ * <==== DEADLOCK ====>
+ *
+ * A non-sleeping get allows the thread to instantly drop the event
+ * if the device is suspending, safely bypassing the synchronize_irq()
+ * deadlock.
+ *
+ * Devlinks guarantee client devices cannot resume or issue DMA until
+ * the SMMU has fully transitioned to RPM_ACTIVE, ensuring no valid
+ * events can be generated during transient states like RPM_RESUMING.
+ */
+ if (!arm_smmu_rpm_get_if_active(smmu)) {
+ dev_warn_ratelimited(smmu->dev, "client device is suspended, dropping event\n");
+ return IRQ_HANDLED;
+ }
+
do {
while (!queue_remove_raw(q, evt)) {
arm_smmu_decode_event(smmu, evt, &event);
@@ -2480,6 +2532,7 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
/* Sync our overflow flag, as we believe we're up to speed */
queue_sync_cons_ovf(q);
+ arm_smmu_rpm_put(smmu);
return IRQ_HANDLED;
}
@@ -2518,6 +2571,18 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
struct arm_smmu_ll_queue *llq = &q->llq;
u64 evt[PRIQ_ENT_DWORDS];
+ /*
+ * Use non-sleeping get to avoid deadlock. Devlinks prevent page
+ * requests from arriving during transient states like RPM_RESUMING.
+ * (see the detailed comment in arm_smmu_evtq_thread)
+ * Page requests require active client masters, which are held in
+ * runtime suspend until the SMMU is fully RPM_ACTIVE.
+ */
+ if (!arm_smmu_rpm_get_if_active(smmu)) {
+ dev_warn_ratelimited(smmu->dev, "client device is suspended, dropping page request\n");
+ return IRQ_HANDLED;
+ }
+
do {
while (!queue_remove_raw(q, evt))
arm_smmu_handle_ppr(smmu, evt);
@@ -2528,6 +2593,7 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
/* Sync our overflow flag, as we believe we're up to speed */
queue_sync_cons_ovf(q);
+ arm_smmu_rpm_put(smmu);
return IRQ_HANDLED;
}
@@ -2582,8 +2648,40 @@ static irqreturn_t arm_smmu_handle_gerror(struct arm_smmu_device *smmu)
static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
{
struct arm_smmu_device *smmu = dev;
+ irqreturn_t ret;
- return arm_smmu_handle_gerror(smmu);
+ /*
+ * Global Errors are only processed if the SMMU is active.
+ *
+ * When the STOP_FLAG is set, the SMMU is disabled (CR0.SMMUEN = 0)
+ * Any errors captured during the quiesce/drain phase (suspend)
+ * will be handled by the explicit arm_smmu_handle_gerror() call at
+ * the end of the arm_smmu_runtime_suspend() callback before power
+ * down.
+ *
+ * During resume (RPM_RESUMING), the only operations that could
+ * trigger a gerror are the CMDQ invalidation commands which are
+ * only issued after the STOP_FLAG has already been cleared.
+ *
+ * Thus, no legitimate gerrors can ever occur while STOP_FLAG is
+ * set. Any interrupt received while !arm_smmu_is_active() is
+ * strictly spurious. Hence, returning IRQ_NONE correctly signals
+ * an unhandled IRQ without missing any valid hardware errors.
+ *
+ * A lockless check is favoured here over a dynamic PM core check
+ * since the runtime_pm_get_if_active would return false during
+ * transient states like RPM_RESUMING & ignore level-triggered
+ * interrupts.
+ */
+ if (!arm_smmu_is_active(smmu)) {
+ dev_err(smmu->dev,
+ "Ignoring gerror interrupt because the SMMU is suspended\n");
+ return IRQ_NONE;
+ }
+
+ ret = arm_smmu_handle_gerror(smmu);
+
+ return ret;
}
static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
@@ -2666,6 +2764,10 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master,
struct arm_smmu_cmd cmd;
struct arm_smmu_cmdq_batch cmds;
+ /* Shouldn't hit the WARN if there's no devlink inconsistency */
+ if (WARN_ON_ONCE(!arm_smmu_is_active(master->smmu)))
+ return 0;
+
cmd = arm_smmu_make_cmd_atc_inv_all(0, IOMMU_NO_PASID);
arm_smmu_cmdq_batch_init_cmd(master->smmu, &cmds, &cmd);
for (i = 0; i < master->num_streams; i++)
@@ -2912,7 +3014,16 @@ static void __arm_smmu_domain_inv_range(struct arm_smmu_invs *invs,
if (cmds.num &&
(next == end || arm_smmu_invs_end_batch(cur, next))) {
+
+ /*
+ * Concurrent suspend races are benign: the cmdq allocation cmpxchg
+ * loop acts as the serialization point to safely drop the batch
+ * without MMIO accesses. Concurrent resume is caught by the HW
+ * reset cache invalidation, ensuring state consistency.
+ */
arm_smmu_cmdq_batch_submit(smmu, &cmds);
+
+ /* Drop this batch to ensure the next one's fresh */
cmds.num = 0;
}
cur = next;
@@ -5135,14 +5246,37 @@ static int arm_smmu_device_disable(struct arm_smmu_device *smmu)
static void arm_smmu_disable_action(void *data)
{
struct arm_smmu_device *smmu = data;
+ int ret;
/* If the SMMU is already suspended, nothing to do */
if (!pm_runtime_suspended(smmu->dev)) {
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0) {
+ dev_err(smmu->dev, "failed to resume device for disable: %d\n", ret);
+ goto disable_pm;
+ }
+
if (smmu->impl_ops && smmu->impl_ops->device_disable)
smmu->impl_ops->device_disable(smmu);
arm_smmu_device_disable(smmu);
+
+ /*
+ * A PM ref was only acquired via arm_smmu_rpm_get() to
+ * guarantee power for the MMIO register writes in
+ * arm_smmu_device_disable().
+ *
+ * Drop the PM ref, we intentionally do NOT invoke a
+ * synchronous put/suspend here since the HW was already
+ * disabled above. Calling put_sync would invoke
+ * arm_smmu_runtime_suspend(), redundantly attempting to
+ * do the same. Any pending autosuspend timer is cleanly
+ * cancelled by pm_runtime_disable() immediately below.
+ * The core code ensures the power management here.
+ */
+ arm_smmu_rpm_put(smmu);
}
+disable_pm:
if (pm_runtime_enabled(smmu->dev)) {
pm_runtime_dont_use_autosuspend(smmu->dev);
pm_runtime_disable(smmu->dev);
@@ -5984,12 +6118,27 @@ static void arm_smmu_device_remove(struct platform_device *pdev)
static void arm_smmu_device_shutdown(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+ int ret;
/* If the SMMU is already suspended, nothing to do */
if (pm_runtime_suspended(&pdev->dev))
return;
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to resume device for shutdown: %d\n", ret);
+ return;
+ }
+
arm_smmu_device_disable(smmu);
+ /*
+ * Drop the PM ref acquired for the MMIO disable write.
+ * An asynchronous put is intentional here as the HW is already
+ * disabled (CR0 disabled), and the system is in the middle of
+ * reboot/poweroff. Initiating a synchronous runtime suspend here
+ * would redundantly disable SMMU and delay system powerdown.
+ */
+ arm_smmu_rpm_put(smmu);
}
static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
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 d0a3d497915c..a018913d3bb8 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -1249,6 +1249,9 @@ static inline bool arm_smmu_is_active(struct arm_smmu_device *smmu)
return !Q_STOP(READ_ONCE(smmu->cmdq.q.llq.prod));
}
+int arm_smmu_rpm_get(struct arm_smmu_device *smmu);
+void arm_smmu_rpm_put(struct arm_smmu_device *smmu);
+
#ifdef CONFIG_ARM_SMMU_V3_SVA
bool arm_smmu_sva_supported(struct arm_smmu_device *smmu);
void arm_smmu_sva_notifier_synchronize(void);
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v10 15/15] iommu/arm-smmu-v3: Add KUnit unit tests for Runtime PM
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
` (13 preceding siblings ...)
2026-09-08 17:17 ` [PATCH v10 14/15] iommu/arm-smmu-v3: Invoke pm_runtime before hw access Pranjal Shrivastava
@ 2026-09-08 17:17 ` Pranjal Shrivastava
14 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 17:17 UTC (permalink / raw)
To: iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
Thomas Gleixner, driver-core, Pranjal Shrivastava
Introduce kunit selftests to verify the Runtime PM elision gating,
post-suspend elisions and progress on resumption under active
invalidation load. Simulate concurrent HW suspension using a timer.
Mock all HW registers and CMDQ buffers by allocating them on RAM.
Make the mock CMDQ self-consuming to avoid hitting queue_full scenarios.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 215 ++++++++++++++++++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
2 files changed, 216 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
index add671363c82..97c1e907717d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
@@ -3,6 +3,10 @@
* Copyright 2024 Google LLC.
*/
#include <kunit/test.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/sched.h>
+#include <linux/timer.h>
#include <linux/io-pgtable.h>
#include "arm-smmu-v3.h"
@@ -771,6 +775,215 @@ static void arm_smmu_v3_invs_test(struct kunit *test)
kfree(test_b);
}
+struct arm_smmu_mock_cmdq {
+ /*
+ * Mock register in RAM. Typed as __le32 to match writel_relaxed()
+ * endianness without calling readl_* accessors directly on RAM.
+ */
+ __le32 mock_prod_reg;
+};
+
+/* Helper to allocate a self-consuming mock cmdq */
+static void arm_smmu_v3_test_init_mock_cmdq(struct kunit *test,
+ struct arm_smmu_device *smmu,
+ struct arm_smmu_mock_cmdq *mock)
+{
+ struct arm_smmu_cmdq *cmdq = &smmu->cmdq;
+ unsigned long *mock_valid_map;
+ u64 *mock_base;
+
+ mock_base = kunit_kzalloc(test, 1024 * sizeof(struct arm_smmu_cmd), GFP_KERNEL);
+ mock_valid_map = kunit_kzalloc(test, BITS_TO_LONGS(1024) * sizeof(long), GFP_KERNEL);
+
+ KUNIT_ASSERT_NOT_NULL(test, mock_base);
+ KUNIT_ASSERT_NOT_NULL(test, mock_valid_map);
+
+ smmu->features = 0;
+ /* 1024 entries */
+ cmdq->q.llq.max_n_shift = 10;
+ cmdq->q.ent_dwords = CMDQ_ENT_DWORDS;
+ cmdq->q.base = (__le64 *)mock_base;
+ cmdq->valid_map = (atomic_long_t *)mock_valid_map;
+
+ /* Self-Consuming, prod == cons always ensures queue empty */
+ cmdq->q.prod_reg = (__force void __iomem *)&mock->mock_prod_reg;
+ cmdq->q.cons_reg = (__force void __iomem *)&mock->mock_prod_reg;
+
+ atomic_set(&cmdq->q.llq.atomic.prod, 0);
+ atomic_set(&cmdq->q.llq.atomic.cons, 0);
+ atomic_set(&cmdq->owner_prod, 0);
+ mock->mock_prod_reg = cpu_to_le32(0);
+}
+
+struct arm_smmu_test_timer_context {
+ struct arm_smmu_device *smmu;
+ struct timer_list timer;
+ bool suspended;
+};
+
+static void arm_smmu_v3_test_rpm_timer_callback(struct timer_list *t)
+{
+ struct arm_smmu_test_timer_context *ctx =
+ timer_container_of(ctx, t, timer);
+ struct arm_smmu_cmdq *cmdq = &ctx->smmu->cmdq;
+
+ /* Simulate a concurrent suspend event interrupting the invalidations */
+ atomic_or(CMDQ_PROD_STOP_FLAG, &cmdq->q.llq.atomic.prod);
+ WRITE_ONCE(ctx->suspended, true);
+}
+
+/*
+ * Verify SMMU PM Runtime gating, elision, and post-suspend resumption
+ * safety sequentially under active stress.
+ */
+static void arm_smmu_v3_rpm_test_stress_race(struct kunit *test)
+{
+ struct arm_smmu_cmd cmd = arm_smmu_make_cmd_cfgi_all();
+ struct arm_smmu_test_timer_context timer_ctx = {0};
+ struct arm_smmu_device mock_smmu = smmu;
+ struct arm_smmu_cmdq *cmdq = &mock_smmu.cmdq;
+ struct arm_smmu_mock_cmdq mock = {0};
+ u32 stopped_prod;
+ int i;
+
+ arm_smmu_v3_test_init_mock_cmdq(test, &mock_smmu, &mock);
+
+ timer_ctx.smmu = &mock_smmu;
+
+ timer_setup_on_stack(&timer_ctx.timer, arm_smmu_v3_test_rpm_timer_callback, 0);
+ mod_timer(&timer_ctx.timer, jiffies + msecs_to_jiffies(10));
+
+ /* Execute the unmap storm until the timer triggers */
+ while (!READ_ONCE(timer_ctx.suspended)) {
+ if (arm_smmu_cmdq_issue_cmdlist(&mock_smmu, cmdq, &cmd, 1, false))
+ break;
+ usleep_range(50, 100);
+ }
+
+ timer_shutdown_sync(&timer_ctx.timer);
+ timer_destroy_on_stack(&timer_ctx.timer);
+
+ /* Establish the post-storm prod_reg index */
+ stopped_prod = le32_to_cpu(mock.mock_prod_reg);
+
+ /*
+ * Attempt multiple unmaps while the SMMU is disabled (STOP_GATE is set)
+ * Every single invalidation must get elided and return 0. The prod_reg
+ * shall remain completely frozen after all of these submissions.
+ */
+ for (i = 0; i < 1000; i++) {
+ if (arm_smmu_cmdq_issue_cmdlist(&mock_smmu, cmdq, &cmd, 1, false))
+ break;
+ }
+ KUNIT_EXPECT_EQ(test, stopped_prod, le32_to_cpu(mock.mock_prod_reg));
+
+ /*
+ * Clear the STOP_FLAG (resume the SMMU). A new invalidation must
+ * now successfully commit prod_idx & move the prod_reg by exactly 1.
+ */
+ atomic_andnot(CMDQ_PROD_STOP_FLAG, &cmdq->q.llq.atomic.prod);
+ KUNIT_EXPECT_EQ(test, 0, arm_smmu_cmdq_issue_cmdlist(&mock_smmu, cmdq, &cmd, 1, false));
+ KUNIT_EXPECT_EQ(test, Q_POS(&cmdq->q.llq, stopped_prod + 1),
+ le32_to_cpu(mock.mock_prod_reg));
+}
+
+struct arm_smmu_test_kthread_context {
+ struct arm_smmu_device *smmu;
+ int error;
+};
+
+static int arm_smmu_v3_test_kthread_worker(void *data)
+{
+ struct arm_smmu_cmd cmd = arm_smmu_make_cmd_cfgi_all();
+ struct arm_smmu_test_kthread_context *ctx = data;
+ struct arm_smmu_cmdq *cmdq = &ctx->smmu->cmdq;
+
+ while (!kthread_should_stop()) {
+ if (arm_smmu_cmdq_issue_cmdlist(ctx->smmu, cmdq, &cmd, 1, false))
+ WRITE_ONCE(ctx->error, 1);
+ usleep_range(50, 100);
+ }
+ return 0;
+}
+
+KUNIT_DEFINE_ACTION_WRAPPER(kunit_action_kthread_stop, kthread_stop, struct task_struct *);
+
+static void arm_smmu_v3_rpm_test_kthread_race(struct kunit *test)
+{
+ struct arm_smmu_test_kthread_context *ctx;
+ struct arm_smmu_device *mock_smmu;
+ struct arm_smmu_mock_cmdq *mock;
+ struct task_struct *thread1, *thread2;
+ u32 stopped_prod;
+ int i;
+
+ ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ mock_smmu = kunit_kzalloc(test, sizeof(*mock_smmu), GFP_KERNEL);
+ mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ctx);
+ KUNIT_ASSERT_NOT_NULL(test, mock_smmu);
+ KUNIT_ASSERT_NOT_NULL(test, mock);
+
+ *mock_smmu = smmu;
+ ctx->smmu = mock_smmu;
+ arm_smmu_v3_test_init_mock_cmdq(test, mock_smmu, mock);
+
+ thread1 = kthread_run(arm_smmu_v3_test_kthread_worker, ctx, "smmu_w1");
+ if (IS_ERR(thread1)) {
+ KUNIT_FAIL(test, "Failed to spawn kthread 1\n");
+ return;
+ }
+ if (kunit_add_action_or_reset(test, kunit_action_kthread_stop, thread1)) {
+ KUNIT_FAIL(test, "Failed to register cleanup for kthread 1\n");
+ return;
+ }
+
+ thread2 = kthread_run(arm_smmu_v3_test_kthread_worker, ctx, "smmu_w2");
+ if (IS_ERR(thread2)) {
+ KUNIT_FAIL(test, "Failed to spawn kthread 2\n");
+ return;
+ }
+ if (kunit_add_action_or_reset(test, kunit_action_kthread_stop, thread2)) {
+ KUNIT_FAIL(test, "Failed to register cleanup for kthread 2\n");
+ return;
+ }
+
+ /* Wait for worker threads to start and issue initial commands */
+ for (i = 0; i < 20; i++) {
+ if (le32_to_cpu(READ_ONCE(mock->mock_prod_reg)) > 0)
+ break;
+ usleep_range(1000, 2000);
+ }
+ KUNIT_EXPECT_GT(test, le32_to_cpu(READ_ONCE(mock->mock_prod_reg)), 0);
+
+ /* Gate the CMDQ */
+ atomic_or(CMDQ_PROD_STOP_FLAG, &mock_smmu->cmdq.q.llq.atomic.prod);
+
+ /* Wait for in-flight submissions to settle */
+ for (i = 0; i < 50; i++) {
+ stopped_prod = le32_to_cpu(READ_ONCE(mock->mock_prod_reg));
+ usleep_range(1000, 2000);
+ if (stopped_prod == le32_to_cpu(READ_ONCE(mock->mock_prod_reg)))
+ break;
+ }
+
+ usleep_range(1000, 2000);
+ KUNIT_EXPECT_EQ(test, stopped_prod, le32_to_cpu(READ_ONCE(mock->mock_prod_reg)));
+
+ /* Open the gate and wait for worker threads to resume */
+ atomic_andnot(CMDQ_PROD_STOP_FLAG, &mock_smmu->cmdq.q.llq.atomic.prod);
+ for (i = 0; i < 20; i++) {
+ if (le32_to_cpu(READ_ONCE(mock->mock_prod_reg)) != stopped_prod)
+ break;
+ usleep_range(1000, 2000);
+ }
+ KUNIT_EXPECT_NE(test, stopped_prod, le32_to_cpu(READ_ONCE(mock->mock_prod_reg)));
+
+ kunit_release_action(test, kunit_action_kthread_stop, thread2);
+ kunit_release_action(test, kunit_action_kthread_stop, thread1);
+ KUNIT_EXPECT_EQ(test, 0, READ_ONCE(ctx->error));
+}
+
static struct kunit_case arm_smmu_v3_test_cases[] = {
KUNIT_CASE(arm_smmu_v3_write_ste_test_bypass_to_abort),
KUNIT_CASE(arm_smmu_v3_write_ste_test_abort_to_bypass),
@@ -797,6 +1010,8 @@ static struct kunit_case arm_smmu_v3_test_cases[] = {
KUNIT_CASE(arm_smmu_v3_write_cd_test_sva_clear),
KUNIT_CASE(arm_smmu_v3_write_cd_test_sva_release),
KUNIT_CASE(arm_smmu_v3_invs_test),
+ KUNIT_CASE(arm_smmu_v3_rpm_test_stress_race),
+ KUNIT_CASE(arm_smmu_v3_rpm_test_kthread_race),
{},
};
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 8e4ce5eb2228..12d191fe57ed 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -991,6 +991,7 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
return ret;
}
+EXPORT_SYMBOL_IF_KUNIT(arm_smmu_cmdq_issue_cmdlist);
static int arm_smmu_cmdq_issue_cmd_p(struct arm_smmu_device *smmu,
struct arm_smmu_cmd *cmd, bool sync)
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite()
2026-09-08 17:17 ` [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite() Pranjal Shrivastava
@ 2026-09-08 19:40 ` Thomas Gleixner
2026-09-08 20:15 ` Pranjal Shrivastava
2026-09-08 22:55 ` Jason Gunthorpe
0 siblings, 2 replies; 23+ messages in thread
From: Thomas Gleixner @ 2026-09-08 19:40 UTC (permalink / raw)
To: Pranjal Shrivastava, iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
driver-core, Pranjal Shrivastava, Jason Gunthorpe, Marc Zyngier
On Tue, Sep 08 2026 at 17:17, Pranjal Shrivastava wrote:
...
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Jason. You really should know better by now :(
> +/**
> + * platform_device_msi_rewrite - Rewrite the MSI config for a platform device
> + * @dev: The device for which to rewrite interrupt
> + * @irq: The interrupt number to be rewritten.
> + * @write_msi_msg: Callback to write the interrupt message for @dev
> + *
> + * Rewrites the cached MSI message for a platform device.
> + *
> + * Note: Platform MSI does not automatically cache composed messages. The caller's
> + * @write_msi_msg callback is expected to cache the message (e.g. into desc->msg)
> + * during initial configuration so it can be rewritten on resume.
> + */
> +void platform_device_msi_rewrite(struct device *dev, unsigned int irq,
> + irq_write_msi_msg_t write_msi_msg)
Why is this a platform device specific function and why does this need to
hand in the write_msi_msg() callback, which is already known through the
interrupt descriptor and the top level interrupt chip?
I spent an awful lot of time and effort to get rid of these platform MSI
layering violations and now you start adding the same mess again.
Not going to happen.
> +{
> + struct msi_desc *desc;
> + struct msi_msg msg;
> +
> + if (!irq || !write_msi_msg)
> + return;
Oh well.
> + desc = irq_get_msi_desc(irq);
> + if (!desc) {
> + dev_err(dev, "Failed to get MSI descriptor for irq %u\n", irq);
> + return;
> + }
Doing this without having the underlying interrupt descriptor locked is
a recipe for an undebuggable disaster waiting to happen. It might be
"safe" in the context you are calling it but it's absolutely not safe in
general.
> + __get_cached_msi_msg(desc, &msg);
> + if (!msg.address_hi && !msg.address_lo) {
> + dev_warn(dev, "No cached MSI message found for irq %u\n", irq);
That's just wrong. A message with a zero address is valid, e.g. when an
interrupt is shut down. So if there is random crap after resume in the
message store and the interrupt is valid, but not requested, then the
cached message still has to be written even if it is zero.
So this want's to be a function in the MSI core code. Also this is not a
per interrupt problem it is obviously a per device domain problem.
Simply because the device provides the message store for all MSI interrupts
which originate from that same device and therefore _all_ MSI interrupts
are affected by that, no?
So this all can be solved at the device domain level without sprinkling
per interrupt invocations including conditionals all over the place.
Something like the completely untested below should just work.
Thanks,
tglx
---
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -669,6 +669,8 @@ void msi_domain_free_irqs_all(struct dev
struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
+void msi_device_domain_restore_msi_msgs(struct device *dev, unsigned int domid);
+
/* Per device platform MSI */
int platform_device_msi_init_and_alloc_irqs(struct device *dev, unsigned int nvec,
irq_write_msi_msg_t write_msi_msg);
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -1775,3 +1775,34 @@ bool msi_device_has_isolated_msi(struct
return arch_is_isolated_msi();
}
EXPORT_SYMBOL_GPL(msi_device_has_isolated_msi);
+
+void msi_device_domain_restore_msi_msgs(struct device *dev, unsigned int domid)
+{
+ if (!dev->msi.data)
+ return;
+
+ guard(msi_descs_lock)(dev);
+ struct irq_domain *domain = msi_get_device_domain(dev, domid);
+
+ if (!domain || !irq_domain_is_msi_device(domain))
+ return;
+
+ struct xarray *xa = &dev->msi.data->__domains[domid].store;
+ struct msi_domain_info *info = domain->host_data;
+ struct msi_desc *msi_desc;
+ unsigned long idx;
+
+ xa_for_each_range(xa, idx, msi_desc, 0, info->hwsize) {
+ /* Only handle MSI entries which have an interrupt associated */
+ if (!msi_desc_match(msi_desc, MSI_DESC_ASSOCIATED))
+ continue;
+
+ scoped_irqdesc_get_and_lock(msi_desc->irq, 0) {
+ struct irq_data *data = irq_desc_get_irq_data(scoped_irqdesc);
+ struct msi_msg msg = msi_desc->msg;
+
+ if (data->chip)
+ irq_chip_write_msi_msg(data, &msg);
+ }
+ }
+}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config
2026-09-08 17:17 ` [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config Pranjal Shrivastava
@ 2026-09-08 19:56 ` Thomas Gleixner
2026-09-08 20:23 ` Pranjal Shrivastava
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Gleixner @ 2026-09-08 19:56 UTC (permalink / raw)
To: Pranjal Shrivastava, iommu
Cc: Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
driver-core, Pranjal Shrivastava
On Tue, Sep 08 2026 at 17:17, Pranjal Shrivastava wrote:
> +static void __maybe_unused arm_smmu_resume_msis(struct arm_smmu_device *smmu)
> +{
> + /* Clear the MSI address regs as they reset to unknown value */
> + writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
> + writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
> +
> + if (smmu->features & ARM_SMMU_FEAT_PRI)
> + writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
> +
> + if (!(smmu->features & ARM_SMMU_FEAT_MSI))
> + return;
> +
> + if (!smmu->dev->msi.domain) {
> + dev_err(smmu->dev, "msi_domain absent during resume\n");
> + smmu->features &= ~ARM_SMMU_FEAT_MSI;
> + return;
If dev->msi.domain == NULL then arm_smmu_setup_msis() already cleared
the MSI feature bit. Has it magically been set again or does resume run
before init or does dev->msi.domain magically disappear during suspend?
I'm all for defensive programming, but this is voodoo and not structured
defense.
Aside of that dev->msi.domain is the patently wrong condition. For
devices which instantiate a MSI device domain dev->msi.domain points to
the MSI parent domain and not to the actual relevant device domain.
You can't query that easily by chasing pointers (for a reason), but
there is no point to do so. See below and the patch I sent you.
> + platform_device_msi_rewrite(smmu->dev, smmu->gerr_irq, arm_smmu_write_msi_msg);
> + platform_device_msi_rewrite(smmu->dev, smmu->evtq.q.irq, arm_smmu_write_msi_msg);
> +
> + if (smmu->features & ARM_SMMU_FEAT_PRI)
> + platform_device_msi_rewrite(smmu->dev, smmu->priq.q.irq, arm_smmu_write_msi_msg);
And that's exactly the point I made about sprinkling this stuff all over
the place and thereby violating all layering rules.
Done correctly this whole function boils down to:
static void __maybe_unused arm_smmu_resume_msis(struct arm_smmu_device *smmu)
{
/* Clear the MSI address regs as they reset to unknown value */
writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
if (smmu->features & ARM_SMMU_FEAT_PRI)
writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
msi_device_domain_restore_msi_msgs(smmu->dev, 0);
}
It just works simply because the function returns early when there is no
domain or the domain is not a MSI device domain, which is correct
because there is nothing to do when nothing is set up.
But that results in too comprehensible code I fear.
Thanks,
tglx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite()
2026-09-08 19:40 ` Thomas Gleixner
@ 2026-09-08 20:15 ` Pranjal Shrivastava
2026-09-08 20:16 ` Pranjal Shrivastava
2026-09-09 9:20 ` Thomas Gleixner
2026-09-08 22:55 ` Jason Gunthorpe
1 sibling, 2 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 20:15 UTC (permalink / raw)
To: Thomas Gleixner
Cc: iommu, Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
driver-core, Jason Gunthorpe, Marc Zyngier
On Tue, Sep 08, 2026 at 09:40:49PM +0200, Thomas Gleixner wrote:
> On Tue, Sep 08 2026 at 17:17, Pranjal Shrivastava wrote:
>
> ...
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Jason. You really should know better by now :(
>
Apologies for the confusion, Jason just directed me to avoid open coding
this in the iommu driver and have the irq core handle it instead. [1]
Jason had nothing to do with the specific implementation / design here,
that misintepretation / mess is mine :(
I'm relatively less familiar with the MSI pieces, and gave it a go
(probably should've prefixed this patch with RFC). Sorry about that!
> > +/**
> > + * platform_device_msi_rewrite - Rewrite the MSI config for a platform device
> > + * @dev: The device for which to rewrite interrupt
> > + * @irq: The interrupt number to be rewritten.
> > + * @write_msi_msg: Callback to write the interrupt message for @dev
> > + *
> > + * Rewrites the cached MSI message for a platform device.
> > + *
> > + * Note: Platform MSI does not automatically cache composed messages. The caller's
> > + * @write_msi_msg callback is expected to cache the message (e.g. into desc->msg)
> > + * during initial configuration so it can be rewritten on resume.
> > + */
> > +void platform_device_msi_rewrite(struct device *dev, unsigned int irq,
> > + irq_write_msi_msg_t write_msi_msg)
>
> Why is this a platform device specific function and why does this need to
> hand in the write_msi_msg() callback, which is already known through the
> interrupt descriptor and the top level interrupt chip?
>
> I spent an awful lot of time and effort to get rid of these platform MSI
> layering violations and now you start adding the same mess again.
>
> Not going to happen.
>
Ack. I'll address the layering violations
> > +{
> > + struct msi_desc *desc;
> > + struct msi_msg msg;
> > +
> > + if (!irq || !write_msi_msg)
> > + return;
>
> Oh well.
>
> > + desc = irq_get_msi_desc(irq);
> > + if (!desc) {
> > + dev_err(dev, "Failed to get MSI descriptor for irq %u\n", irq);
> > + return;
> > + }
>
> Doing this without having the underlying interrupt descriptor locked is
> a recipe for an undebuggable disaster waiting to happen. It might be
> "safe" in the context you are calling it but it's absolutely not safe in
> general.
>
Ack. I was thinking about races but I assumed the descriptor shoudln't
change but that's a "happy" / unsafe assumption.
> > + __get_cached_msi_msg(desc, &msg);
> > + if (!msg.address_hi && !msg.address_lo) {
> > + dev_warn(dev, "No cached MSI message found for irq %u\n", irq);
>
> That's just wrong. A message with a zero address is valid, e.g. when an
> interrupt is shut down. So if there is random crap after resume in the
> message store and the interrupt is valid, but not requested, then the
> cached message still has to be written even if it is zero.
>
> So this want's to be a function in the MSI core code. Also this is not a
> per interrupt problem it is obviously a per device domain problem.
> Simply because the device provides the message store for all MSI interrupts
> which originate from that same device and therefore _all_ MSI interrupts
> are affected by that, no?
>
> So this all can be solved at the device domain level without sprinkling
> per interrupt invocations including conditionals all over the place.
>
Ack. I was wondering if the irq core should also cache the message for
platform MSIs like it's done for PCI ? Would that be a bad idea? Or is
it this way by design? (I'm having to cache the msg in the iommu driver
atm).
> Something like the completely untested below should just work.
>
Thanks for sharing this! I'll give it a go.
> Thanks,
>
> tglx
> ---
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -669,6 +669,8 @@ void msi_domain_free_irqs_all(struct dev
>
> struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
>
> +void msi_device_domain_restore_msi_msgs(struct device *dev, unsigned int domid);
> +
> /* Per device platform MSI */
> int platform_device_msi_init_and_alloc_irqs(struct device *dev, unsigned int nvec,
> irq_write_msi_msg_t write_msi_msg);
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -1775,3 +1775,34 @@ bool msi_device_has_isolated_msi(struct
> return arch_is_isolated_msi();
> }
> EXPORT_SYMBOL_GPL(msi_device_has_isolated_msi);
> +
> +void msi_device_domain_restore_msi_msgs(struct device *dev, unsigned int domid)
> +{
> + if (!dev->msi.data)
> + return;
> +
> + guard(msi_descs_lock)(dev);
> + struct irq_domain *domain = msi_get_device_domain(dev, domid);
> +
> + if (!domain || !irq_domain_is_msi_device(domain))
> + return;
> +
> + struct xarray *xa = &dev->msi.data->__domains[domid].store;
> + struct msi_domain_info *info = domain->host_data;
> + struct msi_desc *msi_desc;
> + unsigned long idx;
> +
> + xa_for_each_range(xa, idx, msi_desc, 0, info->hwsize) {
> + /* Only handle MSI entries which have an interrupt associated */
> + if (!msi_desc_match(msi_desc, MSI_DESC_ASSOCIATED))
> + continue;
> +
> + scoped_irqdesc_get_and_lock(msi_desc->irq, 0) {
> + struct irq_data *data = irq_desc_get_irq_data(scoped_irqdesc);
> + struct msi_msg msg = msi_desc->msg;
> +
> + if (data->chip)
> + irq_chip_write_msi_msg(data, &msg);
> + }
> + }
> +}
>
>
>
>
Thanks,
Praan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite()
2026-09-08 20:15 ` Pranjal Shrivastava
@ 2026-09-08 20:16 ` Pranjal Shrivastava
2026-09-09 9:20 ` Thomas Gleixner
1 sibling, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 20:16 UTC (permalink / raw)
To: Thomas Gleixner
Cc: iommu, Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
driver-core, Jason Gunthorpe, Marc Zyngier
On Tue, Sep 08, 2026 at 08:15:17PM +0000, Pranjal Shrivastava wrote:
> On Tue, Sep 08, 2026 at 09:40:49PM +0200, Thomas Gleixner wrote:
> > On Tue, Sep 08 2026 at 17:17, Pranjal Shrivastava wrote:
> >
> > ...
> > > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> >
> > Jason. You really should know better by now :(
> >
>
> Apologies for the confusion, Jason just directed me to avoid open coding
> this in the iommu driver and have the irq core handle it instead. [1]
> Jason had nothing to do with the specific implementation / design here,
> that misintepretation / mess is mine :(
>
Missed this link. (Sorry!)
[1] https://lore.kernel.org/all/178767577112.3356902.7778967514008843854.b4-review@b4/
Thanks,
Praan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config
2026-09-08 19:56 ` Thomas Gleixner
@ 2026-09-08 20:23 ` Pranjal Shrivastava
0 siblings, 0 replies; 23+ messages in thread
From: Pranjal Shrivastava @ 2026-09-08 20:23 UTC (permalink / raw)
To: Thomas Gleixner
Cc: iommu, Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
driver-core
On Tue, Sep 08, 2026 at 09:56:33PM +0200, Thomas Gleixner wrote:
> On Tue, Sep 08 2026 at 17:17, Pranjal Shrivastava wrote:
> > +static void __maybe_unused arm_smmu_resume_msis(struct arm_smmu_device *smmu)
> > +{
> > + /* Clear the MSI address regs as they reset to unknown value */
> > + writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
> > + writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
> > +
> > + if (smmu->features & ARM_SMMU_FEAT_PRI)
> > + writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
> > +
> > + if (!(smmu->features & ARM_SMMU_FEAT_MSI))
> > + return;
> > +
> > + if (!smmu->dev->msi.domain) {
> > + dev_err(smmu->dev, "msi_domain absent during resume\n");
> > + smmu->features &= ~ARM_SMMU_FEAT_MSI;
> > + return;
>
> If dev->msi.domain == NULL then arm_smmu_setup_msis() already cleared
> the MSI feature bit. Has it magically been set again or does resume run
> before init or does dev->msi.domain magically disappear during suspend?
>
> I'm all for defensive programming, but this is voodoo and not structured
> defense.
>
> Aside of that dev->msi.domain is the patently wrong condition. For
> devices which instantiate a MSI device domain dev->msi.domain points to
> the MSI parent domain and not to the actual relevant device domain.
>
> You can't query that easily by chasing pointers (for a reason), but
> there is no point to do so. See below and the patch I sent you.
>
Guilty of voodoo defense. You're completely right—FEAT_MSI was already
vleared on fallback during probe, and checking the parent domain here was
misguided. Dropping that entirely. (I added the check here and then
updated the setup_msis() part).
> > + platform_device_msi_rewrite(smmu->dev, smmu->gerr_irq, arm_smmu_write_msi_msg);
> > + platform_device_msi_rewrite(smmu->dev, smmu->evtq.q.irq, arm_smmu_write_msi_msg);
> > +
> > + if (smmu->features & ARM_SMMU_FEAT_PRI)
> > + platform_device_msi_rewrite(smmu->dev, smmu->priq.q.irq, arm_smmu_write_msi_msg);
>
> And that's exactly the point I made about sprinkling this stuff all over
> the place and thereby violating all layering rules.
>
> Done correctly this whole function boils down to:
>
> static void __maybe_unused arm_smmu_resume_msis(struct arm_smmu_device *smmu)
> {
> /* Clear the MSI address regs as they reset to unknown value */
> writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
> writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
>
> if (smmu->features & ARM_SMMU_FEAT_PRI)
> writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
>
> msi_device_domain_restore_msi_msgs(smmu->dev, 0);
> }
>
> It just works simply because the function returns early when there is no
> domain or the domain is not a MSI device domain, which is correct
> because there is nothing to do when nothing is set up.
>
> But that results in too comprehensible code I fear.
>
I certainly won't complain about code being too comprehensible! :)
This is much cleaner than what I had.
I'll drop my implementation of patch 7 (platform_device_msi_rewrite)
completely in favor of your core msi_device_domain_restore_msi_msgs()
helper, simplify patch 8 to this and take it for a spin.
Thanks,
Praan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite()
2026-09-08 19:40 ` Thomas Gleixner
2026-09-08 20:15 ` Pranjal Shrivastava
@ 2026-09-08 22:55 ` Jason Gunthorpe
1 sibling, 0 replies; 23+ messages in thread
From: Jason Gunthorpe @ 2026-09-08 22:55 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Pranjal Shrivastava, iommu, Will Deacon, Joerg Roedel,
Robin Murphy, Mostafa Saleh, Nicolin Chen, Daniel Mentz,
Ashish Mhetre, linux-arm-kernel, Greg Kroah-Hartman, rafael,
Danilo Krummrich, driver-core, Marc Zyngier
On Tue, Sep 08, 2026 at 09:40:49PM +0200, Thomas Gleixner wrote:
> On Tue, Sep 08 2026 at 17:17, Pranjal Shrivastava wrote:
>
> ...
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Jason. You really should know better by now :(
I know better enough to have pointed out that an open coded version of
this in a driver should not be open coded in a driver and be made into
proper helper, so you can see it :)
> > + desc = irq_get_msi_desc(irq);
> > + if (!desc) {
> > + dev_err(dev, "Failed to get MSI descriptor for irq %u\n", irq);
> > + return;
> > + }
>
> Doing this without having the underlying interrupt descriptor locked is
> a recipe for an undebuggable disaster waiting to happen. It might be
> "safe" in the context you are calling it but it's absolutely not safe in
> general.
Which is why we are here, I was wondering if the open coded
implementation was even OK..
> @@ -1775,3 +1775,34 @@ bool msi_device_has_isolated_msi(struct
> return arch_is_isolated_msi();
> }
> EXPORT_SYMBOL_GPL(msi_device_has_isolated_msi);
> +
> +void msi_device_domain_restore_msi_msgs(struct device *dev, unsigned int domid)
> +{
Wonderful!
Thanks,
Jason
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite()
2026-09-08 20:15 ` Pranjal Shrivastava
2026-09-08 20:16 ` Pranjal Shrivastava
@ 2026-09-09 9:20 ` Thomas Gleixner
1 sibling, 0 replies; 23+ messages in thread
From: Thomas Gleixner @ 2026-09-09 9:20 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: iommu, Will Deacon, Joerg Roedel, Robin Murphy, Jason Gunthorpe,
Mostafa Saleh, Nicolin Chen, Daniel Mentz, Ashish Mhetre,
linux-arm-kernel, Greg Kroah-Hartman, rafael, Danilo Krummrich,
driver-core, Jason Gunthorpe, Marc Zyngier
On Tue, Sep 08 2026 at 20:15, Pranjal Shrivastava wrote:
> On Tue, Sep 08, 2026 at 09:40:49PM +0200, Thomas Gleixner wrote:
>> So this all can be solved at the device domain level without sprinkling
>> per interrupt invocations including conditionals all over the place.
>>
> Ack. I was wondering if the irq core should also cache the message for
> platform MSIs like it's done for PCI ? Would that be a bad idea? Or is
> it this way by design? (I'm having to cache the msg in the iommu driver
> atm).
Sure. This can be done in irq_chip_write_msi_msg() before invoking the
chip callback. That makes the PCI caching redundant, but it can't be
removed because of other historical layering violations in that
context. So adding it to the core requires to add a comment why this has
to stay in the PCI write function.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-09-09 9:21 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 17:16 [PATCH v10 00/15] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 01/15] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 02/15] iommu/arm-smmu-v3: Add Q_POS() macro Pranjal Shrivastava
2026-09-08 17:16 ` [PATCH v10 03/15] iommu/arm-smmu-v3: Add arm_smmu_drain_queue() helper Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 04/15] iommu/tegra241-cmdqv: Add a helper to drain VCMDQs Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 05/15] iommu/arm-smmu-v3: Add a helper to drain cmd queues Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 06/15] iommu/tegra241-cmdqv: Restore PROD and CONS after resume Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 07/15] platform-msi: Introduce platform_device_msi_rewrite() Pranjal Shrivastava
2026-09-08 19:40 ` Thomas Gleixner
2026-09-08 20:15 ` Pranjal Shrivastava
2026-09-08 20:16 ` Pranjal Shrivastava
2026-09-09 9:20 ` Thomas Gleixner
2026-09-08 22:55 ` Jason Gunthorpe
2026-09-08 17:17 ` [PATCH v10 08/15] iommu/arm-smmu-v3: Cache and restore MSI config Pranjal Shrivastava
2026-09-08 19:56 ` Thomas Gleixner
2026-09-08 20:23 ` Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 09/15] iommu/arm-smmu-v3: Factor out arm_smmu_handle_gerror() Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 10/15] iommu/arm-smmu-v3: Add CMDQ_PROD_STOP_FLAG to gate CMDQ submissions Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 11/15] iommu/tegra241-cmdqv: Add a helper to quiesce VCMDQs Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 12/15] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 13/15] iommu/arm-smmu-v3: Enable pm_runtime and setup devlinks Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 14/15] iommu/arm-smmu-v3: Invoke pm_runtime before hw access Pranjal Shrivastava
2026-09-08 17:17 ` [PATCH v10 15/15] iommu/arm-smmu-v3: Add KUnit unit tests for Runtime PM Pranjal Shrivastava
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox