* [PATCHv4 0/3] pci slot reset handling fixes
@ 2026-02-12 22:41 Keith Busch
2026-02-12 22:41 ` [PATCHv4 1/3] pci: rename __pci_bus_reset and __pci_slot_reset Keith Busch
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Keith Busch @ 2026-02-12 22:41 UTC (permalink / raw)
To: linux-pci, helgaas, alex, dan.j.williams; +Cc: lukas, 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
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCHv4 1/3] pci: rename __pci_bus_reset and __pci_slot_reset 2026-02-12 22:41 [PATCHv4 0/3] pci slot reset handling fixes Keith Busch @ 2026-02-12 22:41 ` Keith Busch 2026-02-12 22:41 ` [PATCHv4 2/3] pci: allow all bus devices to use the same slot Keith Busch 2026-02-12 22:41 ` [PATCHv4 3/3] pci: make reset_subordinate hotplug safe Keith Busch 2 siblings, 0 replies; 10+ messages in thread From: Keith Busch @ 2026-02-12 22:41 UTC (permalink / raw) To: linux-pci, helgaas, alex, dan.j.williams; +Cc: lukas, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHv4 2/3] pci: allow all bus devices to use the same slot 2026-02-12 22:41 [PATCHv4 0/3] pci slot reset handling fixes Keith Busch 2026-02-12 22:41 ` [PATCHv4 1/3] pci: rename __pci_bus_reset and __pci_slot_reset Keith Busch @ 2026-02-12 22:41 ` Keith Busch 2026-02-13 7:26 ` Ilpo Järvinen 2026-02-13 7:27 ` Ilpo Järvinen 2026-02-12 22:41 ` [PATCHv4 3/3] pci: make reset_subordinate hotplug safe Keith Busch 2 siblings, 2 replies; 10+ messages in thread From: Keith Busch @ 2026-02-12 22:41 UTC (permalink / raw) To: linux-pci, helgaas, alex, dan.j.williams; +Cc: lukas, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot 2026-02-12 22:41 ` [PATCHv4 2/3] pci: allow all bus devices to use the same slot Keith Busch @ 2026-02-13 7:26 ` Ilpo Järvinen 2026-02-13 14:10 ` Keith Busch 2026-02-13 7:27 ` Ilpo Järvinen 1 sibling, 1 reply; 10+ messages in thread From: Ilpo Järvinen @ 2026-02-13 7:26 UTC (permalink / raw) To: Keith Busch Cc: linux-pci, helgaas, alex, dan.j.williams, Lukas Wunner, Keith Busch 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; > }; > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot 2026-02-13 7:26 ` Ilpo Järvinen @ 2026-02-13 14:10 ` Keith Busch 0 siblings, 0 replies; 10+ messages in thread From: Keith Busch @ 2026-02-13 14:10 UTC (permalink / raw) To: Ilpo Järvinen Cc: Keith Busch, linux-pci, helgaas, alex, dan.j.williams, Lukas Wunner 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot 2026-02-12 22:41 ` [PATCHv4 2/3] pci: allow all bus devices to use the same slot Keith Busch 2026-02-13 7:26 ` Ilpo Järvinen @ 2026-02-13 7:27 ` Ilpo Järvinen 1 sibling, 0 replies; 10+ messages in thread From: Ilpo Järvinen @ 2026-02-13 7:27 UTC (permalink / raw) To: Keith Busch Cc: linux-pci, helgaas, alex, dan.j.williams, Lukas Wunner, Keith Busch 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; > }; > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv4 3/3] pci: make reset_subordinate hotplug safe 2026-02-12 22:41 [PATCHv4 0/3] pci slot reset handling fixes Keith Busch 2026-02-12 22:41 ` [PATCHv4 1/3] pci: rename __pci_bus_reset and __pci_slot_reset Keith Busch 2026-02-12 22:41 ` [PATCHv4 2/3] pci: allow all bus devices to use the same slot Keith Busch @ 2026-02-12 22:41 ` Keith Busch 2026-02-13 3:41 ` dan.j.williams 2026-02-13 7:32 ` Ilpo Järvinen 2 siblings, 2 replies; 10+ messages in thread From: Keith Busch @ 2026-02-12 22:41 UTC (permalink / raw) To: linux-pci, helgaas, alex, dan.j.williams; +Cc: lukas, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv4 3/3] pci: make reset_subordinate hotplug safe 2026-02-12 22:41 ` [PATCHv4 3/3] pci: make reset_subordinate hotplug safe Keith Busch @ 2026-02-13 3:41 ` dan.j.williams 2026-02-13 7:32 ` Ilpo Järvinen 1 sibling, 0 replies; 10+ messages in thread From: dan.j.williams @ 2026-02-13 3:41 UTC (permalink / raw) To: Keith Busch, linux-pci, helgaas, alex, dan.j.williams; +Cc: lukas, Keith Busch 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> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv4 3/3] pci: make reset_subordinate hotplug safe 2026-02-12 22:41 ` [PATCHv4 3/3] pci: make reset_subordinate hotplug safe Keith Busch 2026-02-13 3:41 ` dan.j.williams @ 2026-02-13 7:32 ` Ilpo Järvinen 2026-02-13 14:34 ` Keith Busch 1 sibling, 1 reply; 10+ messages in thread From: Ilpo Järvinen @ 2026-02-13 7:32 UTC (permalink / raw) To: Keith Busch Cc: linux-pci, helgaas, alex, dan.j.williams, Lukas Wunner, Keith Busch 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv4 3/3] pci: make reset_subordinate hotplug safe 2026-02-13 7:32 ` Ilpo Järvinen @ 2026-02-13 14:34 ` Keith Busch 0 siblings, 0 replies; 10+ messages in thread From: Keith Busch @ 2026-02-13 14:34 UTC (permalink / raw) To: Ilpo Järvinen Cc: Keith Busch, linux-pci, helgaas, alex, dan.j.williams, Lukas Wunner 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-02-13 14:34 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-12 22:41 [PATCHv4 0/3] pci slot reset handling fixes Keith Busch 2026-02-12 22:41 ` [PATCHv4 1/3] pci: rename __pci_bus_reset and __pci_slot_reset Keith Busch 2026-02-12 22:41 ` [PATCHv4 2/3] pci: allow all bus devices to use the same slot Keith Busch 2026-02-13 7:26 ` Ilpo Järvinen 2026-02-13 14:10 ` Keith Busch 2026-02-13 7:27 ` Ilpo Järvinen 2026-02-12 22:41 ` [PATCHv4 3/3] pci: make reset_subordinate hotplug safe Keith Busch 2026-02-13 3:41 ` dan.j.williams 2026-02-13 7:32 ` Ilpo Järvinen 2026-02-13 14:34 ` Keith Busch
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.