[PATCH v6 00/17] iommu/arm-smmu-v3: Quarantine device upon ATC invalidation timeout

[PATCH v6 00/17] iommu/arm-smmu-v3: Quarantine device upon ATC invalidation timeout

[Nicolin Chen]

Hi all,

This series addresses a critical vulnerability and stability issue where an
unresponsive PCIe device failing to process ATC (Address Translation Cache)
invalidation requests leads to silent data corruption and continuous SMMU
CMDQ error spam.

[ As Jason pointed out, because this series fundamentally introduces a new
  RAS feature to quarantine and recover from hardware faults and relies on
  a recently accepted SMMU driver rework, it is not treated as a standard
  bug fix. Thus, most of the patches here don't carry a "Fixes" tag. ]

Currently, when an ATC invalidation times out, the SMMUv3 driver skips the
CMDQ_ERR_CERROR_ATC_INV_IDX error. This leaves the device's ATS cache state
desynchronized from the SMMU: the device cache may retain stale ATC entries
for memory pages that the OS has already reclaimed and reassigned, creating
a direct vector for data corruption. Furthermore, the driver might continue
issuing ATC_INV commands, resulting in constant CMDQ errors:
    unexpected global error reported (0x00000001), this could be serious
    CMDQ error (cons 0x0302bb84): ATC invalidate timeout
    unexpected global error reported (0x00000001), this could be serious
    CMDQ error (cons 0x0302bb88): ATC invalidate timeout
    unexpected global error reported (0x00000001), this could be serious
    CMDQ error (cons 0x0302bb8c): ATC invalidate timeout
    ...

To resolve this, introduce a mechanism to quarantine a broken device in the
SMMUv3 driver and the IOMMU core. To achieve this, add preparatory changes:
 - Pass in PCI reset result to pci_dev_reset_iommu_done()
 - Co-clear pending CMDQ_ERR from the cmdq issuer under a raw_spinlock_t,
   so an ATC_INV timeout flagged in cmdq->atc_sync_timeouts is definitive
   when the issuer reads its bit after CMD_SYNC poll

On the SMMUv3 driver side, retry the timedout ATC_INV batch to identify the
faulty device(s). Perform a surgical STE update, and flag the ATS as broken
to reject further ATS/ATC requests at HW level and suppress timeout spam.

This is on Github:
https://github.com/nicolinc/iommufd/commits/smmuv3_atc_timeout-v6

Changelog
v6:
 * Rebase on v7.3-rc1
 * Use "LLM" for the Assisted-by tags
 * Add Reviewed-by tags from Baolu and Jason
 * [PCI] Move the port register read to the probe path
 * [PCI] Drop the patch propagating error code in quirks.c
 * [iommu] Rename enum gdev_blocked to enum blocked_reason
 * [iommu] Pass the correct old domain when releasing a blocked device
 * [smmuv3] Drop the patch adding arm_smmu_cmdq_batch_force_sync (merged)
v5:
 https://lore.kernel.org/all/cover.1783044582.git.nicolinc@nvidia.com/
 * Rebase on v7.2-rc1
 * [PCI] Probe the underlying bus reset in cxl_reset_bus_function()
 * [PCI] Add quirk_flr_err() to stop the reset cascade on FLR timeout
 * [iommu] Drop iommu_report_device_broken() and its preparatory patches
 * [smmuv3] Drop master->ats_broken bool
 * [smmuv3] Drop master->ats_broken_lock
 * [smmuv3] Drop master->ats_invs scratch
 * [smmuv3] Introduce INV_TYPE_ATS_BROKEN marker
 * [smmuv3] Add arm_smmu_cmdq_batch_force_sync()
 * [smmuv3] Don't rb_erase() a never-inserted stream node
 * [smmuv3] Add streams_lock for atomic SID->master lookup
 * [smmuv3] Drop "Serialize STE.EATS and ats_broken updates"
 * [smmuv3] Drop "Move arm_smmu_invs_for_each_entry to header"
 * [smmuv3] Recheck CMDQ_ERR in tegra241_vintf0_handle_error()
 * [smmuv3] Thread arm_smmu_master_domain on a per-master list
 * [smmuv3] Drop pci_disable_ats() from arm_smmu_quarantine_ats()
 * [smmuv3] Limit the quarantine() to ARM_SMMU_FEAT_COHERENCY only
 * [smmuv3] Rework arm_smmu_quarantine_ats() to walk master_domains
 * [smmuv3] Rework arm_smmu_cmdq_batch_retry(): per-unique-SID retry
 * [smmuv3] Rework arm_smmu_inv_cmp() to treat ATS variants as one class
 * [smmuv3] Rework the issuer-side atc_sync_timeouts test with smp_rmb()
 * [smmuv3] Drop "Co-clear pending CMDQ_ERR when queue_has_space() fails"
 * [smmuv3] Drop "Keep smmu pointer in arm_smmu_inv but add master for ATS"
v4:
 https://lore.kernel.org/all/cover.1779161849.git.nicolinc@nvidia.com/
 * Rebase on Joerg's IOMMU "fixes" branch
 * Rebase on Jason's SMMUv3 cmd_ent series
   https://lore.kernel.org/all/0-v2-47b2bf710ad5+716ac-smmu_no_cmdq_ent_jgg@nvidia.com/
 * [PCI] Don't suspend IOMMU in probe mode
 * [iommu] kfree_rcu() iommu_group
 * [iommu] Convert gdev->blocked to enum gdev_blocked
 * [iommu] Use disable_work_sync() to fix UAF and ref leak
 * [iommu] Gate done() transitions to preserve BLOCKED_BROKEN
 * [iommu] Decrement recovery_cnt when unplugging a blocked gdev
 * [iommu] Drop racy dev_has_iommu() in iommu_report_device_broken()
 * [iommu] Add gdev->broken_pending to skip worker after racing recovery
 * [smmuv3] Add master->ats_invs scratch
 * [smmuv3] Add arm_smmu_cmdq_batch_issue() wrapper
 * [smmuv3] Force per-flush sync for has_ats batches
 * [smmuv3] Serialize STE.EATS and ats_broken updates
 * [smmuv3] Co-clear pending CMDQ_ERR from cmdq issuer
 * [smmuv3] Add invs and has_ats to arm_smmu_cmdq_batch
 * [smmuv3] Move arm_smmu_invs_for_each_entry to header
 * [smmuv3] Set master->ats_broken after clearing STE.EATS
 * [smmuv3] Issue CFGI_STE via arm_smmu_cmdq_issue_cmd_with_sync()
 * [smmuv3] Keep "smmu" pointer in arm_smmu_inv but add "master" for ATS
v3:
 https://lore.kernel.org/all/cover.1776381841.git.nicolinc@nvidia.com/
 * Rebase on arm/smmu/updates branch + bug fix
 * Update commit messages and inline comments
 * [iommu] Drop unnecessary ops validation
 * [iommu] Add missed function stub when !CONFIG_IOMMU_API
 * [iommu] Change iommu_report_device_broken() to per gdev
 * [iommu] Separate quarantine from pci_dev_reset_prepare()
 * [iommu] Check reset failure in pci_dev_reset_iommu_done()
 * [smmuv3] Fix STE update with try_cmpxchg64()
 * [smmuv3] Fix "continue" bug when skipping ATC commands
 * [smmuv3] Replace atomic_t prod_err with a lockless bitmap
 * [smmuv3] Drop master->invs_domain; disable ATS per-master directly
 * [smmuv3] Return -EIO for ATC timeout v.s. -ETIMEDOUT for poll timeout
 * [smmuv3] Replace INV_TYPE_ATS_DISABLED with per-master ats_broken flag
v2:
 https://lore.kernel.org/all/cover.1773774441.git.nicolinc@nvidia.com/
 * Rebase on arm_smmu_invs-v13 series
 * Bisect batched atc invalidation commands
 * Drop the direct pci_reset_function() call
 * Move the work queue from SMMUv3 to the core
 * Proceed a surgical STE update to disable EATS
 * Wait for pci_dev_reset_iommu_done() to signal a recovery
v1:
 https://lore.kernel.org/all/cover.1772686998.git.nicolinc@nvidia.com/

Thanks
Nicolin


Nicolin Chen (17):
  PCI: Don't suspend IOMMU when probing reset capability
  PCI/CXL: Probe the underlying bus reset in cxl_reset_bus_function()
  iommu: Convert gdev->blocked from bool to enum blocked_reason
  iommu: Pass in gdev's blocked state to iommu_deinit_device()
  iommu: Pass in reset result to pci_dev_reset_iommu_done()
  iommu/arm-smmu-v3: Don't rb_erase() a never-inserted stream node
  iommu/arm-smmu-v3: Track ATC invalidation timeouts in a bitmap
  iommu/arm-smmu-v3: Skip remaining GERROR causes on SFM
  iommu/arm-smmu-v3: Introduce per-cmdq cmdq_err_handler callback
  iommu/arm-smmu-v3: Recheck CMDQ_ERR in tegra241_vintf0_handle_error()
  iommu/arm-smmu-v3: Co-clear pending CMDQ_ERR when CMD_SYNC times out
  iommu/arm-smmu-v3: Introduce arm_smmu_cmdq_batch_issue() wrapper
  iommu/arm-smmu-v3: Add streams_lock for atomic-context SID->master
    lookup
  iommu/arm-smmu-v3: Add has_ats to struct arm_smmu_cmdq_batch
  iommu/arm-smmu-v3: Add INV_TYPE_ATS_BROKEN for quarantined masters
  iommu/arm-smmu-v3: Thread arm_smmu_master_domain on a per-master list
  iommu/arm-smmu-v3: Quarantine ATS after an ATC invalidation timeout

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  33 +-
 include/linux/iommu.h                         |   5 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 477 ++++++++++++++++--
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    |  41 +-
 drivers/iommu/iommu.c                         |  87 +++-
 drivers/pci/pci-acpi.c                        |   2 +-
 drivers/pci/pci.c                             |  37 +-
 drivers/pci/quirks.c                          |  13 +-
 8 files changed, 606 insertions(+), 89 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.43.0

[PATCH v6 01/17] PCI: Don't suspend IOMMU when probing reset capability

[Nicolin Chen]

reset_method_store() in drivers/pci/pci-sysfs.c discovers supported reset
methods by calling reset_fn(pdev, PCI_RESET_PROBE, ...) without holding a
device_lock, since the probe path is expected to query the device's reset
capability without changing device state.

However, pci_reset_bus_function() and __pci_dev_specific_reset() violate
that contract after pci_dev_reset_iommu_prepare/done() were added, which
moves the device into a blocking domain and abruptly aborts any in-flight
DMA. Doing this for a probe -- a state-query call that does not even hold
device_lock -- can cause driver timeouts and data loss on a DMAing device.

The peer reset helpers all handle this correctly: they short-circuit on a
probe input before touching the IOMMU.

Skip pci_dev_reset_iommu_prepare()/_done() entirely when probe is set. The
inner reset routines already implement their own probe semantics, and they
perform the capability checks and return without changing device state.

Fixes: f5b16b802174 ("PCI: Suspend iommu function prior to resetting a device")
Cc: stable@vger.kernel.org
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/pci/pci.c    | 13 ++++++++-----
 drivers/pci/quirks.c | 13 ++++++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f80..1eba412b7e0de 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4996,10 +4996,12 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
 	if (bridge && pcie_is_cxl(bridge) && cxl_sbr_masked(bridge))
 		return -ENOTTY;
 
-	rc = pci_dev_reset_iommu_prepare(dev);
-	if (rc) {
-		pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", rc);
-		return rc;
+	if (!probe) {
+		rc = pci_dev_reset_iommu_prepare(dev);
+		if (rc) {
+			pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", rc);
+			return rc;
+		}
 	}
 
 	rc = pci_dev_reset_slot_function(dev, probe);
@@ -5008,7 +5010,8 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
 
 	rc = pci_parent_bus_reset(dev, probe);
 done:
-	pci_dev_reset_iommu_done(dev);
+	if (!probe)
+		pci_dev_reset_iommu_done(dev);
 	return rc;
 }
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index de9bbccda21fd..7858a063929d9 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4253,14 +4253,17 @@ static int __pci_dev_specific_reset(struct pci_dev *dev, bool probe,
 {
 	int ret;
 
-	ret = pci_dev_reset_iommu_prepare(dev);
-	if (ret) {
-		pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", ret);
-		return ret;
+	if (!probe) {
+		ret = pci_dev_reset_iommu_prepare(dev);
+		if (ret) {
+			pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", ret);
+			return ret;
+		}
 	}
 
 	ret = i->reset(dev, probe);
-	pci_dev_reset_iommu_done(dev);
+	if (!probe)
+		pci_dev_reset_iommu_done(dev);
 	return ret;
 }
 
-- 
2.43.0

[PATCH v6 02/17] PCI/CXL: Probe the underlying bus reset in cxl_reset_bus_function()

[Nicolin Chen]

cxl_reset_bus_function() reports "supported" to a probe after checking only
that the upstream bridge carries a CXL port DVSEC. The underlying bus reset
can still be unavailable, e.g. on a bus shared with other devices, so both
the reset_methods[] array and the reset_method sysfs node end up listing a
"cxl_bus" that is guaranteed to fail with -ENOTTY when it is attempted.

Probe the underlying pci_dev_reset_slot_function() and then, if it is not
applicable, pci_parent_bus_reset(). These are the same two checks that the
actual reset runs, so a shared-bus CXL device no longer advertises a method
that can never succeed.

Probing via pci_reset_bus_function() would not work: its cxl_sbr_masked()
check rejects every CXL port with a masked SBR, while the do-reset path in
this function unmasks the SBR before resetting. Such a port would wrongly
probe as unsupported.

Move the port control register read above the probe branch as well, so that
probe runs the same checks. That read asserts no reset, and a failure means
the device cannot be reset this way, which is a probe-time answer.

Also pass an explicit PCI_RESET_DO_RESET at the do-reset call site, since
probe is always false at that point.

Fixes: 53c49b6e6dd2e ("PCI/CXL: Add 'cxl_bus' reset method for devices below CXL Ports")
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/pci/pci.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1eba412b7e0de..a6e0ecfd207a7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5029,13 +5029,21 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
 	if (!dvsec)
 		return -ENOTTY;
 
-	if (probe)
-		return 0;
-
 	rc = pci_read_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL, &reg);
 	if (rc)
 		return -ENOTTY;
 
+	/*
+	 * Do not probe via pci_reset_bus_function(), which would reject a
+	 * masked SBR that the do-reset path below unmasks before resetting.
+	 */
+	if (probe) {
+		rc = pci_dev_reset_slot_function(dev, PCI_RESET_PROBE);
+		if (rc != -ENOTTY)
+			return rc;
+		return pci_parent_bus_reset(dev, PCI_RESET_PROBE);
+	}
+
 	rc = pci_dev_reset_iommu_prepare(dev);
 	if (rc) {
 		pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", rc);
@@ -5050,7 +5058,7 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
 				      val);
 	}
 
-	rc = pci_reset_bus_function(dev, probe);
+	rc = pci_reset_bus_function(dev, PCI_RESET_DO_RESET);
 
 	if (reg != val)
 		pci_write_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL,
-- 
2.43.0

[PATCH v6 03/17] iommu: Convert gdev->blocked from bool to enum blocked_reason

[Nicolin Chen]

The gdev->blocked flag tracks whether a device is individually being held
in the group->blocking_domain while group->domain is retained. Up to now,
a PCI reset in flight is the only producer, so a bool suffices.

Subsequent changes will add more reasons to keep a device blocked, e.g. a
failed-reset case that must not auto-unblock, or a driver-side quarantine
for a hardware fault. These reasons are cleared by different events, which
a single bool cannot encode.

Convert the "bool blocked" into "enum blocked_reason blocked", provisioned
with two initial values: BLOCKED_NONE and BLOCKED_RESETTING, covering the
existing use cases. All readers keep the "if (gdev->blocked)" form, since
BLOCKED_NONE == 0.

This is a pure type change with no behavior change. Follow-on changes will
add new enum values along with their producers.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommu.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cd1bca7ede9af..379132e2d6cf6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -73,16 +73,20 @@ struct iommu_group {
 	void *owner;
 };
 
+enum blocked_reason {
+	BLOCKED_NONE = 0, /* Not blocked */
+	BLOCKED_RESETTING, /* PCI reset in flight */
+};
+
 struct group_device {
 	struct list_head list;
 	struct device *dev;
 	char *name;
 	/*
 	 * Device is blocked for a pending recovery while its group->domain is
-	 * retained. This can happen when:
-	 *  - Device is undergoing a reset
+	 * retained.
 	 */
-	bool blocked;
+	enum blocked_reason blocked;
 	unsigned int reset_depth;
 };
 
@@ -4072,7 +4076,7 @@ int pci_dev_reset_iommu_prepare(struct pci_dev *pdev)
 	 * the correct domain in iommu_driver_get_domain_for_dev() that might be
 	 * called in a set_dev_pasid callback function.
 	 */
-	gdev->blocked = true;
+	gdev->blocked = BLOCKED_RESETTING;
 
 	/*
 	 * Stage PASID domains at blocking_domain while retaining pasid_array.
@@ -4198,7 +4202,7 @@ void pci_dev_reset_iommu_done(struct pci_dev *pdev)
 	 * the correct domain in iommu_driver_get_domain_for_dev() that might be
 	 * called in a set_dev_pasid callback function.
 	 */
-	gdev->blocked = false;
+	gdev->blocked = BLOCKED_NONE;
 
 	/*
 	 * Re-attach PASID domains back to the domains retained in pasid_array.
-- 
2.43.0

[PATCH v6 04/17] iommu: Pass in gdev's blocked state to iommu_deinit_device()

[Nicolin Chen]

iommu_deinit_device() parks the device on the driver's release_domain and
passes group->domain to its attach_dev() callback as the old domain. That
is only correct while the device is still attached to group->domain.

A subsequent change keeps a device blocked on group->blocking_domain after
a failed reset, so the old domain has to follow the device's blocked state.
The group_device holding that state is freed by __iommu_group_free_device()
before iommu_deinit_device() runs, so read it at the call site and pass it
in.

No functional change.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommu.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 379132e2d6cf6..344288e930c40 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -550,7 +550,7 @@ static int iommu_init_device(struct device *dev)
 	return ret;
 }
 
-static void iommu_deinit_device(struct device *dev)
+static void iommu_deinit_device(struct device *dev, bool blocked)
 {
 	struct iommu_group *group = dev->iommu_group;
 	const struct iommu_ops *ops = dev_iommu_ops(dev);
@@ -703,7 +703,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 	list_del(&gdev->list);
 	__iommu_group_free_device(group, gdev);
 err_put_group:
-	iommu_deinit_device(dev);
+	iommu_deinit_device(dev, false);
 	mutex_unlock(&group->mutex);
 	iommu_group_put(group);
 
@@ -759,13 +759,15 @@ static void __iommu_group_remove_device(struct device *dev)
 
 	mutex_lock(&group->mutex);
 	for_each_group_device(group, device) {
+		bool blocked = device->blocked;
+
 		if (device->dev != dev)
 			continue;
 
 		list_del(&device->list);
 		__iommu_group_free_device(group, device);
 		if (dev_has_iommu(dev))
-			iommu_deinit_device(dev);
+			iommu_deinit_device(dev, blocked);
 		else
 			dev->iommu_group = NULL;
 		break;
-- 
2.43.0

[PATCH v6 05/17] iommu: Pass in reset result to pci_dev_reset_iommu_done()

[Nicolin Chen]

IOMMU drivers handle ATC cache maintenance. They may encounter ATC-related
errors (e.g., ATC invalidation timeout), indicating that the ATC cache may
have stale entries that can corrupt the memory. In this case, IOMMU driver
has no choice but to block the device's ATS function and wait for a device
recovery.

The pci_dev_reset_iommu_done() called at the end of a reset function could
serve as a reliable signal to the IOMMU subsystem that the physical device
cache is completely clean. However, the function is called unconditionally
even if the reset operation had actually failed, which would re-attach the
faulty device back to a normal translation domain. And this will leave the
system highly exposed, creating vulnerabilities for data corruption:
    IOMMU blocks RID/ATS
    pci_reset_function():
        pci_dev_reset_iommu_prepare(); // Block RID/ATS
        __reset(); // Failed (ATC is still stale)
        pci_dev_reset_iommu_done(); // Unblock RID/ATS (ah-ha)

Instead, pass in @reset_result to pci_dev_reset_iommu_done() from callers:
    IOMMU blocks RID/ATS
    pci_reset_function():
        pci_dev_reset_iommu_prepare(); // Block RID/ATS
        rc = __reset();
        pci_dev_reset_iommu_done(rc); // Unblock or quarantine

On a successful reset, done() restores the device to its RID/PASID domains
and decrements group->recovery_cnt. On failure, the device remains blocked,
and concurrent domain attachment will be rejected until a successful reset.

Note: -ENOTTY is overloaded with different meanings by PCI reset functions.
Some of them indicate "reset was not attempted", while others indicate "try
the next reset method and the current method failed". IOMMU that must react
these two outcomes separately has no choice but to keep the device blocked
on -ENOTTY as well. Leave an inline FIXME and warning.

This introduces a new situation where a blocked device is being unplugged.
Decrement the group->recovery_cnt accordingly. Such a device stays on the
group->blocking_domain, so hand that to the release_domain attach_dev() as
the old domain in place of group->domain.

Suggested-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 include/linux/iommu.h  |  5 ++--
 drivers/iommu/iommu.c  | 65 +++++++++++++++++++++++++++++++++++++++---
 drivers/pci/pci-acpi.c |  2 +-
 drivers/pci/pci.c      | 10 +++----
 drivers/pci/quirks.c   |  2 +-
 5 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ac43b8b93f14a..78d53f1024db1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1224,7 +1224,7 @@ void iommu_free_global_pasid(ioasid_t pasid);
 
 /* PCI device reset functions */
 int pci_dev_reset_iommu_prepare(struct pci_dev *pdev);
-void pci_dev_reset_iommu_done(struct pci_dev *pdev);
+void pci_dev_reset_iommu_done(struct pci_dev *pdev, int reset_result);
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -1554,7 +1554,8 @@ static inline int pci_dev_reset_iommu_prepare(struct pci_dev *pdev)
 	return 0;
 }
 
