Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Pavol Sakac <sakacpav@amazon.de>
To: Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>, <iommu@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>, <linux-pci@vger.kernel.org>,
	<nh-open-source@amazon.com>
Subject: [RFC PATCH 3/3] iommu: set up the default domain outside iommu_probe_device_lock
Date: Fri, 11 Sep 2026 14:58:34 +0200	[thread overview]
Message-ID: <20260911125907.67105-3-sakacpav@amazon.de> (raw)
In-Reply-To: <20260911-vfopt-s2-v1-0-fff3db7e01c2@amazon.de>

Default domain allocation, attach and direct-mapping setup for a group's
first device still run inline under iommu_probe_device_lock, though they
need only group->mutex: iommu_group_store_type() does this at runtime
under group->mutex alone, and bus_iommu_probe() already defers it via the
group_list. With publication moved out it is the largest term left under
the hold, and parallel VF initialisation convoys behind it.

Use the group_list deferral for the singleton probe path too:
iommu_probe_device() passes a list and runs the setup after the global
unlock, under group->mutex. The global lock's original purpose - commit
01657bc14a39 ("iommu: Avoid races around device probe") - kept two devices
of one group from double-initialising it; that invariant moves to
group->mutex, where the setup is rechecked: of two racing siblings the
first to find the group unset sets it up for every member, and the rest
skip.

A failed setup unwinds only the calling device, like a failed publication;
siblings wait in -EPROBE_DEFER until a later probe finalises the group, or
until a new member joins if the failing call had claimed a bus-scanned
group's queued entry - the same terminal shape a failed bus_iommu_probe()
drain leaves today. A domain the failed setup published stays attached
(the first attach is IOMMU_SET_DOMAIN_MUST_SUCCEED), so members left
behind get the DMA ops it owed them.

bus_iommu_probe() rechecks group->default_domain, as a hotplug probe can
finalise a queued group before the drain takes the mutex.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/iommu/iommu.c | 48 ++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a5e3327aeaca..48fe22bbd1bc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -657,10 +657,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 		goto err_put_group;
 	}
 
-	/*
-	 * The gdev must be in the list before calling
-	 * iommu_setup_default_domain()
-	 */
+	/* List the gdev first: the finaliser covers every listed member. */
 	list_add_tail(&gdev->list, &group->devices);
 	WARN_ON(group->default_domain && !group->domain);
 	if (group->domain || group->default_domain) {
@@ -676,15 +673,10 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 						0);
 		if (ret)
 			goto err_remove_gdev;
-	} else if (!group->default_domain && !group_list) {
-		ret = iommu_setup_default_domain(group, 0);
-		if (ret)
-			goto err_remove_gdev;
 	} else if (!group->default_domain) {
 		/*
-		 * With a group_list argument we defer the default_domain setup
-		 * to the caller by providing a de-duplicated list of groups
-		 * that need further setup.
+		 * Defer setup to the caller draining group_list; both in-tree
+		 * callers pass one.
 		 */
 		if (list_empty(&group->entry))
 			list_add_tail(&group->entry, group_list);
@@ -713,6 +705,7 @@ int iommu_probe_device(struct device *dev)
 	const struct iommu_ops *ops;
 	struct group_device *gdev, *own;
 	struct iommu_group *group;
+	LIST_HEAD(group_list);
 	int ret;
 
 	mutex_lock(&iommu_probe_device_lock);
@@ -721,7 +714,7 @@ int iommu_probe_device(struct device *dev)
 		mutex_unlock(&iommu_probe_device_lock);
 		goto probe_finalize;
 	}
-	ret = __iommu_probe_device(dev, NULL);
+	ret = __iommu_probe_device(dev, &group_list);
 	if (!ret) {
 		/*
 		 * Not every caller pins the device, so pin the group across
@@ -735,6 +728,9 @@ int iommu_probe_device(struct device *dev)
 		return ret;
 
 	mutex_lock(&group->mutex);
+	/* Only the caller that queued the entry may unlink it. */
+	if (!list_empty(&group_list))
+		list_del_init(&group->entry);
 	/* iommu_group_link_device() is all-or-nothing. */
 	for_each_group_device(group, gdev) {
 		if (gdev->linked)
@@ -743,6 +739,20 @@ int iommu_probe_device(struct device *dev)
 		if (ret)
 			goto err_remove_device;
 	}
+	/*
+	 * First thread to find the group unset sets it up for every member;
+	 * binding needs a default domain, so no DMA races the window.
+	 */
+	if (!list_empty(&group->devices) &&
+	    !group->domain && !group->default_domain) {
+		ret = iommu_setup_default_domain(group, 0);
+		if (ret)
+			goto err_remove_device;
+		for_each_group_device(group, gdev)
+			if (dev_has_iommu(gdev->dev))
+				iommu_setup_dma_ops(gdev->dev,
+						    group->default_domain);
+	}
 	mutex_unlock(&group->mutex);
 	iommu_group_put(group);
 
@@ -765,8 +775,15 @@ int iommu_probe_device(struct device *dev)
 	if (own) {
 		list_del(&own->list);
 		__iommu_group_free_device(group, own);
-		iommu_deinit_device(dev);
 	}
+	/* Members left behind stay attached; give them the DMA ops owed. */
+	if (group->default_domain)
+		for_each_group_device(group, gdev)
+			if (dev_has_iommu(gdev->dev))
+				iommu_setup_dma_ops(gdev->dev,
+						    group->default_domain);
+	if (own)
+		iommu_deinit_device(dev);
 	mutex_unlock(&group->mutex);
 	if (own)
 		iommu_group_put(group);	/* iommu_init_device()'s reference */
@@ -2077,7 +2094,10 @@ static int bus_iommu_probe(const struct bus_type *bus)
 		 * that the cross-group default domain type and the setup of the
 		 * IOMMU_RESV_DIRECT will work correctly in non-hotpug scenarios.
 		 */
-		ret = iommu_setup_default_domain(group, 0);
+		ret = 0;
+		/* A hotplug probe may have finalised this group meanwhile. */
+		if (!group->default_domain)
+			ret = iommu_setup_default_domain(group, 0);
 		if (ret) {
 			mutex_unlock(&group->mutex);
 			return ret;
-- 
2.47.3


  parent reply	other threads:[~2026-09-11 13:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 12:58 [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Pavol Sakac
2026-09-11 12:58 ` [RFC PATCH 1/3] iommu: split sysfs link publication out of iommu_group_alloc_device() Pavol Sakac
2026-09-11 13:14   ` sashiko-bot
2026-09-11 12:58 ` [RFC PATCH 2/3] iommu: create device sysfs links outside iommu_probe_device_lock Pavol Sakac
2026-09-11 13:13   ` sashiko-bot
2026-09-11 12:58 ` Pavol Sakac [this message]
2026-09-11 13:15   ` [RFC PATCH 3/3] iommu: set up the default domain " sashiko-bot
2026-09-11 17:45 ` [RFC PATCH 0/3] iommu: Reduce iommu_probe_device_lock contention Robin Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260911125907.67105-3-sakacpav@amazon.de \
    --to=sakacpav@amazon.de \
    --cc=bhelgaas@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=nh-open-source@amazon.com \
    --cc=robin.murphy@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox