Re: [PATCH v11 1/9] PCI: Allow per function PCI slots

Farhan Ali
2026-03-24T23:08:39Z

On 3/24/2026 2:55 PM, Bjorn Helgaas wrote:
> On Mon, Mar 16, 2026 at 12:15:36PM -0700, Farhan Ali wrote:
>> On s390 systems, which use a machine level hypervisor, PCI devices are
>> always accessed through a form of PCI pass-through which fundamentally
>> operates on a per PCI function granularity. This is also reflected in the
>> s390 PCI hotplug driver which creates hotplug slots for individual PCI
>> functions. Its reset_slot() function, which is a wrapper for
>> zpci_hot_reset_device(), thus also resets individual functions.
> I think this "pass-through" is from the hypervisor to Linux, i.e.,
> what we think of as the host kernel, right?

Yes, on s390x we have PR/SM hypervisor which would this passthrough to a
Linux. The Linux would be running in a LPAR (Logical Partition) created
by the PR/SM hypervisor. For end users an LPAR is the 'host' for all
practical purposes.

>
>> 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().
> This alludes to the patch fixing a reset issue, but I think it should
> be more prominent, e.g., the reset and leak fixes could be a separate
> paragraph. The subject line should also mention at least the reset
> fix.

Will fix this. I will move what we fix into a separate paragraph.

>
>> Add a flag for struct pci_slot to allow per function PCI slots for
>> functions managed through a hypervisor, which exposes individual PCI
>> functions while retaining the topology. Since we can use all 8 bits
>> for slot 'number' (for ARI devices), change slot 'number' u16 to
>> account for special values -1 and PCI_SLOT_ALL_DEVICES.
>> ...
>> static ssize_t address_read_file(struct pci_slot *slot, char *buf)
>> {
>> - if (slot->number == 0xff)
>> + if (slot->number == (u16)-1)
> This "-1" is mentioned in the commit log, but I don't know where it
> came from. I guess we must assign -1 as a default somewhere? Could
> this be a #define to connect that assignment with this test?

The -1 is used a placeholder and from what I could tell
rpaphp_register_slot() would be the only one to use this. Would you
prefer this to be a #define?

Thanks

Farhan

>> return sysfs_emit(buf, "%04x:%02x\n",
>> pci_domain_nr(slot->bus),
>> slot->bus->number);