Linux PCI subsystem development
 help / color / mirror / Atom feed
* [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel
@ 2026-09-11 12:11 Pavol Sakac
  2026-09-11 12:28 ` [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn() Pavol Sakac
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:11 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

In the context of kexec-based live update (LUO) used with
datacenter-level hardware for hosting virtualized instances, once
SR-IOV enablement is put in the hot path it becomes a downtime
bottleneck on modern hardware with 100s-1000s of VFs per PF, and begs
for optimization.

This series (S1) introduces parallel VF initialization to put stress on
all participating subsystems; the 4 subsequent series then attack and
optimize the serialization bottlenecks one by one, with the primary
objective of reducing initialization time to a minimum.

On real hardware - a large dual-socket arm64 Neoverse V2 server with
thousands of VFs - the five series together cut SR-IOV initialization
by 65%. Due to HW/SW combination, the iommu_probe_device_lock residual
is not present. This series alone accounts for less than 5% of that,
so the reduction comes from the four series that follow removing the
serialization that keeps the parallelism from paying off.

Lock statistics and SR-IOV init time for 4x PF (NVMe, 255 VFs each)
enabled serially, using the public reproducer described below:

  lock_stat:
  Lock                     wait: Before     After   contentions: Before   After
  iommu_probe_device_lock          0 ms  25507 ms                     0    1143
  &root->kernfs_rwsem             18 ms    942 ms                  3117   93208
  &vfio.group_lock                 0 ms    425 ms                     0     497

  Stage                 SR-IOV init time:
  S0 (baseline)         3027 ms
  S1 (this series)       999 ms

Reproducer:

Runs an x86_64 QEMU/KVM guest with all PCI config space accesses routed
through ECAM, avoiding pci_config_lock, which otherwise serializes the
legacy port-I/O (0xCF8/0xCFC) accesses used for config offsets below
0x100. Guest topology: 48 vCPUs (2 sockets x 24 cores, one thread per
core, pinned 1:1 to host physical cores) and 2 NUMA nodes. After boot it
enables SR-IOV on 4 PFs sequentially (sriov_numvfs, 255 VFs each) and
measures init time on a kernel built without CONFIG_LOCK_STAT, then
separately collects /proc/lock_stat data on a kernel built with
CONFIG_LOCK_STAT=y.

In all series, I lean primarily on lock_stat numbers to defend the
improvements. In the reproducer, the residual iommu_probe_device_lock
dominates the window and masks the later series' wall-time gains;
reducing that lock further is out of scope for this set, but the
dominant residual source is named in S2 and can be followed up in the
future.

Looking for feedback on the overall design shape of the optimizations.

The full set of series building on top of this one:
- S2 iommu_probe_device_lock optimization:
  https://lore.kernel.org/r/20260911-vfopt-s2-v1-0-fff3db7e01c2@amazon.de
- S3 driver core: cut per-node lock traffic in bulk device
  registration (kernfs_rwsem write-taken once per node instead of
  twice, batched inode IDs, indexed glue dirs):
  https://lore.kernel.org/r/20260911-vfopt-s3-v1-0-66e3602f76f7@amazon.de
- S4 vfio: create the group chardev outside vfio.group_lock,
  eliminating its contention:
  https://lore.kernel.org/r/20260911-vfopt-s4-v1-0-98ba1d2ef7ab@amazon.de
- S5 remove the kernfs_rwsem bottleneck by staged sysfs registration:
  an opted-in device's whole subtree is published in one write hold
  instead of one per node:
  https://lore.kernel.org/r/20260911-vfopt-s5-v1-0-fa4cacdb6ca8@amazon.de

This set of series replaces a previous attempt to optimize VF init:
https://lore.kernel.org/lkml/20260702174033.32116-1-sakacpav@amazon.de/

The reproducer is available as a docker image that orchestrates
builds in a QEMU guest and prints results as a table (x86_64 Linux
host with /dev/kvm assumed):

  docker run --device /dev/kvm ghcr.io/pavsa/linux-parallel-sriov-vf-init-bench:7.3-base

Pavol Sakac (8):
  PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn()
  PCI/IOV: Create virtfn buses up front in sriov_add_vfs()
  PCI/PM: Convert pci_bridge_d3_update() recursion to iteration
  PCI/PM: Serialize pci_bridge_d3_update()
  powerpc/pci: Serialize pcibios_bus_add_device()
  PCI/IOV: Let sriov_add_vfs() own the failure unwind
  PCI/IOV: Initialize virtual functions in parallel
  PCI: Probe inline from node-local workqueue workers

 arch/powerpc/kernel/pci-common.c |  14 ++-
 drivers/pci/iov.c                | 189 +++++++++++++++++++++++++++----
 drivers/pci/pci-driver.c         |  18 ++-
 drivers/pci/pci.c                |  87 +++++++++-----
 4 files changed, 252 insertions(+), 56 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.47.3


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn()
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
@ 2026-09-11 12:28 ` Pavol Sakac
  2026-09-11 12:45   ` sashiko-bot
  2026-09-11 12:29 ` [RFC PATCH 2/8] PCI/IOV: Create virtfn buses up front in sriov_add_vfs() Pavol Sakac
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:28 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

pci_iov_add_virtfn() creates the virtfn bus, scans and registers the VF,
and removes the bus again on failure. A later commit creates all virtfn
buses up front, so separate the bus-lifetime concern from the VF-add
concern: move the VF add into a new __pci_iov_add_virtfn() taking the bus
as an argument, and keep bus creation and on-failure bus removal in the
public wrapper.

No functional change intended.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/iov.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 9d408fb8ac25..1826d32a2364 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -343,25 +343,17 @@ static struct pci_dev *pci_iov_scan_device(struct pci_dev *dev, int id,
 	return virtfn;
 }
 
-int pci_iov_add_virtfn(struct pci_dev *dev, int id)
+static int __pci_iov_add_virtfn(struct pci_dev *dev, struct pci_bus *bus,
+				int id)
 {
-	struct pci_bus *bus;
 	struct pci_dev *virtfn;
 	struct resource *res;
 	int rc, i;
 	u64 size;
 
-	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id));
-	if (!bus) {
-		rc = -ENOMEM;
-		goto failed;
-	}
-
 	virtfn = pci_iov_scan_device(dev, id, bus);
