[PATCH v1 00/13] PCI/IOV: Improve reset handling with VFs enabled

[PATCH v1 00/13] PCI/IOV: Improve reset handling with VFs enabled

[Lukasz Laguna]

This series improves reset handling for SR-IOV devices. Today there
are a few gaps when resets happen with VFs enabled:

 1. VF state is lost across PF reset. The PCI core saves and restores
    the PF's config state, including its SR-IOV capability, and the
    kernel representation of each VF (struct pci_dev / struct device)
    survives the reset, but the VFs' hardware state does not. From the
    PCI point of view, the VFs do not exist between the PF reset and
    the restore of the PF IOV state. Their config space is lost, and
    their drivers are never notified about the reset.

 2. VFs are prepared in the wrong order during bus and slot reset. VFs
    are handled like any other device on the bus, and because the walk
    is top-down, the PF's .reset_prepare() runs before its VFs'
    callbacks.

 3. The "bus" reset method is unavailable for a PF with VFs enabled.
    pci_parent_bus_reset() accepts a device only if no other devices
    are on its bus. A PF's VFs sit on the same bus as the PF, so
    enabling VFs makes the PF fail this check. The VFs are in the PF's
    reset domain, so they should not block a bus reset of the PF.

This series addresses these gaps as follows:

 - Patches 1-3 add the PCI core helpers used by the rest of the series.

 - Patch 4 fixes (1) and (2). It locks all VFs together with the PF
   around a function reset, saves and restores the VFs' config state,
   and calls the VF drivers' .reset_prepare() / .reset_done()
   callbacks, so both architected and device-specific state can be
   preserved. During bus and slot resets, VFs are skipped in the device
   walk and handled through their PF. VFs are prepared before the PF
   and restored after it. As a result, pci_reset_function_locked() now
   requires PF callers to hold the device locks of all enabled VFs as
   well.

 - Patches 5-7 adapt PF drivers to the new core behavior. ionic and
   pds_core call pci_reset_function_locked() from their error resume
   handlers, so they now need to lock the VFs around it. i40e no longer
   needs to restore the VFs' MSI state in its .reset_done(), since the
   PCI core now restores the VFs' config state.

 - Patch 8 fixes (3). pci_parent_bus_reset() now ignores the PF's own
   VFs when it checks for other devices on the bus.

 - Patches 9-13 replace open-coded VF lookups with the new helpers.
   PCI/TSM and ixgbe use the VF iterators, and ice, liquidio and xe
   use pci_iov_get_virtfn(). These patches depend on patches 1-2 and
   are independent of the reset changes.

Lukasz Laguna (12):
  PCI/IOV: Introduce pci_iov_get_virtfn()
  PCI/IOV: Introduce VF iterators
  PCI/IOV: Introduce VF locking helpers
  ionic: Lock VFs around pci_reset_function_locked()
  pds_core: Lock VFs around pci_reset_function_locked()
  i40e: Drop VF MSI state restore from PF reset_done
  PCI/IOV: Allow bus reset_method for PF with VFs enabled
  PCI/TSM: Use generic VF iterators
  ixgbe: Use generic VF iterator
  ice: Use pci_iov_get_virtfn()
  liquidio: Use pci_iov_get_virtfn()
  drm/xe/pf: Use pci_iov_get_virtfn()

Michał Winiarski (1):
  PCI/IOV: Allow VF drivers to handle resets

 drivers/gpu/drm/xe/xe_pci_sriov.c             |   4 +-
 drivers/net/ethernet/amd/pds_core/main.c      |   5 +-
 .../ethernet/cavium/liquidio/octeon_mailbox.c |  10 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   |   3 -
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    |  26 ----
 .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |   3 -
 drivers/net/ethernet/intel/ice/ice_sriov.c    |  20 +--
 .../net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  25 +--
 .../ethernet/pensando/ionic/ionic_bus_pci.c   |   5 +-
 drivers/pci/iov.c                             |  88 ++++++++++-
 drivers/pci/pci.c                             | 146 +++++++++++++++++-
 drivers/pci/tsm.c                             |  25 +--
 include/linux/pci.h                           |  63 ++++++++
 13 files changed, 308 insertions(+), 115 deletions(-)

-- 
2.43.0

[PATCH v1 01/13] PCI/IOV: Introduce pci_iov_get_virtfn()

[Lukasz Laguna]

Add pci_iov_get_virtfn() to look up a VF of a PF by its VF ID. It
returns the matching pci_dev with a reference held, or NULL if no device
is present. The caller must drop the reference with pci_dev_put().

Assisted-by: LLM
Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/pci/iov.c   | 25 +++++++++++++++++++++----
 include/linux/pci.h |  4 ++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 9185934a900f..eb019201faed 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -51,6 +51,25 @@ int pci_iov_vf_id(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_iov_vf_id);
 
+/**
+ * pci_iov_get_virtfn() - Get the VF of a PF by its VF ID
+ * @dev: PCI PF
+ * @id: VF ID
+ *
+ * Find the VF with ID @id that belongs to PF @dev.
+ *
+ * The caller must drop the reference on the returned VF with pci_dev_put().
+ *
+ * Return: a pointer to the VF with ID @id, or %NULL if no such VF is present.
+ */
+struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id)
+{
+	return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
+					   pci_iov_virtfn_bus(dev, id),
+					   pci_iov_virtfn_devfn(dev, id));
+}
+EXPORT_SYMBOL_GPL(pci_iov_get_virtfn);
+
 /**
  * pci_iov_get_pf_drvdata - Return the drvdata of a PF
  * @dev: VF pci_dev
@@ -380,9 +399,7 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id)
 	char buf[VIRTFN_ID_LEN];
 	struct pci_dev *virtfn;
 
-	virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
-					     pci_iov_virtfn_bus(dev, id),
-					     pci_iov_virtfn_devfn(dev, id));
+	virtfn = pci_iov_get_virtfn(dev, id);
 	if (!virtfn)
 		return;
 
@@ -399,7 +416,7 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id)
 	pci_stop_and_remove_bus_device(virtfn);
 	virtfn_remove_bus(dev->bus, virtfn->bus);
 
-	/* balance pci_get_domain_bus_and_slot() */
+	/* balance pci_iov_get_virtfn() */
 	pci_dev_put(virtfn);
 	pci_dev_put(dev);
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f3e9c6c9c5e0..ac161f9f930c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2599,6 +2599,7 @@ void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar);
 int pci_iov_virtfn_bus(struct pci_dev *dev, int id);
 int pci_iov_virtfn_devfn(struct pci_dev *dev, int id);
 int pci_iov_vf_id(struct pci_dev *dev);
+struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id);
 void *pci_iov_get_pf_drvdata(struct pci_dev *dev, struct pci_driver *pf_driver);
 int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
 void pci_disable_sriov(struct pci_dev *dev);
