[PATCH] PCI/sysfs: Avoid spurious runtime PM wakeup on config space accesses

[PATCH] PCI/sysfs: Avoid spurious runtime PM wakeup on config space accesses

[Krzysztof Wilczyński]

Currently, the boundary checks in pci_read_config() and
pci_write_config() reject only offsets beyond the effective
configuration space size.

Thus, an access at an offset exactly equal to that size passes
the check, has its length clamped to zero, and then invokes
pci_config_pm_runtime_get() and pci_config_pm_runtime_put()
around transfer blocks that do nothing.

This is a problem because pci_config_pm_runtime_get() synchronously
resumes the upstream bridge through pm_runtime_get_sync(), and resumes
the device itself through pm_runtime_resume() when it is in D3cold,
only for the handler to return zero immediately afterwards.  Such a
spurious wakeup wastes power and adds needless resume latency.

The sysfs core already clamps accesses against the attribute size set
through the bin_size() callback, which reports either 256 or 4096
bytes.  As such, the affected accesses are reads at offset 64 (or 128
for CardBus devices) through files opened without CAP_SYS_ADMIN, and
reads and writes at the exact configuration space size on devices
where a quirk sets a non-standard size.

Therefore, reject accesses at the boundary offset as well, so that
they return early before any runtime PM involvement, matching the
procfs implementations in proc_bus_pci_read() and proc_bus_pci_write().

The value returned to userspace at these offsets remains zero,
so the change is not visible to userspace.

Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
---
 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 5ec0b245a69b..a4f6559c619c 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -718,7 +718,7 @@ static ssize_t pci_read_config(struct file *filp, struct kobject *kobj,
 	else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
 		size = 128;
 
-	if (off > size)
+	if (off >= size)
 		return 0;
 	if (off + count > size) {
 		size -= off;
@@ -799,7 +799,7 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
 		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
 	}
 
-	if (off > dev->cfg_size)
+	if (off >= dev->cfg_size)
 		return 0;
 	if (off + count > dev->cfg_size) {
 		size = dev->cfg_size - off;
-- 
2.55.0

Re: [PATCH] PCI/sysfs: Avoid spurious runtime PM wakeup on config space accesses

[sashiko-bot]

> Currently, the boundary checks in pci_read_config() and
> pci_write_config() reject only offsets beyond the effective
> configuration space size.
> 
> Thus, an access at an offset exactly equal to that size passes
> the check, has its length clamped to zero, and then invokes
> pci_config_pm_runtime_get() and pci_config_pm_runtime_put()
> around transfer blocks that do nothing.
> 
> This is a problem because pci_config_pm_runtime_get() synchronously
> resumes the upstream bridge through pm_runtime_get_sync(), and resumes
> the device itself through pm_runtime_resume() when it is in D3cold,
> only for the handler to return zero immediately afterwards.  Such a
> spurious wakeup wastes power and adds needless resume latency.
> 
> The sysfs core already clamps accesses against the attribute size set
> through the bin_size() callback, which reports either 256 or 4096
> bytes.  As such, the affected accesses are reads at offset 64 (or 128
> for CardBus devices) through files opened without CAP_SYS_ADMIN, and
> reads and writes at the exact configuration space size on devices
> where a quirk sets a non-standard size.
> 
> Therefore, reject accesses at the boundary offset as well, so that
> they return early before any runtime PM involvement, matching the
> procfs implementations in proc_bus_pci_read() and proc_bus_pci_write().
> 
> The value returned to userspace at these offsets remains zero,
> so the change is not visible to userspace.
> 
> Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720204356.1501749-1-kwilczynski@kernel.org?part=1