From c243e6c470c4695965cc8287767925bc1d9a7867 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Wed, 5 Aug 2026 09:55:14 -0700 Subject: [PATCH 1/5] PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder value Introduce a constant for placeholder value and update the kerneldoc for pci_create_slot() to reference PCI_SLOT_PLACEHOLDER instead of -1 throughout. No functional change. Suggested-by: Bjorn Helgaas Signed-off-by: Farhan Ali Signed-off-by: Bjorn Helgaas Cc: Madhavan Srinivasan Cc: Tyrel Datwyler Cc: linuxppc-dev@lists.ozlabs.org Link: https://patch.msgid.link/20260805165518.794-2-alifm@linux.ibm.com --- drivers/pci/hotplug/pnv_php.c | 2 +- drivers/pci/hotplug/rpaphp_slot.c | 2 +- drivers/pci/slot.c | 21 +++++++++++---------- include/linux/pci.h | 3 +++ 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c index ff92a5c301b8..37299d59f906 100644 --- a/drivers/pci/hotplug/pnv_php.c +++ b/drivers/pci/hotplug/pnv_php.c @@ -808,7 +808,7 @@ static struct pnv_php_slot *pnv_php_alloc_slot(struct device_node *dn) if (dn->child && PCI_DN(dn->child)) php_slot->slot_no = PCI_SLOT(PCI_DN(dn->child)->devfn); else - php_slot->slot_no = -1; /* Placeholder slot */ + php_slot->slot_no = PCI_SLOT_PLACEHOLDER; /* Placeholder slot */ kref_init(&php_slot->kref); php_slot->state = PNV_PHP_STATE_INITIALIZED; diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c index 67362e5b9971..92eabf5f61b9 100644 --- a/drivers/pci/hotplug/rpaphp_slot.c +++ b/drivers/pci/hotplug/rpaphp_slot.c @@ -84,7 +84,7 @@ int rpaphp_register_slot(struct slot *slot) struct hotplug_slot *php_slot = &slot->hotplug_slot; u32 my_index; int retval; - int slotno = -1; + int slotno = PCI_SLOT_PLACEHOLDER; dbg("%s registering slot:path[%pOF] index[%x], name[%s] pdomain[%x] type[%d]\n", __func__, slot->dn, slot->index, slot->name, diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index 6d5cd37bfb1e..42ff66461f74 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -37,7 +37,7 @@ static const struct sysfs_ops pci_slot_sysfs_ops = { static ssize_t address_read_file(struct pci_slot *slot, char *buf) { - if (slot->number == 0xff) + if (slot->number == PCI_SLOT_PLACEHOLDER) return sysfs_emit(buf, "%04x:%02x\n", pci_domain_nr(slot->bus), slot->bus->number); @@ -210,7 +210,7 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr) /** * pci_create_slot - create or increment refcount for physical PCI slot * @parent: struct pci_bus of parent bridge - * @slot_nr: PCI_SLOT(pci_dev->devfn), -1 for placeholder, or + * @slot_nr: PCI_SLOT(pci_dev->devfn), PCI_SLOT_PLACEHOLDER for placeholder, or * PCI_SLOT_ALL_DEVICES * @name: user visible string presented in /sys/bus/pci/slots/ * @hotplug: set if caller is hotplug driver, NULL otherwise @@ -236,15 +236,16 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr) * In most cases, @pci_bus, @slot_nr will be sufficient to uniquely identify * a slot. There is one notable exception - pSeries (rpaphp), where the * @slot_nr cannot be determined until a device is actually inserted into - * the slot. In this scenario, the caller may pass -1 for @slot_nr. + * the slot. In this scenario, the caller may pass PCI_SLOT_PLACEHOLDER for @slot_nr. * * The following semantics are imposed when the caller passes @slot_nr == - * -1. First, we no longer check for an existing %struct pci_slot, as there - * may be many slots with @slot_nr of -1. The other change in semantics is - * user-visible, which is the 'address' parameter presented in sysfs will - * consist solely of a dddd:bb tuple, where dddd is the PCI domain of the - * %struct pci_bus and bb is the bus number. In other words, the devfn of - * the 'placeholder' slot will not be displayed. + * PCI_SLOT_PLACEHOLDER. First, we no longer check for an existing %struct + * pci_slot, as there may be many slots with @slot_nr of + * PCI_SLOT_PLACEHOLDER. The other change in semantics is user-visible, + * which is the 'address' parameter presented in sysfs will consist solely + * of a dddd:bb tuple, where dddd is the PCI domain of the %struct pci_bus + * and bb is the bus number. In other words, the devfn of the 'placeholder' + * slot will not be displayed. * * Bus-wide slots: * For PCIe hotplug, the physical slot encompasses the entire secondary @@ -267,7 +268,7 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, mutex_lock(&pci_slot_mutex); - if (slot_nr == -1) + if (slot_nr == PCI_SLOT_PLACEHOLDER) goto placeholder; /* diff --git a/include/linux/pci.h b/include/linux/pci.h index ebb5b9d76360..3824a8dd551e 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -81,6 +81,9 @@ */ #define PCI_SLOT_ALL_DEVICES 0xfe +/* Used to identify a slot as a placeholder */ +#define PCI_SLOT_PLACEHOLDER 0xff + /* pci_slot represents a physical slot */ struct pci_slot { struct pci_bus *bus; /* Bus this slot is on */ From dcc5bec09e23bbc4f9de055a11fce9937244f2c8 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Wed, 5 Aug 2026 09:55:15 -0700 Subject: [PATCH 2/5] PCI: Allow per function PCI slots to fix slot reset on s390 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. Currently, the pci_create_slot() 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. This creates a problem when resetting a function through the hotplug driver's slot_reset() interface. 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(). 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 PCI_SLOT_PLACEHOLDER and PCI_SLOT_ALL_DEVICES. Fixes: 44510d6fa0c0 ("s390/pci: Handling multifunctions") Suggested-by: Niklas Schnelle Signed-off-by: Farhan Ali Signed-off-by: Bjorn Helgaas Reviewed-by: Niklas Schnelle Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20260805165518.794-3-alifm@linux.ibm.com --- drivers/pci/pci.c | 5 +++-- drivers/pci/slot.c | 29 +++++++++++++++++++++++------ include/linux/pci.h | 7 ++++--- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 77b17b13ee61..350bae907ebf 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4897,8 +4897,9 @@ static int pci_reset_hotplug_slot(struct hotplug_slot *hotplug, bool probe) static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe) { - if (dev->multifunction || dev->subordinate || !dev->slot || - dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET) + if (dev->subordinate || !dev->slot || + dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET || + (dev->multifunction && !dev->slot->per_func_slot)) return -ENOTTY; return pci_reset_hotplug_slot(dev->slot->hotplug, probe); diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index 42ff66461f74..897223f01f6a 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -72,6 +72,23 @@ static ssize_t cur_speed_read_file(struct pci_slot *slot, char *buf) return bus_speed_read(slot->bus->cur_bus_speed, buf); } +static bool pci_dev_matches_slot(struct pci_dev *dev, struct pci_slot *slot) +{ + if (slot->per_func_slot) + return dev->devfn == slot->number; + + return slot->number == PCI_SLOT_ALL_DEVICES || + PCI_SLOT(dev->devfn) == slot->number; +} + +static bool pci_slot_enabled_per_func(void) +{ + if (IS_ENABLED(CONFIG_S390)) + return true; + + return false; +} + static void pci_slot_release(struct kobject *kobj) { struct pci_dev *dev; @@ -82,8 +99,7 @@ static void pci_slot_release(struct kobject *kobj) down_read(&pci_bus_sem); list_for_each_entry(dev, &slot->bus->devices, bus_list) - if (slot->number == PCI_SLOT_ALL_DEVICES || - PCI_SLOT(dev->devfn) == slot->number) + if (pci_dev_matches_slot(dev, slot)) dev->slot = NULL; up_read(&pci_bus_sem); @@ -187,8 +203,7 @@ void pci_dev_assign_slot(struct pci_dev *dev) mutex_lock(&pci_slot_mutex); list_for_each_entry(slot, &dev->bus->slots, list) - if (slot->number == PCI_SLOT_ALL_DEVICES || - PCI_SLOT(dev->devfn) == slot->number) + if (pci_dev_matches_slot(dev, slot)) dev->slot = slot; mutex_unlock(&pci_slot_mutex); } @@ -299,6 +314,9 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, slot->bus = pci_bus_get(parent); slot->number = slot_nr; + if (pci_slot_enabled_per_func()) + slot->per_func_slot = 1; + slot->kobj.kset = pci_slots_kset; slot_name = make_slot_name(name); @@ -319,8 +337,7 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, down_read(&pci_bus_sem); list_for_each_entry(dev, &parent->devices, bus_list) - if (slot_nr == PCI_SLOT_ALL_DEVICES || - PCI_SLOT(dev->devfn) == slot_nr) + if (pci_dev_matches_slot(dev, slot)) dev->slot = slot; up_read(&pci_bus_sem); diff --git a/include/linux/pci.h b/include/linux/pci.h index 3824a8dd551e..8593384489db 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -79,17 +79,18 @@ * and, if ARI Forwarding is enabled, functions may appear to be on multiple * devices. */ -#define PCI_SLOT_ALL_DEVICES 0xfe +#define PCI_SLOT_ALL_DEVICES 0xfeff /* Used to identify a slot as a placeholder */ -#define PCI_SLOT_PLACEHOLDER 0xff +#define PCI_SLOT_PLACEHOLDER 0xffff /* pci_slot represents a physical slot */ struct pci_slot { struct pci_bus *bus; /* Bus this slot is on */ struct list_head list; /* Node in list of slots */ struct hotplug_slot *hotplug; /* Hotplug info (move here) */ - unsigned char number; /* Device nr, or PCI_SLOT_ALL_DEVICES */ + u16 number; /* Device nr, or PCI_SLOT_ALL_DEVICES */ + unsigned int per_func_slot:1; /* Allow per function slot */ struct kobject kobj; }; From e18d1abc3bff0a24a5299f3c150ab9546e9439aa Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Wed, 5 Aug 2026 09:55:16 -0700 Subject: [PATCH 3/5] PCI: Avoid saving config space state if inaccessible The current reset process saves the device's config space state before reset and restores it afterward. However errors may occur unexpectedly and it may then be impossible to save config space because the device may be inaccessible (e.g. DPC). This results in saving invalid values that get written back to the device during state restoration. With a reset we want to recover/restore the device into a functional state. So avoid saving the state of the config space when the device config space is inaccessible. Signed-off-by: Farhan Ali [bhelgaas: comment] Signed-off-by: Bjorn Helgaas Reviewed-by: Niklas Schnelle Reviewed-by: Bjorn Helgaas Link: https://patch.msgid.link/20260805165518.794-4-alifm@linux.ibm.com --- drivers/pci/pci.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 350bae907ebf..5e14afc4aa19 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -713,6 +713,28 @@ u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16 dvsec) } EXPORT_SYMBOL_GPL(pci_find_dvsec_capability); +static bool pci_dev_config_accessible(struct pci_dev *dev, char *msg) +{ + u32 val; + + /* + * If a device's config space is inaccessible, reads typically + * return ~0. Since Device and Vendor ID are always ~0 for VFs, + * check the Command and Status registers instead. + * + * N.B. This is racy because the device may become inaccessible + * before the next access. + */ + pci_read_config_dword(dev, PCI_COMMAND, &val); + if (PCI_POSSIBLE_ERROR(val)) { + pci_warn(dev, "Device config space inaccessible; unable to %s\n", + msg); + return false; + } + + return true; +} + /** * pci_find_parent_resource - return resource region of parent bus of given * region @@ -5059,6 +5081,9 @@ static void pci_dev_save_and_disable(struct pci_dev *dev) */ pci_set_power_state(dev, PCI_D0); + if (!pci_dev_config_accessible(dev, "save state")) + return; + pci_save_state(dev); /* * Disable the device by clearing the Command register, except for From ad05c16b290be93e0ff02151b22c5d137f655908 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Wed, 5 Aug 2026 09:55:17 -0700 Subject: [PATCH 4/5] PCI: Fail FLR when config space is inaccessible If a device is in an error state, its config space may not be accssible. Add additional check to validate if a device's config space is accessible before doing an FLR reset. Signed-off-by: Farhan Ali Signed-off-by: Bjorn Helgaas Reviewed-by: Benjamin Block Reviewed-by: Niklas Schnelle Link: https://patch.msgid.link/20260805165518.794-5-alifm@linux.ibm.com --- drivers/pci/pci.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 5e14afc4aa19..a73d1a3a8939 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4385,6 +4385,9 @@ int pcie_flr(struct pci_dev *dev) { int ret; + if (!pci_dev_config_accessible(dev, "FLR")) + return -ENOTTY; + if (!pci_wait_for_pending_transaction(dev)) pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n"); From 231c7a57d19304beb0931e6cbe3a4929daf49747 Mon Sep 17 00:00:00 2001 From: Farhan Ali Date: Wed, 5 Aug 2026 09:55:18 -0700 Subject: [PATCH 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages The current MSI-X restoration path assumes the Command register Memory bit is enabled when writing MSI-X messages. But it's possible the last saved and restored state of a device may not have the Memory bit enabled, even if a device driver later enables Memory bit and MSI-X. Attempting to access Memory space without Memory bit enabled can lead to Unsupported Request (UR) from the device. Fix this by enabling Memory bit and restore it afterwards. Fixes: 41017f0cac92 ("[PATCH] PCI: MSI(X) save/restore for suspend/resume") Signed-off-by: Farhan Ali [bhelgaas: comment] Signed-off-by: Bjorn Helgaas Reviewed-by: Thomas Gleixner Reviewed-by: Niklas Schnelle Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20260805165518.794-6-alifm@linux.ibm.com --- drivers/pci/msi/msi.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c index 209373c92e9e..80a9db417dc8 100644 --- a/drivers/pci/msi/msi.c +++ b/drivers/pci/msi/msi.c @@ -870,6 +870,7 @@ void __pci_restore_msix_state(struct pci_dev *dev) { struct msi_desc *entry; bool write_msg; + u16 cmd; if (!dev->msix_enabled) return; @@ -879,6 +880,14 @@ void __pci_restore_msix_state(struct pci_dev *dev) pci_msix_clear_and_set_ctrl(dev, 0, PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL); + /* + * The restored device state may not have Memory Space enabled. + * Since the MSI-X Table and PBA are in Memory Space, enable it + * while restoring them. + */ + pci_read_config_word(dev, PCI_COMMAND, &cmd); + pci_write_config_word(dev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY); + write_msg = arch_restore_msi_irqs(dev); scoped_guard (msi_descs_lock, &dev->dev) { @@ -889,6 +898,7 @@ void __pci_restore_msix_state(struct pci_dev *dev) } } + pci_write_config_word(dev, PCI_COMMAND, cmd); pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0); }