[PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x

[PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x

[Farhan Ali]

Hi Bjorn,

This patch set includes only the PCI patches of the original series for
error recovery for vfio-pci devices on s390x [1]. Breaking up the patch
series into PCI and VFIO only patches to make merging easier based on
discussion with Alex [2].

Thanks
Farhan

[1] https://lore.kernel.org/all/20260520171113.1111-1-alifm@linux.ibm.com/
[2] https://lore.kernel.org/all/20260602163344.1eda12d2@shazbot.org/

ChangeLog
---------
v22: https://lore.kernel.org/all/20260720192505.2957-1-alifm@linux.ibm.com/
v22 -> v23:
  - Split placeholder constant into a separate patch (patch 1).
  - Rebase on 7.2-rc6

v21: https://lore.kernel.org/all/20260630164807.643-1-alifm@linux.ibm.com/
v21 -> v22
  - Ammend commit message for patch 1.
  - Rebase on 7.2-rc4

v20 https://lore.kernel.org/all/20260622171840.1618-1-alifm@linux.ibm.com/
v20 -> v21
  - Amend commit message to include Fixes tag and cc stable (patch 4).
  - Rebase on 7.2-rc1.

v19 https://lore.kernel.org/all/20260615183524.2880-1-alifm@linux.ibm.com/
v19 -> v20
  - Unconditionally enable Memory bit while restoring MSI-X (patch 4).
  Fixes an issue found with sashiko. 

v18 https://lore.kernel.org/all/20260603181647.2215-1-alifm@linux.ibm.com/
v18 -> v19
  - Move config space accessible check to pcie_flr() function (based on
  discussion of Sashiko review)

  - Fix a gap in MSI-X restoration (patch 4).

  - Rebase on 7.1-rc7

v17 -> v18
  - Rebase on 7.1-rc6.

Farhan Ali (5):
  PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder
    value
  PCI: Allow per function PCI slots to fix slot reset on s390
  PCI: Avoid saving config space state if inaccessible
  PCI: Fail FLR when config space is inaccessible
  PCI/MSI: Enable memory decoding before restoring MSI-X messages

 drivers/pci/hotplug/pnv_php.c     |  2 +-
 drivers/pci/hotplug/rpaphp_slot.c |  2 +-
 drivers/pci/msi/msi.c             | 10 +++++++
 drivers/pci/pci.c                 | 32 ++++++++++++++++++--
 drivers/pci/slot.c                | 50 +++++++++++++++++++++----------
 include/linux/pci.h               |  8 +++--
 6 files changed, 82 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH v23 1/5] PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder value

[Farhan Ali]

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.

Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Farhan Ali <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/<name>
  * @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 64b308b6e61c..b628787e9485 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 */
-- 
2.43.0

[PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390

[Farhan Ali]

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")
Cc: stable@vger.kernel.org
Suggested-by: Niklas Schnelle <schnelle@linux.ibm.com>
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Farhan Ali <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 b628787e9485..43f80d6189a7 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;
 };
 
-- 
2.43.0

[PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible

[Farhan Ali]

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.

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 drivers/pci/pci.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 350bae907ebf..e8d7de77241a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -713,6 +713,27 @@ 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 device's config space is inaccessible it can return ~0 for
+	 * any reads. Since VFs can also return ~0 for Device and Vendor ID
+	 * check Command and Status registers. Note that this is racy
+	 * because the device may become inaccessible partway through
+	 * 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 +5080,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
-- 
2.43.0

[PATCH v23 4/5] PCI: Fail FLR when config space is inaccessible

[Farhan Ali]

If a device is in an error state, then it's config space may not be
accssible. Add additional check to validate if a device's config space is
accessible before doing an FLR reset.

Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Farhan Ali <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 e8d7de77241a..9a9d021301c4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4384,6 +4384,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");
 
-- 
2.43.0

[PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages

[Farhan Ali]

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")
Cc: stable@vger.kernel.org
Reviewed-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Farhan Ali <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..79c7e84d314b 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 decoding enabled
+	 * in the Command register. Since the MSI-X was enabled for the
+	 * device, enable Memory decoding before restoring MSI-X.
+	 */
+	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);
 }
 
-- 
2.43.0

Re: [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x

[Farhan Ali]

Hi Bjorn,

Polite ping on this version.

Thanks

Farhan

On 8/5/2026 9:55 AM, Farhan Ali wrote:
> Hi Bjorn,
>
> This patch set includes only the PCI patches of the original series for
> error recovery for vfio-pci devices on s390x [1]. Breaking up the patch
> series into PCI and VFIO only patches to make merging easier based on
> discussion with Alex [2].
>
> Thanks
> Farhan
>
> [1] https://lore.kernel.org/all/20260520171113.1111-1-alifm@linux.ibm.com/
> [2] https://lore.kernel.org/all/20260602163344.1eda12d2@shazbot.org/
>
> ChangeLog
> ---------
> v22: https://lore.kernel.org/all/20260720192505.2957-1-alifm@linux.ibm.com/
> v22 -> v23:
>    - Split placeholder constant into a separate patch (patch 1).
>    - Rebase on 7.2-rc6
>
> v21: https://lore.kernel.org/all/20260630164807.643-1-alifm@linux.ibm.com/
> v21 -> v22
>    - Ammend commit message for patch 1.
>    - Rebase on 7.2-rc4
>
> v20 https://lore.kernel.org/all/20260622171840.1618-1-alifm@linux.ibm.com/
> v20 -> v21
>    - Amend commit message to include Fixes tag and cc stable (patch 4).
>    - Rebase on 7.2-rc1.
>
> v19 https://lore.kernel.org/all/20260615183524.2880-1-alifm@linux.ibm.com/
> v19 -> v20
>    - Unconditionally enable Memory bit while restoring MSI-X (patch 4).
>    Fixes an issue found with sashiko.
>
> v18 https://lore.kernel.org/all/20260603181647.2215-1-alifm@linux.ibm.com/
> v18 -> v19
>    - Move config space accessible check to pcie_flr() function (based on
>    discussion of Sashiko review)
>
>    - Fix a gap in MSI-X restoration (patch 4).
>
>    - Rebase on 7.1-rc7
>
> v17 -> v18
>    - Rebase on 7.1-rc6.
>
> Farhan Ali (5):
>    PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder
>      value
>    PCI: Allow per function PCI slots to fix slot reset on s390
>    PCI: Avoid saving config space state if inaccessible
>    PCI: Fail FLR when config space is inaccessible
>    PCI/MSI: Enable memory decoding before restoring MSI-X messages
>
>   drivers/pci/hotplug/pnv_php.c     |  2 +-
>   drivers/pci/hotplug/rpaphp_slot.c |  2 +-
>   drivers/pci/msi/msi.c             | 10 +++++++
>   drivers/pci/pci.c                 | 32 ++++++++++++++++++--
>   drivers/pci/slot.c                | 50 +++++++++++++++++++++----------
>   include/linux/pci.h               |  8 +++--
>   6 files changed, 82 insertions(+), 22 deletions(-)
>

Re: [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages

[Bjorn Helgaas]

On Wed, Aug 05, 2026 at 09:55:18AM -0700, Farhan Ali wrote:
> 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")
> Cc: stable@vger.kernel.org
> Reviewed-by: Thomas Gleixner <tglx@kernel.org>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Farhan Ali <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..79c7e84d314b 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 decoding enabled
> +	 * in the Command register. Since the MSI-X was enabled for the
> +	 * device, enable Memory decoding before restoring MSI-X.

PCI_COMMAND_MEMORY must be set because the MSI-X Table and PBA are in
Memory space (in a BAR), right?  I think a more direct way of saying
this would be:

  * 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);
>  }
>  
> -- 
> 2.43.0
> 

Re: [PATCH v23 4/5] PCI: Fail FLR when config space is inaccessible

[Bjorn Helgaas]

On Wed, Aug 05, 2026 at 09:55:17AM -0700, Farhan Ali wrote:
> If a device is in an error state, then it's config space may not be
> accssible. Add additional check to validate if a device's config space is
> accessible before doing an FLR reset.
> 
> Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Farhan Ali <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 e8d7de77241a..9a9d021301c4 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4384,6 +4384,9 @@ int pcie_flr(struct pci_dev *dev)
>  {
>  	int ret;
>  
> +	if (!pci_dev_config_accessible(dev, "FLR"))
> +		return -ENOTTY;

I'm not really keen on this racy check to begin with (though I know I
acked it earlier :)), and also a little hesitant about doing it only
here and not in a more generic place, since several of the reset
methods are susceptible to the same issue.

