linux-kernel - Re: [PATCH] PCI/PM: Avoid redundant delays on D3hot->D3cold

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251006135222.GD2912318@black.igk.intel.com>
Date: Mon, 6 Oct 2025 15:52:22 +0200
From: Mika Westerberg <mika.westerberg@...ux.intel.com>
To: Brian Norris <briannorris@...omium.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	Brian Norris <briannorris@...gle.com>, stable@...r.kernel.org
Subject: Re: [PATCH] PCI/PM: Avoid redundant delays on D3hot->D3cold

Hi,

On Fri, Oct 03, 2025 at 03:40:09PM -0700, Brian Norris wrote:
> From: Brian Norris <briannorris@...gle.com>
> 
> When transitioning to D3cold, __pci_set_power_state() will first
> transition a device to D3hot. If the device was already in D3hot, this
> will add excess work:
> (a) read/modify/write PMCSR; and
> (b) excess delay (pci_dev_d3_sleep()).

How come the device is already in D3hot when __pci_set_power_state() is
called? IIRC PCI core will transition the device to low power state so that
it passes there the deepest possible state, and at that point the device is
still in D0. Then __pci_set_power_state() puts it into D3hot and then turns
if the power resource -> D3cold.

What I'm missing here?

> For (b), we already performed the necessary delay on the previous D3hot
> entry; this was extra noticeable when evaluating runtime PM transition
> latency.
> 
> Check whether we're already in the target state before continuing.
> 
> Note that __pci_set_power_state() already does this same check for other
> state transitions, but D3cold is special because __pci_set_power_state()
> converts it to D3hot for the purposes of PMCSR.
> 
> This seems to be an oversight in commit 0aacdc957401 ("PCI/PM: Clean up
> pci_set_low_power_state()").
> 
> Fixes: 0aacdc957401 ("PCI/PM: Clean up pci_set_low_power_state()")
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Brian Norris <briannorris@...gle.com>
> Signed-off-by: Brian Norris <briannorris@...omium.org>

BTW, I think only one SoB from you is enough ;-)