Linux PCI subsystem development
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Keith Busch <kbusch@meta.com>
Cc: linux-pci@vger.kernel.org, helgaas@kernel.org, alex@shazbot.org,
	 dan.j.williams@intel.com, Lukas Wunner <lukas@wunner.de>,
	 Keith Busch <kbusch@kernel.org>
Subject: Re: [PATCHv4 2/3] pci: allow all bus devices to use the same slot
Date: Fri, 13 Feb 2026 09:27:45 +0200 (EET)	[thread overview]
Message-ID: <8338f25a-eba8-b8a9-4e5a-b18fa5192047@linux.intel.com> (raw)
In-Reply-To: <20260212224112.1913980-3-kbusch@meta.com>

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;
>  };
>  
> 

  parent reply	other threads:[~2026-02-13  7:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=8338f25a-eba8-b8a9-4e5a-b18fa5192047@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=alex@shazbot.org \
    --cc=dan.j.williams@intel.com \
    --cc=helgaas@kernel.org \
    --cc=kbusch@kernel.org \
    --cc=kbusch@meta.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    /path/to/YOUR_REPLY

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

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