-static inline void pci_dev_reset_iommu_done(struct pci_dev *pdev)
+static inline void pci_dev_reset_iommu_done(struct pci_dev *pdev,
+					    int reset_result)
 {
 }
 #endif /* CONFIG_IOMMU_API */
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 344288e930c40..a921f0ccb11c0 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -76,6 +76,7 @@ struct iommu_group {
 enum blocked_reason {
 	BLOCKED_NONE = 0, /* Not blocked */
 	BLOCKED_RESETTING, /* PCI reset in flight */
+	BLOCKED_RESET_FAILED, /* PCI reset failed */
 };
 
 struct group_device {
@@ -589,7 +590,8 @@ static void iommu_deinit_device(struct device *dev, bool blocked)
 			release_domain = ops->identity_domain;
 
 		release_domain->ops->attach_dev(release_domain, dev,
-						group->domain);
+						blocked ? group->blocking_domain :
+							  group->domain);
 	}
 
 	if (ops->release_device)
@@ -764,6 +766,9 @@ static void __iommu_group_remove_device(struct device *dev)
 		if (device->dev != dev)
 			continue;
 
+		/* Must drop the recovery_cnt when removing a blocked device */
+		if (blocked && !WARN_ON(group->recovery_cnt == 0))
+			group->recovery_cnt--;
 		list_del(&device->list);
 		__iommu_group_free_device(group, device);
 		if (dev_has_iommu(dev))
@@ -4027,7 +4032,12 @@ EXPORT_SYMBOL_NS_GPL(iommu_replace_group_handle, "IOMMUFD_INTERNAL");
  * reset is finished, pci_dev_reset_iommu_done() can restore everything.
  *
  * Caller must use pci_dev_reset_iommu_prepare() with pci_dev_reset_iommu_done()
