All of lore.kernel.org
 help / color / mirror / Atom feed
From: <dan.j.williams@intel.com>
To: Keith Busch <kbusch@kernel.org>, <dan.j.williams@intel.com>
Cc: Keith Busch <kbusch@meta.com>, <linux-pci@vger.kernel.org>,
	<helgaas@kernel.org>, <alex@shazbot.org>, <lukas@wunner.de>,
	<guojinhui.liam@bytedance.com>, <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCHv3 2/4] pci: allow all bus devices to use the same slot
Date: Tue, 10 Feb 2026 12:51:55 -0800	[thread overview]
Message-ID: <698b9a6b2c175_2e57100de@dwillia2-mobl4.notmuch> (raw)
In-Reply-To: <aYuU9eI28ubuC9sD@kbusch-mbp>

Keith Busch wrote:
> On Tue, Feb 10, 2026 at 12:00:56PM -0800, dan.j.williams@intel.com wrote:
> > Keith Busch wrote:
> > 
> > > this new feature is 0xfe. This will not clash with any actual slot
> > > number since they are limited to 5 bits.
> > 
> > Can you say a bit more about why other hotplug controller drivers can
> > continue to get away with a single device number as a hotplug slot
> > target?  For example, would you expect that the ACPI hotplug driver is
> > broken if an ARI device is added?
> 
> Right, I don't think acpiphp would work correctly for such a device. I
> didn't change that out an abundance of caution to not introduce
> regressions to platforms I can't readily test.
> 
> I've limited this patch to pciehp since that's the only hotplug driver
> that hard-codes the "devnr" value to 0, and ensures there's only one
> pciehp "hotplug_slot" per bridge. pciehp is uniquely clear that this
> patch is safe.
>
> The remaining hotplug_slot users derive devnr and can make additional
> hotplug_slots for each device on the bus, so I can't tell if it is safe
> to make the others subscribe to this proposed behavior.

Ok, I figured as much, maybe just put these notes in the changelog.

> > It looks like the goal of this sentinel is to turn all
> > pci_try_reset_slot() into pci_try_reset_bus() only for the native PCIe
> > hotplug controller, or did I miss a detail?
> 
> Yes, pretty much that. Since every device on the subordinate bus belongs
> to the same pciehp slot now, we'll get the same locking, notification,
> and state restore as what pci_try_reset_bus() does.
>  
> > > @@ -41,6 +41,10 @@ static ssize_t address_read_file(struct pci_slot *slot, char *buf)
> > >  		return sysfs_emit(buf, "%04x:%02x\n",
> > >  				  pci_domain_nr(slot->bus),
> > >  				  slot->bus->number);
> > > +	if (slot->number == PCI_SLOT_ALL_DEVICES)
> > > +		return sysfs_emit(buf, "%04x:%02x:00\n",
> > > +				  pci_domain_nr(slot->bus),
> > > +				  slot->bus->number);
> > 
> > Why not treat the PCI_SLOT_ALL_DEVICES case the same as the 0xff case?
> 
> Just preserving backward compatibility with what this file would show
> for pciehp slots prior to this patch.

Makes sense, perhaps a comment:

/*
 * Preserve legacy ABI expectations that hotplug drivers that manage
 * multiple devices per slot emit 0 for the device number.
 */

...just to remember that this is due to caution.

With that you can add:

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

  reply	other threads:[~2026-02-10 20:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05 21:25 [PATCHv3 0/4] pci slot reset handling fixes Keith Busch
2026-02-05 21:25 ` [PATCHv3 1/4] pci: rename __pci_bus_reset and __pci_slot_reset Keith Busch
2026-02-06 17:22   ` Keith Busch
2026-02-10 20:44   ` dan.j.williams
2026-02-05 21:25 ` [PATCHv3 2/4] pci: allow all bus devices to use the same slot Keith Busch
2026-02-10 20:00   ` dan.j.williams
2026-02-10 20:28     ` Keith Busch
2026-02-10 20:51       ` dan.j.williams [this message]
2026-02-05 21:25 ` [PATCHv3 3/4] pci: remove slot specific lock/unlock and save/restore Keith Busch
2026-02-10 22:03   ` dan.j.williams
2026-02-10 23:25     ` Keith Busch
2026-02-10 23:48       ` dan.j.williams
2026-02-10 23:46   ` Alex Williamson
2026-02-11  0:12     ` Keith Busch
2026-02-11 15:22       ` Alex Williamson
2026-02-11 15:54         ` Keith Busch
2026-02-05 21:25 ` [PATCHv3 4/4] pci: make reset_subordinate hotplug safe Keith Busch
2026-02-10 22:14   ` dan.j.williams

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=698b9a6b2c175_2e57100de@dwillia2-mobl4.notmuch \
    --to=dan.j.williams@intel.com \
    --cc=alex@shazbot.org \
    --cc=guojinhui.liam@bytedance.com \
    --cc=helgaas@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --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 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.