-	if (IS_ERR(virtfn)) {
-		rc = PTR_ERR(virtfn);
-		goto failed0;
-	}
+	if (IS_ERR(virtfn))
+		return PTR_ERR(virtfn);
 
 	virtfn->dev.parent = dev->dev.parent;
 	virtfn->multifunction = 0;
@@ -393,9 +385,22 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 failed1:
 	pci_stop_and_remove_bus_device(virtfn);
 	pci_dev_put(dev);
-failed0:
-	virtfn_remove_bus(dev->bus, bus);
-failed:
+
+	return rc;
+}
+
+int pci_iov_add_virtfn(struct pci_dev *dev, int id)
+{
+	struct pci_bus *bus;
+	int rc;
+
+	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id));
+	if (!bus)
+		return -ENOMEM;
+
+	rc = __pci_iov_add_virtfn(dev, bus, id);
+	if (rc)
+		virtfn_remove_bus(dev->bus, bus);
 
 	return rc;
 }
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 2/8] PCI/IOV: Create virtfn buses up front in sriov_add_vfs()
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
  2026-09-11 12:28 ` [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn() Pavol Sakac
@ 2026-09-11 12:29 ` Pavol Sakac
  2026-09-11 12:46   ` sashiko-bot
  2026-09-11 12:29 ` [RFC PATCH 3/8] PCI/PM: Convert pci_bridge_d3_update() recursion to iteration Pavol Sakac
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:29 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

virtfn_add_bus() is find-then-create and therefore assumes external
serialization. Create every virtfn bus before adding any VF, taking the
bus-create path out of the per-VF loop so a later commit can run the
per-VF adds concurrently against a stable bus set. Bus numbers are
already fixed before the loop, since pci_iov_set_numvfs() has latched
offset and stride.

The cleanup pass must remove only buses this enable created, because
virtfn_remove_bus() checks that a bus is empty rather than who created
it, so virtfn_add_bus() now reports whether it created the bus and
sriov_add_vfs() records that per VF id in a bitmap. Return early for
num_vfs == 0: kvcalloc(0, ...) returns ZERO_SIZE_PTR and a later commit
dereferences buses[0] unconditionally.

Several ids can share one bus number; only the creating id's bit is set.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/iov.c | 60 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 1826d32a2364..dda9303516f5 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/log2.h>
 #include <linux/pci.h>
@@ -124,10 +125,14 @@ static int compute_max_vf_buses(struct pci_dev *dev)
 	return rc;
 }
 
-static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
+static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr,
+				      bool *created)
 {
 	struct pci_bus *child;
 
+	if (created)
+		*created = false;
+
 	if (bus->number == busnr)
 		return bus;
 
@@ -140,6 +145,8 @@ static struct pci_bus *virtfn_add_bus(struct pci_bus *bus, int busnr)
 		return NULL;
 
 	pci_bus_insert_busn_res(child, busnr, busnr);
+	if (created)
+		*created = true;
 
 	return child;
 }
@@ -394,7 +401,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 	struct pci_bus *bus;
 	int rc;
 
-	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id));
+	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id), NULL);
 	if (!bus)
 		return -ENOMEM;
 
@@ -632,22 +639,67 @@ int __weak pcibios_sriov_disable(struct pci_dev *pdev)
 
 static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
 {
+	unsigned long *created_buses;
+	struct pci_bus **buses;
+	struct pci_bus *bus;
 	unsigned int i;
 	int rc;
 
-	if (dev->no_vf_scan)
+	if (!num_vfs || dev->no_vf_scan)
 		return 0;
 
+	buses = kvcalloc(num_vfs, sizeof(*buses), GFP_KERNEL);
+	if (!buses)
+		return -ENOMEM;
+
+	created_buses = bitmap_zalloc(num_vfs, GFP_KERNEL);
+	if (!created_buses) {
+		kvfree(buses);
+		return -ENOMEM;
+	}
+
 	for (i = 0; i < num_vfs; i++) {
-		rc = pci_iov_add_virtfn(dev, i);
+		bool created;
+
+		buses[i] = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, i),
+					  &created);
+		if (!buses[i]) {
+			rc = -ENOMEM;
+			goto remove_buses;
+		}
+		if (created)
+			__set_bit(i, created_buses);
+	}
+
+	for (i = 0; i < num_vfs; i++) {
+		rc = __pci_iov_add_virtfn(dev, buses[i], i);
 		if (rc)
 			goto failed;
 	}
+
+	bitmap_free(created_buses);
+	kvfree(buses);
 	return 0;
 failed:
 	while (i--)
 		pci_iov_remove_virtfn(dev, i);
 
+remove_buses:
+	/*
+	 * Remove only buses this enable created: several ids can share one
+	 * bus and only the creating id is recorded; virtfn_remove_bus()
+	 * checks emptiness, not ownership.  Re-look each up by number --
+	 * the VF unwind above may already have freed it.
+	 */
+	for_each_set_bit(i, created_buses, num_vfs) {
+		bus = pci_find_bus(pci_domain_nr(dev->bus),
+				   pci_iov_virtfn_bus(dev, i));
+		if (bus)
+			virtfn_remove_bus(dev->bus, bus);
+	}
+
+	bitmap_free(created_buses);
+	kvfree(buses);
 	return rc;
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 3/8] PCI/PM: Convert pci_bridge_d3_update() recursion to iteration
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
  2026-09-11 12:28 ` [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn() Pavol Sakac
  2026-09-11 12:29 ` [RFC PATCH 2/8] PCI/IOV: Create virtfn buses up front in sriov_add_vfs() Pavol Sakac
