From: Niklas Schnelle <schnelle@linux.ibm.com>
To: Farhan Ali <alifm@linux.ibm.com>, Keith Busch <kbusch@kernel.org>
Cc: linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, helgaas@kernel.org, lukas@wunner.de,
alex@shazbot.org, clg@redhat.com, stable@vger.kernel.org,
mjrosato@linux.ibm.com
Subject: Re: [PATCH v9 1/9] PCI: Allow per function PCI slots
Date: Thu, 19 Feb 2026 22:37:06 +0100 [thread overview]
Message-ID: <873a948a74df082bb22672cd58136337ffb54083.camel@linux.ibm.com> (raw)
In-Reply-To: <54301c56-1add-4029-b6ed-b441e4fda8dc@linux.ibm.com>
On Tue, 2026-02-17 at 14:26 -0800, Farhan Ali wrote:
> On 2/17/2026 1:14 PM, Keith Busch wrote:
> > On Tue, Feb 17, 2026 at 10:22:49AM -0800, Farhan Ali wrote:
> > > Currently, the kernel's PCI_SLOT() macro assigns the same pci_slot object
> > > to multifunction devices. This approach worked fine on s390 systems that
> > > only exposed virtual functions as individual PCI domains to the operating
> > > system. Since commit 44510d6fa0c0 ("s390/pci: Handling multifunctions")
> > > s390 supports exposing the topology of multifunction PCI devices by
> > > grouping them in a shared PCI domain. When attempting to reset a function
> > > through the hotplug driver, the shared slot assignment causes the wrong
> > > function to be reset instead of the intended one. It also leaks memory as
> > > we do create a pci_slot object for the function, but don't correctly free
> > > it in pci_slot_release().
> > I think leakage is because s390 is passing in devfn when pci_create_slot
> > is expecting devnr, so things are not getting matched and assigned as
> > expected.
> >
> > If you're able to make this change, it will clash with one existing
> > thing, and another proposal I've got at v5 now(*). Specifically, this
> > would allow all 8 bits to be used for the pci_slot 'number' when it's
> > currently expected to be limited to 5 bits. 0xff is special, and I'm
> > proposing another special value. If we are going to allow the slot
> > numbers to use all 8 bits, I think we need to change the type from u8 to
> > u16 so that there is space to encode such special values.
> >
> > * https://lore.kernel.org/linux-pci/20260217160836.2709885-3-kbusch@meta.com/
>
> I am open to suggestions on how we can better model a pci_slot per
> function. And yeah, I think it makes sense to update the pci_slot
> 'number' to u16.
>
> Thanks
>
> Farhan
It may give some better context to emphasize that s390 has been using a
per PCI function hotplug slot model since commit 7441b0627e22
("s390/pci: PCI hotplug support via SCLP") from 2012. Quoting the
commit description here:
"The hotplug controller creates a slot for every PCI function in
stand-by or configured state. The PCI functions are named after
the PCI function ID (fid). By writing to the power attribute
in /sys/bus/pci/slots/<fid>/power the PCI function
is moved to stand-by or configured state."
So this isn't a new concept or something we can really change since
users and documentation relies on this for over a decade. It's only
that for the longest time the mismatch between the hotplug slot and the
pci slot went unnoticed because it really only causes trouble in very
specific circumstances even after we've started exposing the devfn part
of the geographic PCI addresses for SR-IOV capable multi-function
devices e.g. for the two PFs of ConnectX series NICs.
Thanks,
Niklas
next prev parent reply other threads:[~2026-02-19 21:38 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-17 18:22 [PATCH v9 0/9] Error recovery for vfio-pci devices on s390x Farhan Ali
2026-02-17 18:22 ` [PATCH v9 1/9] PCI: Allow per function PCI slots Farhan Ali
2026-02-17 21:14 ` Keith Busch
2026-02-17 22:26 ` Farhan Ali
2026-02-19 21:37 ` Niklas Schnelle [this message]
2026-02-17 18:22 ` [PATCH v9 2/9] s390/pci: Add architecture specific resource/bus address translation Farhan Ali
2026-02-17 18:22 ` [PATCH v9 3/9] PCI: Avoid saving config space state in reset path Farhan Ali
2026-02-17 19:11 ` Keith Busch
2026-02-17 19:55 ` Farhan Ali
2026-02-18 19:02 ` Keith Busch
2026-02-18 19:35 ` Bjorn Helgaas
2026-02-18 21:48 ` Farhan Ali
2026-02-19 0:20 ` Bjorn Helgaas
2026-02-19 18:06 ` Farhan Ali
2026-02-20 20:53 ` Alex Williamson
2026-02-17 18:22 ` [PATCH v9 4/9] PCI: Add additional checks for flr reset Farhan Ali
2026-02-17 18:22 ` [PATCH v9 5/9] s390/pci: Update the logic for detecting passthrough device Farhan Ali
2026-02-17 18:22 ` [PATCH v9 6/9] s390/pci: Store PCI error information for passthrough devices Farhan Ali
2026-02-17 18:22 ` [PATCH v9 7/9] vfio-pci/zdev: Add a device feature for error information Farhan Ali
2026-02-17 18:22 ` [PATCH v9 8/9] vfio: Add a reset_done callback for vfio-pci driver Farhan Ali
2026-02-17 18:22 ` [PATCH v9 9/9] vfio: Remove the pcie check for VFIO_PCI_ERR_IRQ_INDEX Farhan Ali
[not found] <20260217181959.1536-1-alifm@linux.ibm.com>
2026-02-17 18:19 ` [PATCH v9 1/9] PCI: Allow per function PCI slots Farhan Ali
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=873a948a74df082bb22672cd58136337ffb54083.camel@linux.ibm.com \
--to=schnelle@linux.ibm.com \
--cc=alex@shazbot.org \
--cc=alifm@linux.ibm.com \
--cc=clg@redhat.com \
--cc=helgaas@kernel.org \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mjrosato@linux.ibm.com \
--cc=stable@vger.kernel.org \
/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