Re: [PATCH v2 03/11] iommu/arm-smmu-v3: Add arm_smmu_drain_queue_for_iopf() helper

[Pranjal Shrivastava <praan@google.com>]

On Thu, May 28, 2026 at 12:59:31AM -0700, Nicolin Chen wrote:
> A poll-until-empty form does not converge on the shared EVTQ or PRIQ since
> other masters keep advancing PROD with unrelated traffic. A fixed snapshot
> bounds the wait even under sustained unrelated load, since the target does
> not move with subsequent enqueues.
>

[...]

> 
> This is a prerequisite to apply bug fix calling iopf_queue_flush_dev().
> 
> Fixes: cfea71aea921 ("iommu/arm-smmu-v3: Put iopf enablement in the domain attach path")
> Cc: stable@vger.kernel.org # v6.16
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.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 cf41b3cf5985f..4794a15f351c4 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -894,6 +894,33 @@ static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu,
>  					   cmds->num, true);
>  }
>  
> +/* Drain an SMMU EVTQ or PRIQ to a PROD snapshot taken on entry */
> +static int arm_smmu_drain_queue_for_iopf(struct arm_smmu_device *smmu,
> +					 struct arm_smmu_queue *q)
> +{
> +	struct arm_smmu_queue_poll qp;
> +	u32 prod, cons;
> +	int ret = 0;
> +
> +	/* Snapshot PROD; entries [old_cons, prod) are the cohort to drain */
> +	prod = readl_relaxed(q->prod_reg);
> +	queue_poll_init(smmu, &qp);
> +	qp.wfe = false; /* No SEV on EVTQ/PRIQ PROD advance */
> +	/* Read MMIO each iteration; llq->cons is the threaded handler's */
> +	do {
> +		cons = readl_relaxed(q->cons_reg);
> +		if (__queue_empty(&q->llq, cons, prod) ||
> +		    __queue_consumed(&q->llq, cons, prod))
> +			return 0;
> +		cond_resched();
> +	} while (!(ret = queue_poll(&qp)));
> +
> +	dev_warn_ratelimited(smmu->dev,
> +			     "queue drain timed out at prod=0x%x cons=0x%x\n",
> +			     prod, cons);
> +	return ret;
> +}
> +
>  static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
>  				   struct iommu_page_response *resp)
>  {

Hi Nicolin,

The following is a unified drain_queue helper, that also attempts to
address Sashiko's concerns. This applies on top of my runtime PM v9
series [1], please see if it works for you:

Subject: [PATCH] iommu/arm-smmu-v3: Add a unified arm_smmu_drain_queue()  helper

Introduce a unified arm_smmu_drain_queue() helper to replace queue
specific polling loops. The helper handles both snapshot-based cohort
draining (for IOPF) and full queue draining (required for Runtime PM).

The Runtime PM explicitly gates the CMDQ (via CMDQ_PROD_STOP_FLAG) prior
to draining, this allows a single, snapshot-based target to natively
handle all drain scenarios.

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 68 +++++++++++++++------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  4 +-
 2 files changed, 52 insertions(+), 20 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 9106ffb75f7a..2deed6e3bf9b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1057,39 +1057,70 @@ static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu,
 					   cmds->num, true);
 }
 
+static bool queue_consumed(struct arm_smmu_ll_queue *q, u32 cons, u32 target)
+{
+	if (cons == target)
+		return true;
+	if (Q_WRP(q, cons) == Q_WRP(q, target))
+		return Q_IDX(q, cons) > Q_IDX(q, target);
+	return Q_IDX(q, cons) < Q_IDX(q, target);
+}
+
 /*
- * Currently, only used for CMDQ.
- * TODO: Update to handle PRIQ & EVTQ when PRI support is added
+ * Drains a queue until the consumer reaches the provided 'prod' snapshot.
+ * For CMDQ, the consumer is the SMMU hardware (polled via MMIO).
+ * For EVTQ/PRIQ, the consumer is the CPU IRQ thread (polled via software llq).
  */
-int arm_smmu_queue_poll_until_empty(struct arm_smmu_device *smmu,
-				    struct arm_smmu_queue *q)
+int arm_smmu_drain_queue(struct arm_smmu_device *smmu,
+			 struct arm_smmu_queue *q, u32 prod)
 {
 	struct arm_smmu_queue_poll qp;
-	struct arm_smmu_ll_queue *llq = &q->llq;
-	int ret = 0;
+	/* Secondary CMDQs don't support WFE*/
+	bool is_cmdq = (q == &smmu->cmdq.q);
+	u32 cons;
 
 	queue_poll_init(smmu, &qp);
+	qp.wfe = is_cmdq;
+
+	do {
+		if (is_cmdq) {
+			cons = readl_relaxed(q->cons_reg);
+			WRITE_ONCE(q->llq.cons, cons);
+		} else {
+			cons = READ_ONCE(q->llq.cons);
+		}
+
+		if (queue_consumed(&q->llq, cons, prod))
+			return 0;
+
+		cond_resched();
+	} while (!queue_poll(&qp));
 
 	/*
-	 * The events are only generated when CMDQ becomes non-full.
-	 * Thus, wfe can't be used here, mark it false explicitly.
+	 * Check once more after timeout: cond_resched() could have delayed us
+	 * until after the timer expired while the consumer finished draining.
 	 */
-	qp.wfe = false;
-
-	do {
-		WRITE_ONCE(llq->cons, readl_relaxed(q->cons_reg));
-		if (queue_empty(llq))
-			break;
+	if (is_cmdq) {
+		cons = readl_relaxed(q->cons_reg);
+		WRITE_ONCE(q->llq.cons, cons);
+	} else {
+		cons = READ_ONCE(q->llq.cons);
+	}
 
-		ret = queue_poll(&qp);
+	if (queue_consumed(&q->llq, cons, prod))
+		return 0;
 
-	} while (!ret);
+	dev_warn_ratelimited(smmu->dev,
+			     "queue drain timed out at prod=0x%x cons=0x%x\n",
+			     prod, cons);
 
-	return ret;
+	return -ETIMEDOUT;
 }
 
 static int arm_smmu_drain_queues(struct arm_smmu_device *smmu)
 {
+	struct arm_smmu_cmdq *cmdq = &smmu->cmdq;
+	u32 target;
 	int ret;
 
 	/*
@@ -1097,7 +1128,8 @@ static int arm_smmu_drain_queues(struct arm_smmu_device *smmu)
 	 * from contexts (like suspend) where the caller has already
 	 * ensured that new command submissions are fully closed.
 	 */
-	ret = arm_smmu_queue_poll_until_empty(smmu, &smmu->cmdq.q);
+	target = atomic_read(&cmdq->q.llq.atomic.prod) & CMDQ_PROD_IDX_MASK;
+	ret = arm_smmu_drain_queue(smmu, &cmdq->q, target);
 
 	if (ret)
 		goto out;
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 15b5d5fad627..eddf6f41b570 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -1176,8 +1176,8 @@ 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_queue_poll_until_empty(struct arm_smmu_device *smmu,
-				    struct arm_smmu_queue *q);
+int arm_smmu_drain_queue(struct arm_smmu_device *smmu,
+			 struct arm_smmu_queue *q, u32 prod);
 
 static inline bool arm_smmu_master_canwbs(struct arm_smmu_master *master)
 {
-- 

[1] https://lore.kernel.org/all/20260728210928.1050849-1-praan@google.com/


Thanks,
Praan