- * before/after the core-level reset routine, to decrement the recovery_cnt.
+ * before/after the core-level reset routine. On a successful reset, done() will
+ * decrement group->recovery_cnt and restore domains. On a failure, recovery_cnt
+ * is left intact and the device stays blocked.
+ *
+ * Callers must skip pci_dev_reset_iommu_prepare/done() entirely when no reset
+ * is attempted (e.g. probe mode).
  *
  * Return: 0 on success or negative error code if the preparation failed.
  *
@@ -4057,6 +4067,10 @@ int pci_dev_reset_iommu_prepare(struct pci_dev *pdev)
 	if (gdev->reset_depth++)
 		return 0;
 
+	/* Device might be already blocked for a quarantine */
+	if (gdev->blocked)
+		return 0;
+
 	ret = __iommu_group_alloc_blocking_domain(group);
 	if (ret) {
 		gdev->reset_depth--;
@@ -4138,20 +4152,28 @@ static bool group_device_dma_alias_is_blocked(struct iommu_group *group,
 /**
  * pci_dev_reset_iommu_done() - Restore IOMMU after a PCI device reset is done
  * @pdev: PCI device that has finished a reset routine
+ * @reset_result: Return code from the reset routine
  *
  * After a PCIe device finishes a reset routine, it wants to restore its IOMMU
  * activity, including new translation and cache invalidation, by re-attaching
  * all RID/PASID of the device back to the domains retained in the core-level
  * structure.
  *
- * Caller must pair it with a successful pci_dev_reset_iommu_prepare().
+ * This is a pairing function for pci_dev_reset_iommu_prepare(). Caller passes
+ * the reset return value to @reset_result. On a failed reset, the device will
+ * remain blocked as a quarantine measure, with group->recovery_cnt intact, to
+ * protect system memory until a subsequent successful reset.
+ *
+ * Callers must skip pci_dev_reset_iommu_prepare/done() entirely when no reset
+ * is attempted (e.g. probe mode).
  *
  * Note that, although unlikely, there is a risk that re-attaching domains might
  * fail due to some unexpected happening like OOM.
  */
-void pci_dev_reset_iommu_done(struct pci_dev *pdev)
+void pci_dev_reset_iommu_done(struct pci_dev *pdev, int reset_result)
 {
 	struct iommu_group *group = pdev->dev.iommu_group;
+	enum blocked_reason old_blocked;
 	struct group_device *gdev;
 	unsigned long pasid;
 	void *entry;
@@ -4174,6 +4196,37 @@ void pci_dev_reset_iommu_done(struct pci_dev *pdev)
 	if (WARN_ON(!group->blocking_domain))
 		return;
 
+	/*
+	 * A reset failure implies that the device might be unreliable. E.g. its
+	 * device cache might retain stale entries, which might result in memory
+	 * corruption. Thus, do not unblock the device until a successful reset.
+	 */
+	if (reset_result) {
+		/*
+		 * FIXME: the int-return values from the PCI reset functions are
+		 * not consistent: some reset functions use -ENOTTY to indicate
+		 * "no reset was attempted" (in which case IOMMU should revert a
+		 * prepare), while others use -ENOTTY to indicate "reset failed;
+		 * try the next reset method" (in which case IOMMU should keep
+		 * the device blocked). Without fixing the PCI return result, we
+		 * cannot tell the difference between the two cases. Warn it.
+		 */
+		if (reset_result == -ENOTTY)
+			dev_warn_ratelimited(
+				&pdev->dev,
+				"Reset may have been skipped. Keep it blocked conservatively\n");
+		else
+			dev_err_ratelimited(
+				&pdev->dev,
+				"Reset failed. Keep it blocked to protect memory\n");
+		if (gdev->blocked == BLOCKED_RESETTING)
+			gdev->blocked = BLOCKED_RESET_FAILED;
+		return;
+	}
+
+	if (WARN_ON(!gdev->blocked))
+		return;
+
 	if (group_device_dma_alias_is_blocked(group, gdev)) {
 		/*
 		 * FIXME: DMA aliased devices share the same RID, which would be
@@ -4204,6 +4257,7 @@ void pci_dev_reset_iommu_done(struct pci_dev *pdev)
 	 * the correct domain in iommu_driver_get_domain_for_dev() that might be
 	 * called in a set_dev_pasid callback function.
 	 */
+	old_blocked = gdev->blocked;
 	gdev->blocked = BLOCKED_NONE;
 
 	/*
@@ -4225,6 +4279,9 @@ void pci_dev_reset_iommu_done(struct pci_dev *pdev)
 
 	if (!WARN_ON(group->recovery_cnt == 0))
 		group->recovery_cnt--;
+
+	if (old_blocked > BLOCKED_RESETTING)
+		pci_info(pdev, "Device is unblocked after successful reset\n");
 }
 EXPORT_SYMBOL_GPL(pci_dev_reset_iommu_done);
 
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 42d545edd7fad..0afb7075bfc86 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -977,7 +977,7 @@ int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
 		ret = -ENOTTY;
 	}
 
-	pci_dev_reset_iommu_done(dev);
+	pci_dev_reset_iommu_done(dev, ret);
 	return ret;
 }
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a6e0ecfd207a7..b58eec5e82ca8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4423,7 +4423,7 @@ int pcie_flr(struct pci_dev *dev)
 
 	ret = pci_dev_wait(dev, "FLR", PCIE_RESET_READY_POLL_MS);
 done:
-	pci_dev_reset_iommu_done(dev);
+	pci_dev_reset_iommu_done(dev, ret);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(pcie_flr);
@@ -4501,7 +4501,7 @@ static int pci_af_flr(struct pci_dev *dev, bool probe)
 
 	ret = pci_dev_wait(dev, "AF_FLR", PCIE_RESET_READY_POLL_MS);
 done:
-	pci_dev_reset_iommu_done(dev);
+	pci_dev_reset_iommu_done(dev, ret);
 	return ret;
 }
 
@@ -4555,7 +4555,7 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
 	pci_dev_d3_sleep(dev);
 
 	ret = pci_dev_wait(dev, "PM D3hot->D0", PCIE_RESET_READY_POLL_MS);
-	pci_dev_reset_iommu_done(dev);
+	pci_dev_reset_iommu_done(dev, ret);
 	return ret;
 }
 
@@ -5011,7 +5011,7 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
 	rc = pci_parent_bus_reset(dev, probe);
 done:
 	if (!probe)
-		pci_dev_reset_iommu_done(dev);
+		pci_dev_reset_iommu_done(dev, rc);
 	return rc;
 }
 
@@ -5064,7 +5064,7 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
 		pci_write_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL,
 				      reg);
 
-	pci_dev_reset_iommu_done(dev);
+	pci_dev_reset_iommu_done(dev, rc);
 	return rc;
 }
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 7858a063929d9..64ffc44a6cff2 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4263,7 +4263,7 @@ static int __pci_dev_specific_reset(struct pci_dev *dev, bool probe,
 
 	ret = i->reset(dev, probe);
 	if (!probe)
-		pci_dev_reset_iommu_done(dev);
+		pci_dev_reset_iommu_done(dev, ret);
 	return ret;
 }
 
-- 
2.43.0

[PATCH v6 06/17] iommu/arm-smmu-v3: Don't rb_erase() a never-inserted stream node

[Nicolin Chen]

arm_smmu_insert_master() skips inserting a stream whose StreamID duplicates
one the same master already owns (bridged PCI devices can present duplicate
IDs), leaving that master->streams[i].node zeroed and unlinked from the
smmu->streams rb-tree.

Both the insert error-rollback loop and arm_smmu_remove_master() then call
rb_erase() on every master->streams[i].node unconditionally. rb_erase() on
a zeroed node sees a NULL parent, treats the node as the tree root and sets
root->rb_node = NULL, silently emptying the whole SID tree and breaking SID
lookups (and DMA) for every other master on the SMMU.

Mark each node with RB_CLEAR_NODE() after sort_nonatomic() reorders the
array, since sorting relocates the entries and would leave the earlier
self-referential RB_CLEAR_NODE() pointer stale. An un-inserted node then
stays RB_EMPTY_NODE() and is skipped in both erase loops; inserted nodes
are linked by rb_find_add() and erased as before.

Fixes: b00d24997a11 ("iommu/arm-smmu-v3: Fix iommu_device_probe bug due to duplicated stream ids")
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 14 ++++++++++++--
 1 file changed, 12 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 5732f3ba0122d..082da3dc09e56 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4122,6 +4122,13 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 	     sizeof(master->streams[0]), arm_smmu_stream_id_cmp,
 	     NULL);
 
+	/*
+	 * Clear after sorting: RB_CLEAR_NODE() records the node's own address,
+	 * which sort_nonatomic() invalidates by relocating the entries.
+	 */
+	for (i = 0; i < fwspec->num_ids; i++)
+		RB_CLEAR_NODE(&master->streams[i].node);
+
 	mutex_lock(&smmu->streams_mutex);
 	for (i = 0; i < fwspec->num_ids; i++) {
 		struct arm_smmu_stream *new_stream = &master->streams[i];
@@ -4154,7 +4161,9 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 
 	if (ret) {
 		for (i--; i >= 0; i--)
-			rb_erase(&master->streams[i].node, &smmu->streams);
+			if (!RB_EMPTY_NODE(&master->streams[i].node))
+				rb_erase(&master->streams[i].node,
+					 &smmu->streams);
 		kfree(master->streams);
 		kfree(master->build_invs);
 	}
@@ -4174,7 +4183,8 @@ static void arm_smmu_remove_master(struct arm_smmu_master *master)
 
 	mutex_lock(&smmu->streams_mutex);
 	for (i = 0; i < fwspec->num_ids; i++)
-		rb_erase(&master->streams[i].node, &smmu->streams);
+		if (!RB_EMPTY_NODE(&master->streams[i].node))
+			rb_erase(&master->streams[i].node, &smmu->streams);
 	mutex_unlock(&smmu->streams_mutex);
 
 	kfree(master->streams);
-- 
2.43.0

Re: [PATCH v6 06/17] iommu/arm-smmu-v3: Don't rb_erase() a never-inserted stream node

[Nicolin Chen]

Hi Will,

On Wed, Sep 23, 2026 at 01:11:25PM -0700, Nicolin Chen wrote:
> arm_smmu_insert_master() skips inserting a stream whose StreamID duplicates
> one the same master already owns (bridged PCI devices can present duplicate
> IDs), leaving that master->streams[i].node zeroed and unlinked from the
> smmu->streams rb-tree.
> 
> Both the insert error-rollback loop and arm_smmu_remove_master() then call
> rb_erase() on every master->streams[i].node unconditionally. rb_erase() on
> a zeroed node sees a NULL parent, treats the node as the tree root and sets
> root->rb_node = NULL, silently emptying the whole SID tree and breaking SID
> lookups (and DMA) for every other master on the SMMU.
> 
> Mark each node with RB_CLEAR_NODE() after sort_nonatomic() reorders the
> array, since sorting relocates the entries and would leave the earlier
> self-referential RB_CLEAR_NODE() pointer stale. An un-inserted node then
> stays RB_EMPTY_NODE() and is skipped in both erase loops; inserted nodes
> are linked by rb_find_add() and erased as before.
> 
> Fixes: b00d24997a11 ("iommu/arm-smmu-v3: Fix iommu_device_probe bug due to duplicated stream ids")
> Assisted-by: LLM
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>

There have been two other versions of this fix being sent by other
developers in this cycle; both of them were seemingly for the same
Sashiko finding as this one.

To avoid more versions sent by more folks, maybe we can prioritize
this patch considering that this series might need more time going
through the reviews?

Thanks
Nicolin

Re: [PATCH v6 06/17] iommu/arm-smmu-v3: Don't rb_erase() a never-inserted stream node

[Mostafa Saleh]

On Wed, Sep 23, 2026 at 01:11:25PM -0700, Nicolin Chen wrote:
> arm_smmu_insert_master() skips inserting a stream whose StreamID duplicates
> one the same master already owns (bridged PCI devices can present duplicate
> IDs), leaving that master->streams[i].node zeroed and unlinked from the
> smmu->streams rb-tree.
> 
> Both the insert error-rollback loop and arm_smmu_remove_master() then call
> rb_erase() on every master->streams[i].node unconditionally. rb_erase() on
> a zeroed node sees a NULL parent, treats the node as the tree root and sets
> root->rb_node = NULL, silently emptying the whole SID tree and breaking SID
> lookups (and DMA) for every other master on the SMMU.
> 
> Mark each node with RB_CLEAR_NODE() after sort_nonatomic() reorders the
> array, since sorting relocates the entries and would leave the earlier
> self-referential RB_CLEAR_NODE() pointer stale. An un-inserted node then
> stays RB_EMPTY_NODE() and is skipped in both erase loops; inserted nodes
> are linked by rb_find_add() and erased as before.
> 
> Fixes: b00d24997a11 ("iommu/arm-smmu-v3: Fix iommu_device_probe bug due to duplicated stream ids")
> Assisted-by: LLM
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 14 ++++++++++++--
>  1 file changed, 12 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 5732f3ba0122d..082da3dc09e56 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -4122,6 +4122,13 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
>  	     sizeof(master->streams[0]), arm_smmu_stream_id_cmp,
>  	     NULL);
>  
> +	/*
> +	 * Clear after sorting: RB_CLEAR_NODE() records the node's own address,
> +	 * which sort_nonatomic() invalidates by relocating the entries.
> +	 */
> +	for (i = 0; i < fwspec->num_ids; i++)
> +		RB_CLEAR_NODE(&master->streams[i].node);
> +
>  	mutex_lock(&smmu->streams_mutex);
>  	for (i = 0; i < fwspec->num_ids; i++) {
>  		struct arm_smmu_stream *new_stream = &master->streams[i];
> @@ -4154,7 +4161,9 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
>  
>  	if (ret) {
>  		for (i--; i >= 0; i--)
> -			rb_erase(&master->streams[i].node, &smmu->streams);
> +			if (!RB_EMPTY_NODE(&master->streams[i].node))
> +				rb_erase(&master->streams[i].node,
> +					 &smmu->streams);
>  		kfree(master->streams);
>  		kfree(master->build_invs);
>  	}
> @@ -4174,7 +4183,8 @@ static void arm_smmu_remove_master(struct arm_smmu_master *master)
>  
>  	mutex_lock(&smmu->streams_mutex);
>  	for (i = 0; i < fwspec->num_ids; i++)
> -		rb_erase(&master->streams[i].node, &smmu->streams);
> +		if (!RB_EMPTY_NODE(&master->streams[i].node))
> +			rb_erase(&master->streams[i].node, &smmu->streams);
>  	mutex_unlock(&smmu->streams_mutex);
>  
>  	kfree(master->streams);
> -- 
> 2.43.0
> 

[PATCH v6 07/17] iommu/arm-smmu-v3: Track ATC invalidation timeouts in a bitmap

[Nicolin Chen]

An ATC invalidation timeout is a fatal error. While the SMMUv3 hardware is
aware of the timeout via a GERROR interrupt, the driver thread issuing the
commands lacks a direct mechanism to verify whether its specific batch was
the cause or not, as polling the CMD_SYNC status doesn't natively return a
failure code, making it very difficult to coordinate per-device recovery.

Introduce an atc_sync_timeouts bitmap in the cmdq structure to bridge this
gap. When the ISR detects an ATC timeout, set the bit corresponding to the
physical CMDQ index of the faulting CMD_SYNC command.

On the issuer side, test the bit after the poll completes or times out, and
return -EIO when it is set, so the caller can quarantine the device. Order
the test after the poll with an smp_rmb(), since the poll may use a relaxed
load. Test before clearing, to spare the shared cache line an atomic RMW in
the common no-timeout case.

Clear any stale bit before publishing a new CMD_SYNC in a reused slot, so
the GERROR handler can only mark the new command.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 70 ++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)

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 50f8321e979ce..b8c3fd7aeea45 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -703,6 +703,7 @@ struct arm_smmu_cmdq {
 	atomic_long_t			*valid_map;
 	atomic_t			owner_prod;
 	atomic_t			lock;
+	unsigned long			*atc_sync_timeouts;
 	bool				(*supports_cmd)(struct arm_smmu_cmd *cmd);
 };
 
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 082da3dc09e56..eff60674c9f05 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -352,7 +352,10 @@ void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
 		 * at the CMD_SYNC. Attempt to complete other pending commands
 		 * by repeating the CMD_SYNC, though we might well end up back
 		 * here since the ATC invalidation may still be pending.
+		 *
+		 * Mark the faulty batch in the bitmap for the issuer to match.
 		 */
+		set_bit(Q_IDX(&q->llq, cons), cmdq->atc_sync_timeouts);
 		return;
 	case CMDQ_ERR_CERROR_ILL_IDX:
 	default:
@@ -759,6 +762,14 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
 		queue_write(Q_ENT(&cmdq->q, prod), cmd_sync.data,
 			    ARRAY_SIZE(cmd_sync.data));
 
+		/*
+		 * Clear any stale ATC-timeout bit left in the slot from a prior
+		 * wraparound, before the slot becomes visible to the SMMU. Must
+		 * do this prior to step 3, to prevent a potential race with the
+		 * GERROR ISR calling set_bit() for our own CMD_SYNC.
+		 */
+		clear_bit(Q_IDX(&llq, prod), cmdq->atc_sync_timeouts);
+
 		/*
 		 * In order to determine completion of our CMD_SYNC, we must
 		 * ensure that the queue can't wrap twice without us noticing.
@@ -805,9 +816,61 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
 
 	/* 5. If we are inserting a CMD_SYNC, we must wait for it to complete */
 	if (sync) {
+		u32 sync_prod;
+
 		llq.prod = queue_inc_prod_n(&llq, n);
+		sync_prod = llq.prod;
 		ret = arm_smmu_cmdq_poll_until_sync(smmu, cmdq, &llq);
-		if (ret) {
+
+		/*
+		 * Ensure that the read in __arm_smmu_cmdq_poll_until_msi() or
+		 * __arm_smmu_cmdq_poll_until_consumed() are completed, before
+		 * testing the atc_sync_timeouts bitmap below.
+		 *
+		 * Without it, the test_bit() below could be reordered before
+		 * the relaxed reads in the two poll functions, missing a bit
+		 * that is set before the CMD_SYNC completion. The wmb in the
+		 * writel(GERRORN) ensures that the set_bit() in the ISR must
+		 * be completed, followed by the SMMU consuming the CMD_SYNC.
+		 *
+		 *  [CPU0 - issuer]             | [CPU1 - GERROR ISR]
+		 *                              | __arm_smmu_cmdq_skip_err() {
+		 *                              |   set_bit(atc_sync_timeouts);
+		 *                              | }
+		 *                              | writel(gerror, GERRORN);
+		 *                              |   // wmb: SMMU then resumes,
+		 *  // completion generated by  |   // consuming the CMD_SYNC
+		 *  // the consumed CMD_SYNC    |
+		 *  read CMD_SYNC completion;   |
+		 *  smp_rmb(); // ensure reads  |
+		 *             // are completed |
+		 *  test_bit(atc_sync_timeouts);|
+		 */
+		smp_rmb();
+
+		/*
+		 * Test atc_sync_timeouts first and see if there is ATC timeout
+		 * resulted from this cmdlist. Return -EIO to separate from the
+		 * ARM_SMMU_POLL_TIMEOUT_US software timeout. Use a non-atomic
+		 * test_bit() first, sparing an atomic RMW in the common case.
+		 *
+		 * FIXME possible unhandled ATC invalidation timeout scenario:
+		 * PCI Completion Timeout can be set to a range longer than the
+		 * ARM_SMMU_POLL_TIMEOUT_US software timeout. -ETIMEDOUT can be
+		 * returned by arm_smmu_cmdq_poll_until_sync() while the ATC_INV
+		 * is still pending and not yet reflected in GERROR, so the bit
+		 * on atc_sync_timeouts is not set. In this case, we can hardly
+		 * do anything here, since the command queue HW is still pending
+		 * on the ATC command.
+		 */
+		if (test_bit(Q_IDX(&llq, sync_prod), cmdq->atc_sync_timeouts)) {
+			clear_bit(Q_IDX(&llq, sync_prod),
+				  cmdq->atc_sync_timeouts);
+			dev_err_ratelimited(smmu->dev,
+					    "CMD_SYNC for ATC_INV timeout at prod=0x%08x\n",
+					    sync_prod);
+			ret = -EIO;
+		} else if (ret) {
 			dev_err_ratelimited(smmu->dev,
 					    "CMD_SYNC timeout at 0x%08x [hwprod 0x%08x, hwcons 0x%08x]\n",
 					    llq.prod,
@@ -4476,6 +4539,11 @@ int arm_smmu_cmdq_init(struct arm_smmu_device *smmu,
 	if (!cmdq->valid_map)
 		return -ENOMEM;
 
+	cmdq->atc_sync_timeouts =
+		devm_bitmap_zalloc(smmu->dev, nents, GFP_KERNEL);
+	if (!cmdq->atc_sync_timeouts)
+		return -ENOMEM;
+
 	return 0;
 }
 
-- 
2.43.0

[PATCH v6 08/17] iommu/arm-smmu-v3: Skip remaining GERROR causes on SFM

[Nicolin Chen]

When the SMMU enters Service Failure Mode (SFM), arm_smmu_device_disable()
clears CR0 and the SMMU stops processing requests entirely. The remaining
GERROR causes (MSI write aborts, PRIQ/EVTQ aborts, CMDQ_ERR) are moot at
that point: the cmdq is dead so arm_smmu_cmdq_skip_err() would just twiddle
bookkeeping for a queue nobody's reading, and the per-cause dev_warn lines
add little diagnostic value beyond the SFM message itself.

Ack the GERROR before arm_smmu_device_disable() and return. Acking before
the multi-ms disable wait keeps a level-triggered IRQ source from re-firing
the handler. The writel+return here duplicates the non-SFM tail because a
subsequent commit will give the two paths different locking. SFM is one-way
and the SMMU does not generate new GERROR causes, so the ack is final.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +++
 1 file changed, 3 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 eff60674c9f05..24f286e994e6e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2380,8 +2380,11 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
 		 active);
 
 	if (active & GERROR_SFM_ERR) {
+		/* SMMU is being disabled, so other errors don't matter */
+		writel(gerror, smmu->base + ARM_SMMU_GERRORN);
 		dev_err(smmu->dev, "device has entered Service Failure Mode!\n");
 		arm_smmu_device_disable(smmu);
+		return IRQ_HANDLED;
 	}
 
 	if (active & GERROR_MSI_GERROR_ABT_ERR)
-- 
2.43.0

[PATCH v6 09/17] iommu/arm-smmu-v3: Introduce per-cmdq cmdq_err_handler callback

[Nicolin Chen]

A subsequent change will need arm_smmu_cmdq_issue_cmdlist() to co-clear a
pending CMDQ_ERR after a CMD_SYNC poll timeout. And this needs to be done
for both smmu->cmdq and tegra241-cmdq.

Add a cmdq_err_handler and a paired cmdq_err_lock to struct arm_smmu_cmdq.
arm_smmu_gerror_handler() now takes the per-cmdq cmdq_err_lock when acking
CMDQ_ERR. It already covers a concurrent ack from cmdq_err_handler via its
existing early-exit on no-active-bits.

Impl functions and caller will be added in the subsequent change.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h    | 12 ++++++++++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c    | 17 +++++++++++++----
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c |  2 +-
 3 files changed, 24 insertions(+), 7 deletions(-)

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 b8c3fd7aeea45..ca8f52142b07f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -698,6 +698,10 @@ struct arm_smmu_queue_poll {
 	bool				wfe;
 };
 
+struct arm_smmu_cmdq;
+typedef void (*arm_smmu_cmdq_err_fn)(struct arm_smmu_device *smmu,
+				     struct arm_smmu_cmdq *cmdq);
+
 struct arm_smmu_cmdq {
 	struct arm_smmu_queue		q;
 	atomic_long_t			*valid_map;
@@ -705,6 +709,10 @@ struct arm_smmu_cmdq {
 	atomic_t			lock;
 	unsigned long			*atc_sync_timeouts;
 	bool				(*supports_cmd)(struct arm_smmu_cmd *cmd);
+
+	/* Drain a pending CMDQ_ERR; will hold cmdq_err_lock with irqsave */
+	arm_smmu_cmdq_err_fn		cmdq_err_handler;
+	raw_spinlock_t			cmdq_err_lock;
 };
 
 static inline bool arm_smmu_cmdq_supports_cmd(struct arm_smmu_cmdq *cmdq,
@@ -1170,8 +1178,8 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
 			    struct arm_smmu_queue *q, void __iomem *page,
 			    unsigned long prod_off, unsigned long cons_off,
 			    size_t dwords, const char *name);
-int arm_smmu_cmdq_init(struct arm_smmu_device *smmu,
-		       struct arm_smmu_cmdq *cmdq);
+int arm_smmu_cmdq_init(struct arm_smmu_device *smmu, struct arm_smmu_cmdq *cmdq,
+		       arm_smmu_cmdq_err_fn cmdq_err_handler);
 
 static inline bool arm_smmu_master_canwbs(struct arm_smmu_master *master)
 {
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 24f286e994e6e..409ee1be76815 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2367,13 +2367,18 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
 {
 	u32 gerror, gerrorn, active;
 	struct arm_smmu_device *smmu = dev;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&smmu->cmdq.cmdq_err_lock, flags);
 
 	gerror = readl_relaxed(smmu->base + ARM_SMMU_GERROR);
 	gerrorn = readl_relaxed(smmu->base + ARM_SMMU_GERRORN);
 
 	active = gerror ^ gerrorn;
-	if (!(active & GERROR_ERR_MASK))
+	if (!(active & GERROR_ERR_MASK)) {
+		raw_spin_unlock_irqrestore(&smmu->cmdq.cmdq_err_lock, flags);
 		return IRQ_NONE; /* No errors pending */
+	}
 
 	dev_warn(smmu->dev,
 		 "unexpected global error reported (0x%08x), this could be serious\n",
@@ -2382,6 +2387,7 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
 	if (active & GERROR_SFM_ERR) {
 		/* SMMU is being disabled, so other errors don't matter */
 		writel(gerror, smmu->base + ARM_SMMU_GERRORN);
+		raw_spin_unlock_irqrestore(&smmu->cmdq.cmdq_err_lock, flags);
 		dev_err(smmu->dev, "device has entered Service Failure Mode!\n");
 		arm_smmu_device_disable(smmu);
 		return IRQ_HANDLED;
@@ -2409,6 +2415,7 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
 		arm_smmu_cmdq_skip_err(smmu);
 
 	writel(gerror, smmu->base + ARM_SMMU_GERRORN);
+	raw_spin_unlock_irqrestore(&smmu->cmdq.cmdq_err_lock, flags);
 	return IRQ_HANDLED;
 }
 
@@ -4529,13 +4536,15 @@ int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
 	return 0;
 }
 
-int arm_smmu_cmdq_init(struct arm_smmu_device *smmu,
-		       struct arm_smmu_cmdq *cmdq)
+int arm_smmu_cmdq_init(struct arm_smmu_device *smmu, struct arm_smmu_cmdq *cmdq,
+		       arm_smmu_cmdq_err_fn cmdq_err_handler)
 {
 	unsigned int nents = 1 << cmdq->q.llq.max_n_shift;
 
 	atomic_set(&cmdq->owner_prod, 0);
 	atomic_set(&cmdq->lock, 0);
+	raw_spin_lock_init(&cmdq->cmdq_err_lock);
+	cmdq->cmdq_err_handler = cmdq_err_handler;
 
 	cmdq->valid_map = (atomic_long_t *)devm_bitmap_zalloc(smmu->dev, nents,
 							      GFP_KERNEL);
@@ -4575,7 +4584,7 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 	if (ret)
 		return ret;
 
-	ret = arm_smmu_cmdq_init(smmu, &smmu->cmdq);
+	ret = arm_smmu_cmdq_init(smmu, &smmu->cmdq, NULL);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 6644075c1431e..a595769e7ecd6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -676,7 +676,7 @@ static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
 	q->q_base = q->base_dma & VCMDQ_ADDR;
 	q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
 
-	return arm_smmu_cmdq_init(smmu, cmdq);
+	return arm_smmu_cmdq_init(smmu, cmdq, NULL);
 }
 
 /* VINTF Logical VCMDQ Resource Helpers */
-- 
2.43.0

[PATCH v6 10/17] iommu/arm-smmu-v3: Recheck CMDQ_ERR in tegra241_vintf0_handle_error()

[Nicolin Chen]

A subsequent change will allow cmdq_err_handler to ack a pending CMDQ_ERR
concurrently with tegra241_vintf0_handle_error(). Take cmdq_err_lock around
the gerror read and ack, and recheck (gerror ^ gerrorn) & GERROR_CMDQ_ERR
before calling __arm_smmu_cmdq_skip_err() so a concurrent ack doesn't cause
us to skip_err on an already-handled error.

arm_smmu_gerror_handler() already covers this via its existing early-exit
on no-active-bits.

tegra241_vcmdq_hw_deinit() acks the same GERROR/GERRORN pair unlocked, and
the error IRQ is live from probe, so a latched-error ISR can race a VCMDQ
deinit during a device reset. Take the lock around that ack as well. Since
a user-owned VCMDQ never goes through arm_smmu_cmdq_init() yet does reach
tegra241_vcmdq_hw_deinit(), initialize its cmdq_err_lock at allocation.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index a595769e7ecd6..cf008bee213c4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -323,7 +323,8 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 			unsigned long map_bit = __ffs64(map);
 			unsigned long lidx = 64 * i + map_bit;
 			struct tegra241_vcmdq *vcmdq;
-			u32 gerror;
+			unsigned long flags;
+			u32 gerror, gerrorn;
 
 			map &= ~BIT_ULL(map_bit);
 
@@ -335,9 +336,16 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 			if (!vcmdq)
 				continue;
 
+			raw_spin_lock_irqsave(&vcmdq->cmdq.cmdq_err_lock, flags);
 			gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
-			__arm_smmu_cmdq_skip_err(&cmdqv->smmu, &vcmdq->cmdq);
+			gerrorn = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERRORN));
+
+			if ((gerror ^ gerrorn) & GERROR_CMDQ_ERR)
+				__arm_smmu_cmdq_skip_err(&cmdqv->smmu,
+							 &vcmdq->cmdq);
 			writel(gerror, REG_VCMDQ_PAGE0(vcmdq, GERRORN));
+			raw_spin_unlock_irqrestore(&vcmdq->cmdq.cmdq_err_lock,
+						   flags);
 		}
 	}
 }
@@ -477,6 +485,7 @@ static void tegra241_vcmdq_hw_deinit(struct tegra241_vcmdq *vcmdq)
 {
 	char header[64], *h = lvcmdq_error_header(vcmdq, header, 64);
 	u32 gerrorn, gerror;
+	unsigned long flags;
 
 	if (vcmdq_write_config(vcmdq, 0)) {
 		dev_err(vcmdq->cmdqv->dev,
@@ -492,6 +501,7 @@ static void tegra241_vcmdq_hw_deinit(struct tegra241_vcmdq *vcmdq)
 	writeq_relaxed(0, REG_VCMDQ_PAGE1(vcmdq, BASE));
 	writeq_relaxed(0, REG_VCMDQ_PAGE1(vcmdq, CONS_INDX_BASE));
 
+	raw_spin_lock_irqsave(&vcmdq->cmdq.cmdq_err_lock, flags);
 	gerrorn = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERRORN));
 	gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
 	if (gerror != gerrorn) {
@@ -499,6 +509,7 @@ static void tegra241_vcmdq_hw_deinit(struct tegra241_vcmdq *vcmdq)
 			 "%suncleared error detected, resetting\n", h);
 		writel(gerror, REG_VCMDQ_PAGE0(vcmdq, GERRORN));
 	}
+	raw_spin_unlock_irqrestore(&vcmdq->cmdq.cmdq_err_lock, flags);
 
 	dev_dbg(vcmdq->cmdqv->dev, "%sdeinited\n", h);
 }
@@ -1206,6 +1217,8 @@ static int tegra241_vintf_alloc_lvcmdq_user(struct iommufd_hw_queue *hw_queue,
 	vcmdq->cmdq.q.q_base = base_addr_pa & VCMDQ_ADDR;
 	vcmdq->cmdq.q.q_base |= log2size;
 
+	raw_spin_lock_init(&vcmdq->cmdq.cmdq_err_lock);
+
 	ret = tegra241_vcmdq_hw_init_user(vcmdq);
 	if (ret)
 		goto unmap_lvcmdq;
-- 
2.43.0

[PATCH v6 11/17] iommu/arm-smmu-v3: Co-clear pending CMDQ_ERR when CMD_SYNC times out

[Nicolin Chen]

Once arm_smmu_cmdq_poll_until_sync() returns, arm_smmu_cmdq_issue_cmdlist()
tests its CMD_SYNC slot in atc_sync_timeouts to decide whether there was an
ATC_INV timeout.

On the other hand, when that poll timed out, the GERROR ISR might have been
delayed past the poll deadline, so the atc_sync_timeouts test could miss an
ATC_INV timeout, classifying it as a generic CMD_SYNC timeout and bypassing
the per-device quarantine.

Add two cmdq_err_handler impl functions:
 - arm_smmu_cmdq_err_handler() reads SMMU GERROR/GERRORN.
 - tegra241_vcmdq_handle_cmdq_err() reads VCMDQ GERROR/GERRORN.

Co-clear any pending CMDQ_ERR in the issuer, when the polling on a CMD_SYNC
times out. Each cmdq impl serializes the synchronous drain against its own
IRQ handler with cmdq->cmdq_err_lock.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 31 ++++++++++++++++++-
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 24 +++++++++++++-
 2 files changed, 53 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 409ee1be76815..3081ef407fd69 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -822,6 +822,15 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
 		sync_prod = llq.prod;
 		ret = arm_smmu_cmdq_poll_until_sync(smmu, cmdq, &llq);
 
+		/*
+		 * When the poll above timed out, the GERROR ISR might have been
+		 * delayed past the poll deadline, so the atc_sync_timeouts test
+		 * below could miss our ATC_INV timeout. Thus, drain any pending
+		 * CMDQ_ERR synchronously first via the per-cmdq callback.
+		 */
+		if (ret && cmdq->cmdq_err_handler)
+			cmdq->cmdq_err_handler(smmu, cmdq);
+
 		/*
 		 * Ensure that the read in __arm_smmu_cmdq_poll_until_msi() or
 		 * __arm_smmu_cmdq_poll_until_consumed() are completed, before
@@ -2363,6 +2372,26 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
 
 static int arm_smmu_device_disable(struct arm_smmu_device *smmu);
 
+/* Drain a pending CMDQ_ERR, used by arm_smmu_cmdq_issue_cmdlist() */
+static void arm_smmu_cmdq_err_handler(struct arm_smmu_device *smmu,
+				      struct arm_smmu_cmdq *cmdq)
+{
+	u32 gerror, gerrorn;
+
+	guard(raw_spinlock_irqsave)(&cmdq->cmdq_err_lock);
+
+	gerror = readl_relaxed(smmu->base + ARM_SMMU_GERROR);
+	gerrorn = readl_relaxed(smmu->base + ARM_SMMU_GERRORN);
+
+	if (!((gerror ^ gerrorn) & GERROR_CMDQ_ERR))
+		return;
+
+	__arm_smmu_cmdq_skip_err(smmu, cmdq);
+
+	/* Toggle only the CMDQ_ERR bit; other bits are left for the ISR. */
+	writel(gerrorn ^ GERROR_CMDQ_ERR, smmu->base + ARM_SMMU_GERRORN);
+}
+
 static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
 {
 	u32 gerror, gerrorn, active;
@@ -4584,7 +4613,7 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 	if (ret)
 		return ret;
 
-	ret = arm_smmu_cmdq_init(smmu, &smmu->cmdq, NULL);
+	ret = arm_smmu_cmdq_init(smmu, &smmu->cmdq, arm_smmu_cmdq_err_handler);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index cf008bee213c4..690de008f4dcc 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -350,6 +350,28 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 	}
 }
 
+/* Drain a pending CMDQ_ERR, used by arm_smmu_cmdq_issue_cmdlist() */
+static void tegra241_vcmdq_handle_cmdq_err(struct arm_smmu_device *smmu,
+					   struct arm_smmu_cmdq *cmdq)
+{
+	struct tegra241_vcmdq *vcmdq =
+		container_of(cmdq, struct tegra241_vcmdq, cmdq);
+	u32 gerror, gerrorn;
+
+	guard(raw_spinlock_irqsave)(&cmdq->cmdq_err_lock);
+
+	gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
+	gerrorn = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERRORN));
+
+	if (!((gerror ^ gerrorn) & GERROR_CMDQ_ERR))
+		return;
+
+	__arm_smmu_cmdq_skip_err(smmu, cmdq);
+
+	/* Toggle only the CMDQ_ERR bit on this VCMDQ's GERRORN */
+	writel(gerrorn ^ GERROR_CMDQ_ERR, REG_VCMDQ_PAGE0(vcmdq, GERRORN));
+}
+
 /*
  * The CMDQV error interrupt is edge-triggered, so a pending VINTF error fires
  * this ISR once and does not re-assert. An unacked guest therefore cannot
@@ -687,7 +709,7 @@ static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
 	q->q_base = q->base_dma & VCMDQ_ADDR;
 	q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
 
-	return arm_smmu_cmdq_init(smmu, cmdq, NULL);
+	return arm_smmu_cmdq_init(smmu, cmdq, tegra241_vcmdq_handle_cmdq_err);
 }
 
 /* VINTF Logical VCMDQ Resource Helpers */
-- 
2.43.0

[PATCH v6 12/17] iommu/arm-smmu-v3: Introduce arm_smmu_cmdq_batch_issue() wrapper

[Nicolin Chen]

Both arm_smmu_cmdq_batch_submit() and arm_smmu_cmdq_batch_add_cmd_p() call
arm_smmu_cmdq_issue_cmdlist() to flush batches. A future change will retry
the issued commands on -EIO, using the arm_smmu_invs carried in the batch.
So, a single hook point is preferred.

Introduce an arm_smmu_cmdq_batch_issue() wrapper, so a retry logic will be
simply filled into the wrapper.

No functional changes.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 17 +++++++++++------
 1 file changed, 11 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 3081ef407fd69..c3c31d4010a0a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -965,6 +965,14 @@ static void arm_smmu_cmdq_batch_init_cmd(struct arm_smmu_device *smmu,
 	cmds->cmdq = arm_smmu_get_cmdq(smmu, cmd);
 }
 
+static int arm_smmu_cmdq_batch_issue(struct arm_smmu_device *smmu,
+				     struct arm_smmu_cmdq_batch *cmds,
+				     bool sync)
+{
+	return arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
+					   cmds->num, sync);
+}
+
 static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
 					   struct arm_smmu_cmdq_batch *cmds,
 					   struct arm_smmu_cmd *cmd)
