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

Message-ID: <20250124051509.djzjoml2zcq2xvvz@thinkpad>
Date: Fri, 24 Jan 2025 10:45:09 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Johan Hovold <johan@...nel.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 Tue, Jan 21, 2025 at 02:18:49PM +0100, Johan Hovold wrote:
> 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.
> 

There are multiple pieces needed to be stitch together to enable runtime PM in
the PCIe topology. And each one of them seemed to be controversial enough (due
to common code etc...). So that's the reason they were sent separately. Though
I must admit that the full picture and the limitation of not being able to
exercise the runtime PM should've been mentioned.

> 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.
> 

Sure. I've pinged Ulf offline and he promised to look into it asap.

- Mani

-- 
மணிவண்ணன் சதாசிவம்