Message-ID: <0f6021b500c74db33af8118210dd7a2b2fd31b3c.1756682135.git.nicolinc@nvidia.com>
Date: Sun, 31 Aug 2025 16:31:58 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: <joro@...tes.org>, <jgg@...dia.com>, <bhelgaas@...gle.com>
CC: <suravee.suthikulpanit@....com>, <will@...nel.org>,
	<robin.murphy@....com>, <sven@...nel.org>, <j@...nau.net>,
	<alyssa@...enzweig.io>, <neal@...pa.dev>, <robin.clark@....qualcomm.com>,
	<m.szyprowski@...sung.com>, <krzk@...nel.org>, <alim.akhtar@...sung.com>,
	<dwmw2@...radead.org>, <baolu.lu@...ux.intel.com>, <kevin.tian@...el.com>,
	<yong.wu@...iatek.com>, <matthias.bgg@...il.com>,
	<angelogioacchino.delregno@...labora.com>, <tjeznach@...osinc.com>,
	<paul.walmsley@...ive.com>, <palmer@...belt.com>, <aou@...s.berkeley.edu>,
	<alex@...ti.fr>, <heiko@...ech.de>, <schnelle@...ux.ibm.com>,
	<mjrosato@...ux.ibm.com>, <gerald.schaefer@...ux.ibm.com>,
	<orsonzhai@...il.com>, <baolin.wang@...ux.alibaba.com>,
	<zhang.lyra@...il.com>, <wens@...e.org>, <jernej.skrabec@...il.com>,
	<samuel@...lland.org>, <jean-philippe@...aro.org>, <rafael@...nel.org>,
	<lenb@...nel.org>, <yi.l.liu@...el.com>, <cwabbott0@...il.com>,
	<quic_pbrahma@...cinc.com>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <asahi@...ts.linux.dev>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-arm-msm@...r.kernel.org>,
	<linux-samsung-soc@...r.kernel.org>, <linux-mediatek@...ts.infradead.org>,
	<linux-riscv@...ts.infradead.org>, <linux-rockchip@...ts.infradead.org>,
	<linux-s390@...r.kernel.org>, <linux-sunxi@...ts.linux.dev>,
	<linux-tegra@...r.kernel.org>, <virtualization@...ts.linux.dev>,
	<linux-acpi@...r.kernel.org>, <linux-pci@...r.kernel.org>,
	<patches@...ts.linux.dev>, <vsethi@...dia.com>, <helgaas@...nel.org>,
	<etzhao1900@...il.com>
Subject: [PATCH v4 6/7] iommu: Introduce iommu_dev_reset_prepare() and iommu_dev_reset_done()

PCIe permits a device to ignore ATS invalidation TLPs, while processing a
reset. This creates a problem visible to the OS where an ATS invalidation
command will time out. E.g. an SVA domain will have no coordination with a
reset event and can racily issue ATS invalidations to a resetting device.

The OS should do something to mitigate this as we do not want production
systems to be reporting critical ATS failures, especially in a hypervisor
environment. Broadly, OS could arrange to ignore the timeouts, block page
table mutations to prevent invalidations, or disable and block ATS.

The PCIe spec in sec 10.3.1 IMPLEMENTATION NOTE recommends to disable and
block ATS before initiating a Function Level Reset. It also mentions that
other reset methods could have the same vulnerability as well.

Provide a callback from the PCI subsystem that will enclose the reset and
have the iommu core temporarily change all the attached domain to BLOCKED.
After attaching a BLOCKED domain, IOMMU hardware would fence any incoming
ATS queries. And IOMMU drivers should also synchronously stop issuing new
ATS invalidations and wait for all ATS invalidations to complete. This can
avoid any ATS invaliation timeouts.

However, if there is a domain attachment/replacement happening during an
ongoing reset, ATS routines may be re-activated between the two function
calls. So, introduce a new pending_reset flag in group_device, and reject
any concurrent attach_dev/set_dev_pasid call during a reset for a concern
of compatibility failure.

