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

[Farhan Ali]

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