But I guess in your use case, FLR is the typical method used and maybe
we can worry about the others later.

>  	if (!pci_wait_for_pending_transaction(dev))
>  		pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n");
>  
> -- 
> 2.43.0
> 

Re: [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages

[Farhan Ali]

On 8/12/2026 3:07 PM, Bjorn Helgaas wrote:
> On Wed, Aug 05, 2026 at 09:55:18AM -0700, Farhan Ali wrote:
>> 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")
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Thomas Gleixner <tglx@kernel.org>
>> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
>> Signed-off-by: Farhan Ali <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..79c7e84d314b 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 decoding enabled
>> +	 * in the Command register. Since the MSI-X was enabled for the
>> +	 * device, enable Memory decoding before restoring MSI-X.
> PCI_COMMAND_MEMORY must be set because the MSI-X Table and PBA are in
> Memory space (in a BAR), right?

Yes, since restoring MSI-X would need to access the BAR, we need to set 
PCI_COMMAND_MEMORY.


> I think a more direct way of saying
> this would be:
>
>    * 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.

Sure, we can improve this with what you suggested. Would you prefer me 
to re-spin with the updated comment?

Thanks

Farhan

>> +	 */
>> +	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);
>>   }
>>   
>> -- 
>> 2.43.0
>>

Re: [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x

[Bjorn Helgaas]

On Wed, Aug 05, 2026 at 09:55:13AM -0700, Farhan Ali wrote:
> Hi Bjorn,
> 
> This patch set includes only the PCI patches of the original series for
> error recovery for vfio-pci devices on s390x [1]. Breaking up the patch
> series into PCI and VFIO only patches to make merging easier based on
> discussion with Alex [2].
> 
> Thanks
> Farhan
> 
> [1] https://lore.kernel.org/all/20260520171113.1111-1-alifm@linux.ibm.com/
> [2] https://lore.kernel.org/all/20260602163344.1eda12d2@shazbot.org/
> 
> ChangeLog
> ---------
> v22: https://lore.kernel.org/all/20260720192505.2957-1-alifm@linux.ibm.com/
> v22 -> v23:
>   - Split placeholder constant into a separate patch (patch 1).
>   - Rebase on 7.2-rc6

(No need to rebase; I always apply on -rc1 regardless)

> Farhan Ali (5):
>   PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder
>     value
>   PCI: Allow per function PCI slots to fix slot reset on s390
>   PCI: Avoid saving config space state if inaccessible
>   PCI: Fail FLR when config space is inaccessible
>   PCI/MSI: Enable memory decoding before restoring MSI-X messages
> 
>  drivers/pci/hotplug/pnv_php.c     |  2 +-
>  drivers/pci/hotplug/rpaphp_slot.c |  2 +-
>  drivers/pci/msi/msi.c             | 10 +++++++
>  drivers/pci/pci.c                 | 32 ++++++++++++++++++--
>  drivers/pci/slot.c                | 50 +++++++++++++++++++++----------
>  include/linux/pci.h               |  8 +++--
>  6 files changed, 82 insertions(+), 22 deletions(-)

Applied to pci/slot for v7.3, thanks!

Re: [PATCH v23 4/5] PCI: Fail FLR when config space is inaccessible

[Farhan Ali]

On 8/12/2026 3:34 PM, Bjorn Helgaas wrote:
> On Wed, Aug 05, 2026 at 09:55:17AM -0700, Farhan Ali wrote:
>> If a device is in an error state, then it's config space may not be
>> accssible. Add additional check to validate if a device's config space is
>> accessible before doing an FLR reset.
>>
>> Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
>> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
>> Signed-off-by: Farhan Ali <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 e8d7de77241a..9a9d021301c4 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -4384,6 +4384,9 @@ int pcie_flr(struct pci_dev *dev)
>>   {
>>   	int ret;
>>   
>> +	if (!pci_dev_config_accessible(dev, "FLR"))
>> +		return -ENOTTY;
> I'm not really keen on this racy check to begin with (though I know I
> acked it earlier :)), and also a little hesitant about doing it only
> here and not in a more generic place, since several of the reset
> methods are susceptible to the same issue.
>
> But I guess in your use case, FLR is the typical method used and maybe
> we can worry about the others later.

