[PATCHv4 0/3] pci slot reset handling fixes

[PATCHv4 0/3] pci slot reset handling fixes

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

Changes from previous version:

 * Dropped the patch that unifies bus and slot reset device locking and
   config space restoration. There were good intentions for proposing
   it, but it's not necessary anymore for the pciehp case at hand and
   it introduces risks by conflating what is actually affected when you
   request a slot reset.

 * Fixed up some typos, added requested code comments

 * Fixed up the compilation linking mistake in patch 1

 * I changed a parameter name in the last patch for the common bridge
   reset method. Previously called "masked", but it's really the case
   that both methods want to mask hotplug events. The real difference
   between thw two options is whether we save+restore the affected
   devices. The error handling path doesn't want that because the error
   handler is responsible for it. But everyone else wants the reset
   handler to take care of it automatically.

 * Added reviews

Keith Busch (3):
  pci: rename __pci_bus_reset and __pci_slot_reset
  pci: allow all bus devices to use the same slot
  pci: make reset_subordinate hotplug safe

 drivers/pci/hotplug/pciehp_core.c |  3 +-
 drivers/pci/pci-sysfs.c           |  3 +-
 drivers/pci/pci.c                 | 94 ++++++++++++++++++++-----------
 drivers/pci/pci.h                 |  2 +-
 drivers/pci/slot.c                | 27 +++++++--
 include/linux/pci.h               |  8 ++-
 6 files changed, 96 insertions(+), 41 deletions(-)

-- 
2.47.3

[PATCHv4 1/3] pci: rename __pci_bus_reset and __pci_slot_reset

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

