All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes
@ 2026-10-25  0:08 Matthew Wood
  2026-10-25  0:08 ` [PATCH 1/5] PCI: Add disconnected device checks to user config reads Matthew Wood
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Matthew Wood @ 2026-10-25  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, kbusch

Many PCI sysfs attributes that read from config space are using kernel
accessors that don't wait for pci_cfg_access_lock() holders before reading:

- serial_number
- max_link_width
- current_link_*
- *_bus_number

If userspace attempts to read from these attributes while config space is not
available (e.g. during a reset) there's no telling what values will be
returned: All ones, zeros, or garbage.

Looking at attribute addition history it seems like this was an oversight
instead of a specific decision so this series tries to resolve the attributes
above to match the sysfs attributes that do use user accessors: config & vpd.

Patch 1 is a prerequisite for the rest of the series. The kernel
accessors these attributes use today skip devices marked disconnected
but the user accessors don't, so switching over alone would make the
attributes start sending config reads to disconnected devices. Patch 1
adds the same pci_dev_is_disconnected() check to the user read
accessors.

Note: That check also affects their existing callers. On a disconnected
device, VPD reads and writes, VFIO config reads, and VFIO writes to
PCI_COMMAND now fail with -ENODEV, rather than returning ~0 data or,
for VPD writes, timing out with a "firmware bug" warning.
pciconfig_read() fails with -EIO, and the sysfs "config" file and
/proc/bus/pci still return ~0 data.

Changes were successfully tested by continuously reading sysfs attributes
while a reset was issued.

Matthew Wood (5):
  PCI: Add disconnected device checks to user config reads
  PCI/sysfs: Use user config accessors for bus number attributes
  PCI: Add pcie_capability_user_read_{word,dword}()
  PCI/sysfs: Use user config accessors for link attributes
  PCI/sysfs: Use user config accessors for serial_number

 drivers/pci/access.c    | 57 +++++++++++++++++++++++++++++++++++++++--
 drivers/pci/pci-sysfs.c | 35 ++++++++++++++++++-------
 drivers/pci/pci.h       |  2 ++
 3 files changed, 83 insertions(+), 11 deletions(-)


base-commit: f4319f548f7d8cb595ac1c91bfd459876399eeb3
--
2.53.0-Meta

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] PCI: Add disconnected device checks to user config reads
  2026-10-25  0:08 [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes Matthew Wood
@ 2026-10-25  0:08 ` Matthew Wood
  2026-10-25  0:53   ` sashiko-bot
  2026-10-25  0:08 ` [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes Matthew Wood
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Matthew Wood @ 2026-10-25  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, kbusch, Nikita Vetoshkin

Since commit 4b1038834739 ("PCI: Don't attempt config access to
disconnected devices") the kernel config read accessors skip devices
marked disconnected, returning PCIBIOS_DEVICE_NOT_FOUND and ~0 data.
The user config read accessors still issue the read, and if the host
bridge completes it with ~0 data and no error, they report success.

Check pci_dev_is_disconnected() in pci_user_read_config_*() as well,
failing with -ENODEV and ~0 data. Do the check after waiting for any
pci_cfg_access_lock() holder so a reader blocked behind a reset also
sees a device that was disconnected while it waited.

This prepares for converting sysfs attributes from the kernel
accessors to the user accessors so they keep skipping disconnected
devices.

On a disconnected device, existing callers of the user read accessors
now behave as follows:

  - The sysfs "config" file and /proc/bus/pci ignore the return value
    and return the same ~0 data as before
  - pciconfig_read() fails with -EIO
  - VPD reads and VFIO config reads fail with -ENODEV instead of
    returning ~0 data
  - VPD writes fail with -ENODEV instead of timing out with a
    "firmware bug" warning
  - VFIO writes to PCI_COMMAND fail with -ENODEV, since VFIO reads the
    current value first

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Tested-by: Nikita Vetoshkin <nekto0n@meta.com>
---
 drivers/pci/access.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index b123da16b63b..18a206bb2a94 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -237,8 +237,11 @@ int pci_user_read_config_##size						\
 	raw_spin_lock_irq(&pci_lock);					\
 	if (unlikely(dev->block_cfg_access))				\
 		pci_wait_cfg(dev);					\
-	ret = dev->bus->ops->read(dev->bus, dev->devfn,			\
-				  pos, sizeof(type), &data);		\
+	if (pci_dev_is_disconnected(dev))				\
+		ret = PCIBIOS_DEVICE_NOT_FOUND;				\
+	else								\
+		ret = dev->bus->ops->read(dev->bus, dev->devfn,		\
+					  pos, sizeof(type), &data);	\
 	raw_spin_unlock_irq(&pci_lock);					\
 	if (ret)							\
 		PCI_SET_ERROR_RESPONSE(val);				\
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes
  2026-10-25  0:08 [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes Matthew Wood
  2026-10-25  0:08 ` [PATCH 1/5] PCI: Add disconnected device checks to user config reads Matthew Wood
@ 2026-10-25  0:08 ` Matthew Wood
  2026-10-25  0:52   ` sashiko-bot
  2026-10-26 15:00   ` Keith Busch
  2026-10-25  0:08 ` [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}() Matthew Wood
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Matthew Wood @ 2026-10-25  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, kbusch, Nikita Vetoshkin

secondary_bus_number and subordinate_bus_number read config space with
pci_read_config_byte(). The kernel config accessors don't check
block_cfg_access, so a read that races with a reset goes to the device
mid-reset and can fail or return a bogus bus number.

Use pci_user_read_config_byte() instead, as the "config" attribute does,
so the read waits for any pci_cfg_access_lock() holder (e.g.
pci_reset_function()) to finish.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Tested-by: Nikita Vetoshkin <nekto0n@meta.com>
---
 drivers/pci/pci-sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 6669e9ad5bd3..b37a285d717d 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -267,7 +267,7 @@ static ssize_t secondary_bus_number_show(struct device *dev,
 	int err;
 
 	pci_config_pm_runtime_get(pci_dev);
-	err = pci_read_config_byte(pci_dev, PCI_SECONDARY_BUS, &sec_bus);
+	err = pci_user_read_config_byte(pci_dev, PCI_SECONDARY_BUS, &sec_bus);
 	pci_config_pm_runtime_put(pci_dev);
 
 	if (err)
@@ -286,7 +286,7 @@ static ssize_t subordinate_bus_number_show(struct device *dev,
 	int err;
 
 	pci_config_pm_runtime_get(pci_dev);
-	err = pci_read_config_byte(pci_dev, PCI_SUBORDINATE_BUS, &sub_bus);
+	err = pci_user_read_config_byte(pci_dev, PCI_SUBORDINATE_BUS, &sub_bus);
 	pci_config_pm_runtime_put(pci_dev);
 
 	if (err)
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}()
  2026-10-25  0:08 [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes Matthew Wood
  2026-10-25  0:08 ` [PATCH 1/5] PCI: Add disconnected device checks to user config reads Matthew Wood
  2026-10-25  0:08 ` [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes Matthew Wood
@ 2026-10-25  0:08 ` Matthew Wood
  2026-10-25  0:51   ` sashiko-bot
  2026-10-26 15:20   ` Keith Busch
  2026-10-25  0:08 ` [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes Matthew Wood
  2026-10-25  0:08 ` [PATCH 5/5] PCI/sysfs: Use user config accessors for serial_number Matthew Wood
  4 siblings, 2 replies; 14+ messages in thread
From: Matthew Wood @ 2026-10-25  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, kbusch, Nikita Vetoshkin

pcie_capability_read_word() and pcie_capability_read_dword() use the
kernel config accessors which don't check block_cfg_access, so they
can't be used for reads on behalf of userspace that should wait for a
reset (or other pci_cfg_access_lock() holder) to finish.

Add pcie_capability_user_read_word() and
pcie_capability_user_read_dword() helpers, which provide the same function
using the user config accessors.

These are for use within the PCI core, so they are declared in
drivers/pci/pci.h and not exported.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Tested-by: Nikita Vetoshkin <nekto0n@meta.com>
---
 drivers/pci/access.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h    |  2 ++
 2 files changed, 52 insertions(+)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 18a206bb2a94..ff5a16236758 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -490,6 +490,56 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
 }
 EXPORT_SYMBOL(pcie_capability_read_dword);
 
+/*
+ * Same as pcie_capability_read_{word,dword}(), but using the user config
+ * accessors.  Returns 0 on success, negative values indicate error.
+ */
+int pcie_capability_user_read_word(struct pci_dev *dev, int pos, u16 *val)
+{
+	int ret;
+
+	*val = 0;
+	if (pos & 1)
+		return -EINVAL;
+
+	if (pcie_capability_reg_implemented(dev, pos)) {
+		ret = pci_user_read_config_word(dev, pci_pcie_cap(dev) + pos,
+						val);
+		if (ret)
+			*val = 0;
+		return ret;
+	}
+
+	if (pci_is_pcie(dev) && pcie_downstream_port(dev) &&
+	    pos == PCI_EXP_SLTSTA)
+		*val = PCI_EXP_SLTSTA_PDS;
+
+	return 0;
+}
+
+int pcie_capability_user_read_dword(struct pci_dev *dev, int pos, u32 *val)
+{
+	int ret;
+
+	*val = 0;
+	if (pos & 3)
+		return -EINVAL;
+
+	if (pcie_capability_reg_implemented(dev, pos)) {
+		ret = pci_user_read_config_dword(dev, pci_pcie_cap(dev) + pos,
+						 val);
+		if (ret)
+			*val = 0;
+		return ret;
+	}
+
+	if (pci_is_pcie(dev) && pcie_downstream_port(dev) &&
+	    pos == PCI_EXP_SLTSTA)
+		*val = PCI_EXP_SLTSTA_PDS;
+
+	return 0;
+}
+
 int pcie_capability_write_word(struct pci_dev *dev, int pos, u16 val)
 {
 	if (pos & 1)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 77efa4c5c960..7c5a2ebc5706 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -128,6 +128,8 @@ extern struct mutex pci_rescan_remove_lock;
 bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
 bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
 bool pcie_cap_has_rtctl(const struct pci_dev *dev);
+int pcie_capability_user_read_word(struct pci_dev *dev, int pos, u16 *val);
+int pcie_capability_user_read_dword(struct pci_dev *dev, int pos, u32 *val);
 
 /* Standard Capability finder */
 /**
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes
  2026-10-25  0:08 [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes Matthew Wood
                   ` (2 preceding siblings ...)
  2026-10-25  0:08 ` [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}() Matthew Wood
@ 2026-10-25  0:08 ` Matthew Wood
  2026-10-25  0:52   ` sashiko-bot
  2026-10-26 15:21   ` Keith Busch
  2026-10-25  0:08 ` [PATCH 5/5] PCI/sysfs: Use user config accessors for serial_number Matthew Wood
  4 siblings, 2 replies; 14+ messages in thread
From: Matthew Wood @ 2026-10-25  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, kbusch, Nikita Vetoshkin

max_link_width, current_link_speed and current_link_width read the PCIe
Link Capabilities and Link Status registers with the kernel config
accessors. Those don't check block_cfg_access so a read that races
with a reset goes to the device mid-reset and can fail or return a bogus
value.

Use pcie_capability_user_read_{word,dword}() instead so the read waits for
any pci_cfg_access_lock() holder (e.g. pci_reset_function()) to finish. For
max_link_width, this means inlining the pcie_get_width_cap() logic
instead of creating an additional helper with this single calling location.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Tested-by: Nikita Vetoshkin <nekto0n@meta.com>
---
 drivers/pci/pci-sysfs.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index b37a285d717d..a272f8583a8f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -208,14 +208,16 @@ static ssize_t max_link_width_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	ssize_t ret;
+	u32 lnkcap;
 
 	/* We read PCI_EXP_LNKCAP, so we need the device to be accessible. */
 	pci_config_pm_runtime_get(pdev);
-	ret = sysfs_emit(buf, "%u\n", pcie_get_width_cap(pdev));
+	pcie_capability_user_read_dword(pdev, PCI_EXP_LNKCAP, &lnkcap);
 	pci_config_pm_runtime_put(pdev);
 
-	return ret;
+	return sysfs_emit(buf, "%u\n", lnkcap ?
+			  FIELD_GET(PCI_EXP_LNKCAP_MLW, lnkcap) :
+			  PCIE_LNK_WIDTH_UNKNOWN);
 }
 static DEVICE_ATTR_RO(max_link_width);
 
@@ -228,7 +230,8 @@ static ssize_t current_link_speed_show(struct device *dev,
 	enum pci_bus_speed speed;
 
 	pci_config_pm_runtime_get(pci_dev);
-	err = pcie_capability_read_word(pci_dev, PCI_EXP_LNKSTA, &linkstat);
+	err = pcie_capability_user_read_word(pci_dev, PCI_EXP_LNKSTA,
+					     &linkstat);
 	pci_config_pm_runtime_put(pci_dev);
 
 	if (err)
@@ -248,7 +251,8 @@ static ssize_t current_link_width_show(struct device *dev,
 	int err;
 
 	pci_config_pm_runtime_get(pci_dev);
-	err = pcie_capability_read_word(pci_dev, PCI_EXP_LNKSTA, &linkstat);
+	err = pcie_capability_user_read_word(pci_dev, PCI_EXP_LNKSTA,
+					     &linkstat);
 	pci_config_pm_runtime_put(pci_dev);
 
 	if (err)
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] PCI/sysfs: Use user config accessors for serial_number
  2026-10-25  0:08 [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes Matthew Wood
                   ` (3 preceding siblings ...)
  2026-10-25  0:08 ` [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes Matthew Wood
@ 2026-10-25  0:08 ` Matthew Wood
  2026-10-25  0:57   ` sashiko-bot
  4 siblings, 1 reply; 14+ messages in thread
From: Matthew Wood @ 2026-10-25  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, kbusch, Nikita Vetoshkin

serial_number_show() reads the device serial number with pci_get_dsn(),
which uses the kernel config accessors which don't check
block_cfg_access, so a read that races with a reset goes to the device
mid-reset and can fail with -EIO or report an all-ones serial number.

Do the capability walk and DSN reads with the user config accessors
instead, as the "config" attribute does, so each access waits for any
pci_cfg_access_lock() holder (e.g. pci_reset_function()) to finish.
Also wrap the reads in pci_config_pm_runtime_{get,put}() like the other
attributes that read config space.

A reset can still start between two accesses, but the next access just
waits for it to finish, and the DSN capability is unchanged by a reset.

Unlike pci_find_ext_capability(), the walk doesn't check dev->cfg_size
first. serial_number is only visible if pci_get_dsn() found the DSN
capability when the device was added, and cfg_size is final by then.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
Tested-by: Nikita Vetoshkin <nekto0n@meta.com>
---
 drivers/pci/pci-sysfs.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index a272f8583a8f..cbdd56b92d1a 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -696,10 +696,23 @@ static ssize_t serial_number_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
-	u64 dsn;
+	u32 dword;
+	u64 dsn = 0;
 	u8 bytes[8];
+	int pos;
+
+	pci_config_pm_runtime_get(pci_dev);
+	pos = PCI_FIND_NEXT_EXT_CAP(pci_user_read_config, 0,
+				    PCI_EXT_CAP_ID_DSN, NULL, pci_dev);
+	if (pos) {
+		pos += 4;
+		pci_user_read_config_dword(pci_dev, pos, &dword);
+		dsn = (u64)dword;
+		pci_user_read_config_dword(pci_dev, pos + 4, &dword);
+		dsn |= ((u64)dword) << 32;
+	}
+	pci_config_pm_runtime_put(pci_dev);
 
-	dsn = pci_get_dsn(pci_dev);
 	if (!dsn)
 		return -EIO;
 
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}()
  2026-10-25  0:08 ` [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}() Matthew Wood
@ 2026-10-25  0:51   ` sashiko-bot
  2026-10-26 15:20   ` Keith Busch
  1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-25  0:51 UTC (permalink / raw)
  To: Matthew Wood; +Cc: linux-pci

> pcie_capability_read_word() and pcie_capability_read_dword() use the
> kernel config accessors which don't check block_cfg_access, so they
> can't be used for reads on behalf of userspace that should wait for a
> reset (or other pci_cfg_access_lock() holder) to finish.
> 
> Add pcie_capability_user_read_word() and
> pcie_capability_user_read_dword() helpers, which provide the same function
> using the user config accessors.
> 
> These are for use within the PCI core, so they are declared in
> drivers/pci/pci.h and not exported.
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025000836.58294-1-thepacketgeek@gmail.com?part=3


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes
  2026-10-25  0:08 ` [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes Matthew Wood
@ 2026-10-25  0:52   ` sashiko-bot
  2026-10-26 15:21   ` Keith Busch
  1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-25  0:52 UTC (permalink / raw)
  To: Matthew Wood; +Cc: linux-pci

> max_link_width, current_link_speed and current_link_width read the PCIe
> Link Capabilities and Link Status registers with the kernel config
> accessors. Those don't check block_cfg_access so a read that races
> with a reset goes to the device mid-reset and can fail or return a bogus
> value.
> 
> Use pcie_capability_user_read_{word,dword}() instead so the read waits for
> any pci_cfg_access_lock() holder (e.g. pci_reset_function()) to finish. For
> max_link_width, this means inlining the pcie_get_width_cap() logic
> instead of creating an additional helper with this single calling location.
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025000836.58294-1-thepacketgeek@gmail.com?part=4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes
  2026-10-25  0:08 ` [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes Matthew Wood
@ 2026-10-25  0:52   ` sashiko-bot
  2026-10-26 15:00   ` Keith Busch
  1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-25  0:52 UTC (permalink / raw)
  To: Matthew Wood; +Cc: linux-pci

> secondary_bus_number and subordinate_bus_number read config space with
> pci_read_config_byte(). The kernel config accessors don't check
> block_cfg_access, so a read that races with a reset goes to the device
> mid-reset and can fail or return a bogus bus number.
> 
> Use pci_user_read_config_byte() instead, as the "config" attribute does,
> so the read waits for any pci_cfg_access_lock() holder (e.g.
> pci_reset_function()) to finish.
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025000836.58294-1-thepacketgeek@gmail.com?part=2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] PCI: Add disconnected device checks to user config reads
  2026-10-25  0:08 ` [PATCH 1/5] PCI: Add disconnected device checks to user config reads Matthew Wood
@ 2026-10-25  0:53   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-25  0:53 UTC (permalink / raw)
  To: Matthew Wood; +Cc: linux-pci

> Since commit 4b1038834739 ("PCI: Don't attempt config access to
> disconnected devices") the kernel config read accessors skip devices
> marked disconnected, returning PCIBIOS_DEVICE_NOT_FOUND and ~0 data.
> The user config read accessors still issue the read, and if the host
> bridge completes it with ~0 data and no error, they report success.
> 
> Check pci_dev_is_disconnected() in pci_user_read_config_*() as well,
> failing with -ENODEV and ~0 data. Do the check after waiting for any
> pci_cfg_access_lock() holder so a reader blocked behind a reset also
> sees a device that was disconnected while it waited.
> 
> This prepares for converting sysfs attributes from the kernel
> accessors to the user accessors so they keep skipping disconnected
> devices.
> 
> On a disconnected device, existing callers of the user read accessors
> now behave as follows:
> 
>   - The sysfs "config" file and /proc/bus/pci ignore the return value
>     and return the same ~0 data as before
>   - pciconfig_read() fails with -EIO
>   - VPD reads and VFIO config reads fail with -ENODEV instead of
>     returning ~0 data
>   - VPD writes fail with -ENODEV instead of timing out with a
>     "firmware bug" warning
>   - VFIO writes to PCI_COMMAND fail with -ENODEV, since VFIO reads the
>     current value first
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025000836.58294-1-thepacketgeek@gmail.com?part=1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] PCI/sysfs: Use user config accessors for serial_number
  2026-10-25  0:08 ` [PATCH 5/5] PCI/sysfs: Use user config accessors for serial_number Matthew Wood
@ 2026-10-25  0:57   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-25  0:57 UTC (permalink / raw)
  To: Matthew Wood; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI/sysfs: serial_number_show() ignores pci_user_read_config_dword() errors
--

commit c9ac0517dd6b036d6799a82dae1da82995d72a99
Author: Matthew Wood <thepacketgeek@gmail.com>

PCI/sysfs: Use user config accessors for serial_number

This commit updates serial_number_show() to walk capabilities and read the
Device Serial Number using pci_user_read_config_dword() instead of
pci_get_dsn(), wrapped in pci_config_pm_runtime_get() and
pci_config_pm_runtime_put(), so config reads wait for in-progress device
resets.

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index a272f8583a8fe..cbdd56b92d1aa 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -696,10 +696,23 @@ static ssize_t serial_number_show(struct device *dev,
>  				  struct device_attribute *attr, char *buf)
>  {
>  	struct pci_dev *pci_dev = to_pci_dev(dev);
> -	u64 dsn;
> +	u32 dword;
> +	u64 dsn = 0;
>  	u8 bytes[8];
> +	int pos;
> +
> +	pci_config_pm_runtime_get(pci_dev);
> +	pos = PCI_FIND_NEXT_EXT_CAP(pci_user_read_config, 0,
> +				    PCI_EXT_CAP_ID_DSN, NULL, pci_dev);
> +	if (pos) {
> +		pos += 4;
> +		pci_user_read_config_dword(pci_dev, pos, &dword);
> +		dsn = (u64)dword;
> +		pci_user_read_config_dword(pci_dev, pos + 4, &dword);
> +		dsn |= ((u64)dword) << 32;
> +	}

[Severity: Medium]
What happens if either of the pci_user_read_config_dword() calls in
serial_number_show() fails after PCI_FIND_NEXT_EXT_CAP() locates the DSN
capability?

Each pci_user_read_config_dword() call independently acquires and releases
pci_lock, so a surprise removal / hot-unplug (marking the device
disconnected) or a PCI bus read error can occur between finding the
capability and reading the two DSN dwords.

In drivers/pci/access.c, PCI_USER_READ_CONFIG() sets the output value to all
ones on failure:

drivers/pci/access.c:PCI_USER_READ_CONFIG() {
	...
	if (pci_dev_is_disconnected(dev))
		ret = PCIBIOS_DEVICE_NOT_FOUND;
	else
		ret = dev->bus->ops->read(dev->bus, dev->devfn,
					  pos, sizeof(type), &data);
	raw_spin_unlock_irq(&pci_lock);
	if (ret)
		PCI_SET_ERROR_RESPONSE(val);
	else
		*val = (type)data;

	return pcibios_err_to_errno(ret);
}

Because the return values of pci_user_read_config_dword() are not checked in
serial_number_show(), a failed read sets dword to 0xffffffff and folds it
into dsn. That makes dsn non-zero, bypassing the subsequent !dsn check and
emitting an all-ones (ff-ff-ff-ff-ff-ff-ff-ff) or partially all-ones serial
number to userspace instead of returning an error.

Should serial_number_show() check the return values of both
pci_user_read_config_dword() calls before constructing dsn?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025000836.58294-1-thepacketgeek@gmail.com?part=5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes
  2026-10-25  0:08 ` [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes Matthew Wood
  2026-10-25  0:52   ` sashiko-bot
@ 2026-10-26 15:00   ` Keith Busch
  1 sibling, 0 replies; 14+ messages in thread
From: Keith Busch @ 2026-10-26 15:00 UTC (permalink / raw)
  To: Matthew Wood; +Cc: Bjorn Helgaas, linux-pci, linux-kernel, Nikita Vetoshkin

On Sat, Oct 24, 2026 at 05:08:30PM -0700, Matthew Wood wrote:
> secondary_bus_number and subordinate_bus_number read config space with
> pci_read_config_byte(). The kernel config accessors don't check
> block_cfg_access, so a read that races with a reset goes to the device
> mid-reset and can fail or return a bogus bus number.
> 
> Use pci_user_read_config_byte() instead, as the "config" attribute does,
> so the read waits for any pci_cfg_access_lock() holder (e.g.
> pci_reset_function()) to finish.

Looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}()
  2026-10-25  0:08 ` [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}() Matthew Wood
  2026-10-25  0:51   ` sashiko-bot
@ 2026-10-26 15:20   ` Keith Busch
  1 sibling, 0 replies; 14+ messages in thread
From: Keith Busch @ 2026-10-26 15:20 UTC (permalink / raw)
  To: Matthew Wood; +Cc: Bjorn Helgaas, linux-pci, linux-kernel, Nikita Vetoshkin

On Sat, Oct 24, 2026 at 05:08:31PM -0700, Matthew Wood wrote:
> pcie_capability_read_word() and pcie_capability_read_dword() use the
> kernel config accessors which don't check block_cfg_access, so they
> can't be used for reads on behalf of userspace that should wait for a
> reset (or other pci_cfg_access_lock() holder) to finish.
> 
> Add pcie_capability_user_read_word() and
> pcie_capability_user_read_dword() helpers, which provide the same function
> using the user config accessors.
> 
> These are for use within the PCI core, so they are declared in
> drivers/pci/pci.h and not exported.

Looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes
  2026-10-25  0:08 ` [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes Matthew Wood
  2026-10-25  0:52   ` sashiko-bot
@ 2026-10-26 15:21   ` Keith Busch
  1 sibling, 0 replies; 14+ messages in thread
From: Keith Busch @ 2026-10-26 15:21 UTC (permalink / raw)
  To: Matthew Wood; +Cc: Bjorn Helgaas, linux-pci, linux-kernel, Nikita Vetoshkin

On Sat, Oct 24, 2026 at 05:08:32PM -0700, Matthew Wood wrote:
> max_link_width, current_link_speed and current_link_width read the PCIe
> Link Capabilities and Link Status registers with the kernel config
> accessors. Those don't check block_cfg_access so a read that races
> with a reset goes to the device mid-reset and can fail or return a bogus
> value.
> 
> Use pcie_capability_user_read_{word,dword}() instead so the read waits for
> any pci_cfg_access_lock() holder (e.g. pci_reset_function()) to finish. For
> max_link_width, this means inlining the pcie_get_width_cap() logic
> instead of creating an additional helper with this single calling location.

Looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-10-26 15:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25  0:08 [PATCH 0/5] PCI/sysfs: Wait for config access lock in sysfs attributes Matthew Wood
2026-10-25  0:08 ` [PATCH 1/5] PCI: Add disconnected device checks to user config reads Matthew Wood
2026-10-25  0:53   ` sashiko-bot
2026-10-25  0:08 ` [PATCH 2/5] PCI/sysfs: Use user config accessors for bus number attributes Matthew Wood
2026-10-25  0:52   ` sashiko-bot
2026-10-26 15:00   ` Keith Busch
2026-10-25  0:08 ` [PATCH 3/5] PCI: Add pcie_capability_user_read_{word,dword}() Matthew Wood
2026-10-25  0:51   ` sashiko-bot
2026-10-26 15:20   ` Keith Busch
2026-10-25  0:08 ` [PATCH 4/5] PCI/sysfs: Use user config accessors for link attributes Matthew Wood
2026-10-25  0:52   ` sashiko-bot
2026-10-26 15:21   ` Keith Busch
2026-10-25  0:08 ` [PATCH 5/5] PCI/sysfs: Use user config accessors for serial_number Matthew Wood
2026-10-25  0:57   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.