linux-kernel - Re: [PATCH] PCI/sysfs: Ensure devices are powered for config reads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aMHE3hmnGTYBrK0E@google.com>
Date: Wed, 10 Sep 2025 11:35:10 -0700
From: Brian Norris <briannorris@...omium.org>
To: Ethan Zhao <etzhao1900@...il.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] PCI/sysfs: Ensure devices are powered for config reads

On Fri, Aug 22, 2025 at 09:11:25AM +0800, Ethan Zhao wrote:
> On 8/21/2025 11:28 PM, Brian Norris wrote:
> > On Thu, Aug 21, 2025 at 08:41:28PM +0800, Ethan Zhao wrote:
> > > Hold a PM reference by pci_config_pm_runtime_get() and then write some
> > > data to the PCIe config space, no objection.
> > > 
> > > To know about the linkspeed etc capabilities/not status, how about
> > > creating a cached version of these caps, no need to change their
> > > power state.
> > 
> > For static values like the "max" attributes, maybe that's fine.
> > 
> > But Linux is not always the one changing the link speed. I've seen PCI
> > devices that autonomously request link-speed changes, and AFAICT, the
> > only way we'd know in host software is to go reread the config
> > registers. So caching just produces cache invalidation problems.
> Maybe you meant the link-speed status, that would be volatile based on
> link retraining.

Yes.

> Here we are talking about some non-volatile capabilities value no
> invalidation needed to their cached variables.

I missed the "not status" part a few lines up.

So yes, I agree it's possible to make some of these (but not all) use a
cache. I could perhaps give that a shot, if it's acknowledged that the
non-cacheable attributes are worth fixing.

>
> > > If there is aggressive power saving requirement, and the polling
> > > of these caps will make up wakeup/poweron bugs.
> > 
> > If you're worried about wakeup frequency, I think that's a matter of
> > user space / system administraction to decide -- if it doesn't want to
> > potentially wake up the link, it shouldn't be poking at config-based
> IMHO, sysfs interface is part of KABI, you change its behavior , you
> definitely would break some running binaries. there is alternative
> way to avoid re-cooking binaries or waking up administrator to modify
> their configuration/script in the deep night. you already got it.

That's not how KABI works. Just because there's a potentially-observable
difference doesn't mean we're "breaking" the ABI. You'd have to
demonstrate an actual use case that is breaking. I don't see how it's
"broken" to wake up a device when the API is asking for a value that can
only be retrieved while awake. Sure, it's potentially a small change in
power consumption, but that can apply to almost any kind of change.

My claim is that this is a currently broken area, and that it is
impossible to use these interfaces on a system that aggressively enters
D3cold. If a system observes any difference from this change, then it
was broken before. Bugfixes are not inherently KABI breakages just
because they can be observed.

Brian