Make the code a little easier to navigate with more descriptive
function names. The two renamed functions here "try" to do to a reset,
so make that clear in the name to distinguish them from other similarly
named functions.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/pci/pci-sysfs.c |  2 +-
 drivers/pci/pci.c       | 10 +++++-----
 drivers/pci/pci.h       |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 363187ba4f56c..b44e884fd5372 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -563,7 +563,7 @@ static ssize_t reset_subordinate_store(struct device *dev,
 		return -EINVAL;
 
 	if (val) {
-		int ret = __pci_reset_bus(bus);
+		int ret = pci_try_reset_bus(bus);
 
 		if (ret)
 			return ret;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f3244630bfd05..5ab0b22dc7274 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5542,7 +5542,7 @@ int pci_probe_reset_slot(struct pci_slot *slot)
 EXPORT_SYMBOL_GPL(pci_probe_reset_slot);
 
 /**
- * __pci_reset_slot - Try to reset a PCI slot
+ * pci_try_reset_slot - Try to reset a PCI slot
  * @slot: PCI slot to reset
  *
  * A PCI bus may host multiple slots, each slot may support a reset mechanism
@@ -5556,7 +5556,7 @@ EXPORT_SYMBOL_GPL(pci_probe_reset_slot);
  *
  * Same as above except return -EAGAIN if the slot cannot be locked
  */
-static int __pci_reset_slot(struct pci_slot *slot)
+static int pci_try_reset_slot(struct pci_slot *slot)
 {
 	int rc;
 
@@ -5645,12 +5645,12 @@ int pci_probe_reset_bus(struct pci_bus *bus)
 EXPORT_SYMBOL_GPL(pci_probe_reset_bus);
 
 /**
- * __pci_reset_bus - Try to reset a PCI bus
+ * pci_try_reset_bus - Try to reset a PCI bus
  * @bus: top level PCI bus to reset
  *
  * Same as above except return -EAGAIN if the bus cannot be locked
  */
-int __pci_reset_bus(struct pci_bus *bus)
+int pci_try_reset_bus(struct pci_bus *bus)
 {
 	int rc;
 
@@ -5679,7 +5679,7 @@ int __pci_reset_bus(struct pci_bus *bus)
 int pci_reset_bus(struct pci_dev *pdev)
 {
 	return (!pci_probe_reset_slot(pdev->slot)) ?
-	    __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus);
+	    pci_try_reset_slot(pdev->slot) : pci_try_reset_bus(pdev->bus);
 }
 EXPORT_SYMBOL_GPL(pci_reset_bus);
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index c8a0522e2e1f5..d1350d54b932d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -231,7 +231,7 @@ bool pci_reset_supported(struct pci_dev *dev);
 void pci_init_reset_methods(struct pci_dev *dev);
 int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
 int pci_bus_error_reset(struct pci_dev *dev);
-int __pci_reset_bus(struct pci_bus *bus);
+int pci_try_reset_bus(struct pci_bus *bus);
 
 struct pci_cap_saved_data {
 	u16		cap_nr;
-- 
2.47.3

[PATCHv4 2/3] pci: allow all bus devices to use the same slot

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

A pcie hotplug slot applies to the entire subordinate bus. Thus, pciehp
only allocates a single hotplug_slot for the bridge to that bus. The pci
slot, though, would only match to functions on device 0, meaning all
device beyond that are not matched to any slot even though they share
it. A slot reset will break all the missing devices because the handling
skips them.

Allow a slot to be created to claim all devices on a bus, not just a
matching device. This is done by introducing a sentinel value, named
PCI_SLOT_ALL_DEVICES, which then has the pci slot match to any device on
the bus. This fixes slot resets for pciehp.

Since 0xff already has special meaning, the chosen value for this new
feature is 0xfe. This will not clash with any actual slot number since
they are limited to 5 bits.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/pci/hotplug/pciehp_core.c |  3 ++-
 drivers/pci/slot.c                | 27 +++++++++++++++++++++++----
 include/linux/pci.h               |  8 +++++++-
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index f59baa9129709..d80346d567049 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -79,7 +79,8 @@ static int init_slot(struct controller *ctrl)
 	snprintf(name, SLOT_NAME_SIZE, "%u", PSN(ctrl));
 
 	retval = pci_hp_initialize(&ctrl->hotplug_slot,
-				   ctrl->pcie->port->subordinate, 0, name);
+				   ctrl->pcie->port->subordinate,
+				   PCI_SLOT_ALL_DEVICES, name);
 	if (retval) {
 		ctrl_err(ctrl, "pci_hp_initialize failed: error %d\n", retval);
 		kfree(ops);
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index 50fb3eb595fe6..bf6f265454a84 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -42,6 +42,15 @@ static ssize_t address_read_file(struct pci_slot *slot, char *buf)
 				  pci_domain_nr(slot->bus),
 				  slot->bus->number);
 
+	/*
+	 * Preserve legacy ABI expectations that hotplug drivers that manage
+	 * multiple devices per slot emit 0 for the device number.
+	 */
+	if (slot->number == PCI_SLOT_ALL_DEVICES)
+		return sysfs_emit(buf, "%04x:%02x:00\n",
+				  pci_domain_nr(slot->bus),
+				  slot->bus->number);
+
 	return sysfs_emit(buf, "%04x:%02x:%02x\n",
 			  pci_domain_nr(slot->bus),
 			  slot->bus->number,
@@ -73,7 +82,8 @@ static void pci_slot_release(struct kobject *kobj)
 
 	down_read(&pci_bus_sem);
 	list_for_each_entry(dev, &slot->bus->devices, bus_list)
-		if (PCI_SLOT(dev->devfn) == slot->number)
+		if (slot->number == PCI_SLOT_ALL_DEVICES ||
+		    PCI_SLOT(dev->devfn) == slot->number)
 			dev->slot = NULL;
 	up_read(&pci_bus_sem);
 
@@ -166,7 +176,8 @@ void pci_dev_assign_slot(struct pci_dev *dev)
 
 	mutex_lock(&pci_slot_mutex);
 	list_for_each_entry(slot, &dev->bus->slots, list)
-		if (PCI_SLOT(dev->devfn) == slot->number)
+		if (slot->number == PCI_SLOT_ALL_DEVICES ||
+		    PCI_SLOT(dev->devfn) == slot->number)
 			dev->slot = slot;
 	mutex_unlock(&pci_slot_mutex);
 }
@@ -188,7 +199,7 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr)
 /**
  * pci_create_slot - create or increment refcount for physical PCI slot
  * @parent: struct pci_bus of parent bridge
- * @slot_nr: PCI_SLOT(pci_dev->devfn) or -1 for placeholder
+ * @slot_nr: PCI_SLOT(pci_dev->devfn), -1 for placeholder, or PCI_SLOT_ALL_DEVICES
  * @name: user visible string presented in /sys/bus/pci/slots/<name>
  * @hotplug: set if caller is hotplug driver, NULL otherwise
  *
@@ -222,6 +233,13 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr)
  * consist solely of a dddd:bb tuple, where dddd is the PCI domain of the
  * %struct pci_bus and bb is the bus number. In other words, the devfn of
  * the 'placeholder' slot will not be displayed.
+ *
+ * Bus-wide slots:
+ * For PCIe hotplug, the physical slot encompasses the entire subordinate
+ * bus, not just a single device number. Pass @slot_nr == PCI_SLOT_ALL_DEVICES
+ * to create a slot that matches all devices on the bus. Unlike placeholder
+ * slots, bus-wide slots go through normal slot lookup and reuse existing
+ * slots if present.
  */
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 				 const char *name,
@@ -285,7 +303,8 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 
 	down_read(&pci_bus_sem);
 	list_for_each_entry(dev, &parent->devices, bus_list)
-		if (PCI_SLOT(dev->devfn) == slot_nr)
+		if (slot_nr == PCI_SLOT_ALL_DEVICES ||
+		    PCI_SLOT(dev->devfn) == slot_nr)
 			dev->slot = slot;
 	up_read(&pci_bus_sem);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index edf792a79193f..7073519bcc1ad 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -72,12 +72,18 @@
 /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */
 #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff)
 
+/*
+ * PCI_SLOT_ALL_DEVICES indicates a slot that covers all devices on the bus.
+ * Used for PCIe hotplug where the physical slot is the entire subordinate bus.
+ */
+#define PCI_SLOT_ALL_DEVICES	0xfe
+
 /* pci_slot represents a physical slot */
 struct pci_slot {
 	struct pci_bus		*bus;		/* Bus this slot is on */
 	struct list_head	list;		/* Node in list of slots */
 	struct hotplug_slot	*hotplug;	/* Hotplug info (move here) */
-	unsigned char		number;		/* PCI_SLOT(pci_dev->devfn) */
+	unsigned char		number;		/* Device nr, or PCI_SLOT_ALL_DEVICES */
 	struct kobject		kobj;
 };
 
-- 
2.47.3

[PATCHv4 3/3] pci: make reset_subordinate hotplug safe

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

Use the slot reset method when resetting the bridge if the bus contains
hot plug slots. This fixes spurious hot plug events that are triggered
by the secondary bus reset that bypasses the slot's detection disabling.

Resetting a bridge's subordinate bus can be done like this:

  # echo 1 > /sys/bus/pci/devices/0000:50:01.0/reset_subordinate

Prior to this patch, an example kernel message may show something like:

  pcieport 0000:50:01.0: pciehp: Slot(40): Link Down

With this change, the pciehp driver ignores the link event during the
reset, so may show this message instead:

  pcieport 0000:50:01.0: pciehp: Slot(40): Link Down/Up ignored

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/pci/pci-sysfs.c |  3 +-
 drivers/pci/pci.c       | 86 +++++++++++++++++++++++++++--------------
 drivers/pci/pci.h       |  2 +-
 3 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index b44e884fd5372..6187b0f3e2833 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -553,7 +553,6 @@ static ssize_t reset_subordinate_store(struct device *dev,
 				const char *buf, size_t count)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	struct pci_bus *bus = pdev->subordinate;
 	unsigned long val;
 
 	if (!capable(CAP_SYS_ADMIN))
@@ -563,7 +562,7 @@ static ssize_t reset_subordinate_store(struct device *dev,
 		return -EINVAL;
 
 	if (val) {
-		int ret = pci_try_reset_bus(bus);
+		int ret = pci_try_reset_bridge(pdev);
 
 		if (ret)
 			return ret;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5ab0b22dc7274..c535cd7f45013 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -54,6 +54,10 @@ unsigned int pci_pm_d3hot_delay;
 
 static void pci_pme_list_scan(struct work_struct *work);
 
+#define PCI_RESET_RESTORE true
+#define PCI_RESET_NO_RESTORE false
+static int pci_reset_bridge(struct pci_dev *bridge, bool restore);
+
 static LIST_HEAD(pci_pme_list);
 static DEFINE_MUTEX(pci_pme_list_mutex);
 static DECLARE_DELAYED_WORK(pci_pme_work, pci_pme_list_scan);
@@ -5600,36 +5604,10 @@ static int pci_bus_reset(struct pci_bus *bus, bool probe)
 /**
  * pci_bus_error_reset - reset the bridge's subordinate bus
  * @bridge: The parent device that connects to the bus to reset
- *
- * This function will first try to reset the slots on this bus if the method is
- * available. If slot reset fails or is not available, this will fall back to a
- * secondary bus reset.
  */
 int pci_bus_error_reset(struct pci_dev *bridge)
 {
-	struct pci_bus *bus = bridge->subordinate;
-	struct pci_slot *slot;
-
-	if (!bus)
-		return -ENOTTY;
-
-	mutex_lock(&pci_slot_mutex);
-	if (list_empty(&bus->slots))
-		goto bus_reset;
-
-	list_for_each_entry(slot, &bus->slots, list)
-		if (pci_probe_reset_slot(slot))
-			goto bus_reset;
-
-	list_for_each_entry(slot, &bus->slots, list)
-		if (pci_slot_reset(slot, PCI_RESET_DO_RESET))
-			goto bus_reset;
-
-	mutex_unlock(&pci_slot_mutex);
-	return 0;
-bus_reset:
-	mutex_unlock(&pci_slot_mutex);
-	return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
+	return pci_reset_bridge(bridge, PCI_RESET_NO_RESTORE);
 }
 
 /**
@@ -5650,7 +5628,7 @@ EXPORT_SYMBOL_GPL(pci_probe_reset_bus);
  *
  * Same as above except return -EAGAIN if the bus cannot be locked
  */
-int pci_try_reset_bus(struct pci_bus *bus)
+static int pci_try_reset_bus(struct pci_bus *bus)
 {
 	int rc;
 
@@ -5670,6 +5648,58 @@ int pci_try_reset_bus(struct pci_bus *bus)
 	return rc;
 }
 
+/**
+ * pci_reset_bridge - reset a bridge's subordinate bus
+ * @bridge: bridge that connects to the bus to reset
+ * @restore: true if affected device states need to be restored after the reset
+ *
+ * This function will first try to reset the slots on this bus if the method is
+ * available. If slot reset fails or is not available, this will fall back to a
+ * secondary bus reset.
+ */
+static int pci_reset_bridge(struct pci_dev *bridge, bool restore)
+{
+	struct pci_bus *bus = bridge->subordinate;
+	struct pci_slot *slot;
+
+	if (!bus)
+		return -ENOTTY;
+
+	mutex_lock(&pci_slot_mutex);
+	if (list_empty(&bus->slots))
+		goto bus_reset;
+
+	list_for_each_entry(slot, &bus->slots, list)
+		if (pci_probe_reset_slot(slot))
+			goto bus_reset;
+
+	list_for_each_entry(slot, &bus->slots, list) {
+		int ret;
+
+		if (restore)
+			ret = pci_try_reset_slot(slot);
+		else
+			ret = pci_slot_reset(slot, PCI_RESET_DO_RESET);
+
+		if (ret)
+			goto bus_reset;
+	}
+
+	mutex_unlock(&pci_slot_mutex);
+	return 0;
+bus_reset:
+	mutex_unlock(&pci_slot_mutex);
+
+	if (restore)
+		return pci_try_reset_bus(bus);
+	return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
+}
+
+int pci_try_reset_bridge(struct pci_dev *bridge)
+{
+	return pci_reset_bridge(bridge, PCI_RESET_RESTORE);
+}
+
 /**
  * pci_reset_bus - Try to reset a PCI bus
  * @pdev: top level PCI device to reset via slot/bus
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d1350d54b932d..9e363ad22e161 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -231,7 +231,7 @@ bool pci_reset_supported(struct pci_dev *dev);
 void pci_init_reset_methods(struct pci_dev *dev);
 int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
 int pci_bus_error_reset(struct pci_dev *dev);
-int pci_try_reset_bus(struct pci_bus *bus);
+int pci_try_reset_bridge(struct pci_dev *bridge);
 
 struct pci_cap_saved_data {
 	u16		cap_nr;
-- 
2.47.3

Re: [PATCHv4 3/3] pci: make reset_subordinate hotplug safe

[dan.j.williams]

Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Use the slot reset method when resetting the bridge if the bus contains
> hot plug slots. This fixes spurious hot plug events that are triggered
> by the secondary bus reset that bypasses the slot's detection disabling.

Did you want to copy that useful error report with the GHES details etc
from http://lore.kernel.org/20260205212533.1512153-4-kbusch@meta.com? I
thought it was useful to clarify this can fix panics.

>   # echo 1 > /sys/bus/pci/devices/0000:50:01.0/reset_subordinate
> 
> Prior to this patch, an example kernel message may show something like:
> 
>   pcieport 0000:50:01.0: pciehp: Slot(40): Link Down
> 
> With this change, the pciehp driver ignores the link event during the
> reset, so may show this message instead:
> 
>   pcieport 0000:50:01.0: pciehp: Slot(40): Link Down/Up ignored
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
[..]
> @@ -5670,6 +5648,58 @@ int pci_try_reset_bus(struct pci_bus *bus)
>  	return rc;
>  }
>  
> +/**
> + * pci_reset_bridge - reset a bridge's subordinate bus
> + * @bridge: bridge that connects to the bus to reset
> + * @restore: true if affected device states need to be restored after the reset

Maybe be more explicit and say:

@restore: when true use a reset method that invokes pci_dev_restore() post reset

...because pci_dev_restore() does a little bit more than just state
restoration.

With or without those minor fixups:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot

[Ilpo Järvinen]

On Thu, 12 Feb 2026, Keith Busch wrote:

> From: Keith Busch <kbusch@kernel.org>
> 
> A pcie hotplug slot applies to the entire subordinate bus. Thus, pciehp
> only allocates a single hotplug_slot for the bridge to that bus. The pci
> slot, though, would only match to functions on device 0, meaning all
> device beyond that are not matched to any slot even though they share
> it. A slot reset will break all the missing devices because the handling
> skips them.
> 
> Allow a slot to be created to claim all devices on a bus, not just a
> matching device. This is done by introducing a sentinel value, named
> PCI_SLOT_ALL_DEVICES, which then has the pci slot match to any device on
> the bus. This fixes slot resets for pciehp.
> 
> Since 0xff already has special meaning, the chosen value for this new
> feature is 0xfe. This will not clash with any actual slot number since
> they are limited to 5 bits.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/pci/hotplug/pciehp_core.c |  3 ++-
>  drivers/pci/slot.c                | 27 +++++++++++++++++++++++----
>  include/linux/pci.h               |  8 +++++++-
>  3 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
> index f59baa9129709..d80346d567049 100644
> --- a/drivers/pci/hotplug/pciehp_core.c
> +++ b/drivers/pci/hotplug/pciehp_core.c
> @@ -79,7 +79,8 @@ static int init_slot(struct controller *ctrl)
>  	snprintf(name, SLOT_NAME_SIZE, "%u", PSN(ctrl));
>  
>  	retval = pci_hp_initialize(&ctrl->hotplug_slot,
> -				   ctrl->pcie->port->subordinate, 0, name);
> +				   ctrl->pcie->port->subordinate,
> +				   PCI_SLOT_ALL_DEVICES, name);
>  	if (retval) {
>  		ctrl_err(ctrl, "pci_hp_initialize failed: error %d\n", retval);
>  		kfree(ops);
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 50fb3eb595fe6..bf6f265454a84 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -42,6 +42,15 @@ static ssize_t address_read_file(struct pci_slot *slot, char *buf)
>  				  pci_domain_nr(slot->bus),
>  				  slot->bus->number);
>  
> +	/*
> +	 * Preserve legacy ABI expectations that hotplug drivers that manage
> +	 * multiple devices per slot emit 0 for the device number.
> +	 */
> +	if (slot->number == PCI_SLOT_ALL_DEVICES)
> +		return sysfs_emit(buf, "%04x:%02x:00\n",
> +				  pci_domain_nr(slot->bus),
> +				  slot->bus->number);

This fits to two lines.

Add braces as it's a multi-line block.

-- 
 i.

> +
>  	return sysfs_emit(buf, "%04x:%02x:%02x\n",
>  			  pci_domain_nr(slot->bus),
>  			  slot->bus->number,
> @@ -73,7 +82,8 @@ static void pci_slot_release(struct kobject *kobj)
>  
>  	down_read(&pci_bus_sem);
>  	list_for_each_entry(dev, &slot->bus->devices, bus_list)
> -		if (PCI_SLOT(dev->devfn) == slot->number)
> +		if (slot->number == PCI_SLOT_ALL_DEVICES ||
> +		    PCI_SLOT(dev->devfn) == slot->number)
>  			dev->slot = NULL;
>  	up_read(&pci_bus_sem);
>  
> @@ -166,7 +176,8 @@ void pci_dev_assign_slot(struct pci_dev *dev)
>  
>  	mutex_lock(&pci_slot_mutex);
>  	list_for_each_entry(slot, &dev->bus->slots, list)
> -		if (PCI_SLOT(dev->devfn) == slot->number)
> +		if (slot->number == PCI_SLOT_ALL_DEVICES ||
> +		    PCI_SLOT(dev->devfn) == slot->number)
>  			dev->slot = slot;
>  	mutex_unlock(&pci_slot_mutex);
>  }
> @@ -188,7 +199,7 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr)
>  /**
>   * pci_create_slot - create or increment refcount for physical PCI slot
>   * @parent: struct pci_bus of parent bridge
> - * @slot_nr: PCI_SLOT(pci_dev->devfn) or -1 for placeholder
> + * @slot_nr: PCI_SLOT(pci_dev->devfn), -1 for placeholder, or PCI_SLOT_ALL_DEVICES
>   * @name: user visible string presented in /sys/bus/pci/slots/<name>
>   * @hotplug: set if caller is hotplug driver, NULL otherwise
>   *
> @@ -222,6 +233,13 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr)
>   * consist solely of a dddd:bb tuple, where dddd is the PCI domain of the
>   * %struct pci_bus and bb is the bus number. In other words, the devfn of
>   * the 'placeholder' slot will not be displayed.
> + *
> + * Bus-wide slots:
> + * For PCIe hotplug, the physical slot encompasses the entire subordinate
> + * bus, not just a single device number. Pass @slot_nr == PCI_SLOT_ALL_DEVICES
> + * to create a slot that matches all devices on the bus. Unlike placeholder
> + * slots, bus-wide slots go through normal slot lookup and reuse existing
> + * slots if present.
>   */
>  struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  				 const char *name,
> @@ -285,7 +303,8 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  
>  	down_read(&pci_bus_sem);
>  	list_for_each_entry(dev, &parent->devices, bus_list)
> -		if (PCI_SLOT(dev->devfn) == slot_nr)
> +		if (slot_nr == PCI_SLOT_ALL_DEVICES ||
> +		    PCI_SLOT(dev->devfn) == slot_nr)
>  			dev->slot = slot;
>  	up_read(&pci_bus_sem);
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index edf792a79193f..7073519bcc1ad 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -72,12 +72,18 @@
>  /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */
>  #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff)
>  
> +/*
> + * PCI_SLOT_ALL_DEVICES indicates a slot that covers all devices on the bus.
> + * Used for PCIe hotplug where the physical slot is the entire subordinate bus.
> + */
> +#define PCI_SLOT_ALL_DEVICES	0xfe
> +
>  /* pci_slot represents a physical slot */
>  struct pci_slot {
>  	struct pci_bus		*bus;		/* Bus this slot is on */
>  	struct list_head	list;		/* Node in list of slots */
>  	struct hotplug_slot	*hotplug;	/* Hotplug info (move here) */
> -	unsigned char		number;		/* PCI_SLOT(pci_dev->devfn) */
> +	unsigned char		number;		/* Device nr, or PCI_SLOT_ALL_DEVICES */
>  	struct kobject		kobj;
>  };
>  
> 

Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot

[Ilpo Järvinen]

On Thu, 12 Feb 2026, Keith Busch wrote:

> From: Keith Busch <kbusch@kernel.org>
> 

Capitalize pci: in the subject.

-- 
 i.

> A pcie hotplug slot applies to the entire subordinate bus. Thus, pciehp
> only allocates a single hotplug_slot for the bridge to that bus. The pci
> slot, though, would only match to functions on device 0, meaning all
> device beyond that are not matched to any slot even though they share
> it. A slot reset will break all the missing devices because the handling
> skips them.
> 
> Allow a slot to be created to claim all devices on a bus, not just a
> matching device. This is done by introducing a sentinel value, named
> PCI_SLOT_ALL_DEVICES, which then has the pci slot match to any device on
> the bus. This fixes slot resets for pciehp.
> 
> Since 0xff already has special meaning, the chosen value for this new
> feature is 0xfe. This will not clash with any actual slot number since
> they are limited to 5 bits.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/pci/hotplug/pciehp_core.c |  3 ++-
>  drivers/pci/slot.c                | 27 +++++++++++++++++++++++----
>  include/linux/pci.h               |  8 +++++++-
>  3 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
> index f59baa9129709..d80346d567049 100644
> --- a/drivers/pci/hotplug/pciehp_core.c
> +++ b/drivers/pci/hotplug/pciehp_core.c
> @@ -79,7 +79,8 @@ static int init_slot(struct controller *ctrl)
>  	snprintf(name, SLOT_NAME_SIZE, "%u", PSN(ctrl));
>  
>  	retval = pci_hp_initialize(&ctrl->hotplug_slot,
> -				   ctrl->pcie->port->subordinate, 0, name);
> +				   ctrl->pcie->port->subordinate,
> +				   PCI_SLOT_ALL_DEVICES, name);
>  	if (retval) {
>  		ctrl_err(ctrl, "pci_hp_initialize failed: error %d\n", retval);
>  		kfree(ops);
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 50fb3eb595fe6..bf6f265454a84 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -42,6 +42,15 @@ static ssize_t address_read_file(struct pci_slot *slot, char *buf)
>  				  pci_domain_nr(slot->bus),
>  				  slot->bus->number);
>  
> +	/*
> +	 * Preserve legacy ABI expectations that hotplug drivers that manage
> +	 * multiple devices per slot emit 0 for the device number.
> +	 */
> +	if (slot->number == PCI_SLOT_ALL_DEVICES)
> +		return sysfs_emit(buf, "%04x:%02x:00\n",
> +				  pci_domain_nr(slot->bus),
> +				  slot->bus->number);
> +
>  	return sysfs_emit(buf, "%04x:%02x:%02x\n",
>  			  pci_domain_nr(slot->bus),
>  			  slot->bus->number,
> @@ -73,7 +82,8 @@ static void pci_slot_release(struct kobject *kobj)
>  
>  	down_read(&pci_bus_sem);
>  	list_for_each_entry(dev, &slot->bus->devices, bus_list)
> -		if (PCI_SLOT(dev->devfn) == slot->number)
> +		if (slot->number == PCI_SLOT_ALL_DEVICES ||
> +		    PCI_SLOT(dev->devfn) == slot->number)
>  			dev->slot = NULL;
>  	up_read(&pci_bus_sem);
>  
> @@ -166,7 +176,8 @@ void pci_dev_assign_slot(struct pci_dev *dev)
>  
>  	mutex_lock(&pci_slot_mutex);
>  	list_for_each_entry(slot, &dev->bus->slots, list)
> -		if (PCI_SLOT(dev->devfn) == slot->number)
> +		if (slot->number == PCI_SLOT_ALL_DEVICES ||
> +		    PCI_SLOT(dev->devfn) == slot->number)
>  			dev->slot = slot;
>  	mutex_unlock(&pci_slot_mutex);
>  }
> @@ -188,7 +199,7 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr)
>  /**
>   * pci_create_slot - create or increment refcount for physical PCI slot
>   * @parent: struct pci_bus of parent bridge
> - * @slot_nr: PCI_SLOT(pci_dev->devfn) or -1 for placeholder
> + * @slot_nr: PCI_SLOT(pci_dev->devfn), -1 for placeholder, or PCI_SLOT_ALL_DEVICES
>   * @name: user visible string presented in /sys/bus/pci/slots/<name>
>   * @hotplug: set if caller is hotplug driver, NULL otherwise
>   *
> @@ -222,6 +233,13 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr)
>   * consist solely of a dddd:bb tuple, where dddd is the PCI domain of the
>   * %struct pci_bus and bb is the bus number. In other words, the devfn of
>   * the 'placeholder' slot will not be displayed.
> + *
> + * Bus-wide slots:
> + * For PCIe hotplug, the physical slot encompasses the entire subordinate
> + * bus, not just a single device number. Pass @slot_nr == PCI_SLOT_ALL_DEVICES
> + * to create a slot that matches all devices on the bus. Unlike placeholder
> + * slots, bus-wide slots go through normal slot lookup and reuse existing
> + * slots if present.
>   */
>  struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  				 const char *name,
> @@ -285,7 +303,8 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>  
>  	down_read(&pci_bus_sem);
>  	list_for_each_entry(dev, &parent->devices, bus_list)
> -		if (PCI_SLOT(dev->devfn) == slot_nr)
> +		if (slot_nr == PCI_SLOT_ALL_DEVICES ||
> +		    PCI_SLOT(dev->devfn) == slot_nr)
>  			dev->slot = slot;
>  	up_read(&pci_bus_sem);
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index edf792a79193f..7073519bcc1ad 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -72,12 +72,18 @@
>  /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */
>  #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff)
>  
> +/*
> + * PCI_SLOT_ALL_DEVICES indicates a slot that covers all devices on the bus.
> + * Used for PCIe hotplug where the physical slot is the entire subordinate bus.
> + */
> +#define PCI_SLOT_ALL_DEVICES	0xfe
> +
>  /* pci_slot represents a physical slot */
>  struct pci_slot {
>  	struct pci_bus		*bus;		/* Bus this slot is on */
>  	struct list_head	list;		/* Node in list of slots */
>  	struct hotplug_slot	*hotplug;	/* Hotplug info (move here) */
> -	unsigned char		number;		/* PCI_SLOT(pci_dev->devfn) */
> +	unsigned char		number;		/* Device nr, or PCI_SLOT_ALL_DEVICES */
>  	struct kobject		kobj;
>  };
>  
> 

Re: [PATCHv4 3/3] pci: make reset_subordinate hotplug safe

[Ilpo Järvinen]

On Thu, 12 Feb 2026, Keith Busch wrote:

> From: Keith Busch <kbusch@kernel.org>
> 
> Use the slot reset method when resetting the bridge if the bus contains
> hot plug slots. This fixes spurious hot plug events that are triggered
> by the secondary bus reset that bypasses the slot's detection disabling.
> 
> Resetting a bridge's subordinate bus can be done like this:
> 
>   # echo 1 > /sys/bus/pci/devices/0000:50:01.0/reset_subordinate
> 
> Prior to this patch, an example kernel message may show something like:
> 
>   pcieport 0000:50:01.0: pciehp: Slot(40): Link Down
> 
> With this change, the pciehp driver ignores the link event during the
> reset, so may show this message instead:
> 
>   pcieport 0000:50:01.0: pciehp: Slot(40): Link Down/Up ignored
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/pci/pci-sysfs.c |  3 +-
>  drivers/pci/pci.c       | 86 +++++++++++++++++++++++++++--------------
>  drivers/pci/pci.h       |  2 +-
>  3 files changed, 60 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index b44e884fd5372..6187b0f3e2833 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -553,7 +553,6 @@ static ssize_t reset_subordinate_store(struct device *dev,
>  				const char *buf, size_t count)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> -	struct pci_bus *bus = pdev->subordinate;
>  	unsigned long val;
>  
>  	if (!capable(CAP_SYS_ADMIN))
> @@ -563,7 +562,7 @@ static ssize_t reset_subordinate_store(struct device *dev,
>  		return -EINVAL;
>  
>  	if (val) {
> -		int ret = pci_try_reset_bus(bus);
> +		int ret = pci_try_reset_bridge(pdev);
>  
>  		if (ret)
>  			return ret;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 5ab0b22dc7274..c535cd7f45013 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -54,6 +54,10 @@ unsigned int pci_pm_d3hot_delay;
>  
>  static void pci_pme_list_scan(struct work_struct *work);
>  
> +#define PCI_RESET_RESTORE true
> +#define PCI_RESET_NO_RESTORE false
> +static int pci_reset_bridge(struct pci_dev *bridge, bool restore);

As this forward declaration is unnecessary after the suggestion below, 
there should be comment before defines to indicate they relate to 
pci_reset_bridge()'s restore parameter.

> +
>  static LIST_HEAD(pci_pme_list);
>  static DEFINE_MUTEX(pci_pme_list_mutex);
>  static DECLARE_DELAYED_WORK(pci_pme_work, pci_pme_list_scan);
> @@ -5600,36 +5604,10 @@ static int pci_bus_reset(struct pci_bus *bus, bool probe)
>  /**
>   * pci_bus_error_reset - reset the bridge's subordinate bus
>   * @bridge: The parent device that connects to the bus to reset
> - *
> - * This function will first try to reset the slots on this bus if the method is
> - * available. If slot reset fails or is not available, this will fall back to a
> - * secondary bus reset.
>   */
>  int pci_bus_error_reset(struct pci_dev *bridge)
>  {
> -	struct pci_bus *bus = bridge->subordinate;
> -	struct pci_slot *slot;
> -
> -	if (!bus)
> -		return -ENOTTY;
> -
> -	mutex_lock(&pci_slot_mutex);
> -	if (list_empty(&bus->slots))
> -		goto bus_reset;
> -
> -	list_for_each_entry(slot, &bus->slots, list)
> -		if (pci_probe_reset_slot(slot))
> -			goto bus_reset;
> -
> -	list_for_each_entry(slot, &bus->slots, list)
> -		if (pci_slot_reset(slot, PCI_RESET_DO_RESET))
> -			goto bus_reset;
> -
> -	mutex_unlock(&pci_slot_mutex);
> -	return 0;
> -bus_reset:
> -	mutex_unlock(&pci_slot_mutex);
> -	return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
> +	return pci_reset_bridge(bridge, PCI_RESET_NO_RESTORE);
>  }

I think this should be moved down below pci_reset_bridge() and the forward 
declaration of pci_reset_bridge() removed.
  
>  /**
> @@ -5650,7 +5628,7 @@ EXPORT_SYMBOL_GPL(pci_probe_reset_bus);
>   *
>   * Same as above except return -EAGAIN if the bus cannot be locked
>   */
> -int pci_try_reset_bus(struct pci_bus *bus)
> +static int pci_try_reset_bus(struct pci_bus *bus)
>  {
>  	int rc;
>  
> @@ -5670,6 +5648,58 @@ int pci_try_reset_bus(struct pci_bus *bus)
>  	return rc;
>  }
>  
> +/**
> + * pci_reset_bridge - reset a bridge's subordinate bus
> + * @bridge: bridge that connects to the bus to reset
> + * @restore: true if affected device states need to be restored after the reset
> + *
> + * This function will first try to reset the slots on this bus if the method is
> + * available. If slot reset fails or is not available, this will fall back to a
> + * secondary bus reset.
> + */
> +static int pci_reset_bridge(struct pci_dev *bridge, bool restore)
> +{
> +	struct pci_bus *bus = bridge->subordinate;
> +	struct pci_slot *slot;
> +
> +	if (!bus)
> +		return -ENOTTY;
> +
> +	mutex_lock(&pci_slot_mutex);
> +	if (list_empty(&bus->slots))
> +		goto bus_reset;
> +
> +	list_for_each_entry(slot, &bus->slots, list)
> +		if (pci_probe_reset_slot(slot))
> +			goto bus_reset;
> +
> +	list_for_each_entry(slot, &bus->slots, list) {
> +		int ret;
> +
> +		if (restore)
> +			ret = pci_try_reset_slot(slot);
> +		else
> +			ret = pci_slot_reset(slot, PCI_RESET_DO_RESET);
> +
> +		if (ret)
> +			goto bus_reset;
> +	}
> +
> +	mutex_unlock(&pci_slot_mutex);
> +	return 0;
> +bus_reset:
> +	mutex_unlock(&pci_slot_mutex);
> +
> +	if (restore)
> +		return pci_try_reset_bus(bus);
> +	return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
> +}
> +
> +int pci_try_reset_bridge(struct pci_dev *bridge)
> +{
> +	return pci_reset_bridge(bridge, PCI_RESET_RESTORE);
> +}
> +
>  /**
>   * pci_reset_bus - Try to reset a PCI bus
>   * @pdev: top level PCI device to reset via slot/bus
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index d1350d54b932d..9e363ad22e161 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -231,7 +231,7 @@ bool pci_reset_supported(struct pci_dev *dev);
>  void pci_init_reset_methods(struct pci_dev *dev);
>  int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
>  int pci_bus_error_reset(struct pci_dev *dev);
> -int pci_try_reset_bus(struct pci_bus *bus);
> +int pci_try_reset_bridge(struct pci_dev *bridge);
>  
>  struct pci_cap_saved_data {
>  	u16		cap_nr;
> 

-- 
 i.

Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot

[Keith Busch]

On Fri, Feb 13, 2026 at 09:26:35AM +0200, Ilpo Järvinen wrote:
> On Thu, 12 Feb 2026, Keith Busch wrote:
> > @@ -42,6 +42,15 @@ static ssize_t address_read_file(struct pci_slot *slot, char *buf)
> >  				  pci_domain_nr(slot->bus),
> >  				  slot->bus->number);
> >  
> > +	/*
> > +	 * Preserve legacy ABI expectations that hotplug drivers that manage
> > +	 * multiple devices per slot emit 0 for the device number.
> > +	 */
> > +	if (slot->number == PCI_SLOT_ALL_DEVICES)
> > +		return sysfs_emit(buf, "%04x:%02x:00\n",
> > +				  pci_domain_nr(slot->bus),
> > +				  slot->bus->number);
> 
> This fits to two lines.
> 
> Add braces as it's a multi-line block.

These suggestions clash with the local contentions of this function.
These lines are exact copy/paste from directly above it, with a tiny
modification for the legacy behavior. Not that I'm advocating for this
particular coding style, but consistency is preferred.

Re: [PATCHv4 3/3] pci: make reset_subordinate hotplug safe

[Keith Busch]

On Fri, Feb 13, 2026 at 09:32:32AM +0200, Ilpo Järvinen wrote:
> On Thu, 12 Feb 2026, Keith Busch wrote:
> >  int pci_bus_error_reset(struct pci_dev *bridge)
> >  {
> > -	struct pci_bus *bus = bridge->subordinate;
> > -	struct pci_slot *slot;
> > -
> > -	if (!bus)
> > -		return -ENOTTY;
> > -
> > -	mutex_lock(&pci_slot_mutex);
> > -	if (list_empty(&bus->slots))
> > -		goto bus_reset;
> > -
> > -	list_for_each_entry(slot, &bus->slots, list)
> > -		if (pci_probe_reset_slot(slot))
> > -			goto bus_reset;
> > -
> > -	list_for_each_entry(slot, &bus->slots, list)
> > -		if (pci_slot_reset(slot, PCI_RESET_DO_RESET))
> > -			goto bus_reset;
> > -
> > -	mutex_unlock(&pci_slot_mutex);
> > -	return 0;
> > -bus_reset:
> > -	mutex_unlock(&pci_slot_mutex);
> > -	return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
> > +	return pci_reset_bridge(bridge, PCI_RESET_NO_RESTORE);
> >  }
> 
> I think this should be moved down below pci_reset_bridge() and the forward 
> declaration of pci_reset_bridge() removed.

Yes, thanks for the suggestion. A cleaner end result for sure.