There are two corner cases that won't work:
1. Alias devices that share the same RID
   Blocking one device also blocks the other alias devices that might not
   want a reset. Given that it's very rare for an alias device to support
   ATS, simply skip the blocking routine.

2. SRIOV devices that its PF is resetting while its VF isn't.
   Both PF and VF should block RID and PASIDs. But, since VF is not aware
   of the reset, it is difficult to block it and reject concurrent attach
   calls, because it's not logically reasonable to reject a VF attachment
   due to a resetting PF unless the VF is resetting too. To address this,
   we won't be able to reject any concurrent attachment as simple as this
   patch does; instead we will need two new compatibility testing ops for
   attach_dev/set_dev_pasid to allowing caching a compatible attach. This
   itself, however, would be a big series. So, for now, skip the blocking
   routine for PF devices, and leave a note.

Reviewed-by: Lu Baolu <baolu.lu@...ux.intel.com>
Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
---
 include/linux/iommu.h |  12 +++
 drivers/iommu/iommu.c | 199 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 211 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6d6d068c3de48..0d8e252929c89 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1169,6 +1169,9 @@ void dev_iommu_priv_set(struct device *dev, void *priv);
 extern struct mutex iommu_probe_device_lock;
 int iommu_probe_device(struct device *dev);
 
+int iommu_dev_reset_prepare(struct device *dev);
+void iommu_dev_reset_done(struct device *dev);
+
 int iommu_device_use_default_domain(struct device *dev);
 void iommu_device_unuse_default_domain(struct device *dev);
 
@@ -1453,6 +1456,15 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
 	return -ENODEV;
 }
 
+static inline int iommu_dev_reset_prepare(struct device *dev)
+{
+	return 0;
+}
+
+static inline void iommu_dev_reset_done(struct device *dev)
+{
+}
+
 static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
 {
 	return NULL;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f08c177f30de8..bcc239f3592f4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -71,12 +71,29 @@ struct group_device {
 	struct list_head list;
 	struct device *dev;
 	char *name;
+	bool pending_reset : 1;
 };
 
 /* Iterate over each struct group_device in a struct iommu_group */
 #define for_each_group_device(group, pos) \
 	list_for_each_entry(pos, &(group)->devices, list)
 
+/* Callers must hold the dev->iommu_group->mutex */
+static struct group_device *device_to_group_device(struct device *dev)
+{
+	struct iommu_group *group = dev->iommu_group;
+	struct group_device *gdev;
+
+	lockdep_assert_held(&group->mutex);
+
+	/* gdev must be in the list */
+	for_each_group_device(group, gdev) {
+		if (gdev->dev == dev)
+			break;
+	}
+	return gdev;
+}
+
 struct iommu_group_attribute {
 	struct attribute attr;
 	ssize_t (*show)(struct iommu_group *group, char *buf);
@@ -2157,6 +2174,12 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
 
 	guard(mutex)(&dev->iommu_group->mutex);
 
+	/*
+	 * There is a concurrent attach while the device is resetting. Defer it
+	 * until iommu_dev_reset_done() attaching the device to group->domain.
+	 */
+	if (device_to_group_device(dev)->pending_reset)
+		return -EBUSY;
 	return __iommu_attach_device(domain, dev, NULL);
 }
 
@@ -2201,6 +2224,16 @@ struct iommu_domain *iommu_get_domain_for_dev_locked(struct device *dev)
 
 	lockdep_assert_held(&group->mutex);
 
+	/*
+	 * Driver handles the low-level __iommu_attach_device(), including the
+	 * one invoked by iommu_dev_reset_done(), in which case the driver must
+	 * get the blocking domain over group->domain caching the one prior to
+	 * iommu_dev_reset_prepare(), so that it wouldn't end up with attaching
+	 * the device from group->domain (old) to group->domain (new).
+	 */
+	if (device_to_group_device(dev)->pending_reset)
+		return group->blocking_domain;
+
 	return group->domain;
 }
 EXPORT_SYMBOL_GPL(iommu_get_domain_for_dev_locked);