@@ -994,14 +1002,12 @@ static void arm_smmu_cmdq_batch_add_cmd_p(struct arm_smmu_device *smmu,
 					  struct arm_smmu_cmd *cmd)
 {
 	if (arm_smmu_cmdq_batch_force_sync(smmu, cmds, cmd)) {
-		arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
-					    cmds->num, true);
+		arm_smmu_cmdq_batch_issue(smmu, cmds, true);
 		arm_smmu_cmdq_batch_init_cmd(smmu, cmds, cmd);
 	}
 
 	if (cmds->num == CMDQ_BATCH_ENTRIES) {
-		arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
-					    cmds->num, false);
+		arm_smmu_cmdq_batch_issue(smmu, cmds, false);
 		arm_smmu_cmdq_batch_init_cmd(smmu, cmds, cmd);
 	}
 
@@ -1017,8 +1023,7 @@ static void arm_smmu_cmdq_batch_add_cmd_p(struct arm_smmu_device *smmu,
 static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu,
 				      struct arm_smmu_cmdq_batch *cmds)
 {
-	return arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
-					   cmds->num, true);
+	return arm_smmu_cmdq_batch_issue(smmu, cmds, true);
 }
 
 static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
-- 
2.43.0

[PATCH v6 13/17] iommu/arm-smmu-v3: Add streams_lock for atomic-context SID->master lookup

