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

[Farhan Ali <alifm@linux.ibm.com>]

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