linux-kernel - Re: [PATCH v7 2/2] PCI: Enable runtime pm of the host bridge

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: <Z4-eufq4M04XHjck@hovoldconsulting.com>
Date: Tue, 21 Jan 2025 14:18:49 +0100
From: Johan Hovold <johan@...nel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: Bjorn Helgaas <helgaas@...nel.org>,
	Krishna Chaitanya Chundru <quic_krichai@...cinc.com>,
	rafael@...nel.org, ulf.hansson@...aro.org,
	Kevin Xie <kevin.xie@...rfivetech.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Markus.Elfring@....de, quic_mrana@...cinc.com,
	m.szyprowski@...sung.com, linux-pm@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	regressions@...ts.linux.dev
Subject: Re: [PATCH v7 2/2] PCI: Enable runtime pm of the host bridge

On Mon, Jan 20, 2025 at 08:58:29PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jan 20, 2025 at 11:29:41AM +0100, Johan Hovold wrote:

> > I'd argue for reverting the offending commit as that is the only way to
> > make sure that the new warning is ever addressed.

> How come reverting becomes the *only* way to address the issue?

I didn't say it was the only way to address the issue, just that it's
the only way to make sure that the new warning gets addressed. Once code
is upstream, some vendors tend to lose interest.

> There seems to
> be nothing wrong with the commit in question and the same pattern in being used
> in other drivers as well. The issue looks to be in the PM core.

After taking a closer look now, I agree that the underlying issue seems
to be in PM core.

Possibly introduced by Rafael's commit 6e176bf8d461 ("PM: sleep: core:
Do not skip callbacks in the resume phase") which moved the set_active()
call from resume_noirq() to resume_early().

> Moreover, the warning is not causing any functional issue as far as I know. So
> just reverting the commit that took so much effort to get merged for the sake of
> hiding a warning doesn't feel right to me.

My point was simply that this commit introduced a new warning in 6.13,
and there is still no fix available. The code is also effectively dead,
well aside from triggering the warning, and runtime suspending the host
controller cannot even be tested with mainline yet (and this was
unfortunately not made clear in the commit message).

The change should have been part of a series that actually enabled the
functionality and not just a potential piece of it which cannot yet be
tested. Also, for Qualcomm controllers, we don't even yet have proper
suspend so it's probably not a good idea to throw runtime PM into the
mix there just yet.

But, sure, a revert would have made more sense last week. I guess you
have a few more weeks to address this now. We can always backport a
revert once rc1 is out.

Johan