[Nicolin Chen]

A subsequent change will look up arm_smmu_master entries by SID from inside
arm_smmu_cmdq_batch_retry(), which runs with invs->rwlock read_lock held in
IRQ-disabled context, and so cannot take the sleeping streams_mutex.

Add a spinlock_t streams_lock that protects rb_root mutations alongside the
existing streams_mutex:
 - atomic-context readers will hold the spinlock alone
 - writers (insert/remove paths) take both

A reader under the streams_lock uses all the streams of the found master,
so the insertion has to be all-or-nothing: make arm_smmu_insert_master()
initialize all the L2 strtabs first and then insert all the stream nodes in
one critical section, making a master found via any single SID always fully
initialized.

Update the lockdep assertion in arm_smmu_find_master() to accept either of
the locks so the helper is callable from both contexts.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 95 ++++++++++++++-------
 2 files changed, 65 insertions(+), 32 deletions(-)

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 ca8f52142b07f..2f8d22cc404ac 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -972,6 +972,8 @@ struct arm_smmu_device {
 
 	struct rb_root			streams;
 	struct mutex			streams_mutex;
+	/* Held during rb_root updates; allows atomic-context lookups */
+	spinlock_t			streams_lock;
 };
 
 struct arm_smmu_stream {
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 c3c31d4010a0a..6f217e916ab0c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2126,7 +2126,8 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
 {
 	struct rb_node *node;
 
-	lockdep_assert_held(&smmu->streams_mutex);
+	lockdep_assert(lockdep_is_held(&smmu->streams_mutex) ||
+		       lockdep_is_held(&smmu->streams_lock));
 
 	node = rb_find(&sid, &smmu->streams, arm_smmu_streams_cmp_key);
 	if (!node)
@@ -4191,6 +4192,51 @@ static int arm_smmu_stream_id_cmp(const void *_l, const void *_r)
 	return cmp_int(*l, *r);
 }
 
+/* Caller must hold the streams_mutex. Publishes all the nodes, or none */
+static int arm_smmu_insert_streams(struct arm_smmu_device *smmu,
+				   struct arm_smmu_master *master)
+{
+	struct arm_smmu_master *existing_master = NULL;
+	u32 existing_sid = 0;
+	unsigned long flags;
+	int ret = 0;
+	int i;
+
+	spin_lock_irqsave(&smmu->streams_lock, flags);
+	for (i = 0; i < master->num_streams; i++) {
+		struct rb_node *existing;
+
+		existing = rb_find_add(&master->streams[i].node,
+				       &smmu->streams,
+				       arm_smmu_streams_cmp_node);
+		if (!existing)
+			continue;
+
+		existing_master = rb_entry(existing, struct arm_smmu_stream,
+					   node)->master;
+
+		/* Bridged PCI devices may end up with duplicated IDs */
+		if (existing_master == master)
+			continue;
+
+		existing_sid = master->streams[i].id;
+		ret = -ENODEV;
+		break;
+	}
+	if (ret)
+		for (i--; i >= 0; i--)
+			if (!RB_EMPTY_NODE(&master->streams[i].node))
+				rb_erase(&master->streams[i].node,
+					 &smmu->streams);
+	spin_unlock_irqrestore(&smmu->streams_lock, flags);
+
+	if (ret)
+		dev_warn(master->dev,
+			 "Aliasing StreamID 0x%x (from %s) unsupported, expect DMA to be broken\n",
+			 existing_sid, dev_name(existing_master->dev));
+	return ret;
+}
+
 static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 				  struct arm_smmu_master *master)
 {
@@ -4237,40 +4283,22 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 		RB_CLEAR_NODE(&master->streams[i].node);
 
 	mutex_lock(&smmu->streams_mutex);
-	for (i = 0; i < fwspec->num_ids; i++) {
-		struct arm_smmu_stream *new_stream = &master->streams[i];
-		struct rb_node *existing;
-		u32 sid = new_stream->id;
 
-		ret = arm_smmu_init_sid_strtab(smmu, sid);
+	/*
+	 * Initialize the L2 strtabs before publishing any stream node, and
+	 * insert all the nodes in one critical section, so an atomic reader
+	 * never sees a partially initialized master.
+	 */
+	for (i = 0; i < fwspec->num_ids; i++) {
+		ret = arm_smmu_init_sid_strtab(smmu, master->streams[i].id);
 		if (ret)
 			break;
-
-		/* Insert into SID tree */
-		existing = rb_find_add(&new_stream->node, &smmu->streams,
-				       arm_smmu_streams_cmp_node);
-		if (existing) {
-			struct arm_smmu_master *existing_master =
-				rb_entry(existing, struct arm_smmu_stream, node)
-					->master;
-
-			/* Bridged PCI devices may end up with duplicated IDs */
-			if (existing_master == master)
-				continue;
-
-			dev_warn(master->dev,
-				 "Aliasing StreamID 0x%x (from %s) unsupported, expect DMA to be broken\n",
-				 sid, dev_name(existing_master->dev));
-			ret = -ENODEV;
-			break;
-		}
 	}
 
+	if (!ret)
+		ret = arm_smmu_insert_streams(smmu, master);
+
 	if (ret) {
-		for (i--; i >= 0; i--)
-			if (!RB_EMPTY_NODE(&master->streams[i].node))
-				rb_erase(&master->streams[i].node,
-					 &smmu->streams);
 		kfree(master->streams);
 		kfree(master->build_invs);
 	}
@@ -4289,9 +4317,11 @@ static void arm_smmu_remove_master(struct arm_smmu_master *master)
 		return;
 
 	mutex_lock(&smmu->streams_mutex);
-	for (i = 0; i < fwspec->num_ids; i++)
-		if (!RB_EMPTY_NODE(&master->streams[i].node))
-			rb_erase(&master->streams[i].node, &smmu->streams);
+	scoped_guard(spinlock_irqsave, &smmu->streams_lock)
+		for (i = 0; i < fwspec->num_ids; i++)
+			if (!RB_EMPTY_NODE(&master->streams[i].node))
+				rb_erase(&master->streams[i].node,
+					 &smmu->streams);
 	mutex_unlock(&smmu->streams_mutex);
 
 	kfree(master->streams);
@@ -4726,6 +4756,7 @@ static int arm_smmu_init_structures(struct arm_smmu_device *smmu)
 	int ret;
 
 	mutex_init(&smmu->streams_mutex);
+	spin_lock_init(&smmu->streams_lock);
 	smmu->streams = RB_ROOT;
 
 	ret = arm_smmu_init_queues(smmu);
-- 
2.43.0

[PATCH v6 14/17] iommu/arm-smmu-v3: Add has_ats to struct arm_smmu_cmdq_batch

[Nicolin Chen]

The arm_smmu_cmdq_batch_add_cmd_p() might flush a sub-batch mid-way, when
the ARM_SMMU_OPT_CMDQ_FORCE_SYNC is set or when a batch is full. To allow
a future change to retry these sub-batch flushes on a timeout and identify
the broken master, the batch needs to know whether it holds an ATC_INV.

Add a "has_ats" flag, set by arm_smmu_cmdq_batch_add_cmd_p() when it queues
an ATC_INV command.

No functional changes.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 ++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +++
 2 files changed, 5 insertions(+)

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 2f8d22cc404ac..6fa4f390ad133 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -724,6 +724,8 @@ static inline bool arm_smmu_cmdq_supports_cmd(struct arm_smmu_cmdq *cmdq,
 struct arm_smmu_cmdq_batch {
 	struct arm_smmu_cmd		cmds[CMDQ_BATCH_ENTRIES];
 	struct arm_smmu_cmdq		*cmdq;
+	/* Set when an ATC_INV is queued; gates the retry-aware sync decision */
+	bool				has_ats;
 	int				num;
 };
 
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 6f217e916ab0c..607246a965a91 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -963,6 +963,7 @@ static void arm_smmu_cmdq_batch_init_cmd(struct arm_smmu_device *smmu,
 {
 	cmds->num = 0;
 	cmds->cmdq = arm_smmu_get_cmdq(smmu, cmd);
+	cmds->has_ats = false;
 }
 
 static int arm_smmu_cmdq_batch_issue(struct arm_smmu_device *smmu,
@@ -1011,6 +1012,8 @@ static void arm_smmu_cmdq_batch_add_cmd_p(struct arm_smmu_device *smmu,
 		arm_smmu_cmdq_batch_init_cmd(smmu, cmds, cmd);
 	}
 
+	if (FIELD_GET(CMDQ_0_OP, cmd->data[0]) == CMDQ_OP_ATC_INV)
+		cmds->has_ats = true;
 	cmds->cmds[cmds->num++] = *cmd;
 }
 
-- 
2.43.0

[PATCH v6 15/17] iommu/arm-smmu-v3: Add INV_TYPE_ATS_BROKEN for quarantined masters

[Nicolin Chen]

Clearing STE.EATS makes the SMMU reject ATS requests from any quarantined
master, but does not stop the driver from issuing ATC invalidations for it.
Those commands would continue to time out.

Introduce INV_TYPE_ATS_BROKEN, which a subsequent change will assign to the
master's ATS invalidation entries. Treat it as an ATS type during iteration
and ordering, but skip it when issuing commands.

The subsequent quarantine path changes inv->type in place via WRITE_ONCE()
while readers hold the invalidation array's read lock. Since a u8 load is
already atomic, arm_smmu_inv_type() uses READ_ONCE() to annotate the race
for KCSAN. Whole-struct copies in merge and purge cannot use READ_ONCE(),
so annotate those copies with data_race().

A stale read at worst issues one more ATC_INV, which times out and causes
the master to be quarantined again.

Treat ATS, ATS_FULL, and ATS_BROKEN as one comparator class. This preserves
the sort position and matching identity when an entry is changed in place.

No functional change yet; the new type is never set anywhere.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 12 +++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +++++++++++++++------
 2 files changed, 35 insertions(+), 10 deletions(-)

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 6fa4f390ad133..1394a14241fd3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -739,6 +739,7 @@ enum arm_smmu_inv_type {
 	INV_TYPE_S2_VMID_S1_CLEAR,
 	INV_TYPE_ATS,
 	INV_TYPE_ATS_FULL,
+	INV_TYPE_ATS_BROKEN,
 };
 
 struct arm_smmu_inv {
@@ -755,9 +756,18 @@ struct arm_smmu_inv {
 	int users; /* users=0 to mark as a trash to be purged */
 };
 
+/* cur->type may flip to INV_TYPE_ATS_BROKEN concurrently with readers */
+static inline u8 arm_smmu_inv_type(const struct arm_smmu_inv *inv)
+{
+	return READ_ONCE(inv->type);
+}
+
 static inline bool arm_smmu_inv_is_ats(const struct arm_smmu_inv *inv)
 {
-	return inv->type == INV_TYPE_ATS || inv->type == INV_TYPE_ATS_FULL;
+	u8 type = arm_smmu_inv_type(inv);
+
+	return type == INV_TYPE_ATS || type == INV_TYPE_ATS_FULL ||
+	       type == INV_TYPE_ATS_BROKEN;
 }
 
 /**
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 607246a965a91..359cbee20ce5a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1094,13 +1094,21 @@ arm_smmu_invs_iter_next(struct arm_smmu_invs *invs, size_t next, size_t *idx)
 static int arm_smmu_inv_cmp(const struct arm_smmu_inv *inv_l,
 			    const struct arm_smmu_inv *inv_r)
 {
+	/*
+	 * Treat all ATS types as one class, so an in-place flip to ATS_BROKEN
+	 * preserves the sort order and still matches the original ATS entry.
+	 */
+	bool are_ats = arm_smmu_inv_is_ats(inv_l) & arm_smmu_inv_is_ats(inv_r);
+	u8 type_l = arm_smmu_inv_type(inv_l);
+	u8 type_r = arm_smmu_inv_type(inv_r);
+
 	if (inv_l->smmu != inv_r->smmu)
 		return cmp_int((uintptr_t)inv_l->smmu, (uintptr_t)inv_r->smmu);
-	if (inv_l->type != inv_r->type)
-		return cmp_int(inv_l->type, inv_r->type);
+	if (!are_ats && type_l != type_r)
+		return cmp_int(type_l, type_r);
 	if (inv_l->id != inv_r->id)
 		return cmp_int(inv_l->id, inv_r->id);
-	if (arm_smmu_inv_is_ats(inv_l))
+	if (are_ats)
 		return cmp_int(inv_l->ssid, inv_r->ssid);
 	return 0;
 }
@@ -1186,11 +1194,12 @@ struct arm_smmu_invs *arm_smmu_invs_merge(struct arm_smmu_invs *invs,
 		return ERR_PTR(-ENOMEM);
 
 	new = new_invs->inv;
+	/* data_race(): a racing quarantine may flip ->type; the u8 is safe */
 	arm_smmu_invs_for_each_cmp(invs, i, to_merge, j, cmp) {
 		if (cmp < 0) {
-			*new = invs->inv[i];
+			*new = data_race(invs->inv[i]);
 		} else if (cmp == 0) {
-			*new = invs->inv[i];
+			*new = data_race(invs->inv[i]);
 			WRITE_ONCE(new->users, READ_ONCE(new->users) + 1);
 		} else {
 			*new = to_merge->inv[j];
@@ -1312,8 +1321,9 @@ struct arm_smmu_invs *arm_smmu_invs_purge(struct arm_smmu_invs *invs)
 	if (!new_invs)
 		return NULL;
 
+	/* data_race(): a racing quarantine may flip ->type; the u8 is safe */
 	arm_smmu_invs_for_each_entry(invs, i, inv) {
-		new_invs->inv[num_invs] = *inv;
+		new_invs->inv[num_invs] = data_race(*inv);
 		if (arm_smmu_inv_is_ats(inv))
 			new_invs->has_ats = true;
 		num_invs++;
@@ -2700,8 +2710,8 @@ static inline bool arm_smmu_invs_end_batch(struct arm_smmu_inv *cur,
 	if (cur->smmu != next->smmu)
 		return true;
 	/* The batch for S2 TLBI must be done before nested S1 ASIDs */
-	if (cur->type != INV_TYPE_S2_VMID_S1_CLEAR &&
-	    next->type == INV_TYPE_S2_VMID_S1_CLEAR)
+	if (arm_smmu_inv_type(cur) != INV_TYPE_S2_VMID_S1_CLEAR &&
+	    arm_smmu_inv_type(next) == INV_TYPE_S2_VMID_S1_CLEAR)
 		return true;
 	/* ATS must be after a sync of the S1/S2 invalidations */
 	if (!arm_smmu_inv_is_ats(cur) && arm_smmu_inv_is_ats(next))
@@ -2737,7 +2747,7 @@ static void __arm_smmu_domain_inv_range(struct arm_smmu_invs *invs,
 		if (!cmds.num)
 			arm_smmu_cmdq_batch_init_cmd(smmu, &cmds, &cmd);
 
-		switch (cur->type) {
+		switch (arm_smmu_inv_type(cur)) {
 		case INV_TYPE_S1_ASID:
 			cmd = arm_smmu_make_cmd_tlbi(cur->size_opcode,
 						     cur->id, 0);
@@ -2771,6 +2781,9 @@ static void __arm_smmu_domain_inv_range(struct arm_smmu_invs *invs,
 				arm_smmu_make_cmd_atc_inv_all(cur->id,
 							      IOMMU_NO_PASID));
 			break;
+		case INV_TYPE_ATS_BROKEN:
+			/* Master is quarantined; skip its ATC_INV */
+			break;
 		default:
 			WARN_ON_ONCE(1);
 			break;
@@ -3326,6 +3339,8 @@ arm_smmu_master_build_inv(struct arm_smmu_master *master,
 		cur->size_opcode = cur->nsize_opcode = CMDQ_OP_ATC_INV;
 		cur->ssid = ssid;
 		break;
+	case INV_TYPE_ATS_BROKEN:
+		break;
 	}
 
 	return cur;
-- 
2.43.0

[PATCH v6 16/17] iommu/arm-smmu-v3: Thread arm_smmu_master_domain on a per-master list

[Nicolin Chen]

A subsequent change needs to enumerate, from the CMDQ error path in atomic
context, every domain a master is attached to so it can mark this master's
ATS entries broken in each domain's invs after an ATC invalidation timeout.
The existing per-domain smmu_domain->devices list tracks the inverse
direction (masters in a given domain), so introduce a per-master list.

Add a second list_head master_elm to arm_smmu_master_domain, threaded onto
a new master->master_domains list under master_domains_lock. The CMDQ error
path walks the list while holding smmu->streams_lock; that path runs under
the invs->rwlock read side, which is itself sleepable on PREEMPT_RT, so a
plain spinlock_t suffices for both. The attach and detach sites now take it
with spin_lock(), nested inside the existing devices_lock critical section
that already disables IRQs; it is a leaf in the lock order, so no inversion
is introduced.

No functional change.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 ++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++++++
 2 files changed, 12 insertions(+)

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 1394a14241fd3..1fa207c44763e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -1035,6 +1035,9 @@ struct arm_smmu_master {
 	struct arm_smmu_vmaster		*vmaster; /* use smmu->streams_mutex */
 	/* Locked by the iommu core using the group mutex */
 	struct arm_smmu_ctx_desc_cfg	cd_table;
+	struct list_head		master_domains;
+	/* Protects master_domains */
+	spinlock_t			master_domains_lock;
 	unsigned int			num_streams;
 	bool				ats_enabled : 1;
 	bool				ste_ats_enabled : 1;
@@ -1129,6 +1132,7 @@ struct arm_smmu_invs *arm_smmu_invs_purge(struct arm_smmu_invs *invs);
 
 struct arm_smmu_master_domain {
 	struct list_head devices_elm;
+	struct list_head master_elm;
 	struct arm_smmu_master *master;
 	/*
 	 * For nested domains the master_domain is threaded onto the S2 parent,
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 359cbee20ce5a..c56fd5c6b6242 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3431,6 +3431,9 @@ static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
 						    ssid, nested_ats_flush);
 	if (master_domain) {
 		list_del(&master_domain->devices_elm);
+		spin_lock(&master->master_domains_lock);
+		list_del(&master_domain->master_elm);
+		spin_unlock(&master->master_domains_lock);
 		if (master->ats_enabled)
 			atomic_dec(&smmu_domain->nr_ats_masters);
 	}
@@ -3681,6 +3684,9 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
 		if (state->ats_enabled)
 			atomic_inc(&smmu_domain->nr_ats_masters);
 		list_add(&master_domain->devices_elm, &smmu_domain->devices);
+		spin_lock(&master->master_domains_lock);
+		list_add(&master_domain->master_elm, &master->master_domains);
+		spin_unlock(&master->master_domains_lock);
 		spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 		arm_smmu_install_new_domain_invs(state);
@@ -4405,6 +4411,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 	master->dev = dev;
 	master->smmu = smmu;
 	dev_iommu_priv_set(dev, master);
+	INIT_LIST_HEAD(&master->master_domains);
+	spin_lock_init(&master->master_domains_lock);
 
 	ret = arm_smmu_insert_master(smmu, master);
 	if (ret)
-- 
2.43.0

[PATCH v6 17/17] iommu/arm-smmu-v3: Quarantine ATS after an ATC invalidation timeout

[Nicolin Chen]

An ATC invalidation timeout is reported on the trailing CMD_SYNC, thereby
identifying the failed batch but not the CMD_ATC_INV or responsible master.

cmds->cmds is sorted by SID, so when an ATS batch returns -EIO, retry one
invalidation for each unique SID to find the unresponsive master. A batch
with only one unique SID needs no retry. This is the common case, since it
is uncommon for an ATS-capable PCI device to have multiple Stream IDs.

For each identified master, clear STE.EATS on all of its SIDs and mark its
ATS invalidation entries in every attached domain as INV_TYPE_ATS_BROKEN,
thereby blocking further ATS requests and preventing repeated ATC timeouts.

The marking must span every domain because a master may be attached at its
RID and at multiple PASIDs. Marking only the timed-out invs would leave its
other invs issuing CMD_ATC_INV commands that continue to time out.

Only mark the entries after the CFGI_STE completes successfully. Otherwise,
leave them active so later invalidations retry quarantine instead of being
suppressed while ATS may remain enabled.

The marking cannot take invs->rwlock for writing because the timeout path
already holds a read lock; taking the write side would ABBA-deadlock this
path against a concurrent timeout. Change the type with WRITE_ONCE(); once
STE.EATS is cleared, a racing invalidation cannot create any new ATC entry
and may at worst time out once more.

Use try_cmpxchg64() to clear STE.EATS without losing a concurrent update to
the STE; a non-atomic fallback could revert concurrent changes, such as an
S1DSS update. On a non-cacheable stream table, try_cmpxchg64() is unsafe,
so limit quarantine to coherent SMMUs. Non-coherent SMMUs retain existing
behavior of reporting every ATC_INV timeout.

Finally, force CMD_SYNC on every sub-batch containing an ATC invalidation
so the timeout is observed by the call that issued it.

Identification is synchronous; each unresponsive SID adds a CMD_SYNC poll
bounded by ARM_SMMU_POLL_TIMEOUT_US. That bound is rarely reached, since a
non-responding ATC_INV completes in error once the device's PCIe Completion
Timeout expires.

Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 202 +++++++++++++++++++-
 1 file changed, 199 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 c56fd5c6b6242..db202b79bf1e6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -118,6 +118,10 @@ 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);
+static struct arm_smmu_ste *
+arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid);
+static struct arm_smmu_domain *
+to_smmu_domain_devices(struct iommu_domain *domain);
 
 static void parse_driver_options(struct arm_smmu_device *smmu)
 {
@@ -966,12 +970,25 @@ static void arm_smmu_cmdq_batch_init_cmd(struct arm_smmu_device *smmu,
 	cmds->has_ats = false;
 }
 
+static void arm_smmu_cmdq_batch_retry(struct arm_smmu_device *smmu,
+				      struct arm_smmu_cmdq_batch *cmds);
+
 static int arm_smmu_cmdq_batch_issue(struct arm_smmu_device *smmu,
 				     struct arm_smmu_cmdq_batch *cmds,
 				     bool sync)
 {
-	return arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
-					   cmds->num, sync);
+	int ret = arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
+					      cmds->num, sync);
+
+	/*
+	 * The CMDQ HW reports an ATC invalidation timeout at the trailing
+	 * CMD_SYNC, not at the failing CMD_ATC_INV. Re-issue each unique ATS
+	 * SID in the batch to identify the unresponsive master and block its
+	 * ATS so subsequent invalidations make forward progress.
+	 */
+	if (ret == -EIO && cmds->has_ats)
+		arm_smmu_cmdq_batch_retry(smmu, cmds);
+	return ret;
 }
 
 static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
@@ -995,6 +1012,10 @@ static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
 	    arm_smmu_erratum_cmd_needs_repeating(&cmds->cmds[0]))
 		return true;
 
+	/* ATC_INV timeout is reported to CMD_SYNC; catch at the call site */
+	if (cmds->num == CMDQ_BATCH_ENTRIES && cmds->has_ats)
+		return true;
+
 	return false;
 }
 
@@ -1068,7 +1089,11 @@ static inline struct arm_smmu_inv *
 arm_smmu_invs_iter_next(struct arm_smmu_invs *invs, size_t next, size_t *idx)
 {
 	while (true) {
-		if (next >= invs->num_invs) {
+		/*
+		 * Lockless readers (arm_smmu_invs_set_ats_broken) pair with the
+		 * WRITE_ONCE() in arm_smmu_invs_unref(); num_invs only shrinks.
+		 */
+		if (next >= READ_ONCE(invs->num_invs)) {
 			*idx = next;
 			return NULL;
 		}
@@ -2558,6 +2583,177 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master,
 	return arm_smmu_cmdq_batch_submit(master->smmu, &cmds);
 }
 
+static void arm_smmu_invs_set_ats_broken(struct arm_smmu_invs *invs,
+					 struct arm_smmu_device *smmu, u32 sid)
+{
+	struct arm_smmu_inv *inv;
+	size_t i;
+
+	/* arm_smmu_atc_inv_master() submits batches with invs=NULL */
+	if (!invs)
+		return;
+
+	/*
+	 * invs->rwlock is deliberately not taken: the caller holds one domain's
+	 * read side for the timed-out batch, then taking another domain's write
+	 * side while a concurrent timeout does the reverse would ABBA-deadlock.
+	 *
+	 * This indicates some potential races, but they are harmless since EATS
+	 * was already cleared:
+	 *  - WRITE_ONCE() may hit a stale invs copy if an attach just installed
+	 *    a new invs, which might result in another ATC_INV timeout.
+	 *  - a concurrent invalidation may still issue an ATC_INV that may time
+	 *    out again.
+	 */
+	arm_smmu_invs_for_each_entry(invs, i, inv) {
+		u8 type = arm_smmu_inv_type(inv);
+
+		if (inv->smmu == smmu && inv->id == sid &&
+		    (type == INV_TYPE_ATS || type == INV_TYPE_ATS_FULL))
+			WRITE_ONCE(inv->type, INV_TYPE_ATS_BROKEN);
+	}
+}
+
+/* Find the master by SID and block its ATS at the SMMU */
+static void arm_smmu_quarantine_ats(struct arm_smmu_device *smmu, u32 stream_id)
+{
+	struct arm_smmu_cmd cmd = arm_smmu_make_cmd_op(CMDQ_OP_CFGI_STE);
+	struct arm_smmu_master_domain *md;
+	struct arm_smmu_cmdq_batch cmds;
+	struct arm_smmu_master *master;
+	struct arm_smmu_invs *invs;
+	unsigned long flags;
+	int i;
+
+	/*
+	 * The in-place STE.EATS clear relies on try_cmpxchg64(), which is UB
+	 * on Non-Cacheable memory. Leave a non-coherent SMMU unquarantined:
+	 * its invalidations keep issuing ATC_INV and reporting the timeouts.
+	 */
+	if (!(smmu->features & ARM_SMMU_FEAT_COHERENCY))
+		return;
+
+	guard(spinlock_irqsave)(&smmu->streams_lock);
+	master = arm_smmu_find_master(smmu, stream_id);
+	/*
+	 * A concurrent hot-unplug can release the master while a stale ATS
+	 * entry for it still lingers in the invs snapshot being walked here.
+	 */
+	if (!master)
+		return;
+
+	/* Clear STE.EATS for every SID and sync to the SMMU */
+	arm_smmu_cmdq_batch_init_cmd(smmu, &cmds, &cmd);
+
+	for (i = 0; i < master->num_streams; i++) {
+		u32 sid = master->streams[i].id;
+		struct arm_smmu_ste *ste = arm_smmu_get_step_for_sid(smmu, sid);
+		__le64 old, new;
+
+		/*
+		 * A concurrent arm_smmu_write_ste() of a domain attachment may
+		 * overwrite the data[1] and set EATS, which is recoverable by
+		 * another ATC_INV issued by its arm_smmu_attach_commit().
+		 */
+		old = READ_ONCE(ste->data[1]);
+		do {
+			new = old & ~cpu_to_le64(STRTAB_STE_1_EATS);
+		} while (!try_cmpxchg64(&ste->data[1], &old, new));
+
+		arm_smmu_cmdq_batch_add_cmd(
+			smmu, &cmds, arm_smmu_make_cmd_cfgi_ste(sid, true));
+	}
+
+	/*
+	 * Only proceed to mark the entries broken if the STE.EATS clear above
+	 * is confirmed; otherwise return so invalidations keep issuing ATC_INV
+	 * (and re-quarantine) until ATS is actually disabled.
+	 */
+	if (arm_smmu_cmdq_batch_submit(smmu, &cmds)) {
+		dev_err_ratelimited(smmu->dev,
+				    "failed to disable ATS for master\n");
+		return;
+	}
+
+	/*
+	 * Mark this master's ATS entries broken in every domain it is attached,
+	 * so later invalidations skip the ATC_INV that would time out again.
+	 */
+	rcu_read_lock();
+	spin_lock_irqsave(&master->master_domains_lock, flags);
+	list_for_each_entry(md, &master->master_domains, master_elm) {
+		struct arm_smmu_domain *smmu_domain =
+			to_smmu_domain_devices(md->domain);
+
+		if (!smmu_domain)
+			continue;
+		invs = rcu_dereference(smmu_domain->invs);
+		for (i = 0; i < master->num_streams; i++)
+			arm_smmu_invs_set_ats_broken(invs, smmu,
+						     master->streams[i].id);
+	}
+	spin_unlock_irqrestore(&master->master_domains_lock, flags);
+	rcu_read_unlock();
+}
+
+/* Re-issue every unique ATS SID in @cmds to identify and quarantine masters. */
+static void arm_smmu_cmdq_batch_retry(struct arm_smmu_device *smmu,
+				      struct arm_smmu_cmdq_batch *cmds)
+{
+	struct arm_smmu_cmd atc = {};
+	u32 last_sid = 0;
+	int nr_sids = 0;
+	int i;
+
+	/*
+	 * Count unique Stream IDs, taking advantage of the sorted commands. An
+	 * ATS-capable PCI device rarely has multiple SIDs, so a batch commonly
+	 * carries a single SID, where a re-issue probe would be pointless.
+	 */
+	for (i = 0; i < cmds->num; i++) {
+		u32 sid;
+
+		/* Only ATC_INV commands can time out */
+		if (FIELD_GET(CMDQ_0_OP, cmds->cmds[i].data[0]) !=
+		    CMDQ_OP_ATC_INV)
+			continue;
+
+		sid = FIELD_GET(CMDQ_ATC_0_SID, cmds->cmds[i].data[0]);
+		if (!nr_sids || sid != last_sid) {
+			nr_sids++;
+			last_sid = sid;
+		}
+	}
+
+	/* The timed-out CMD_SYNC already identifies the lone Stream ID */
+	if (nr_sids == 1) {
+		arm_smmu_quarantine_ats(smmu, last_sid);
+		return;
+	}
+
+	for (i = 0; i < cmds->num; i++) {
+		u32 sid;
+
+		if (FIELD_GET(CMDQ_0_OP, cmds->cmds[i].data[0]) !=
+		    CMDQ_OP_ATC_INV)
+			continue;
+
+		/*
+		 * One retry per Stream ID. So, only try the first command since
+		 * commands are sorted. And each dead master costs one CMD_SYNC,
+		 * bounded by its PCIe Completion Timeout (usually <= 250ms).
+		 */
+		sid = FIELD_GET(CMDQ_ATC_0_SID, cmds->cmds[i].data[0]);
+		if (atc.data[0] &&
+		    sid == FIELD_GET(CMDQ_ATC_0_SID, atc.data[0]))
+			continue;
+
+		atc = cmds->cmds[i];
+		if (arm_smmu_cmdq_issue_cmd_p(smmu, &atc, true) == -EIO)
+			arm_smmu_quarantine_ats(smmu, sid);
+	}
+}
+
 /* IO_PGTABLE API */
 static void arm_smmu_tlb_inv_context(void *cookie)
 {
-- 
2.43.0