@@ -2309,6 +2342,13 @@ static int __iommu_device_set_domain(struct iommu_group *group,
 		dev->iommu->attach_deferred = 0;
 	}
 
+	/*
+	 * There is a concurrent attach while the device is resetting. Defer it
+	 * until iommu_dev_reset_done() attaching the device to group->domain.
+	 */
+	if (gdev->pending_reset)
+		return -EBUSY;
+
 	ret = __iommu_attach_device(new_domain, dev, old_domain);
 	if (ret) {
 		/*
@@ -3394,6 +3434,15 @@ static int __iommu_set_group_pasid(struct iommu_domain *domain,
 	int ret;
 
 	for_each_group_device(group, device) {
+		/*
+		 * There is a concurrent attach while the device is resetting.
+		 * Defer it until iommu_dev_reset_done() attaching the device to
+		 * group->domain.
+		 */
+		if (device->pending_reset) {
+			ret = -EBUSY;
+			goto err_revert;
+		}
 		if (device->dev->iommu->max_pasids > 0) {
 			ret = domain->ops->set_dev_pasid(domain, device->dev,
 							 pasid, old);
@@ -3815,6 +3864,156 @@ int iommu_replace_group_handle(struct iommu_group *group,
 }
 EXPORT_SYMBOL_NS_GPL(iommu_replace_group_handle, "IOMMUFD_INTERNAL");
 
+/**
+ * iommu_dev_reset_prepare() - Block IOMMU to prepare for a device reset
+ * @dev: device that is going to enter a reset routine
+ *
+ * When certain device is entering a reset routine, it wants to block any IOMMU
+ * activity during the reset routine. This includes blocking any translation as
+ * well as cache invalidation too (especially the device cache).
+ *
+ * This function attaches all RID/PASID of the device's to IOMMU_DOMAIN_BLOCKED
+ * allowing any blocked-domain-supporting IOMMU driver to pause translation and
+ * cahce invalidation, but leaves the software domain pointers intact so later
+ * the iommu_dev_reset_done() can restore everything.
+ *
+ * Return: 0 on success or negative error code if the preparation failed.
+ *
+ * Caller must use iommu_dev_reset_prepare() and iommu_dev_reset_done() together
+ * before/after the core-level reset routine, to unclear the pending_reset flag.
+ *
+ * These two functions are designed to be used by PCI reset functions that would
+ * not invoke any racy iommu_release_device(), since PCI sysfs node gets removed
+ * before it notifies with a BUS_NOTIFY_REMOVED_DEVICE. When using them in other
+ * case, callers must ensure there will be no racy iommu_release_device() call,
+ * which otherwise would UAF the dev->iommu_group pointer.
+ */
+int iommu_dev_reset_prepare(struct device *dev)
+{
+	struct iommu_group *group = dev->iommu_group;
+	struct group_device *gdev;
+	unsigned long pasid;
+	void *entry;
+	int ret = 0;
+
+	if (!dev_has_iommu(dev))
+		return 0;
+
+	/*
+	 * FIXME resetting a PF will reset any VF in the hardware level, so this
+	 * should basically block both the PF and its VFs. On the other hand, VF
+	 * software might not go through a reset, so it can run into any regular
+	 * operation like invoking a concurrent attach_dev/set_dev_pasid call.
+	 *
+	 * Due to compatibility concern, any concurrent attach_dev/set_dev_pasid
+	 * is being rejected with -EBUSY. For a PF, this rejection is reasonable
+	 * and simple since a concurrent attachment would not be sane. For a VF,
+	 * however, it would be difficult to justify.
+	 *
+	 * One way to work this out is to have a new op running a compatibility
+	 * test for a concurrent attachment. Then, so long as it is compatible,
+	 * the attachment would be deferred to iommu_dev_reset_done(). Bypass PF
+	 * devices for now.
+	 */
+	if (dev_is_pci(dev) && pci_num_vf(to_pci_dev(dev)) > 0)
+		return 0;
+
+	guard(mutex)(&group->mutex);
+
+	/* We cannot block an RID that is shared with another device */
+	if (dev_is_pci(dev)) {
+		for_each_group_device(group, gdev) {
+			if (gdev->dev != dev && dev_is_pci(gdev->dev) &&
+			    pci_devs_are_dma_aliases(to_pci_dev(gdev->dev),
+						     to_pci_dev(dev)))
+				return 0;
+		}
+	}
+
+	ret = __iommu_group_alloc_blocking_domain(group);
+	if (ret)
+		return ret;
+
+	/* Stage RID domain at blocking_domain while retaining group->domain */
+	if (group->domain != group->blocking_domain) {
+		ret = __iommu_attach_device(group->blocking_domain, dev,
+					    group->domain);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Stage PASID domains at blocking_domain while retaining pasid_array.
+	 *
+	 * The pasid_array is mostly fenced by group->mutex, except one reader
+	 * in iommu_attach_handle_get(), so it's safe to read without xa_lock.
+	 */
+	xa_for_each_start(&group->pasid_array, pasid, entry, 1)
+		iommu_remove_dev_pasid(dev, pasid,
+				       pasid_array_entry_to_domain(entry));
+
+	device_to_group_device(dev)->pending_reset = true;
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_dev_reset_prepare);
+
+/**
+ * iommu_dev_reset_done() - Restore IOMMU after a device reset is finished
+ * @dev: device that has finished a reset routine
+ *
+ * When certain device has finished a reset routine, it wants to restore its
+ * IOMMU activity, including new translation as well as cache invalidation, by
+ * re-attaching all RID/PASID of the device's back to the domains retained in
+ * the core-level structure.
+ *
+ * Caller must pair it with a successfully returned iommu_dev_reset_prepare().
+ *
+ * Note that, although unlikely, there is a risk that re-attaching domains might
+ * fail due to some unexpected happening like OOM.
+ */
+void iommu_dev_reset_done(struct device *dev)
+{
+	struct iommu_group *group = dev->iommu_group;
+	struct group_device *gdev;
+	unsigned long pasid;
+	void *entry;
+
+	if (!dev_has_iommu(dev))
+		return;
+
+	guard(mutex)(&group->mutex);
+
+	gdev = device_to_group_device(dev);
+
+	/* iommu_dev_reset_prepare() was not successfully called */
+	if (WARN_ON(!group->blocking_domain))
+		return;
+
+	/* iommu_dev_reset_prepare() was bypassed for the device */
+	if (!gdev->pending_reset)
+		return;
+
+	/* Re-attach RID domain back to group->domain */
+	if (group->domain != group->blocking_domain) {
+		WARN_ON(__iommu_attach_device(group->domain, dev,
+					      group->blocking_domain));
+	}
+
+	/*
+	 * Re-attach PASID domains back to the domains retained in pasid_array.
+	 *
+	 * The pasid_array is mostly fenced by group->mutex, except one reader
+	 * in iommu_attach_handle_get(), so it's safe to read without xa_lock.
+	 */
+	xa_for_each_start(&group->pasid_array, pasid, entry, 1)
+		WARN_ON(__iommu_set_group_pasid(
+			pasid_array_entry_to_domain(entry), group, pasid,
+			group->blocking_domain));
+
+	gdev->pending_reset = false;
+}
+EXPORT_SYMBOL_GPL(iommu_dev_reset_done);
+
 #if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
 /**
  * iommu_dma_prepare_msi() - Map the MSI page in the IOMMU domain
-- 
2.43.0

Powered by blists - more mailing lists