@@ -2636,6 +2637,9 @@ static inline int pci_iov_vf_id(struct pci_dev *dev)
 	return -ENOSYS;
 }
 
+static inline struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id)
+{ return NULL; }
+
 static inline void *pci_iov_get_pf_drvdata(struct pci_dev *dev,
 					   struct pci_driver *pf_driver)
 {
-- 
2.43.0

[PATCH v1 02/13] PCI/IOV: Introduce VF iterators

[Lukasz Laguna]

Introduce helpers to iterate over the enabled VFs of a PF. Add
pci_get_vf() and pci_get_vf_reverse() to retrieve the next enabled VF
of a PF in ascending or descending VF ID order, along with
for_each_pci_vf(), for_each_pci_vf_reverse(), and
for_each_pci_vf_continue_reverse() built on top of them. The helpers
return each VF with a reference held, matching existing PCI device
lookup APIs.

This provides a common and convenient way to iterate over a PF's enabled
VFs and avoids duplicating the iteration logic in drivers.

Assisted-by: LLM
Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/pci/iov.c   | 63 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h | 57 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index eb019201faed..a437370c4e36 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -70,6 +70,69 @@ struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id)
 }
 EXPORT_SYMBOL_GPL(pci_iov_get_virtfn);
 
+/**
+ * pci_get_vf() - Return the first or next existing VF of a PF
+ * @dev: PCI PF whose VFs are being iterated
+ * @from: Previous VF returned by this function, or %NULL to return the first VF
+ *
+ * Iterate over existing VFs of @dev in ascending VF ID order, skipping VF IDs
+ * with no device present. If a VF is found, its reference count is incremented.
+ * A new iteration is started by passing %NULL as the @from argument. Otherwise,
+ * iteration continues after @from, and the reference count on @from is
+ * decremented.
+ *
+ * Return: a pointer to the first or next VF, or %NULL if no further VF is found.
+ */
+struct pci_dev *pci_get_vf(struct pci_dev *dev, struct pci_dev *from)
+{
+	int id = 0;
+
+	if (from) {
+		id = pci_iov_vf_id(from) + 1;
+		pci_dev_put(from);
+	}
+
+	for (; id < pci_num_vf(dev); id++) {
+		from = pci_iov_get_virtfn(dev, id);
+		if (from)
+			return from;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(pci_get_vf);
+
+/**
+ * pci_get_vf_reverse() - Return the last or previous existing VF of a PF
+ * @dev: PCI PF whose VFs are being iterated
+ * @from: Previous VF returned by this function, or %NULL to return the last VF
+ *
+ * Same semantics as pci_get_vf(), except iteration proceeds in descending VF
+ * ID order.
+ *
+ * Return: a pointer to the last or previous VF, or %NULL if no further VF is
+ * found.
+ */
+struct pci_dev *pci_get_vf_reverse(struct pci_dev *dev, struct pci_dev *from)
+{
+	int id = pci_num_vf(dev);
+
+	if (from) {
+		id = pci_iov_vf_id(from);
+		pci_dev_put(from);
+	}
+
+	while (id > 0) {
+		id--;
+		from = pci_iov_get_virtfn(dev, id);
+		if (from)
+			return from;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(pci_get_vf_reverse);
+
 /**
  * pci_iov_get_pf_drvdata - Return the drvdata of a PF
  * @dev: VF pci_dev
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ac161f9f930c..03fe10ffa19b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -616,6 +616,53 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
 #define for_each_pci_dev_reverse(d) \
 	while ((d = pci_get_device_reverse(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
 
+/**
+ * for_each_pci_vf - iterate over existing VFs of a PF
+ * @dev: PCI PF whose VFs are being iterated
+ * @vf: struct pci_dev * used as a loop cursor
+ *
+ * Iterate over existing VFs in ascending VF ID order, skipping VF IDs with no
+ * device present.
+ *
+ * During each iteration, @vf is returned with a reference held. That
+ * reference is released automatically only when advancing to the next VF.
+ * If the loop is exited early, the caller must drop the last reference.
+ */
+#define for_each_pci_vf(dev, vf) \
+	for ((vf) = NULL; ((vf) = pci_get_vf((dev), (vf))) != NULL; )
+
+/**
+ * for_each_pci_vf_reverse - iterate backwards over existing VFs of a PF
+ * @dev: PCI PF whose VFs are being iterated
+ * @vf: struct pci_dev * used as a loop cursor
+ *
+ * Iterate over existing VFs in descending VF ID order, skipping VF IDs with no
+ * device present.
+ *
+ * During each iteration, @vf is returned with a reference held. That
+ * reference is released automatically only when advancing to the next VF.
+ * If the loop is exited early, the caller must drop the last reference.
+ */
+#define for_each_pci_vf_reverse(dev, vf) \
+	for ((vf) = NULL; ((vf) = pci_get_vf_reverse((dev), (vf))) != NULL; )
+
+/**
+ * for_each_pci_vf_continue_reverse - iterate backwards from the given VF
+ * @dev: PCI PF whose VFs are being iterated
+ * @vf: struct pci_dev * used as a loop cursor
+ *
+ * Continue iterating in descending VF ID order from the preceding @vf.
+ * Before the first iteration, @vf must point to the current VF with a
+ * reference held.
+ *
+ * During each iteration, @vf is returned with a reference held. That
+ * reference is released automatically only when advancing to the next VF.
+ * If the loop is exited early, the caller must drop the last reference.
+ */
+#define for_each_pci_vf_continue_reverse(dev, vf) \
+	for ((vf) = pci_get_vf_reverse((dev), (vf)); (vf) != NULL; \
+	     (vf) = pci_get_vf_reverse((dev), (vf)))
+
 static inline int pci_channel_offline(struct pci_dev *pdev)
 {
 	return (pdev->error_state != pci_channel_io_normal);
@@ -2600,6 +2647,8 @@ int pci_iov_virtfn_bus(struct pci_dev *dev, int id);
 int pci_iov_virtfn_devfn(struct pci_dev *dev, int id);
 int pci_iov_vf_id(struct pci_dev *dev);
 struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id);
+struct pci_dev *pci_get_vf(struct pci_dev *dev, struct pci_dev *from);
+struct pci_dev *pci_get_vf_reverse(struct pci_dev *dev, struct pci_dev *from);
 void *pci_iov_get_pf_drvdata(struct pci_dev *dev, struct pci_driver *pf_driver);
 int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
 void pci_disable_sriov(struct pci_dev *dev);
@@ -2640,6 +2689,14 @@ static inline int pci_iov_vf_id(struct pci_dev *dev)
 static inline struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id)
 { return NULL; }
 
+static inline struct pci_dev *pci_get_vf(struct pci_dev *dev,
+					 struct pci_dev *from)
+{ return NULL; }
+
+static inline struct pci_dev *pci_get_vf_reverse(struct pci_dev *dev,
+						 struct pci_dev *from)
+{ return NULL; }
+
 static inline void *pci_iov_get_pf_drvdata(struct pci_dev *dev,
 					   struct pci_driver *pf_driver)
 {
-- 
2.43.0

[PATCH v1 03/13] PCI/IOV: Introduce VF locking helpers

[Lukasz Laguna]

Add pci_dev_lock_vfs() and pci_dev_unlock_vfs() helpers to lock and
unlock all enabled VFs of a PF device.

This provides a common way to serialize operations involving
subordinate VFs.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/pci/pci.c   | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |  2 ++
 2 files changed, 41 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a62af7320539..f1669f88e768 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5111,6 +5111,45 @@ void pci_dev_unlock(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_dev_unlock);
 
+/**
+ * pci_dev_lock_vfs() - Lock subordinate VFs of a PF device
+ * @dev: PCI device whose VFs should be locked
+ *
+ * Lock all currently enabled VFs in forward iteration order.
+ *
+ * Context: The caller must hold the device lock of @dev.
+ */
+void pci_dev_lock_vfs(struct pci_dev *dev)
+{
+	struct pci_dev *vf;
+
+	device_lock_assert(&dev->dev);
+
+	for_each_pci_vf(dev, vf)
+		pci_dev_lock(vf);
+}
+EXPORT_SYMBOL_GPL(pci_dev_lock_vfs);
+
+/**
+ * pci_dev_unlock_vfs() - Unlock subordinate VFs of a PF device
+ * @dev: PCI device whose VFs should be unlocked
+ *
+ * Unlock all currently enabled VFs in reverse iteration order of
+ * pci_dev_lock_vfs().
+ *
+ * Context: The caller must hold the device lock of @dev and all its VFs.
+ */
+void pci_dev_unlock_vfs(struct pci_dev *dev)
+{
+	struct pci_dev *vf;
+
+	device_lock_assert(&dev->dev);
+
+	for_each_pci_vf_reverse(dev, vf)
+		pci_dev_unlock(vf);
+}
+EXPORT_SYMBOL_GPL(pci_dev_unlock_vfs);
+
 static void pci_dev_save_and_disable(struct pci_dev *dev)
 {
 	const struct pci_error_handlers *err_handler =
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 03fe10ffa19b..718d17035439 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2097,6 +2097,8 @@ void pci_cfg_access_unlock(struct pci_dev *dev);
 void pci_dev_lock(struct pci_dev *dev);
 int pci_dev_trylock(struct pci_dev *dev);
 void pci_dev_unlock(struct pci_dev *dev);
+void pci_dev_lock_vfs(struct pci_dev *dev);
+void pci_dev_unlock_vfs(struct pci_dev *dev);
 DEFINE_GUARD(pci_dev, struct pci_dev *, pci_dev_lock(_T), pci_dev_unlock(_T))
 
 /*
-- 
2.43.0

[PATCH v1 04/13] PCI/IOV: Allow VF drivers to handle resets

[Lukasz Laguna]

From: Michał Winiarski <michal.winiarski@intel.com>

While the kernel representation of VF device (think struct device) is
preserved across PF FLR, and the PF IOV state (among other PCI CFG
state) is saved and restored across reset, the actual VF state is lost,
as from PCI perspective, VF didn't exist in the time between PF FLR and
PF IOV state restore.

Save / restore all subordinate VFs config state, and notify their
drivers using .reset_prepare() / .reset_done() to make sure that both
architected and unarchitected state can be preserved across FLR.

VFs are disabled only after the PF driver's .reset_prepare() and their
state is restored before PF driver's .reset_done(), so they can still
communicate with the PF while it handles the reset.

This also fixes VFs handling during bus and slot reset. Previously, VFs
were managed independently, so reset preparation for VFs was triggered
while walking the bus devices. Since that walk is top-down, the
.reset_prepare() was triggered on PF before it was triggered on the VFs.

As part of this, pci_reset_function_locked() now requires PF callers to
hold the device locks for all enabled VFs as well.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Co-developed-by: Lukasz Laguna <lukasz.laguna@intel.com>
Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/pci/pci.c | 105 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 99 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f1669f88e768..90468e38dc5a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5150,17 +5150,48 @@ void pci_dev_unlock_vfs(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_dev_unlock_vfs);
 
-static void pci_dev_save_and_disable(struct pci_dev *dev)
+static int pci_dev_trylock_vfs(struct pci_dev *dev)
+{
+	struct pci_dev *vf;
+
+	device_lock_assert(&dev->dev);
+
+	for_each_pci_vf(dev, vf) {
+		if (!pci_dev_trylock(vf))
+			goto unlock;
+	}
+
+	return 1;
+
+unlock:
+	for_each_pci_vf_continue_reverse(dev, vf)
+		pci_dev_unlock(vf);
+
+	return 0;
+}
+
+static void pci_dev_vfs_lock_assert(struct pci_dev *dev)
+{
+	struct pci_dev *vf;
+
+	device_lock_assert(&dev->dev);
+
+	for_each_pci_vf(dev, vf)
+		device_lock_assert(&vf->dev);
+}
+
+static void pci_dev_reset_prepare_and_save(struct pci_dev *dev)
 {
 	const struct pci_error_handlers *err_handler =
 			dev->driver ? dev->driver->err_handler : NULL;
 
+	device_lock_assert(&dev->dev);
+
 	/*
 	 * dev->driver->err_handler->reset_prepare() is protected against
 	 * races with ->remove() by the device lock, which must be held by
 	 * the caller.
 	 */
-	device_lock_assert(&dev->dev);
 	if (err_handler && err_handler->reset_prepare)
 		err_handler->reset_prepare(dev);
 	else if (dev->driver)
@@ -5177,6 +5208,17 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
 		return;
 
 	pci_save_state(dev);
+}
+
+static void pci_dev_save_and_disable(struct pci_dev *dev)
+{
+	struct pci_dev *vf;
+
+	for_each_pci_vf(dev, vf)
+		pci_dev_reset_prepare_and_save(vf);
+
+	pci_dev_reset_prepare_and_save(dev);
+
 	/*
 	 * Disable the device by clearing the Command register, except for
 	 * INTx-disable which is set.  This not only disables MMIO and I/O port
@@ -5184,15 +5226,18 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
 	 * DMA from the device including MSI/MSI-X interrupts.  For PCI 2.3
 	 * compliant devices, INTx-disable prevents legacy interrupts.
 	 */
+	for_each_pci_vf(dev, vf)
+		pci_write_config_word(vf, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
+
 	pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
 }
 
-static void pci_dev_restore(struct pci_dev *dev)
+static void pci_dev_reset_done(struct pci_dev *dev)
 {
 	const struct pci_error_handlers *err_handler =
 			dev->driver ? dev->driver->err_handler : NULL;
 
-	pci_restore_state(dev);
+	device_lock_assert(&dev->dev);
 
 	/*
 	 * dev->driver->err_handler->reset_done() is protected against
@@ -5205,6 +5250,23 @@ static void pci_dev_restore(struct pci_dev *dev)
 		pci_warn(dev, "reset done");
 }
 
+static void pci_dev_restore(struct pci_dev *dev)
+{
+	struct pci_dev *vf;
+
+	device_lock_assert(&dev->dev);
+
+	pci_restore_state(dev);
+
+	for_each_pci_vf(dev, vf)
+		pci_restore_state(vf);
+
+	pci_dev_reset_done(dev);
+
+	for_each_pci_vf(dev, vf)
+		pci_dev_reset_done(vf);
+}
+
 /* dev->reset_methods[] is a 0-terminated list of indices into this array */
 const struct pci_reset_fn_method pci_reset_fn_methods[] = {
 	{ },
@@ -5320,6 +5382,10 @@ void pci_init_reset_methods(struct pci_dev *dev)
  * from __pci_reset_function_locked() in that it saves and restores device state
  * over the reset and takes the PCI device lock.
  *
+ * If @dev is a PF with VFs enabled, the VFs are locked as well, their drivers
+ * are notified via reset_prepare() and reset_done(), and their state is saved
+ * and restored over the reset.
+ *
  * Returns 0 if the device function was successfully reset or negative if the
  * device doesn't support resetting a single function.
  */
@@ -5340,11 +5406,13 @@ int pci_reset_function(struct pci_dev *dev)
 		pci_dev_lock(bridge);
 
 	pci_dev_lock(dev);
+	pci_dev_lock_vfs(dev);
 	pci_dev_save_and_disable(dev);
 
 	rc = __pci_reset_function_locked(dev);
 
 	pci_dev_restore(dev);
+	pci_dev_unlock_vfs(dev);
 	pci_dev_unlock(dev);
 
 	if (bridge)
@@ -5368,7 +5436,11 @@ EXPORT_SYMBOL_GPL(pci_reset_function);
  * over the reset.  It also differs from pci_reset_function() in that it
  * requires the PCI device lock to be held.
  *
- * Context: The caller must hold the device lock.
+ * If @dev is a PF with VFs enabled, VFs drivers are notified via
+ * reset_prepare() and reset_done(), and their state is saved and restored over
+ * the reset.
+ *
+ * Context: The caller must hold the device lock of @dev and of all its VFs.
  *
  * Return: 0 if the device function was successfully reset or negative if the
  * device doesn't support resetting a single function.
@@ -5378,6 +5450,7 @@ int pci_reset_function_locked(struct pci_dev *dev)
 	int rc;
 
 	device_lock_assert(&dev->dev);
+	pci_dev_vfs_lock_assert(dev);
 
 	if (!pci_reset_supported(dev))
 		return -ENOTTY;
@@ -5396,7 +5469,7 @@ EXPORT_SYMBOL_GPL(pci_reset_function_locked);
  * pci_try_reset_function - quiesce and reset a PCI device function
  * @dev: PCI device to reset
  *
- * Same as above, except return -EAGAIN if unable to lock device.
+ * Same as above, except return -EAGAIN if unable to lock device or its VFs.
  */
 int pci_try_reset_function(struct pci_dev *dev)
 {
@@ -5408,9 +5481,16 @@ int pci_try_reset_function(struct pci_dev *dev)
 	if (!pci_dev_trylock(dev))
 		return -EAGAIN;
 
+	if (!pci_dev_trylock_vfs(dev)) {
+		rc = -EAGAIN;
+		goto pf_unlock;
+	}
+
 	pci_dev_save_and_disable(dev);
 	rc = __pci_reset_function_locked(dev);
 	pci_dev_restore(dev);
+	pci_dev_unlock_vfs(dev);
+pf_unlock:
 	pci_dev_unlock(dev);
 
 	return rc;
@@ -5564,6 +5644,11 @@ static int pci_slot_trylock(struct pci_slot *slot)
 	return __pci_bus_trylock(slot->bus, slot);
 }
 
+static bool pci_dev_skip_bus_reset(struct pci_dev *dev)
+{
+	return dev->is_virtfn; /* VFs are managed by their PF */
+}
+
 /*
  * Save and disable devices from the top of the tree down while holding
  * the @dev mutex lock for the entire tree.
@@ -5573,6 +5658,8 @@ static void pci_bus_save_and_disable_locked(struct pci_bus *bus)
 	struct pci_dev *dev;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
+		if (pci_dev_skip_bus_reset(dev))
+			continue;
 		pci_dev_save_and_disable(dev);
 		if (dev->subordinate)
 			pci_bus_save_and_disable_locked(dev->subordinate);
@@ -5589,6 +5676,8 @@ static void pci_bus_restore_locked(struct pci_bus *bus)
 	struct pci_dev *dev;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
+		if (pci_dev_skip_bus_reset(dev))
+			continue;
 		pci_dev_restore(dev);
 		if (dev->subordinate) {
 			pci_bridge_wait_for_secondary_bus(dev, "bus reset");
@@ -5608,6 +5697,8 @@ static void pci_slot_save_and_disable_locked(struct pci_slot *slot)
 	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
 		if (!dev->slot || dev->slot != slot)
 			continue;
+		if (pci_dev_skip_bus_reset(dev))
+			continue;
 		pci_dev_save_and_disable(dev);
 		if (dev->subordinate)
 			pci_bus_save_and_disable_locked(dev->subordinate);
@@ -5626,6 +5717,8 @@ static void pci_slot_restore_locked(struct pci_slot *slot)
 	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
 		if (!dev->slot || dev->slot != slot)
 			continue;
+		if (pci_dev_skip_bus_reset(dev))
+			continue;
 		pci_dev_restore(dev);
 		if (dev->subordinate) {
 			pci_bridge_wait_for_secondary_bus(dev, "slot reset");
-- 
2.43.0

[PATCH v1 05/13] ionic: Lock VFs around pci_reset_function_locked()

[Lukasz Laguna]

pci_reset_function_locked() now requires PF callers to hold the device
locks for all enabled VFs as well. Lock VFs around the reset in
ionic_pci_error_resume().

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 05f19489ec5c..a0f501fe33e4 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -505,8 +505,11 @@ static void ionic_pci_error_resume(struct pci_dev *pdev)
 	struct ionic *ionic = pci_get_drvdata(pdev);
 	struct ionic_lif *lif = ionic->lif;
 
-	if (lif && test_bit(IONIC_LIF_F_FW_RESET, lif->state))
+	if (lif && test_bit(IONIC_LIF_F_FW_RESET, lif->state)) {
+		pci_dev_lock_vfs(pdev);
 		pci_reset_function_locked(pdev);
+		pci_dev_unlock_vfs(pdev);
+	}
 }
 
 static const struct pci_error_handlers ionic_err_handler = {
-- 
2.43.0

[PATCH v1 06/13] pds_core: Lock VFs around pci_reset_function_locked()

[Lukasz Laguna]

pci_reset_function_locked() now requires PF callers to hold the device
locks for all enabled VFs as well. Lock VFs around the reset in
pdsc_pci_error_resume().

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/net/ethernet/amd/pds_core/main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
index bb79e7476370..23e03d813fdc 100644
--- a/drivers/net/ethernet/amd/pds_core/main.c
+++ b/drivers/net/ethernet/amd/pds_core/main.c
@@ -583,8 +583,11 @@ static void pdsc_pci_error_resume(struct pci_dev *pdev)
 {
 	struct pdsc *pdsc = pci_get_drvdata(pdev);
 
-	if (test_bit(PDSC_S_FW_DEAD, &pdsc->state))
+	if (test_bit(PDSC_S_FW_DEAD, &pdsc->state)) {
+		pci_dev_lock_vfs(pdev);
 		pci_reset_function_locked(pdev);
+		pci_dev_unlock_vfs(pdev);
+	}
 }
 
 static const struct pci_error_handlers pdsc_err_handler = {
-- 
2.43.0

[PATCH v1 07/13] i40e: Drop VF MSI state restore from PF reset_done

[Lukasz Laguna]

The PCI core now saves and restores the config state of all enabled VFs
around PF reset, including MSI state. Remove the driver-specific
restore, as it's redundant now.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  3 ---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 26 -------------------
 .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |  3 ---
 3 files changed, 32 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 0cd0e5597c90..44258573b07b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -16498,9 +16498,6 @@ static void i40e_pci_error_reset_done(struct pci_dev *pdev)
 		return;
 
 	i40e_reset_and_rebuild(pf, false, false);
-#ifdef CONFIG_PCI_IOV
-	i40e_restore_all_vfs_msi_state(pdev);
-#endif /* CONFIG_PCI_IOV */
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index a26c3d47ec15..f240e6a36887 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -154,32 +154,6 @@ void i40e_vc_notify_reset(struct i40e_pf *pf)
 			     (u8 *)&pfe, sizeof(struct virtchnl_pf_event));
 }
 
-#ifdef CONFIG_PCI_IOV
-void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev)
-{
-	u16 vf_id;
-	u16 pos;
-
-	/* Continue only if this is a PF */
-	if (!pdev->is_physfn)
-		return;
-
-	if (!pci_num_vf(pdev))
-		return;
-
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV);
-	if (pos) {
-		struct pci_dev *vf_dev = NULL;
-
-		pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_id);
-		while ((vf_dev = pci_get_device(pdev->vendor, vf_id, vf_dev))) {
-			if (vf_dev->is_virtfn && vf_dev->physfn == pdev)
-				pci_restore_msi_state(vf_dev);
-		}
-	}
-}
-#endif /* CONFIG_PCI_IOV */
-
 /**
  * i40e_vc_notify_vf_reset
  * @vf: pointer to the VF structure
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index 4e119c0502f3..b6b11de74de5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -148,9 +148,6 @@ int i40e_ndo_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool enable);
 
 void i40e_vc_notify_link_state(struct i40e_pf *pf);
 void i40e_vc_notify_reset(struct i40e_pf *pf);
-#ifdef CONFIG_PCI_IOV
-void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev);
-#endif /* CONFIG_PCI_IOV */
 int i40e_get_vf_stats(struct net_device *netdev, int vf_id,
 		      struct ifla_vf_stats *vf_stats);
 
-- 
2.43.0

[PATCH v1 08/13] PCI/IOV: Allow bus reset_method for PF with VFs enabled

[Lukasz Laguna]

pci_parent_bus_reset() accepts a device for secondary bus reset only when
no other devices are present on that bus.

While that makes sense in most cases, it is too strict for SR-IOV. When
a PF has VFs enabled, those VFs are enumerated on the same bus. That
causes this check to reject the PF, even though its VFs are in the same
reset domain and should not block a PF bus reset.

Ignore PF-owned VFs in this check, so the bus reset method remains
available for PFs with VFs enabled.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 90468e38dc5a..66680a05069d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4946,7 +4946,7 @@ static int pci_parent_bus_reset(struct pci_dev *dev, bool probe)
 		return -ENOTTY;
 
 	list_for_each_entry(pdev, &dev->bus->devices, bus_list)
-		if (pdev != dev)
+		if (pdev != dev && !(pdev->is_virtfn && pci_physfn(pdev) == dev))
 			return -ENOTTY;
 
 	if (probe)
-- 
2.43.0

[PATCH v1 09/13] PCI/TSM: Use generic VF iterators

[Lukasz Laguna]

pci_tsm_walk_fns() and pci_tsm_walk_fns_reverse() implement their own VF
iteration logic. Replace it with for_each_pci_vf() and
for_each_pci_vf_reverse().

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/pci/tsm.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
index 5fdcd7f2e820..01fa6149e481 100644
--- a/drivers/pci/tsm.c
+++ b/drivers/pci/tsm.c
@@ -84,6 +84,7 @@ static void pci_tsm_walk_fns(struct pci_dev *pdev,
 	for (int i = 0; i < 8; i++) {
 		struct pci_dev *pf __free(pci_dev_put) = pci_get_slot(
 			pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), i));
+		struct pci_dev *vf;
 
 		if (!pf)
 			continue;
@@ -93,18 +94,8 @@ static void pci_tsm_walk_fns(struct pci_dev *pdev,
 			cb(pf, data);
 
 		/* walk virtual functions of each pf */
-		for (int j = 0; j < pci_num_vf(pf); j++) {
-			struct pci_dev *vf __free(pci_dev_put) =
-				pci_get_domain_bus_and_slot(
-					pci_domain_nr(pf->bus),
-					pci_iov_virtfn_bus(pf, j),
-					pci_iov_virtfn_devfn(pf, j));
-
-			if (!vf)
-				continue;
-
+		for_each_pci_vf(pf, vf)
 			cb(vf, data);
-		}
 	}
 
 	/*
@@ -128,22 +119,14 @@ static void pci_tsm_walk_fns_reverse(struct pci_dev *pdev,
 	for (int i = 7; i >= 0; i--) {
 		struct pci_dev *pf __free(pci_dev_put) = pci_get_slot(
 			pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), i));
+		struct pci_dev *vf;
 
 		if (!pf)
 			continue;
 
 		/* reverse walk virtual functions */
-		for (int j = pci_num_vf(pf) - 1; j >= 0; j--) {
-			struct pci_dev *vf __free(pci_dev_put) =
-				pci_get_domain_bus_and_slot(
-					pci_domain_nr(pf->bus),
-					pci_iov_virtfn_bus(pf, j),
-					pci_iov_virtfn_devfn(pf, j));
-
-			if (!vf)
-				continue;
+		for_each_pci_vf_reverse(pf, vf)
 			cb(vf, data);
-		}
 
 		/* on exit, caller will run @cb on function 0 */
 		if (i > 0)
-- 
2.43.0

[PATCH v1 10/13] ixgbe: Use generic VF iterator

[Lukasz Laguna]

ixgbe_get_vfs() finds the VFs by walking PCI devices with the VF device
ID and checking that they belong to the PF. Replace it with
for_each_pci_vf(), which iterates over the PF's VFs directly in VF ID
order.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 .../net/ethernet/intel/ixgbe/ixgbe_sriov.c    | 25 +++++--------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 431d77da15a5..67e22dc514f8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -123,28 +123,15 @@ static int __ixgbe_enable_sriov(struct ixgbe_adapter *adapter,
 static void ixgbe_get_vfs(struct ixgbe_adapter *adapter)
 {
 	struct pci_dev *pdev = adapter->pdev;
-	u16 vendor = pdev->vendor;
 	struct pci_dev *vfdev;
 	int vf = 0;
-	u16 vf_id;
-	int pos;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV);
-	if (!pos)
-		return;
-	pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_id);
-
-	vfdev = pci_get_device(vendor, vf_id, NULL);
-	for (; vfdev; vfdev = pci_get_device(vendor, vf_id, vfdev)) {
-		if (!vfdev->is_virtfn)
-			continue;
-		if (vfdev->physfn != pdev)
-			continue;
-		if (vf >= adapter->num_vfs)
-			continue;
-		pci_dev_get(vfdev);
-		adapter->vfinfo[vf].vfdev = vfdev;
-		++vf;
+	for_each_pci_vf(pdev, vfdev) {
+		if (vf >= adapter->num_vfs) {
+			pci_dev_put(vfdev);
+			break;
+		}
+		adapter->vfinfo[vf++].vfdev = pci_dev_get(vfdev);
 	}
 }
 
-- 
2.43.0

[PATCH v1 11/13] ice: Use pci_iov_get_virtfn()

[Lukasz Laguna]

ice_create_vf_entries() finds the VFs by walking PCI devices with the
VF device ID and checking that they belong to the PF. Replace it with
pci_iov_get_virtfn(), which looks up each VF directly by its VF ID.

This also avoids leaking the reference held by pci_get_device() when VF
entry allocation fails.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_sriov.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index e04de0215596..290f07fe2e55 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -686,16 +686,11 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs)
 {
 	struct pci_dev *pdev = pf->pdev;
 	struct ice_vfs *vfs = &pf->vfs;
-	struct pci_dev *vfdev = NULL;
 	struct ice_vf *vf;
-	u16 vf_pdev_id;
-	int err, pos;
+	int err;
 
 	lockdep_assert_held(&vfs->table_lock);
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV);
-	pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_pdev_id);
-
 	for (u16 vf_id = 0; vf_id < num_vfs; vf_id++) {
 		vf = kzalloc_obj(*vf);
 		if (!vf) {
@@ -712,23 +707,12 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs)
 
 		ice_initialize_vf_entry(vf);
 
-		do {
-			vfdev = pci_get_device(pdev->vendor, vf_pdev_id, vfdev);
-		} while (vfdev && vfdev->physfn != pdev);
-		vf->vfdev = vfdev;
+		vf->vfdev = pci_iov_get_virtfn(pdev, vf_id);
 		vf->vf_sw_id = pf->first_sw;
 
-		pci_dev_get(vfdev);
-
 		hash_add_rcu(vfs->table, &vf->entry, vf_id);
 	}
 
