iommu/arm-smmu-v3: Add PRI support

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 [email protected] # v6.16

Assisted-by Claude:claude-opus-4-7

Signed-off-by Nicolin Chen <[email protected]>

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 27 +++++++++++++++++++++
 1 file changed, 27 insertions(+)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +20 -0

index cf41b3cf5985f..4794a15f351c4 100644
@@ -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)));

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.

Quoted 6 lines · Pranjal Shrivastava

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

Quoted 7 lines

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.

[..]

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu, · line 1083

Quoted 2 lines

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -0

 	queue_poll_init(smmu, &qp);
+	qp.wfe = is_cmdq;

The callers of this drain() will be RPM and PRI, either of which needs busy-waiting. So, I have added a different drain() helper. I run Sashiko locally; it also gives a clean result.

Commit:
https://github.com/nicolinc/iommufd/commit/d10325ff40fd45c476bc98d484d8dd2cdc7fdf95

This will be in my pri-v3 series:
https://github.com/nicolinc/iommufd/commits/smmuv3_pri-v3

We have too many SMMUv3 under review now. So, I will post it once some of them are merged.

Nicolin

For PRIQ/EVTQ, my only worry was the HW producing more events, say Device A is detached (we call drain from attach_release) but Device B (still attached) starts an agressive DMA with multiple PRI requests/evts.

Maybe we should go back to snapshot based polling for that?

Quoted 5 lines · Pranjal Shrivastava

For PRIQ/EVTQ, my only worry was the HW producing more events, say Device A is detached (we call drain from attach_release) but Device B (still attached) starts an agressive DMA with multiple PRI requests/evts.

Maybe we should go back to snapshot based polling for that?

If you look closely, "pending" is the snapshot, regardless of the growing prod.

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

Quoted 10 lines · Nicolin Chen

Quoted 5 lines · Pranjal Shrivastava

For PRIQ/EVTQ, my only worry was the HW producing more events, say Device A is detached (we call drain from attach_release) but Device B (still attached) starts an agressive DMA with multiple PRI requests/evts.

Maybe we should go back to snapshot based polling for that?

If you look closely, "pending" is the snapshot, regardless of the growing prod.

Quoted 8 lines

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

Right, but we write back cons to cons_reg only after we see queue_empty() We have a do { .. } while (!queue_empty(llq)); and then we call queue_sync_cons_ovf(q). Same for PRIQ handler.

IIUC the helper relies on cons_reg being updated:

cons = readl_relaxed(q->cons_reg);
drained += Q_POS(&q->llq, cons - prev);

Thus, if we get a storm of pri reqs / evts, we may never leave that loop and queue_sync_cons_ovf(q).

Quoted 29 lines · Pranjal Shrivastava

Quoted 10 lines · Nicolin Chen

Quoted 5 lines · Pranjal Shrivastava

For PRIQ/EVTQ, my only worry was the HW producing more events, say Device A is detached (we call drain from attach_release) but Device B (still attached) starts an agressive DMA with multiple PRI requests/evts.

Maybe we should go back to snapshot based polling for that?

If you look closely, "pending" is the snapshot, regardless of the growing prod.

Quoted 8 lines

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

Right, but we write back cons to cons_reg only after we see queue_empty() We have a do { .. } while (!queue_empty(llq)); and then we call queue_sync_cons_ovf(q). Same for PRIQ handler.

IIUC the helper relies on cons_reg being updated:

cons = readl_relaxed(q->cons_reg);
drained += Q_POS(&q->llq, cons - prev);

Quoted 2 lines

Thus, if we get a storm of pri reqs / evts, we may never leave that loop and queue_sync_cons_ovf(q).

cons_reg is updated by queue_remove_raw() every iteration inside that loop.

queue_sync_cons_ovf() doesn't update cons_reg unless an overflow.

From: Malak Marrid <[email protected]>

To handle IOMMU_FAULT_PAGE_REQ from the PRI queue, arm_smmu_page_response() must issue a CMDQ_OP_PRI_RESP back to the SMMU.

