linux-kernel - Re: [PATCH v5] 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: <ZwWxQQgPSlVa3QH_@surfacebook.localdomain>
Date: Wed, 9 Oct 2024 01:25:05 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Krishna chaitanya chundru <quic_krichai@...cinc.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	manivannan.sadhasivam@...aro.org, Markus.Elfring@....de,
	quic_mrana@...cinc.com, rafael@...nel.org, linux-pm@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	quic_vbadigan@...cinc.com, quic_ramkri@...cinc.com
Subject: Re: [PATCH v5] PCI: Enable runtime pm of the host bridge

Thu, Oct 03, 2024 at 05:10:33PM -0500, Bjorn Helgaas kirjoitti:
> On Thu, Oct 03, 2024 at 11:32:32AM +0530, Krishna chaitanya chundru wrote:
> > The Controller driver is the parent device of the PCIe host bridge,
> > PCI-PCI bridge and PCIe endpoint as shown below.
> > 
> >         PCIe controller(Top level parent & parent of host bridge)
> >                         |
> >                         v
> >         PCIe Host bridge(Parent of PCI-PCI bridge)
> >                         |
> >                         v
> >         PCI-PCI bridge(Parent of endpoint driver)
> >                         |
> >                         v
> >                 PCIe endpoint driver
> > 
> > Now, when the controller device goes to runtime suspend, PM framework
> > will check the runtime PM state of the child device (host bridge) and
> > will find it to be disabled. So it will allow the parent (controller
> > device) to go to runtime suspend. Only if the child device's state was
> > 'active' it will prevent the parent to get suspended.
> > 
> > It is a property of the runtime PM framework that it can only
> > follow continuous dependency chains.  That is, if there is a device
> > with runtime PM disabled in a dependency chain, runtime PM cannot be
> > enabled for devices below it and above it in that chain both at the
> > same time.
> > 
> > Since runtime PM is disabled for host bridge, the state of the child
> > devices under the host bridge is not taken into account by PM framework
> > for the top level parent, PCIe controller. So PM framework, allows
> > the controller driver to enter runtime PM irrespective of the state
> > of the devices under the host bridge. And this causes the topology
> > breakage and also possible PM issues like controller driver goes to
> > runtime suspend while endpoint driver is doing some transfers.
> > 
> > Because of the above, in order to enable runtime PM for a PCIe
> > controller device, one needs to ensure that runtime PM is enabled for
> > all devices in every dependency chain between it and any PCIe endpoint
> > (as runtime PM is enabled for PCIe endpoints).
> > 
> > This means that runtime PM needs to be enabled for the host bridge
> > device, which is present in all of these dependency chains.
> > 
> > After this change, the host bridge device will be runtime-suspended
> > by the runtime PM framework automatically after suspending its last
> > child and it will be runtime-resumed automatically before resuming its
> > first child which will allow the runtime PM framework to track
> > dependencies between the host bridge device and all of its
> > descendants.
> 
> Applied to pci/pm for v6.13, thanks for your patience is working
> through this!

...

> > +	pm_runtime_set_active(&bridge->dev);
> > +	pm_runtime_no_callbacks(&bridge->dev);
> > +	devm_pm_runtime_enable(&bridge->dev);

So, potentially this might lead to a case where PM runtime is enabled and won't
be disabled on the bridge device removal. Is this a problem?

TL;DR:
I have found no explanations why error code from devm_*() call is being ignored.

> >  	return 0;


-- 
With Best Regards,
Andy Shevchenko