-	/* Decrement of refcount done by pci_get_device() inside the loop does
-	 * not touch the last iteration's vfdev, so it has to be done manually
-	 * to balance pci_dev_get() added within the loop.
-	 */
-	pci_dev_put(vfdev);
-
 	return 0;
 
 err_free_entries:
-- 
2.43.0

[PATCH v1 12/13] liquidio: Use pci_iov_get_virtfn()

[Lukasz Laguna]

lio_vf_pci_dev_by_qno() finds the VF by walking PCI devices with the
CN23XX VF device ID and checking that they belong to the PF and match
the VF ID. Replace it with pci_iov_get_virtfn(), which looks up the VF
directly by its VF ID.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
index 697fcdc41e3c..58f25ddd2f6d 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
@@ -28,7 +28,6 @@
 
 static struct pci_dev *lio_vf_pci_dev_by_qno(struct octeon_device *oct, u32 q_no)
 {
-	struct pci_dev *vfdev = NULL;
 	int vfidx;
 
 	if (!oct->sriov_info.rings_per_vf)
@@ -41,14 +40,7 @@ static struct pci_dev *lio_vf_pci_dev_by_qno(struct octeon_device *oct, u32 q_no
 	if (vfidx >= oct->sriov_info.num_vfs_alloced)
 		return NULL;
 
-	while ((vfdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
-				       OCTEON_CN23XX_VF_VID, vfdev))) {
-		if (pci_physfn(vfdev) && pci_physfn(vfdev) == oct->pci_dev &&
-		    pci_iov_vf_id(vfdev) == vfidx)
-			return vfdev;
-	}
-
-	return NULL;
+	return pci_iov_get_virtfn(oct->pci_dev, vfidx);
 }
 
 /**
-- 
2.43.0

[PATCH v1 13/13] drm/xe/pf: Use pci_iov_get_virtfn()

[Lukasz Laguna]

xe_pci_sriov_get_vf_pdev() implements its own VF lookup. Replace it with
pci_iov_get_virtfn(), which also takes the VF's bus number into account.
Previously, the VF was assumed to be on the PF's bus, which is not always
true.

Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
---
 drivers/gpu/drm/xe/xe_pci_sriov.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
index 35e6b53e18ce..d5a4e95b3ffb 100644
--- a/drivers/gpu/drm/xe/xe_pci_sriov.c
+++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
@@ -258,7 +258,5 @@ struct pci_dev *xe_pci_sriov_get_vf_pdev(struct pci_dev *pdev, unsigned int vfid
 	xe_assert(xe, vfid);
 	xe_assert(xe, vfid <= pci_sriov_get_totalvfs(pdev));
 
-	return pci_get_domain_bus_and_slot(pci_domain_nr(pdev->bus),
-					   pdev->bus->number,
-					   pci_iov_virtfn_devfn(pdev, vfid - 1));
+	return pci_iov_get_virtfn(pdev, vfid - 1);
 }
-- 
2.43.0

Re: [PATCH v1 09/13] PCI/TSM: Use generic VF iterators

[sashiko-bot]

> pci_tsm_walk_fns() and pci_tsm_walk_fns_reverse() implement their own VF
> iteration logic. Replace it with for_each_pci_vf() and
> for_each_pci_vf_reverse().
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=9

Re: [PATCH v1 10/13] ixgbe: Use generic VF iterator

[sashiko-bot]

> ixgbe_get_vfs() finds the VFs by walking PCI devices with the VF device
> ID and checking that they belong to the PF. Replace it with
> for_each_pci_vf(), which iterates over the PF's VFs directly in VF ID
> order.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=10

Re: [PATCH v1 11/13] ice: Use pci_iov_get_virtfn()

[sashiko-bot]

> ice_create_vf_entries() finds the VFs by walking PCI devices with the
> VF device ID and checking that they belong to the PF. Replace it with
> pci_iov_get_virtfn(), which looks up each VF directly by its VF ID.
> 
> This also avoids leaking the reference held by pci_get_device() when VF
> entry allocation fails.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=11

Re: [PATCH v1 07/13] i40e: Drop VF MSI state restore from PF reset_done

[sashiko-bot]

> The PCI core now saves and restores the config state of all enabled VFs
> around PF reset, including MSI state. Remove the driver-specific
> restore, as it's redundant now.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=7

Re: [PATCH v1 12/13] liquidio: Use pci_iov_get_virtfn()

[sashiko-bot]

> lio_vf_pci_dev_by_qno() finds the VF by walking PCI devices with the
> CN23XX VF device ID and checking that they belong to the PF and match
> the VF ID. Replace it with pci_iov_get_virtfn(), which looks up the VF
> directly by its VF ID.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=12

Re: [PATCH v1 01/13] PCI/IOV: Introduce pci_iov_get_virtfn()

[sashiko-bot]

> Add pci_iov_get_virtfn() to look up a VF of a PF by its VF ID. It
> returns the matching pci_dev with a reference held, or NULL if no device
> is present. The caller must drop the reference with pci_dev_put().
> 
> Assisted-by: LLM
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=1

Re: [PATCH v1 13/13] drm/xe/pf: Use pci_iov_get_virtfn()

[sashiko-bot]

> xe_pci_sriov_get_vf_pdev() implements its own VF lookup. Replace it with
> pci_iov_get_virtfn(), which also takes the VF's bus number into account.
> Previously, the VF was assumed to be on the PF's bus, which is not always
> true.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=13

Re: [PATCH v1 02/13] PCI/IOV: Introduce VF iterators

[sashiko-bot]

> Introduce helpers to iterate over the enabled VFs of a PF. Add
> pci_get_vf() and pci_get_vf_reverse() to retrieve the next enabled VF
> of a PF in ascending or descending VF ID order, along with
> for_each_pci_vf(), for_each_pci_vf_reverse(), and
> for_each_pci_vf_continue_reverse() built on top of them. The helpers
> return each VF with a reference held, matching existing PCI device
> lookup APIs.
> 
> This provides a common and convenient way to iterate over a PF's enabled
> VFs and avoids duplicating the iteration logic in drivers.
> 
> Assisted-by: LLM
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=2

Re: [PATCH v1 06/13] pds_core: Lock VFs around pci_reset_function_locked()

[sashiko-bot]

> pci_reset_function_locked() now requires PF callers to hold the device
> locks for all enabled VFs as well. Lock VFs around the reset in
> pdsc_pci_error_resume().
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=6

Re: [PATCH v1 05/13] ionic: Lock VFs around pci_reset_function_locked()

[sashiko-bot]

> pci_reset_function_locked() now requires PF callers to hold the device
> locks for all enabled VFs as well. Lock VFs around the reset in
> ionic_pci_error_resume().
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=5

Re: [PATCH v1 03/13] PCI/IOV: Introduce VF locking helpers

[sashiko-bot]

> Add pci_dev_lock_vfs() and pci_dev_unlock_vfs() helpers to lock and
> unlock all enabled VFs of a PF device.
> 
> This provides a common way to serialize operations involving
> subordinate VFs.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=3

Re: [PATCH v1 08/13] PCI/IOV: Allow bus reset_method for PF with VFs enabled

[sashiko-bot]

> pci_parent_bus_reset() accepts a device for secondary bus reset only when
> no other devices are present on that bus.
> 
> While that makes sense in most cases, it is too strict for SR-IOV. When
> a PF has VFs enabled, those VFs are enumerated on the same bus. That
> causes this check to reject the PF, even though its VFs are in the same
> reset domain and should not block a PF bus reset.
> 
> Ignore PF-owned VFs in this check, so the bus reset method remains
> available for PFs with VFs enabled.
> 
> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=8

Re: [PATCH v1 04/13] PCI/IOV: Allow VF drivers to handle resets

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] PCI/IOV: bus and slot resets fail to lock VFs on virtfn bus or other slots
- [High] PCI/IOV: VF reset_prepare() before PF reset_prepare() deadlocks hisi_qm FLR
--

commit f7e0f3b0fa0caf7e168b1c2965897910aa159a0a
Author: Michał Winiarski <michal.winiarski@intel.com>

PCI/IOV: Allow VF drivers to handle resets

This commit saves and restores PCI configuration state for all subordinate
VFs across PF resets and notifies VF drivers via reset_prepare() and
reset_done(). It also updates bus and slot resets so that VFs are managed
through their parent PF rather than walked independently on the bus.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 95452c11dea55..ba8553011daea 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

[ ... ]

> @@ -5164,6 +5195,17 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
>  		return;
>  
>  	pci_save_state(dev);
> +}
> +
> +static void pci_dev_save_and_disable(struct pci_dev *dev)
> +{
> +	struct pci_dev *vf;
> +
> +	for_each_pci_vf(dev, vf)
> +		pci_dev_reset_prepare_and_save(vf);
> +
> +	pci_dev_reset_prepare_and_save(dev);

[Severity: High]
Does calling pci_dev_reset_prepare_and_save(vf) on every VF before calling
pci_dev_reset_prepare_and_save(dev) on the PF deadlock HiSilicon QM resets?

During pci_reset_function(pf), pci_dev_save_and_disable() now runs
reset_prepare() on all VFs and then on the PF before pci_dev_restore() calls
reset_done() on any of them.

In drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c,
hisi_acc_vf_pci_reset_prepare() sets QM_RESETTING on the parent PF's
qm->misc_ctl (hisi_acc_vdev->pf_qm) and only clears it in
hisi_acc_vf_pci_aer_reset_done():

    while (test_and_set_bit(QM_RESETTING, &qm->misc_ctl)) {
        msleep(1);
        if (++delay > QM_RESET_WAIT_TIMEOUT) {
            dev_err(dev, "reset prepare failed\n");
            return;
        }
    }

When the first VF sets QM_RESETTING on pf_qm->misc_ctl, any subsequent VF's
hisi_acc_vf_pci_reset_prepare() spins for 60 seconds and fails, and the PF's
hisi_qm_reset_prepare() -> qm_reset_prepare_ready(pf_qm) ->
qm_wait_reset_finish(pf_qm) in drivers/crypto/hisilicon/qm.c also spins on
pf_qm->misc_ctl for ~80 seconds before timing out with -EBUSY ("FLR not
ready!") without stopping the PF.

A similar self-deadlock occurs when VFs are bound to the host hisi_qm drivers
(hisi_hpre, hisi_sec2, hisi_zip) in drivers/crypto/hisilicon/qm.c:

    static int qm_reset_prepare_ready(struct hisi_qm *qm)
    {
        struct pci_dev *pdev = qm->pdev;
        struct hisi_qm *pf_qm = pci_get_drvdata(pci_physfn(pdev));

        if (qm->ver < QM_HW_V3)
            return qm_wait_reset_finish(pf_qm);

        return qm_wait_reset_finish(qm);
    }

On Kunpeng920 (qm->ver < QM_HW_V3), the first VF's hisi_qm_reset_prepare()
sets QM_RESETTING on pf_qm->misc_ctl, blocking subsequent VFs and the PF's
own hisi_qm_reset_prepare(). On Kunpeng930+ (qm->ver >= QM_HW_V3), each VF's
hisi_qm_reset_prepare() sets QM_RESETTING on vf_qm->misc_ctl; when the PF's
hisi_qm_reset_prepare() then calls qm_try_stop_vfs(qm, QM_PF_FLR_PREPARE,
QM_DOWN), each VF's mailbox handler qm_pf_reset_vf_prepare() calls
qm_reset_prepare_ready(vf_qm) -> qm_wait_reset_finish(vf_qm), which waits for
QM_RESETTING on vf_qm->misc_ctl to clear and times out.

[ ... ]

> @@ -5551,6 +5631,11 @@ static int pci_slot_trylock(struct pci_slot *slot)
>  	return __pci_bus_trylock(slot->bus, slot);
>  }
>  
> +static bool pci_dev_skip_bus_reset(struct pci_dev *dev)
> +{
> +	return dev->is_virtfn; /* VFs are managed by their PF */
> +}
> +
>  /*
>   * Save and disable devices from the top of the tree down while holding
>   * the @dev mutex lock for the entire tree.
> @@ -5560,6 +5645,8 @@ static void pci_bus_save_and_disable_locked(struct pci_bus *bus)
>  	struct pci_dev *dev;
>  
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		if (pci_dev_skip_bus_reset(dev))
> +			continue;
>  		pci_dev_save_and_disable(dev);
>  		if (dev->subordinate)
>  			pci_bus_save_and_disable_locked(dev->subordinate);
> @@ -5576,6 +5663,8 @@ static void pci_bus_restore_locked(struct pci_bus *bus)
>  	struct pci_dev *dev;
>  
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		if (pci_dev_skip_bus_reset(dev))
> +			continue;
>  		pci_dev_restore(dev);
>  		if (dev->subordinate) {
>  			pci_bridge_wait_for_secondary_bus(dev, "bus reset");
> @@ -5595,6 +5684,8 @@ static void pci_slot_save_and_disable_locked(struct pci_slot *slot)
>  	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
>  		if (!dev->slot || dev->slot != slot)
>  			continue;
> +		if (pci_dev_skip_bus_reset(dev))
> +			continue;
>  		pci_dev_save_and_disable(dev);
>  		if (dev->subordinate)
>  			pci_bus_save_and_disable_locked(dev->subordinate);
> @@ -5613,6 +5704,8 @@ static void pci_slot_restore_locked(struct pci_slot *slot)
>  	list_for_each_entry(dev, &slot->bus->devices, bus_list) {
>  		if (!dev->slot || dev->slot != slot)
>  			continue;
> +		if (pci_dev_skip_bus_reset(dev))
> +			continue;
>  		pci_dev_restore(dev);
>  		if (dev->subordinate) {
>  			pci_bridge_wait_for_secondary_bus(dev, "slot reset");

[Severity: High]
Can pci_bus_save_and_disable_locked(), pci_bus_restore_locked(),
pci_slot_save_and_disable_locked(), and pci_slot_restore_locked() operate on
unlocked VFs when VFs reside on an SR-IOV virtual bus or do not match the
PF's struct pci_slot?

When pci_dev_save_and_disable() and pci_dev_restore() run on a PF, they
iterate over all VFs with for_each_pci_vf(dev, vf) and call
pci_dev_reset_prepare_and_save(vf) and pci_dev_reset_done(vf), both of which
require the VF lock to be held and assert device_lock_assert(&vf->dev).

However, __pci_bus_lock(), __pci_bus_trylock(), and __pci_bus_unlock() in
drivers/pci/pci.c only walk bus->devices (filtered by dev->slot == slot for
slot resets) and recurse into dev->subordinate:

    list_for_each_entry(dev, &bus->devices, bus_list) {
        if (slot && (!dev->slot || dev->slot != slot))
            continue;
        if (dev->subordinate) {
            if (!pci_bus_trylock(dev->subordinate))
                goto unlock;
        } else if (!pci_dev_trylock(dev))
            goto unlock;
    }

First, if any VFs reside on an SR-IOV virtual bus created by virtfn_add_bus()
in drivers/pci/iov.c:

    child = pci_add_new_bus(bus, NULL, busnr);

that virtual bus has bridge == NULL, so pf->subordinate is NULL and those VFs
are on the virtual bus's devices list rather than pf->bus->devices. As a
result, __pci_bus_lock() and __pci_bus_trylock() never lock VFs on the
virtual bus.

Second, during a slot reset (pci_slot_trylock() / pci_slot_lock()),
pci_dev_matches_slot() in drivers/pci/slot.c only assigns dev->slot = slot
when the device matches the slot number:

    static bool pci_dev_matches_slot(struct pci_dev *dev, struct pci_slot *slot)
    {
        if (slot->per_func_slot)
            return dev->devfn == slot->number;

        return slot->number == PCI_SLOT_ALL_DEVICES ||
            PCI_SLOT(dev->devfn) == slot->number;
    }

Any VF with a different PCI_SLOT(vf->devfn) (when slot->number !=
PCI_SLOT_ALL_DEVICES) or a different devfn (when slot->per_func_slot is set)
has vf->slot != slot and is skipped by __pci_bus_trylock(slot->bus, slot),
while pci_slot_save_and_disable_locked() and pci_slot_restore_locked() still
visit all VFs of the PF via for_each_pci_vf(dev, vf) without holding their
device or config access locks.

Should __pci_bus_lock(), __pci_bus_trylock(), and __pci_bus_unlock() also
skip VFs with pci_dev_skip_bus_reset(dev) and lock/unlock VFs via their PF
using pci_dev_lock_vfs(), pci_dev_trylock_vfs(), and pci_dev_unlock_vfs()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019083514.838640-1-lukasz.laguna@intel.com?part=4