linux-kernel - Re: [PATCH v4 06/11] PCI/ASPM: Clear aspm_disable as part of __pci_enable_link

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: <20250711230013.GA2309106@bhelgaas>
Date: Fri, 11 Jul 2025 18:00:13 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>,
	Manivannan Sadhasivam <mani@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jingoo Han <jingoohan1@...il.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Rob Herring <robh@...nel.org>, Jeff Johnson <jjohnson@...nel.org>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	linux-pci@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	linux-arm-msm@...r.kernel.org, mhi@...ts.linux.dev,
	linux-wireless@...r.kernel.org, ath11k@...ts.infradead.org,
	qiang.yu@....qualcomm.com, quic_vbadigan@...cinc.com,
	quic_vpernami@...cinc.com, quic_mrana@...cinc.com,
	Jeff Johnson <jeff.johnson@....qualcomm.com>
Subject: Re: [PATCH v4 06/11] PCI/ASPM: Clear aspm_disable as part of
 __pci_enable_link_state()

On Fri, Jul 11, 2025 at 04:38:48PM +0300, Ilpo Järvinen wrote:

> +++ b/include/linux/pci.h
> @@ -1826,8 +1826,8 @@ static inline int pcie_set_target_speed(struct pci_dev *port,
>  #ifdef CONFIG_PCIEASPM
>  int pci_disable_link_state(struct pci_dev *pdev, int state);
>  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
> -int pci_enable_link_state(struct pci_dev *pdev, int state);

AFAICT there's no caller of this at all.  Why do we keep it?

> -int pci_enable_link_state_locked(struct pci_dev *pdev, int state);

We only have two callers of this (pcie-qcom.c and vmd.c, both in
drivers/pci/), so it's not clear to me that it needs to be in
include/linux/pci.h.

I'm a little dubious about it in the first place since I don't think
drivers should be enabling ASPM states on their own, but pcie-qcom.c
and vmd.c are PCIe controller drivers, not PCI device drivers, so I
guess we can live with them for now.

IMO the "someday" goal should be that we get rid of aspm_policy and
enable all the available power saving states by default.  We have
sysfs knobs that administrators can use if necessary, and drivers or
quirks can disable states if they need to work around hardware
defects.

I think the compiled-in aspm_policy default and the module parameters
are basically chicken switches that only exist because aspm.c and some
devices aren't robust enough.

Bjorn