However, either a stall event in the EVTQ or a PRI request in the PRIQ can surface to the IOPF infrastructure with fault.type == IOMMU_FAULT_PAGE_REQ, and a single master can in principle be both stall-capable and PRI-capable

(e.g. FEAT_STALL_FORCE on a PCIe device with PRI), so master state is not a reliable discriminator.

Add IOMMU_FAULT_PAGE_REQUEST_STALLS_TRANS to the generic flags so the fault reporter can mark a page request that is holding the device's transaction:
arm_smmu_handle_event() sets it on STALL events arm_smmu_handle_ppr() leaves it clear for PRI events

Note: streams[0].id remains the RID because arm_smmu_enable_iopf() rejects num_streams != 1.

Co-developed-by Barak Biber <[email protected]>

Signed-off-by Barak Biber <[email protected]>

Co-developed-by Stefan Kaestle <[email protected]>

Signed-off-by Stefan Kaestle <[email protected]>

Signed-off-by Malak Marrid <[email protected]>

Signed-off-by Nicolin Chen <[email protected]>

---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
  include/linux/iommu.h                       |  1 +
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 75 +++++++++++++++------
  3 files changed, 58 insertions(+), 19 deletions(-)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1 -0

index 2bb810e4d5fce..1083621705f16 100644
@@ -1007,6 +1007,7 @@ struct arm_smmu_master {
  	/* Locked by the iommu core using the group mutex */
  	struct arm_smmu_ctx_desc_cfg	cd_table;
  	unsigned int			num_streams;
+	bool				pri_enabled : 1;
  	bool				ats_enabled : 1;
  	bool				ste_ats_enabled : 1;
  	bool				stall_enabled;

include/linux/iommu.h +1 -0

index e587d4ac4d331..83c4dfcf20637 100644
@@ -76,6 +76,7 @@ struct iommu_fault_page_request {
  #define IOMMU_FAULT_PAGE_REQUEST_PASID_VALID	(1 << 0)
  #define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE	(1 << 1)
  #define IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID	(1 << 2)
+#define IOMMU_FAULT_PAGE_REQUEST_STALLS_TRANS	(1 << 3)
  	u32	flags;
  	u32	pasid;
  	u32	grpid;

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +56 -19

index ffc9621cd2288..061f1d46fda0d 100644
@@ -921,32 +921,68 @@ static int arm_smmu_drain_queue_for_iopf(struct arm_smmu_device *smmu,
  	return ret;
  }
 
-static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
+static void arm_smmu_page_response(struct device *dev, struct iopf_fault *evt,
  				   struct iommu_page_response *resp)
  {
  	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-	u8 resume_resp;
+	struct arm_smmu_cmd cmd;
+	int sid;
 
-	if (WARN_ON(!master->stall_enabled))
+	if (WARN_ON_ONCE(evt->fault.type != IOMMU_FAULT_PAGE_REQ))
  		return;
 
-	switch (resp->code) {
-	case IOMMU_PAGE_RESP_INVALID:
-	case IOMMU_PAGE_RESP_FAILURE:
-		resume_resp = CMDQ_RESUME_0_RESP_ABORT;
-		break;
-	case IOMMU_PAGE_RESP_SUCCESS:
-		resume_resp = CMDQ_RESUME_0_RESP_RETRY;
-		break;
-	default:
-		resume_resp = CMDQ_RESUME_0_RESP_TERM;
-		break;
+	/* IOPF is gated to num_streams == 1 in arm_smmu_enable_iopf() */
+	sid = master->streams[0].id;
+
+	if (evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_STALLS_TRANS) {
+		u8 resume_resp;
+
+		if (WARN_ON_ONCE(!master->stall_enabled))
+			return;
+		switch (resp->code) {
+		case IOMMU_PAGE_RESP_INVALID:
+		case IOMMU_PAGE_RESP_FAILURE:
+			resume_resp = CMDQ_RESUME_0_RESP_ABORT;
+			break;
+		case IOMMU_PAGE_RESP_SUCCESS:
+			resume_resp = CMDQ_RESUME_0_RESP_RETRY;
+			break;
+		default:
+			resume_resp = CMDQ_RESUME_0_RESP_TERM;
+			break;
+		}
+		cmd = arm_smmu_make_cmd_resume(sid, resp->grpid, resume_resp);
+	} else {
+		enum pri_resp pri_resp;
+		bool ssv;
+
+		if (WARN_ON_ONCE(!master->pri_enabled))
+			return;
+		/* PCIe allows only one PRG Response per group */
+		if (!(evt->fault.prm.flags &
+		      IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE))
+			return;
+		switch (resp->code) {
+		case IOMMU_PAGE_RESP_SUCCESS:
+			pri_resp = PRI_RESP_SUCC;
+			break;
+		case IOMMU_PAGE_RESP_FAILURE:
+			pri_resp = PRI_RESP_FAIL;
+			break;
+		case IOMMU_PAGE_RESP_INVALID:
+			pri_resp = PRI_RESP_DENY;
+			break;
+		default:
+			WARN_ON(true);
+			return;
+		}
+		ssv = !!(evt->fault.prm.flags &
+			 IOMMU_FAULT_PAGE_REQUEST_PASID_VALID);
+		cmd = arm_smmu_make_cmd_pri_resp(sid, resp->pasid, ssv,
+						 resp->grpid, pri_resp);
  	}
 
-	arm_smmu_cmdq_issue_cmd(master->smmu,
-				arm_smmu_make_cmd_resume(master->streams[0].id,
-							 resp->grpid,
-							 resume_resp));
+	arm_smmu_cmdq_issue_cmd(master->smmu, cmd);
  	/*
  	 * Don't send a SYNC, it doesn't do anything for RESUME or PRI_RESP.
  	 * RESUME consumption guarantees that the stalled transaction will be

@@ -2081,7 +2117,8 @@ static int arm_smmu_handle_event(struct arm_smmu_device *smmu, u64 *evt,
 
  		flt->type = IOMMU_FAULT_PAGE_REQ;
  		flt->prm = (struct iommu_fault_page_request){
-			.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
+			.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE |
+				 IOMMU_FAULT_PAGE_REQUEST_STALLS_TRANS,
  			.grpid = event->stag,
  			.perm = perm,
  			.addr = event->iova,

Quoted 8 lines · Nicolin Chen

From: Malak Marrid <[email protected]>

To handle IOMMU_FAULT_PAGE_REQ from the PRI queue, arm_smmu_page_response() must issue a CMDQ_OP_PRI_RESP back to the SMMU.

However, either a stall event in the EVTQ or a PRI request in the PRIQ can surface to the IOPF infrastructure with fault.type == IOMMU_FAULT_PAGE_REQ, and a single master can in principle be both stall-capable and PRI-capable

No, the SMMU architecture does all it can to specifically forbid this, see 3.12.1 and 16.4, it just can't be made architecturally ILLEGAL to enable stalls for PCIe devices because there's no strict architectural definition for what "a PCIe device" actually is. Similarly with the note in the definition of STE.EATS about the relationship with CD.S - the unwritten implication is that defining specific behaviours would only create an unreasonable burden for hardware validation, for the sake of something that nobody in their right mind should ever do anyway.

The expectation is that RCiEPs which do speak stallable non-PCIe bus protocols will not go to the effort of implementing ATS/PRI capabilities (not least because there's every chance that such protocols simply doesn't have that kind of transaction flow anyway). And conversely that it can be considered an egregious firmware (or system design) error to even claim (let alone force) stall capability for a real PCIe root port which may be deadlocked by blocking its requirement for free-flowing writes. Thus I think we could go so far as to refuse to handle any endpoint which did somehow claim both.

The SMMUv3 driver doesn't handle events on the PRI queue or respond to IOPF faults. This series adds the missing pieces, using the IOPF infrastructure, to convert PRI page requests into iopf_faults and issue CMDQ_OP_PRI_RESP.

The iopf_queue_flush_dev() contract requires the driver to first drain the hardware PRI queue and synchronize using a threaded IRQ handler before the IOPF software flush. This drove the additional commits compared to v1:
- arm_smmu_drain_queue_for_iopf() drains the hardware queue to the PROD snapshot
- arm_smmu_attach_release() moves the teardown outside the global lock
- synchronize_irq() closes the gap before the final flush

This is on Github:
https://github.com/nicolinc/iommufd/commits/smmuv3_pri-v2

FWIW, engineers on the NVIDIA side have managed to verify the PRI feature.

Changelog
v2:
* Allocate evtq.iopf for ARM_SMMU_FEAT_PRI
* Pick up Jean's PRI stubs and PRI export patches
* Enable PRI for PCI devices in arm_smmu_probe_device()
* Add arm_smmu_drain_queue_for_iopf() for EVTQ and PRIQ
* Add arm_smmu_attach_release() to rework the IOPF drain
* Add IOMMU_FAULT_PAGE_REQUEST_STALLS_TRANS for STALL mode
* Gate pci_enable_pri() on FEAT_PRI plus a non-NULL evtq.iopf
* Deny unrecognised-StreamID PRG_LAST in arm_smmu_handle_ppr()
* Disable PRI when no IRQ handler is registered (unique or combined IRQ)
v1:
https://lore.kernel.org/all/[email protected]/

Jean-Philippe Brucker (2):
PCI/ATS: Add PRI stubs
PCI/ATS: Export pci_enable_pri() and pci_reset_pri()

Malak Marrid (1):
iommu/arm-smmu-v3: Submit CMDQ_OP_PRI_RESP for IOPF event

Nicolin Chen (8):
iommu/arm-smmu-v3: Add arm_smmu_attach_release() iommu/arm-smmu-v3: Factor out __queue_empty() and __queue_consumed() iommu/arm-smmu-v3: Add arm_smmu_drain_queue_for_iopf() helper iommu/arm-smmu-v3: Drain in-flight fault handlers iommu/arm-smmu-v3: Support PRI Page Request in arm_smmu_handle_ppr() iommu/arm-smmu-v3: Disable PRI when no IRQ handler is registered iommu/arm-smmu-v3: Allocate IOPF queue for ARM_SMMU_FEAT_PRI iommu/arm-smmu-v3: Enable PRI for PCI device in
arm_smmu_probe_device()

  drivers/iommu/arm/Kconfig                     |   1 +
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   3 +
  include/linux/iommu.h                         |   1 +
  include/linux/pci-ats.h                       |   5 +
  .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     |   1 +
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 244 +++++++++++++++---
  drivers/pci/ats.c                             |   2 +
  7 files changed, 221 insertions(+), 36 deletions(-)

base-commit: 74fa4c177ad09800b007cba043370c887bb1b4e3

One thing I noticed while reviewing: when arm_smmu_priq_thread() detects PRIQ overflow, partial faults (non-LAST pages stored via report_partial_fault()) whose LAST page was lost in the overflow remain permanently in iopf_param->partial. This is a monotonic memory leak — it grows with each overflow event.

Intel VT-d handles this in prq_event_thread() (drivers/iommu/intel/prq.c):

    if (head == tail) {
        iopf_queue_discard_partial(iommu->iopf_queue);
        writel(DMA_PRS_PRO, iommu->reg + DMAR_PRS_REG);
    }

iopf_queue_discard_partial() was written for exactly this scenario. Could we add the same here arm_smmu_priq_thread() (drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c) ?

    if (queue_sync_prod_in(q) == -EOVERFLOW) {
        dev_err(smmu->dev, "PRIQ overflow detected -- requests lost\n");
+       iopf_queue_discard_partial(smmu->evtq.iopf);
    }

At this point all surviving entries have already been consumed by the loop above, so discarding unconditionally is safe — implicitly matching Intel's "head == tail" guard.