linux-kernel - Re: [PATCH v4] PCI/PM: Prevent runtime suspend before devices are fully initialized

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: <aWrjhqC_6I2UNXC5@google.com>
Date: Fri, 16 Jan 2026 17:19:02 -0800
From: Brian Norris <briannorris@...omium.org>
To: Marek Szyprowski <m.szyprowski@...sung.com>,
	"Rafael J . Wysocki" <rafael@...nel.org>
Cc: Bjorn Helgaas <helgaas@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Lukas Wunner <lukas@...ner.de>, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-pm@...r.kernel.org,
	"Rafael J . Wysocki" <rafael@...nel.org>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v4] PCI/PM: Prevent runtime suspend before devices are
 fully initialized

Hi Marek,

On Thu, Jan 15, 2026 at 12:14:49PM +0100, Marek Szyprowski wrote:
> On 14.01.2026 21:10, Brian Norris wrote:
> > On Wed, Jan 14, 2026 at 10:46:41AM +0100, Marek Szyprowski wrote:
> >> On 06.01.2026 23:27, Bjorn Helgaas wrote:
> >>> On Thu, Oct 23, 2025 at 02:09:01PM -0700, Brian Norris wrote:
> >>>> Today, it's possible for a PCI device to be created and
> >>>> runtime-suspended before it is fully initialized. When that happens, the
> >>>> device will remain in D0, but the suspend process may save an
> >>>> intermediate version of that device's state -- for example, without
> >>>> appropriate BAR configuration. When the device later resumes, we'll
> >>>> restore invalid PCI state and the device may not function.
> >>>>
> >>>> Prevent runtime suspend for PCI devices by deferring pm_runtime_enable()
> >>>> until we've fully initialized the device.
> > ...
> >> This patch landed recently in linux-next as commit c796513dc54e
> >> ("PCI/PM: Prevent runtime suspend until devices are fully initialized").
> >> In my tests I found that it sometimes causes the "pci 0000:01:00.0:
> >> runtime PM trying to activate child device 0000:01:00.0 but parent
> >> (0000:00:00.0) is not active" warning on Qualcomm Robotics RB5 board
> >> (arch/arm64/boot/dts/qcom/qrb5165-rb5.dts). This in turn causes a
> >> lockdep warning about console lock, but this is just a consequence of
> >> the runtime pm warning. Reverting $subject patch on top of current
> >> linux-next hides this warning.
> >>
> >> Here is a kernel log:
> >>
> >> pci 0000:01:00.0: [17cb:1101] type 00 class 0xff0000 PCIe Endpoint
> >> pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x000fffff 64bit]
> >> pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
> >> pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 5.0
> >> GT/s PCIe x1 link at 0000:00:00.0 (capable of 7.876 Gb/s with 8.0 GT/s
> >> PCIe x1 link)
> >> pci 0000:01:00.0: Adding to iommu group 13
> >> pci 0000:01:00.0: ASPM: default states L0s L1
> >> pcieport 0000:00:00.0: bridge window [mem 0x60400000-0x604fffff]: assigned
> >> pci 0000:01:00.0: BAR 0 [mem 0x60400000-0x604fffff 64bit]: assigned
> >> pci 0000:01:00.0: runtime PM trying to activate child device
> >> 0000:01:00.0 but parent (0000:00:00.0) is not active
> > Thanks for the report. I'll try to look at reproducing this, or at least
> > getting a better mental model of exactly why this might fail (or,
> > "warn") this way. But if you have the time and desire to try things out
> > for me, can you give v1 a try?
> >
> > https://lore.kernel.org/all/20251016155335.1.I60a53c170a8596661883bd2b4ef475155c7aa72b@changeid/
> >
> > I'm pretty sure it would not invoke the same problem.
> 
> Right, this one works fine.
> 
> > I also suspect v3
> > might not, but I'm less sure:
> >
> > https://lore.kernel.org/all/20251022141434.v3.1.I60a53c170a8596661883bd2b4ef475155c7aa72b@changeid/
> This one too, at least I was not able to reproduce any fail.

Thanks for testing. I'm still not sure exactly how to reproduce your
failure, but it seems as if the root port is being allowed to suspend
before the endpoint is added to the system, and it remains so while the
endpoint is about to probe. device_initial_probe() will be OK with
respect to PM, since it will wake up the port if needed. But this
particular code is not OK, since it doesn't ensure the parent device is
active while preparing the endpoint power state.

I suppose one way to "solve" that is (untested):

--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -380,8 +380,12 @@ void pci_bus_add_device(struct pci_dev *dev)
 		put_device(&pdev->dev);
 	}
 
+	if (dev->dev.parent)
+		pm_runtime_get_sync(dev->dev.parent);
 	pm_runtime_set_active(&dev->dev);
 	pm_runtime_enable(&dev->dev);
+	if (dev->dev.parent)
+		pm_runtime_put(dev->dev.parent);
 
 	if (!dn || of_device_is_available(dn))
 		pci_dev_allow_binding(dev);

Personally, I'm more inclined to go back to v1, since it prepares the
runtime PM status when the device is first discovered. That way, its
ancestors are still active, avoiding these sorts of problems. I'm
frankly not sure of all the reasons Rafael recommended I make the
v1->v3->v4 changes, and now that they cause problems, I'm inclined to
question them again.

Rafael, do you have any thoughts?

Brian