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: <kztudduots342zz7gnx3twtlvjm5pd7sde627zxcsbe6c2imqk@7mmp7vjght5j>
Date: Wed, 28 Jan 2026 08:26:03 -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 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.

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

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