Yeah, I was also hesitant adding it to the other reset methods as I 
don't have hardware to test it. One reason to have the 
pci_dev_config_accessible() function was to be able to use it in other 
reset methods if needed.

Thanks for reviewing and merging the changes!

Thanks

Farhan


>
>>   	if (!pci_wait_for_pending_transaction(dev))
>>   		pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n");
>>   
>> -- 
>> 2.43.0
>>

Re: [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390

[Bjorn Helgaas]

On Wed, Aug 05, 2026 at 09:55:15AM -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.

Sorry to come back to this yet again.  I understand the issue with
the wrong pci_slot being assigned for these s390 functions.

What I don't understand is why we would use slot_reset() in the first
place.  I would expect FLR instead.

The hotplug slot_reset() path is used by pci_reset_bus_function().
But given the order in pci_reset_fn_methods[], we would typically try
pcie_reset_flr() first, and we would only get to
pci_reset_bus_function() if FLR and the other resets are not
available.

Since these are actually multi-function devices, I'm surprised that
they wouldn't advertise FLR support.

> 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")
> Cc: stable@vger.kernel.org
> Suggested-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Farhan Ali <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 b628787e9485..43f80d6189a7 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;
>  };
>  
> -- 
> 2.43.0
> 

Re: [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390

[Niklas Schnelle]

On Thu, 2026-08-13 at 18:25 -0500, Bjorn Helgaas wrote:
> On Wed, Aug 05, 2026 at 09:55:15AM -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.
> 
> Sorry to come back to this yet again.  I understand the issue with
> the wrong pci_slot being assigned for these s390 functions.
> 
> What I don't understand is why we would use slot_reset() in the first
> place.  I would expect FLR instead.
> 
> The hotplug slot_reset() path is used by pci_reset_bus_function().
> But given the order in pci_reset_fn_methods[], we would typically try
> pcie_reset_flr() first, and we would only get to
> pci_reset_bus_function() if FLR and the other resets are not
> available.
> 
> Since these are actually multi-function devices, I'm surprised that
> they wouldn't advertise FLR support.
> 


Hi Bjorn,

Good question. The problem isn't that FLR isn't advertised or
unsupported. Rather we end up needing to use the slot reset when the
platform has put the PCI function in the architected error state which
blocks both MMIO and DMA similar to DPC and which we can only get out
of with the platform specific CLP Set PCI Function Disable/Enable
hypercalls. FLR still works if you have a function that wasn't put in
the error state but for most real world errors as well as some service
scenarios we do end up in the error state where a FLR won't work.

To give an example for a service scenario because it's pretty neat. We
have up to 4 drawers of CPUs acting as a single SMP system as well as
multiple I/O cages with the PCIe cards in them. Now each I/O cage is
connected to two different PCIe root complexes on two different drawers
with only one link active. So one thing we can do is to migrate all
workload off a drawer and then swap over to the alternate root complex
with a single error event and one such zpci_hot_reset_device(). Then
with a CPU drawer evacuated you can actually replace CPUs without any
downtime beyond that reset while staying within a single machine.

Thanks,
Niklas

Re: [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390

[Bjorn Helgaas]

On Fri, Aug 14, 2026 at 10:46:28AM +0200, Niklas Schnelle wrote:
> On Thu, 2026-08-13 at 18:25 -0500, Bjorn Helgaas wrote:
> > On Wed, Aug 05, 2026 at 09:55:15AM -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.
> > 
> > Sorry to come back to this yet again.  I understand the issue with
> > the wrong pci_slot being assigned for these s390 functions.
> > 
> > What I don't understand is why we would use slot_reset() in the first
> > place.  I would expect FLR instead.
> > 
> > The hotplug slot_reset() path is used by pci_reset_bus_function().
> > But given the order in pci_reset_fn_methods[], we would typically try
> > pcie_reset_flr() first, and we would only get to
> > pci_reset_bus_function() if FLR and the other resets are not
> > available.
> > 
> > Since these are actually multi-function devices, I'm surprised that
> > they wouldn't advertise FLR support.
> 
> Good question. The problem isn't that FLR isn't advertised or
> unsupported. Rather we end up needing to use the slot reset when the
> platform has put the PCI function in the architected error state which
> blocks both MMIO and DMA similar to DPC and which we can only get out
> of with the platform specific CLP Set PCI Function Disable/Enable
> hypercalls. FLR still works if you have a function that wasn't put in
> the error state but for most real world errors as well as some service
> scenarios we do end up in the error state where a FLR won't work.

I assume these are standard PCIe devices, but this architected error
state doesn't sound like something from the PCIe spec.

DPC works by disabling the link, but of course that blocks traffic to
all the functions of an MFD, so maybe this is some s390-specific thing
outside the endpoint, e.g., something in a Downstream Port that can
selectively block traffic to/from a specific function?

To get to pci_reset_bus_function() where we can use the slot reset, I
think all the previous methods, including pcie_reset_flr(), must have
failed with -ENOTTY.  But I don't see a place that would do that.
Maybe you remove the other methods from dev->reset_methods[]?

In addition to whatever the CLP Set PCI Function Disable/Enable
hypercall does to unblock traffic to/from the endpoint, I suppose it
does an FLR internally?  It must use some standard PCIe mechanism
because the endpoint doesn't know anything about s390 or the
hypervisor.

I wonder if we should make some kind of direct platform-specific reset
method, or maybe a pcibios_*()-style hook in the pcie_reset_flr() path
instead of this somewhat convoluted pci_slot stuff.  But
s390_pci_hpc.c is pretty simple and maybe it's used for things other
than reset.

Bjorn

Re: [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390

[Niklas Schnelle]

On Fri, 2026-08-14 at 08:26 -0500, Bjorn Helgaas wrote:
> On Fri, Aug 14, 2026 at 10:46:28AM +0200, Niklas Schnelle wrote:
> > On Thu, 2026-08-13 at 18:25 -0500, Bjorn Helgaas wrote:
> > > On Wed, Aug 05, 2026 at 09:55:15AM -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.
> > > 
> > > Sorry to come back to this yet again.  I understand the issue with
> > > the wrong pci_slot being assigned for these s390 functions.
> > > 
> > > What I don't understand is why we would use slot_reset() in the first
> > > place.  I would expect FLR instead.
> > > 
> > > The hotplug slot_reset() path is used by pci_reset_bus_function().
> > > But given the order in pci_reset_fn_methods[], we would typically try
> > > pcie_reset_flr() first, and we would only get to
> > > pci_reset_bus_function() if FLR and the other resets are not
> > > available.
> > > 
> > > Since these are actually multi-function devices, I'm surprised that
> > > they wouldn't advertise FLR support.
> > 
> > Good question. The problem isn't that FLR isn't advertised or
> > unsupported. Rather we end up needing to use the slot reset when the
> > platform has put the PCI function in the architected error state which
> > blocks both MMIO and DMA similar to DPC and which we can only get out
> > of with the platform specific CLP Set PCI Function Disable/Enable
> > hypercalls. FLR still works if you have a function that wasn't put in
> > the error state but for most real world errors as well as some service
> > scenarios we do end up in the error state where a FLR won't work.
> 
> I assume these are standard PCIe devices, but this architected error
> state doesn't sound like something from the PCIe spec.
> 
> DPC works by disabling the link, but of course that blocks traffic to
> all the functions of an MFD, so maybe this is some s390-specific thing
> outside the endpoint, e.g., something in a Downstream Port that can
> selectively block traffic to/from a specific function?

Yes the error state is an s390 concept. It's implemented by firmware
which controls the PCIe root controllers which are hidden from Linux.
This firmware also controls the s390 HW IOMMU, though Linux controls
the translation tables. And it even controls the PCIe switches between
the root port and the endpoint. So firmware can disable MMIO, block DMA
and interrupts on a per function basis without dropping the link. If
the link is dropped it will of course affect the entire multi-function
device or even an entire I/O cage but then firmware will do the PCIe
level resets, link recovery etc. Then it will generate error events for
all affected functions and require a CLP based reset for each
individually. So even in that case we still work through it on a per
PCI function basis.

> 
> To get to pci_reset_bus_function() where we can use the slot reset, I
> think all the previous methods, including pcie_reset_flr(), must have
> failed with -ENOTTY.  But I don't see a place that would do that.
> Maybe you remove the other methods from dev->reset_methods[]?

In our normal error recovery flow we use zpci_hot_reset_device()
directly. But with Farhan's series QEMU now needs to drive the reset
through vfio-pci. This happens on behalf of a guest which itself does
the CLP Set PCI Function Disable.

> 
> In addition to whatever the CLP Set PCI Function Disable/Enable
> hypercall does to unblock traffic to/from the endpoint, I suppose it
> does an FLR internally? 

Yes that's at least one of the options, it can also do PERST#, swap
links to alternate paths retrain links etc.

>  It must use some standard PCIe mechanism
> because the endpoint doesn't know anything about s390 or the
> hypervisor.

Yes, though our machines only support a limited set of devices and
firmware also knows what kind of device is plugged where, so while the
devices don't know about s390 the firmware may do device specific
recovery steps.

> 
> I wonder if we should make some kind of direct platform-specific reset
> method, or maybe a pcibios_*()-style hook in the pcie_reset_flr() path
> instead of this somewhat convoluted pci_slot stuff.  But
> s390_pci_hpc.c is pretty simple and maybe it's used for things other
> than reset.
> 
> Bjorn

s390_pci_hpc.c is also used for "sharing" PCI functions between
different Linux instances. Basically a hotplug slot with the power
attribute reading 0 represents a PCI function which is in a pool of
standby PCI functions which are seen by multiple Linux instances at
once. Once one instance write 1 to power the PCI function is configured
(aka attached) to that Linux instance and becomes exclusively owned and
the hotplug slot becomes invisible/is hot unplugged for all other Linux
instances that could previously see it. On the other hand when one
Linux instance write 0 to a hotplug slot the PCI function is returned
to the pool and other Linux instances get a hotplug slot hot plugged.

Note that even without this patch resetting through s390_pci_hpc.c
worked on a per-function basis since the linking from the hotplug slot
to struct pci_slot was ok which is also why this stayed hidden for so
long.

Thanks,
Niklas