* [PATCH v23 1/5] PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder value
2026-08-05 16:55 [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x Farhan Ali
@ 2026-08-05 16:55 ` Farhan Ali
2026-08-05 17:07 ` sashiko-bot
2026-08-05 16:55 ` [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390 Farhan Ali
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Farhan Ali @ 2026-08-05 16:55 UTC (permalink / raw)
To: linux-s390, linux-kernel, linux-pci
Cc: helgaas, alex, alifm, schnelle, mjrosato, Madhavan Srinivasan,
Tyrel Datwyler, linuxppc-dev, Bjorn Helgaas
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390
2026-08-05 16:55 [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x Farhan Ali
2026-08-05 16:55 ` [PATCH v23 1/5] PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder value Farhan Ali
@ 2026-08-05 16:55 ` Farhan Ali
2026-08-05 17:12 ` sashiko-bot
2026-08-05 16:55 ` [PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible Farhan Ali
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Farhan Ali @ 2026-08-05 16:55 UTC (permalink / raw)
To: linux-s390, linux-kernel, linux-pci
Cc: helgaas, alex, alifm, schnelle, mjrosato, stable
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390
2026-08-05 16:55 ` [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390 Farhan Ali
@ 2026-08-05 17:12 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-05 17:12 UTC (permalink / raw)
To: Farhan Ali
Cc: Heiko Carstens, linux-s390, Vasily Gorbik, Alexander Gordeev,
linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Unconditionally enabling per_func_slot on S390 breaks standard PCIe hotplug (e.g., pciehp), PCI_SLOT_ALL_DEVICES assignment, and physical slot matching.
Pre-existing issues:
- [High] Lockless access to `dev->slot` allows a Use-After-Free if a hotplug slot is concurrently destroyed.
--
--- Patch [2]: [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390 ---
commit 5800507f8f4f03803f3a4540f211363e22c66b48
Author: Farhan Ali <alifm@linux.ibm.com>
PCI: Allow per function PCI slots to fix slot reset on s390
This patch introduces a per_func_slot flag for struct pci_slot to allow
per-function PCI slots on s390. It modifies slot matching logic and changes
the slot number field to a u16 to accommodate special values.
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee615..350bae907ebf8 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))
[Severity: High]
This is a pre-existing issue, but does reading dev->slot here and accessing
dev->slot->per_func_slot allow a use-after-free?
__pci_reset_function_locked() acquires device_lock(&dev->dev) but calls
pci_dev_reset_slot_function(), which reads dev->slot locklessly.
Concurrently, if a hotplug driver is removed, pci_destroy_slot() drops
the kobject refcount and triggers pci_slot_release().
pci_slot_release() frees the slot and sets dev->slot = NULL:
pci_slot_release() {
...
list_for_each_entry(dev, &slot->bus->devices, bus_list)
if (pci_dev_matches_slot(dev, slot))
dev->slot = NULL;
...
kfree(slot);
}
If the reset thread is preempted after checking dev->slot but before
dereferencing dev->slot->per_func_slot, could it dereference the freed
pointer?
> return -ENOTTY;
>
> return pci_reset_hotplug_slot(dev->slot->hotplug, probe);
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 42ff66461f74a..897223f01f6ac 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;
> +}
> +
[ ... ]
> @@ -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;
> +
[Severity: High]
Does unconditionally setting per_func_slot for all slots on S390 break
standard PCIe hotplug topologies?
When generic PCIe hotplug drivers like pciehp create a bus-wide slot, they
pass PCI_SLOT_ALL_DEVICES (now 0xfeff). Because pci_slot_enabled_per_func()
globally forces per_func_slot = 1 on S390, pci_dev_matches_slot() will
evaluate:
dev->devfn == 0xfeff
Since devfn is an 8-bit value (0-255), this will never match, leaving
dev->slot unassigned for all devices on those buses.
Similarly, if a generic driver specifies a physical slot number (like 2),
it will evaluate dev->devfn == 2, which incorrectly matches Function 2 of
Device 0 rather than matching all functions of Device 2.
Will this cause generic hotplug slots to fail matching on S390, breaking
sysfs linkages and PCI resets?
> slot->kobj.kset = pci_slots_kset;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805165518.794-1-alifm@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible
2026-08-05 16:55 [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x Farhan Ali
2026-08-05 16:55 ` [PATCH v23 1/5] PCI: Introduce PCI_SLOT_PLACEHOLDER constant for slot_nr placeholder value Farhan Ali
2026-08-05 16:55 ` [PATCH v23 2/5] PCI: Allow per function PCI slots to fix slot reset on s390 Farhan Ali
@ 2026-08-05 16:55 ` Farhan Ali
2026-08-05 17:05 ` sashiko-bot
2026-08-05 16:55 ` [PATCH v23 4/5] PCI: Fail FLR when config space is inaccessible Farhan Ali
2026-08-05 16:55 ` [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages Farhan Ali
4 siblings, 1 reply; 11+ messages in thread
From: Farhan Ali @ 2026-08-05 16:55 UTC (permalink / raw)
To: linux-s390, linux-kernel, linux-pci
Cc: helgaas, alex, alifm, schnelle, mjrosato, Bjorn Helgaas
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible
2026-08-05 16:55 ` [PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible Farhan Ali
@ 2026-08-05 17:05 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-05 17:05 UTC (permalink / raw)
To: Farhan Ali
Cc: Alexander Gordeev, linux-pci, linux-s390, Vasily Gorbik,
Heiko Carstens
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Function parameter `msg` should be declared as `const char *` instead of `char *`.
--
--- Patch [3]: [PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible ---
commit d2cfd742f2dfe4c4445b198bcb7010bd9ef42491
Author: Farhan Ali <alifm@linux.ibm.com>
PCI: Avoid saving config space state if inaccessible
This commit modifies the PCI reset process to avoid saving the device's
configuration space state if it is inaccessible. It checks the PCI_COMMAND
register to determine accessibility before attempting to save the state.
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 350bae907ebf8..e8d7de77241a6 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)
[Severity: Low]
This isn't a bug, but should the msg parameter be declared as const char *
instead of char *?
The function is called with string literals, which violates const
correctness and can trigger discarded-qualifier compiler warnings.
> +{
> + u32 val;
> +
> + /*
> + * If device's config space is inaccessible it can return ~0 for
[ ... ]
> @@ -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);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805165518.794-1-alifm@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v23 4/5] PCI: Fail FLR when config space is inaccessible
2026-08-05 16:55 [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x Farhan Ali
` (2 preceding siblings ...)
2026-08-05 16:55 ` [PATCH v23 3/5] PCI: Avoid saving config space state if inaccessible Farhan Ali
@ 2026-08-05 16:55 ` Farhan Ali
2026-08-05 17:07 ` sashiko-bot
2026-08-05 16:55 ` [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages Farhan Ali
4 siblings, 1 reply; 11+ messages in thread
From: Farhan Ali @ 2026-08-05 16:55 UTC (permalink / raw)
To: linux-s390, linux-kernel, linux-pci
Cc: helgaas, alex, alifm, schnelle, mjrosato, Benjamin Block
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages
2026-08-05 16:55 [PATCH v23 0/5] [PCI] Error recovery for vfio-pci devices on s390x Farhan Ali
` (3 preceding siblings ...)
2026-08-05 16:55 ` [PATCH v23 4/5] PCI: Fail FLR when config space is inaccessible Farhan Ali
@ 2026-08-05 16:55 ` Farhan Ali
2026-08-05 17:07 ` sashiko-bot
4 siblings, 1 reply; 11+ messages in thread
From: Farhan Ali @ 2026-08-05 16:55 UTC (permalink / raw)
To: linux-s390, linux-kernel, linux-pci
Cc: helgaas, alex, alifm, schnelle, mjrosato, stable, Thomas Gleixner
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages
2026-08-05 16:55 ` [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages Farhan Ali
@ 2026-08-05 17:07 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-05 17:07 UTC (permalink / raw)
To: Farhan Ali
Cc: Vasily Gorbik, linux-s390, Heiko Carstens, Alexander Gordeev,
linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing error check on PCI_COMMAND read leads to permanent register corruption and unintended device enablement on transient PCI errors.
--
--- Patch [5]: [PATCH v23 5/5] PCI/MSI: Enable memory decoding before restoring MSI-X messages ---
commit 6d8ddd25b8ed47c23845049339a3c29cd4e53440
Author: Farhan Ali <alifm@linux.ibm.com>
PCI/MSI: Enable memory decoding before restoring MSI-X messages
This commit temporarily enables the Memory bit in the Command register
before writing MSI-X messages to prevent Unsupported Request errors.
It then restores the original Command register state afterwards.
> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
> index 209373c92e9e1..79c7e84d314b7 100644
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
[ ... ]
> @@ -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);
[Severity: High]
If the device is inaccessible or experiencing a transient error (e.g., an AER
link reset), can this read fail and set cmd to 0xFFFF?
Looking at pci_read_config_word(), it uses PCI_SET_ERROR_RESPONSE() on failure:
int pci_read_config_word(const struct pci_dev *dev, int where, u16 *val)
{
if (pci_dev_is_disconnected(dev)) {
PCI_SET_ERROR_RESPONSE(val);
return PCIBIOS_DEVICE_NOT_FOUND;
}
...
}
If cmd is set to 0xFFFF, the subsequent pci_write_config_word() will write
0xFFFF back to the Command register.
Does this unintentionally enable all device capabilities, including Bus
Mastering (which could lead to rogue DMA) and SERR#?
Should we check the return value of pci_read_config_word() before writing?
> +
> 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);
[Severity: High]
If cmd was populated with the error response 0xFFFF, does this write
permanently commit the corrupted state back to the device?
> pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805165518.794-1-alifm@linux.ibm.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread