linux-kernel - Re: [PATCH 1/3] PCI: host-common: Add shared D3cold eligibility helper for host bridges

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: <bz2dnldnx2z6o3jtm7hdy4f3azmd4ug2sq2pi7anuar7s7xssq@b7cqfkr7pd36>
Date: Thu, 29 Jan 2026 21:30:14 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: Jingoo Han <jingoohan1@...il.com>, 
	Manivannan Sadhasivam <mani@...nel.org>, Lorenzo Pieralisi <lpieralisi@...nel.org>, 
	Krzysztof Wilczyński <kwilczynski@...nel.org>, Rob Herring <robh@...nel.org>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Will Deacon <will@...nel.org>, linux-pci@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, jonathanh@...dia.com
Subject: Re: [PATCH 1/3] PCI: host-common: Add shared D3cold eligibility
 helper for host bridges

On Thu, Jan 29, 2026 at 11:08:57AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 1/28/2026 7:56 PM, Bjorn Andersson wrote:
> > On Wed, Jan 28, 2026 at 05:10:41PM +0530, Krishna Chaitanya Chundru wrote:
> > > Add a common helper, pci_host_common_can_enter_d3cold(), to determine
> > > whether a PCI host bridge can safely transition to D3cold.
> > Please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > 
> > It clearly says that you're supposed to start your commit message with a
> > description of the problem you're solving. In fact, even after reading
> > the entire commit message a few times I only know what the patch does,
> > but it's not clear to me why this patch exists.
> > 
> > > The helper walks all devices on the bridge's bus and only allows the
> > > host bridge to enter D3cold if all PCIe endpoints are already in
> > > PCI_D3hot.
> > The code below does walk the bus, but it doesn't allow/disallow anything
> > as far as I can tell, it queries their type, state, and if they are wake
> > capable?
> > 
> > > For devices that may wake the system, it additionally
> > > requires that the device supports PME wakeup from D3cold(with WAKE#).
> > > Devices without wakeup enabled are not restricted by this check and can
> > > be allowed to keep device in D3cold.
> > > 
> > Again, this code doesn't perform any action, it doesn't
> > allow/disallow/restrict the devices from doing anything, it merely
> > queries a bunch of things across all devices, and the commit message
> > fails to capture why this is useful.
> This is a helper function used by controller drivers, to know if the
> controller
> driver can keep in D3cold state or not. we use endpoint states and their
> wakeup
> capability support to determine d3cold can be supported or not. The return
> value
> of this function will tell controller drivers to know if we can allow D3cold
> or not.

Please make sure that the commit message captures this.

Regards,
Bjorn

> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> > > ---
> > >   drivers/pci/controller/pci-host-common.c | 29 +++++++++++++++++++++++++++++
> > >   drivers/pci/controller/pci-host-common.h |  2 ++
> > >   2 files changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > > index c473e7c03bacad2de07c798768f99652443431e0..225472c5ac82c6c5b44257d68a0fc503ec046ff1 100644
> > > --- a/drivers/pci/controller/pci-host-common.c
> > > +++ b/drivers/pci/controller/pci-host-common.c
> > > @@ -106,5 +106,34 @@ void pci_host_common_remove(struct platform_device *pdev)
> > >   }
> > >   EXPORT_SYMBOL_GPL(pci_host_common_remove);
> > > +static int pci_host_common_check_d3cold(struct pci_dev *pdev, void *userdata)
> > > +{
> > > +	bool *d3cold_allow = userdata;
> > > +
> > > +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
> > > +		return 0;
> > > +
> > > +	if (pdev->current_state != PCI_D3hot)
> > > +		goto exit;
> > > +
> > > +	if (device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold))
> > > +		goto exit;
> > > +
> > > +	return 0;
> > > +exit:
> > > +	*d3cold_allow = false;
> > > +	return -EBUSY;
> > > +}
> > > +
> > > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> > Please add kernel-doc for any EXPORT_SYMBOL() functions, so that it's
> > clear to the next guy what the API does.
> Initially, I had a change in my workspace which has kernel-doc, but after
> seeing this
> file I see none of the exported API's had a kernel-doc. Following it I
> dropped the kernel
> -doc at last minute. I will add this in v2.
> 
> - Krishna Chaitanya.
> > 
> > Regards,
> > Bjorn
> > 
> > > +{
> > > +	bool d3cold_allow = true;
> > > +
> > > +	pci_walk_bus(bridge->bus, pci_host_common_check_d3cold, &d3cold_allow);
> > > +
> > > +	return d3cold_allow;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_host_common_can_enter_d3cold);
> > > +
> > >   MODULE_DESCRIPTION("Common library for PCI host controller drivers");
> > >   MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> > > index b5075d4bd7eb31fbf1dc946ef1a6afd5afb5b3c6..18a731bca058828340bca84776d0e91da1edbbf7 100644
> > > --- a/drivers/pci/controller/pci-host-common.h
> > > +++ b/drivers/pci/controller/pci-host-common.h
> > > @@ -20,4 +20,6 @@ void pci_host_common_remove(struct platform_device *pdev);
> > >   struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
> > >   	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> > > +
> > > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge);
> > >   #endif
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > > 
>