@ 2026-09-11 12:29 ` Pavol Sakac
  2026-09-11 12:40   ` sashiko-bot
  2026-09-11 12:30 ` [RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update() Pavol Sakac
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:29 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

pci_bridge_d3_update() propagates a bridge_d3 change to upstream bridges
by tail recursion: when a bridge's bridge_d3 value changes, the function
calls itself with that bridge as the new device.

Convert the tail recursion into an iterative loop. Each level recomputes
"remove" and d3cold_ok exactly as the recursive call did for its own
device, and the early returns become loop exits. No functional change
intended.

An upcoming change serializes this update with a mutex; the iterative
form lets that mutex be taken once per external call instead of once per
bridge level.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/pci.c | 67 +++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8..c62a315c0b4c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3104,46 +3104,49 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
  */
 void pci_bridge_d3_update(struct pci_dev *dev)
 {
-	bool remove = !device_is_registered(&dev->dev);
 	struct pci_dev *bridge;
-	bool d3cold_ok = true;
 
-	bridge = pci_upstream_bridge(dev);
-	if (!bridge || !pci_bridge_d3_possible(bridge))
-		return;
+	while ((bridge = pci_upstream_bridge(dev)) &&
+	       pci_bridge_d3_possible(bridge)) {
+		bool remove = !device_is_registered(&dev->dev);
+		bool d3cold_ok = true;
 
-	/*
-	 * If D3 is currently allowed for the bridge, removing one of its
-	 * children won't change that.
-	 */
-	if (remove && bridge->bridge_d3)
-		return;
+		/*
+		 * If D3 is currently allowed for the bridge, removing one of
+		 * its children won't change that.
+		 */
+		if (remove && bridge->bridge_d3)
+			break;
 
-	/*
-	 * If D3 is currently allowed for the bridge and a child is added or
-	 * changed, disallowance of D3 can only be caused by that child, so
-	 * we only need to check that single device, not any of its siblings.
-	 *
-	 * If D3 is currently not allowed for the bridge, checking the device
-	 * first may allow us to skip checking its siblings.
-	 */
-	if (!remove)
-		pci_dev_check_d3cold(dev, &d3cold_ok);
+		/*
+		 * If D3 is currently allowed for the bridge and a child is
+		 * added or changed, disallowance of D3 can only be caused by
+		 * that child, so we only need to check that single device,
+		 * not any of its siblings.
+		 *
+		 * If D3 is currently not allowed for the bridge, checking the
+		 * device first may allow us to skip checking its siblings.
+		 */
+		if (!remove)
+			pci_dev_check_d3cold(dev, &d3cold_ok);
 
-	/*
-	 * If D3 is currently not allowed for the bridge, this may be caused
-	 * either by the device being changed/removed or any of its siblings,
-	 * so we need to go through all children to find out if one of them
-	 * continues to block D3.
-	 */
-	if (d3cold_ok && !bridge->bridge_d3)
-		pci_walk_bus(bridge->subordinate, pci_dev_check_d3cold,
-			     &d3cold_ok);
+		/*
+		 * If D3 is currently not allowed for the bridge, this may be
+		 * caused either by the device being changed/removed or any of
+		 * its siblings, so we need to go through all children to find
+		 * out if one of them continues to block D3.
+		 */
+		if (d3cold_ok && !bridge->bridge_d3)
+			pci_walk_bus(bridge->subordinate, pci_dev_check_d3cold,
+				     &d3cold_ok);
+
+		if (bridge->bridge_d3 == d3cold_ok)
+			break;
 
-	if (bridge->bridge_d3 != d3cold_ok) {
 		bridge->bridge_d3 = d3cold_ok;
+
 		/* Propagate change to upstream bridges */
-		pci_bridge_d3_update(bridge);
+		dev = bridge;
 	}
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update()
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
                   ` (2 preceding siblings ...)
  2026-09-11 12:29 ` [RFC PATCH 3/8] PCI/PM: Convert pci_bridge_d3_update() recursion to iteration Pavol Sakac
@ 2026-09-11 12:30 ` Pavol Sakac
  2026-09-11 12:47   ` sashiko-bot
  2026-09-11 12:31 ` [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device() Pavol Sakac
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:30 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

pci_bridge_d3_update() does an unlocked read-modify-write of
bridge->bridge_d3, and its callers are not mutually serialized: the
d3cold_allowed sysfs write and the driver-context D3cold helpers hold
neither pci_rescan_remove_lock nor device_lock. A concurrent write can
lose an update and leave bridge_d3 stale, costing a wrong D3cold decision
rather than memory safety. An upcoming change runs pci_bus_add_device()
for sibling VFs concurrently, making sibling additions concurrent callers
too, so this must land first.

Add a mutex around the whole update, taken once for the propagation
loop. A device with no D3cold-capable port above it returns before the
mutex, so the common add is not funneled through a global lock, and the
loop re-evaluates both conditions under it. The mutex serializes the
updaters against each other only; the d3cold_allowed store itself still
writes an adjacent bit of the same word unlocked, a pre-existing
exposure this change neither widens nor closes. The resulting order is
pci_rescan_remove_lock, device_lock(any) -> pci_bridge_d3_lock ->
pci_bus_sem (read), so pci_bridge_d3_lock must never be acquired while
holding pci_bus_sem and no pci_walk_bus() callback may call into this
path.

The race dates back to commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into
D3 during suspend"), is theoretical with no known report, and so
carries no Fixes: tag and no stable designation; it claims no measured
performance contribution.

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

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c62a315c0b4c..b2a159ef125b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3095,17 +3095,36 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
 }
 
 /*
+ * Serializes pci_bridge_d3_update()'s bridge_d3 read-modify-writes and
+ * their upstream propagation.  Ordering: pci_rescan_remove_lock,
+ * device_lock(any) -> pci_bridge_d3_lock -> pci_bus_sem (read); no
+ * pci_walk_bus() callback may call into this path.
+ */
+static DEFINE_MUTEX(pci_bridge_d3_lock);
+
+/**
  * pci_bridge_d3_update - Update bridge D3 capabilities
  * @dev: PCI device which is changed
  *
  * Update upstream bridge PM capabilities accordingly depending on if the
  * device PM configuration was changed or the device is being removed.  The
  * change is also propagated upstream.
+ *
+ * Context: Process context. Takes and releases pci_bridge_d3_lock.
  */
 void pci_bridge_d3_update(struct pci_dev *dev)
 {
 	struct pci_dev *bridge;
 
+	/*
+	 * Unlocked fast path; the loop condition re-evaluates both checks
+	 * under the lock.
+	 */
+	bridge = pci_upstream_bridge(dev);
+	if (!bridge || !pci_bridge_d3_possible(bridge))
+		return;
+
+	mutex_lock(&pci_bridge_d3_lock);
 	while ((bridge = pci_upstream_bridge(dev)) &&
 	       pci_bridge_d3_possible(bridge)) {
 		bool remove = !device_is_registered(&dev->dev);
@@ -3148,6 +3167,7 @@ void pci_bridge_d3_update(struct pci_dev *dev)
 		/* Propagate change to upstream bridges */
 		dev = bridge;
 	}
+	mutex_unlock(&pci_bridge_d3_lock);
 }
 
 /**
@@ -3157,6 +3177,9 @@ void pci_bridge_d3_update(struct pci_dev *dev)
  * This function can be used in drivers to enable D3cold from the device
  * they handle.  It also updates upstream PCI bridge PM capabilities
  * accordingly.
+ *
+ * Context: Process context. Takes and releases pci_bridge_d3_lock;
+ * must not be called from a pci_walk_bus() callback.
  */
 void pci_d3cold_enable(struct pci_dev *dev)
 {
@@ -3174,6 +3197,9 @@ EXPORT_SYMBOL_GPL(pci_d3cold_enable);
  * This function can be used in drivers to disable D3cold from the device
  * they handle.  It also updates upstream PCI bridge PM capabilities
  * accordingly.
+ *
+ * Context: Process context. Takes and releases pci_bridge_d3_lock;
+ * must not be called from a pci_walk_bus() callback.
  */
 void pci_d3cold_disable(struct pci_dev *dev)
 {
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device()
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
                   ` (3 preceding siblings ...)
  2026-09-11 12:30 ` [RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update() Pavol Sakac
@ 2026-09-11 12:31 ` Pavol Sakac
  2026-09-11 12:55   ` sashiko-bot
  2026-09-11 12:32 ` [RFC PATCH 6/8] PCI/IOV: Let sriov_add_vfs() own the failure unwind Pavol Sakac
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

The pseries and powernv pcibios_bus_add_device() hooks insert the device
into the EEH PE tree with bare list manipulation that
arch/powerpc/kernel/eeh.c itself flags as unlocked, and for SR-IOV the
pseries hook attaches every VF's eeh_dev to the shared physfn PE. Every
caller is serialized by enumeration context today; an upcoming change
runs pci_bus_add_device() for sibling VFs concurrently and makes the race
reachable.

Serialize the platform hook dispatch with an arch-local mutex. Exclusion
against the EEH recovery thread stays carried by pci_rescan_remove_lock,
and on probe-path enables the residue folds into the pre-existing
exposure described in a later patch in this series ("PCI/IOV: Initialize
virtual functions in parallel"). A proper PE-tree lock is a larger EEH
cleanup, so eeh_pe.c is left alone.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 arch/powerpc/kernel/pci-common.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 4fc52c21fe5d..21cccd0e97f8 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1046,6 +1046,15 @@ void pcibios_setup_bus_self(struct pci_bus *bus)
 		phb->controller_ops.dma_bus_setup(bus);
 }
 
+/*
+ * The pseries/powernv hooks update the shared EEH PE tree with no
+ * internal locking; serialize concurrent sibling VF additions.
+ * Removal-side updates do not take this lock: additions are
+ * drained before any unwind, and the sysfs enable and EEH paths
+ * hold pci_rescan_remove_lock.
+ */
+static DEFINE_MUTEX(pcibios_bus_add_device_lock);
+
 void pcibios_bus_add_device(struct pci_dev *dev)
 {
 	struct pci_controller *phb;
@@ -1068,8 +1077,11 @@ void pcibios_bus_add_device(struct pci_dev *dev)
 	if (ppc_md.pci_irq_fixup)
 		ppc_md.pci_irq_fixup(dev);
 
-	if (ppc_md.pcibios_bus_add_device)
+	if (ppc_md.pcibios_bus_add_device) {
+		mutex_lock(&pcibios_bus_add_device_lock);
 		ppc_md.pcibios_bus_add_device(dev);
+		mutex_unlock(&pcibios_bus_add_device_lock);
+	}
 }
 
 int pcibios_device_add(struct pci_dev *dev)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 6/8] PCI/IOV: Let sriov_add_vfs() own the failure unwind
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
                   ` (4 preceding siblings ...)
  2026-09-11 12:31 ` [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device() Pavol Sakac
@ 2026-09-11 12:32 ` Pavol Sakac
  2026-09-11 12:52   ` sashiko-bot
  2026-09-11 12:33 ` [RFC PATCH 7/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
  2026-09-11 12:34 ` [RFC PATCH 8/8] PCI: Probe inline from node-local workqueue workers Pavol Sakac
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:32 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

__pci_iov_add_virtfn() unwinds its own sysfs-link failure with
pci_stop_and_remove_bus_device(), which lockdep-asserts
pci_rescan_remove_lock. The next commit runs __pci_iov_add_virtfn() from
async workers that must never take or require that lock, so the unwind
has to move to the enabling task.

Leave __pci_iov_add_virtfn() reporting only and let each caller unwind
through pci_iov_remove_virtfn(), whose lookup-based design is correct at
every failure stage. sriov_add_vfs() unwinds ids 0..i inclusive on
failure of VF i, since VF i may be registered but not yet linked. The
wrapper unwinds fully before returning, because its EEH caller discards
the return code: the VF is removed through pci_iov_remove_virtfn(),
which also frees the bus it empties, and a bus this call created with
no VF registered on it is removed explicitly.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/iov.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index dda9303516f5..a32b2c295922 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -383,35 +383,42 @@ static int __pci_iov_add_virtfn(struct pci_dev *dev, struct pci_bus *bus,
 	pci_device_add(virtfn, virtfn->bus);
 	rc = pci_iov_sysfs_link(dev, virtfn, id);
 	if (rc)
-		goto failed1;
+		return rc;
 
 	pci_bus_add_device(virtfn);
 
 	return 0;
-
-failed1:
-	pci_stop_and_remove_bus_device(virtfn);
-	pci_dev_put(dev);
-
-	return rc;
 }
 
 int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 {
 	struct pci_bus *bus;
+	bool created;
 	int rc;
 
-	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id), NULL);
+	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id), &created);
 	if (!bus)
 		return -ENOMEM;
 
 	rc = __pci_iov_add_virtfn(dev, bus, id);
-	if (rc)
-		virtfn_remove_bus(dev->bus, bus);
+	if (rc) {
+		pci_iov_remove_virtfn(dev, id);
+		/*
+		 * Same ownership and stale-pointer rules as the
+		 * sriov_add_vfs() bus unwind.
+		 */
+		if (created) {
+			bus = pci_find_bus(pci_domain_nr(dev->bus),
+					   pci_iov_virtfn_bus(dev, id));
+			if (bus)
+				virtfn_remove_bus(dev->bus, bus);
+		}
+	}
 
 	return rc;
 }
 
+/* Unwind primitive for partial adds: a missing VF must stay a silent no-op. */
 void pci_iov_remove_virtfn(struct pci_dev *dev, int id)
 {
 	char buf[VIRTFN_ID_LEN];
@@ -681,8 +688,10 @@ static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
 	kvfree(buses);
 	return 0;
 failed:
-	while (i--)
+	/* VF i may be partially added: unwind ids 0..i inclusive. */
+	do {
 		pci_iov_remove_virtfn(dev, i);
+	} while (i--);
 
 remove_buses:
 	/*
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 7/8] PCI/IOV: Initialize virtual functions in parallel
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
                   ` (5 preceding siblings ...)
  2026-09-11 12:32 ` [RFC PATCH 6/8] PCI/IOV: Let sriov_add_vfs() own the failure unwind Pavol Sakac
@ 2026-09-11 12:33 ` Pavol Sakac
  2026-09-11 12:43   ` sashiko-bot
  2026-09-11 12:34 ` [RFC PATCH 8/8] PCI: Probe inline from node-local workqueue workers Pavol Sakac
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

Serial per-VF scanning and device addition dominate SR-IOV enable time at
large VF counts, and a kexec-based live update re-creates every VF
through this same loop. Distribute the per-VF work over the kernel's
async machinery.

VF0 is added synchronously first because pci_iov_scan_device() writes the
config fields shared by every VF only on the id == 0 pass and reads them
locklessly afterwards. The rest are fanned out over async entries
sharing an on-stack context with an atomic id cursor, scheduled on the
PF's node with async_schedule_node_domain(). A worker that fails records
the first error there and the enabling task unwinds every id descending;
when several fail the temporally first errno is reported where the serial
code reported the lowest failing id's, and nothing in-tree consumes the
distinction.

The workers claim ids from that cursor and keep draining until the range
is exhausted, so entries are capped at one per online CPU rather than one
per VF.

One entry per VF would make the submission cost -- an allocation plus the
global async_lock per entry -- scale with the VF count and fall on the
enabling task, contending with the workers it has already queued.

No worker takes pci_rescan_remove_lock, which the sysfs enable path holds
around the whole sriov_configure() call, and each runs exactly the code
the enabling task ran serially, so no new deadlock class is
constructible. The probe-time pci_enable_sriov() path holds no rescan
lock, so its failure unwind reaches pci_stop_and_remove_bus_device()
unlocked, a pre-existing hole this neither widens nor closes.

Within one enable, sysfs links, uevents and VF binds now occur in
nondeterministic order, and a failed enable may transiently create VFs
past the failing id before unwinding them all. VF probes run in async
context, where a synchronous request_module() WARNs.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/iov.c | 83 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index a32b2c295922..ac2ddda4bf14 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2009 Intel Corporation, Yu Zhao <yu.zhao@intel.com>
  */
 
+#include <linux/async.h>
 #include <linux/bitfield.h>
 #include <linux/bitmap.h>
 #include <linux/bits.h>
@@ -350,6 +351,12 @@ static struct pci_dev *pci_iov_scan_device(struct pci_dev *dev, int id,
 	return virtfn;
 }
 
+/*
+ * Safe to run concurrently for distinct ids only, on pre-created buses
+ * the caller keeps alive; id 0 must complete first
+ * (pci_read_vf_config_common()).  Must not take pci_rescan_remove_lock;
+ * failures are unwound by the caller via pci_iov_remove_virtfn().
+ */
 static int __pci_iov_add_virtfn(struct pci_dev *dev, struct pci_bus *bus,
 				int id)
 {
@@ -644,11 +651,41 @@ int __weak pcibios_sriov_disable(struct pci_dev *pdev)
 	return 0;
 }
 
+/* On-stack; live until async_synchronize_full_domain() drains the workers. */
+struct sriov_add_ctx {
+	struct pci_dev *dev;
+	struct pci_bus **buses;
+	u16 num_vfs;
+	atomic_t next_id;
+	atomic_t error;
+};
+
+static void sriov_add_vf_work(void *data, async_cookie_t cookie)
+{
+	struct sriov_add_ctx *ctx = data;
+	int id;
+	int rc;
+
+	while ((id = atomic_fetch_inc(&ctx->next_id)) < ctx->num_vfs) {
+		rc = __pci_iov_add_virtfn(ctx->dev, ctx->buses[id], id);
+		if (rc)
+			atomic_cmpxchg(&ctx->error, 0, rc);
+	}
+}
+
 static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
 {
+	/*
+	 * EXCLUSIVE: a registered domain joins async_global_pending, so a
+	 * VF probe calling async_synchronize_full() from a worker would
+	 * self-deadlock.
+	 */
+	ASYNC_DOMAIN_EXCLUSIVE(sriov_async_domain);
+	struct sriov_add_ctx ctx;
 	unsigned long *created_buses;
 	struct pci_bus **buses;
 	struct pci_bus *bus;
+	unsigned int nr_workers;
 	unsigned int i;
 	int rc;
 
@@ -678,17 +715,48 @@ static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
 			__set_bit(i, created_buses);
 	}
 
-	for (i = 0; i < num_vfs; i++) {
-		rc = __pci_iov_add_virtfn(dev, buses[i], i);
-		if (rc)
-			goto failed;
+	/* Id 0 writes the shared config fields later ids read locklessly. */
+	rc = __pci_iov_add_virtfn(dev, buses[0], 0);
+	if (rc) {
+		i = 0;
+		goto failed;
+	}
+
+	ctx.dev = dev;
+	ctx.buses = buses;
+	ctx.num_vfs = num_vfs;
+	atomic_set(&ctx.next_id, 1);	/* VF0 was added above */
+	atomic_set(&ctx.error, 0);
+
+	/*
+	 * An entry async_schedule_node_domain() cannot queue (allocation
+	 * failure or async backlog) runs in the caller and drains the
+	 * remaining range -- every id is added exactly once.
+	 */
+	nr_workers = min_t(unsigned int, num_vfs - 1, num_online_cpus());
+	for (i = 0; i < nr_workers; i++)
+		async_schedule_node_domain(sriov_add_vf_work, &ctx,
+					   dev_to_node(&dev->dev),
+					   &sriov_async_domain);
+
+	async_synchronize_full_domain(&sriov_async_domain);
+
+	rc = atomic_read(&ctx.error);
+	if (rc) {
+		i = num_vfs - 1;
+		goto failed;
 	}
 
 	bitmap_free(created_buses);
 	kvfree(buses);
 	return 0;
 failed:
-	/* VF i may be partially added: unwind ids 0..i inclusive. */
+	/*
+	 * Unwind ids 0..i inclusive: i is 0 on the sync VF0 path and
+	 * num_vfs - 1 on the worker path (workers past the first failure
+	 * may have added more); pci_iov_remove_virtfn() copes with
+	 * partial and never-added ids.
+	 */
 	do {
 		pci_iov_remove_virtfn(dev, i);
 	} while (i--);
@@ -832,6 +900,11 @@ static void sriov_del_vfs(struct pci_dev *dev)
 	struct pci_sriov *iov = dev->sriov;
 	int i;
 
+	/*
+	 * Deliberately serial: the parallel-add locking arguments (and the
+	 * powerpc pcibios_bus_add_device() serialization) assume removal
+	 * never runs concurrently.
+	 */
 	for (i = 0; i < iov->num_VFs; i++)
 		pci_iov_remove_virtfn(dev, i);
 }
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 8/8] PCI: Probe inline from node-local workqueue workers
  2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
                   ` (6 preceding siblings ...)
  2026-09-11 12:33 ` [RFC PATCH 7/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
@ 2026-09-11 12:34 ` Pavol Sakac
  2026-09-11 12:40   ` sashiko-bot
  7 siblings, 1 reply; 17+ messages in thread
From: Pavol Sakac @ 2026-09-11 12:34 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, David Matlack, Ilpo Järvinen,
	Krzysztof Wilczyński, Kees Cook, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy, linuxppc-dev,
	Niklas Schnelle, Benjamin Block, Lukas Wunner, Ionut Nechita,
	nh-open-source

pci_call_probe() dispatches each probe with queue_work_on() to a
housekeeping CPU of the device's node, except where it already probes
inline. A VF added by one of the preceding node-placed async VF-add
workers already executes on its device's node, so the dispatch buys it no
locality: it funnels every VF probe through the single CPU
cpumask_any_and() elects and re-serializes the parallel enable at the
probe step.

Call local_pci_probe() directly when the caller is a workqueue worker
(PF_WQ_WORKER) whose current CPU is on the device's node and in the same
HK_TYPE_DOMAIN housekeeping mask the dispatch elects from. The gate is
not VF-specific: any probe already running in a node-local housekeeping
kworker takes it, while a probe from a task that is not a worker keeps
the dispatch.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/pci-driver.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e16aa59dd7ac..f5e12a5044cf 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -392,6 +392,22 @@ static bool pci_physfn_is_probed(struct pci_dev *dev)
 #endif
 }
 
+static bool pci_probe_inline(int node)
+{
+	/* The CPU is a placement hint; an unstable raw read is fine. */
+	int cpu = raw_smp_processor_id();
+	bool ret;
+
+	if (!(current->flags & PF_WQ_WORKER) || cpu_to_node(cpu) != node)
+		return false;
+
+	rcu_read_lock();
+	ret = housekeeping_cpu(cpu, HK_TYPE_DOMAIN);
+	rcu_read_unlock();
+
+	return ret;
+}
+
 static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 			  const struct pci_device_id *id)
 {
@@ -412,7 +428,7 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 	 * device is probed from work_on_cpu() of the Physical device.
 	 */
 	if (node < 0 || node >= MAX_NUMNODES || !node_online(node) ||
-	    pci_physfn_is_probed(dev)) {
+	    pci_physfn_is_probed(dev) || pci_probe_inline(node)) {
 		error = local_pci_probe(&ddi);
 	} else {
 		struct pci_probe_arg arg = { .ddi = &ddi };
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 8/8] PCI: Probe inline from node-local workqueue workers
  2026-09-11 12:34 ` [RFC PATCH 8/8] PCI: Probe inline from node-local workqueue workers Pavol Sakac
@ 2026-09-11 12:40   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:40 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

> pci_call_probe() dispatches each probe with queue_work_on() to a
> housekeeping CPU of the device's node, except where it already probes
> inline. A VF added by one of the preceding node-placed async VF-add
> workers already executes on its device's node, so the dispatch buys it no
> locality: it funnels every VF probe through the single CPU
> cpumask_any_and() elects and re-serializes the parallel enable at the
> probe step.
> 
> Call local_pci_probe() directly when the caller is a workqueue worker
> (PF_WQ_WORKER) whose current CPU is on the device's node and in the same
> HK_TYPE_DOMAIN housekeeping mask the dispatch elects from. The gate is
> not VF-specific: any probe already running in a node-local housekeeping
> kworker takes it, while a probe from a task that is not a worker keeps
> the dispatch.
> 
> Assisted-by: LLM
> Signed-off-by: Pavol Sakac <sakacpav@amazon.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=8


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 3/8] PCI/PM: Convert pci_bridge_d3_update() recursion to iteration
  2026-09-11 12:29 ` [RFC PATCH 3/8] PCI/PM: Convert pci_bridge_d3_update() recursion to iteration Pavol Sakac
@ 2026-09-11 12:40   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:40 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

> pci_bridge_d3_update() propagates a bridge_d3 change to upstream bridges
> by tail recursion: when a bridge's bridge_d3 value changes, the function
> calls itself with that bridge as the new device.
> 
> Convert the tail recursion into an iterative loop. Each level recomputes
> "remove" and d3cold_ok exactly as the recursive call did for its own
> device, and the early returns become loop exits. No functional change
> intended.
> 
> An upcoming change serializes this update with a mutex; the iterative
> form lets that mutex be taken once per external call instead of once per
> bridge level.
> 
> Assisted-by: LLM
> Signed-off-by: Pavol Sakac <sakacpav@amazon.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=3


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 7/8] PCI/IOV: Initialize virtual functions in parallel
  2026-09-11 12:33 ` [RFC PATCH 7/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
@ 2026-09-11 12:43   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:43 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

> Serial per-VF scanning and device addition dominate SR-IOV enable time at
> large VF counts, and a kexec-based live update re-creates every VF
> through this same loop. Distribute the per-VF work over the kernel's
> async machinery.
> 
> VF0 is added synchronously first because pci_iov_scan_device() writes the
> config fields shared by every VF only on the id == 0 pass and reads them
> locklessly afterwards. The rest are fanned out over async entries
> sharing an on-stack context with an atomic id cursor, scheduled on the
> PF's node with async_schedule_node_domain(). A worker that fails records
> the first error there and the enabling task unwinds every id descending;
> when several fail the temporally first errno is reported where the serial
> code reported the lowest failing id's, and nothing in-tree consumes the
> distinction.
> 
> [ ... ]
> past the failing id before unwinding them all. VF probes run in async
> context, where a synchronous request_module() WARNs.
> 
> Assisted-by: LLM
> Signed-off-by: Pavol Sakac <sakacpav@amazon.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=7


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn()
  2026-09-11 12:28 ` [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn() Pavol Sakac
@ 2026-09-11 12:45   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:45 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

> pci_iov_add_virtfn() creates the virtfn bus, scans and registers the VF,
> and removes the bus again on failure. A later commit creates all virtfn
> buses up front, so separate the bus-lifetime concern from the VF-add
> concern: move the VF add into a new __pci_iov_add_virtfn() taking the bus
> as an argument, and keep bus creation and on-failure bus removal in the
> public wrapper.
> 
> No functional change intended.
> 
> Assisted-by: LLM
> Signed-off-by: Pavol Sakac <sakacpav@amazon.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 2/8] PCI/IOV: Create virtfn buses up front in sriov_add_vfs()
  2026-09-11 12:29 ` [RFC PATCH 2/8] PCI/IOV: Create virtfn buses up front in sriov_add_vfs() Pavol Sakac
@ 2026-09-11 12:46   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:46 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

> virtfn_add_bus() is find-then-create and therefore assumes external
> serialization. Create every virtfn bus before adding any VF, taking the
> bus-create path out of the per-VF loop so a later commit can run the
> per-VF adds concurrently against a stable bus set. Bus numbers are
> already fixed before the loop, since pci_iov_set_numvfs() has latched
> offset and stride.
> 
> The cleanup pass must remove only buses this enable created, because
> virtfn_remove_bus() checks that a bus is empty rather than who created
> it, so virtfn_add_bus() now reports whether it created the bus and
> sriov_add_vfs() records that per VF id in a bitmap. Return early for
> num_vfs == 0: kvcalloc(0, ...) returns ZERO_SIZE_PTR and a later commit
> dereferences buses[0] unconditionally.
> 
> Several ids can share one bus number; only the creating id's bit is set.
> 
> Assisted-by: LLM
> Signed-off-by: Pavol Sakac <sakacpav@amazon.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update()
  2026-09-11 12:30 ` [RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update() Pavol Sakac
@ 2026-09-11 12:47   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:47 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

> pci_bridge_d3_update() does an unlocked read-modify-write of
> bridge->bridge_d3, and its callers are not mutually serialized: the
> d3cold_allowed sysfs write and the driver-context D3cold helpers hold
> neither pci_rescan_remove_lock nor device_lock. A concurrent write can
> lose an update and leave bridge_d3 stale, costing a wrong D3cold decision
> rather than memory safety. An upcoming change runs pci_bus_add_device()
> for sibling VFs concurrently, making sibling additions concurrent callers
> too, so this must land first.
> 
> Add a mutex around the whole update, taken once for the propagation
> loop. A device with no D3cold-capable port above it returns before the
> mutex, so the common add is not funneled through a global lock, and the
> loop re-evaluates both conditions under it. The mutex serializes the
> updaters against each other only; the d3cold_allowed store itself still
> writes an adjacent bit of the same word unlocked, a pre-existing
> exposure this change neither widens nor closes. The resulting order is
> pci_rescan_remove_lock, device_lock(any) -> pci_bridge_d3_lock ->
> pci_bus_sem (read), so pci_bridge_d3_lock must never be acquired while
> holding pci_bus_sem and no pci_walk_bus() callback may call into this
> path.
> 
> The race dates back to commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into
> D3 during suspend"), is theoretical with no known report, and so
> carries no Fixes: tag and no stable designation; it claims no measured
> performance contribution.
> 
> Assisted-by: LLM
> Signed-off-by: Pavol Sakac <sakacpav@amazon.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 6/8] PCI/IOV: Let sriov_add_vfs() own the failure unwind
  2026-09-11 12:32 ` [RFC PATCH 6/8] PCI/IOV: Let sriov_add_vfs() own the failure unwind Pavol Sakac
@ 2026-09-11 12:52   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:52 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Signed integer overflow in pci_iov_virtfn_bus() and pci_iov_virtfn_devfn() when computing the routing ID for a virtual function.
- [High] Preexisting lockdep assertion failure when EEH driver removes a Virtual Function without holding the pci_rescan_remove_lock.
--

commit b6383e6825acdb6d59417c0f87bde1ecfbf1a2b6
Author: Pavol Sakac <sakacpav@amazon.de>

PCI/IOV: Let sriov_add_vfs() own the failure unwind

This commit modifies the PCI/IOV virtualization code to handle VF addition
failures gracefully without requiring the pci_rescan_remove_lock in async
workers. It pushes the failure unwind responsibility to the callers using
pci_iov_remove_virtfn().

> Leave __pci_iov_add_virtfn() reporting only and let each caller unwind
> through pci_iov_remove_virtfn(), whose lookup-based design is correct at
> every failure stage. sriov_add_vfs() unwinds ids 0..i inclusive on
> failure of VF i, since VF i may be registered but not yet linked. The
> wrapper unwinds fully before returning, because its EEH caller discards
> the return code: the VF is removed through pci_iov_remove_virtfn(),

[Severity: High]
This isn't a bug introduced by this patch, but does this EEH caller
invocation violate core kernel locking rules?

Looking at the EEH driver, it calls pci_iov_remove_virtfn() to tear down
a VF without holding pci_rescan_remove_lock:

arch/powerpc/kernel/eeh_driver.c:eeh_rmv_device() {
    ...
    if (edev->physfn) {
        ...
        pci_iov_remove_virtfn(edev->physfn, edev->vf_index);
        ...
    }
    ...
}

However, pci_iov_remove_virtfn() calls pci_stop_and_remove_bus_device(),
which explicitly requires pci_rescan_remove_lock to be held, as verified by
lockdep_assert_held(&pci_rescan_remove_lock). Removing a device without
this lock risks concurrent PCI hotplug operations, which could lead to race
conditions and use-after-free bugs during EEH error recovery.

> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index dda9303516f5d..a32b2c2959223 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
[ ... ]
> @@ -394,8 +397,9 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
>  {
>  	struct pci_bus *bus;
> +	bool created;
>  	int rc;
>  
> -	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id), NULL);
> +	bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id), &created);

[Severity: High]
This is a pre-existing issue, but is there a signed integer overflow
vulnerability in the pci_iov_virtfn_bus() offset calculation?

Looking at pci_iov_virtfn_bus(), the calculation relies on multiplying
stride and vf_id:

drivers/pci/iov.c:pci_iov_virtfn_bus() {
    ...
    return dev->bus->number + ((dev->devfn + dev->sriov->offset +
                                dev->sriov->stride * vf_id) >> 8);
}

The dev->sriov->stride is a u16 and vf_id is an int. This evaluates to a
32-bit signed integer. If a malicious or misconfigured PCI device
advertises a large total_VFs and stride in its SR-IOV Extended Capability
structure, couldn't the multiplication overflow and produce a negative bus
offset?

This could underflow the bus number and cause the kernel to insert the new
bus with an out-of-bounds or corrupted identifier, resulting in memory
corruption in the PCI resource tree.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device()
  2026-09-11 12:31 ` [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device() Pavol Sakac
@ 2026-09-11 12:55   ` sashiko-bot
  0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-09-11 12:55 UTC (permalink / raw)
  To: Pavol Sakac; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential NULL pointer dereference of `pdn` in `pseries_pcibios_bus_add_device`.
- [High] Potential NULL pointer dereference of the physical function's EEH device (`edev`) in `pseries_pcibios_bus_add_device`.
--

commit 7133056da4f8622a96d7696deea5dac9e5c2eeb4
Author: Pavol Sakac <sakacpav@amazon.de>

powerpc/pci: Serialize pcibios_bus_add_device()

This commit serializes the pcibios_bus_add_device platform hook dispatch
with an arch-local mutex to prevent race conditions during concurrent sibling
VF additions. This addresses unlocked list manipulations in the EEH PE tree.

> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 4fc52c21fe5d4..21cccd0e97f8b 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c

[ ... ]

> @@ -1068,8 +1077,11 @@ void pcibios_bus_add_device(struct pci_dev *dev)
>  	if (ppc_md.pci_irq_fixup)
>  		ppc_md.pci_irq_fixup(dev);
>  
> -	if (ppc_md.pcibios_bus_add_device)
> +	if (ppc_md.pcibios_bus_add_device) {
> +		mutex_lock(&pcibios_bus_add_device_lock);
>  		ppc_md.pcibios_bus_add_device(dev);

[Severity: High]
This isn't a bug introduced by this patch, but can this hook lead to a NULL
pointer dereference if pdn allocation fails?

If add_sriov_vf_pdns() fails (for example, due to memory pressure), it
returns NULL instead of propagating an error. The PCI core proceeds to
instantiate the Virtual Functions and calls this platform hook.

In arch/powerpc/platforms/pseries/eeh_pseries.c:pseries_pcibios_bus_add_device():

    pdn = pci_get_pdn(pdev);
    pdn->device_id  =  pdev->device;

Could a privileged user trigger a kernel panic by enabling SR-IOV under
memory pressure, turning a recoverable failure into a DoS?

[Severity: High]
This is also a pre-existing issue, but could this hook dereference a NULL
pointer if the physical function lacks an initialized EEH device?

When adding a Virtual Function, pseries_pcibios_bus_add_device() assumes
the physical function's EEH device pointer is valid:

In arch/powerpc/platforms/pseries/eeh_pseries.c:pseries_pcibios_bus_add_device():

    struct eeh_pe *physfn_pe = pci_dev_to_eeh_dev(pdev->physfn)->pe;

If EEH initialization failed for the physical function (for instance, if the
device is not part of a PE or firmware didn't support EEH), its EEH device
pointer remains NULL.

Would writing to the sriov_numvfs sysfs file on such a physical function
cause an unconditional dereference of the NULL edev pointer here?

> +		mutex_unlock(&pcibios_bus_add_device_lock);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=5

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-09-11 12:55 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 12:11 [RFC PATCH 0/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
2026-09-11 12:28 ` [RFC PATCH 1/8] PCI/IOV: Split virtfn bus handling out of pci_iov_add_virtfn() Pavol Sakac
2026-09-11 12:45   ` sashiko-bot
2026-09-11 12:29 ` [RFC PATCH 2/8] PCI/IOV: Create virtfn buses up front in sriov_add_vfs() Pavol Sakac
2026-09-11 12:46   ` sashiko-bot
2026-09-11 12:29 ` [RFC PATCH 3/8] PCI/PM: Convert pci_bridge_d3_update() recursion to iteration Pavol Sakac
2026-09-11 12:40   ` sashiko-bot
2026-09-11 12:30 ` [RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update() Pavol Sakac
2026-09-11 12:47   ` sashiko-bot
2026-09-11 12:31 ` [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device() Pavol Sakac
2026-09-11 12:55   ` sashiko-bot
2026-09-11 12:32 ` [RFC PATCH 6/8] PCI/IOV: Let sriov_add_vfs() own the failure unwind Pavol Sakac
2026-09-11 12:52   ` sashiko-bot
2026-09-11 12:33 ` [RFC PATCH 7/8] PCI/IOV: Initialize virtual functions in parallel Pavol Sakac
2026-09-11 12:43   ` sashiko-bot
2026-09-11 12:34 ` [RFC PATCH 8/8] PCI: Probe inline from node-local workqueue workers Pavol Sakac
2026-09-11 12:40   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox