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: <604ffae3-1bfc-0922-b001-f3338880eb21@linux.intel.com>
Date: Fri, 11 Jul 2025 16:38:48 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
cc: 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, 11 Jul 2025, Krishna Chaitanya Chundru wrote:
> On 7/11/2025 2:51 PM, Ilpo Järvinen wrote:
> > On Fri, 11 Jul 2025, Manivannan Sadhasivam wrote:
> > > On Wed, Jul 09, 2025 at 06:01:22PM GMT, Krishna Chaitanya Chundru wrote:
> > > > On 7/9/2025 2:40 PM, Ilpo Järvinen wrote:

> > > > > 1) Renames pci_enable_link_state() to pci_set_default_link_state()
> > > > > 
> > > > > 1b) If pci_enable_link_state() is still needed after that, a new
> > > > > function
> > > > > is added to symmetrically pair with pci_disable_link_state().
> > > > > 
> > > > > or alternatively,
> > > > > 
> > > > > 2) Changelog justifies very clearly why this change is okay with the
> > > > > existing callers. (And obviously the function comment should be
> > > > > altered to
> > > > > match the functionality in that case too).
> > > > > 
> > > > > If approach 2 is chosen, it should be very carefully reviewed when it
> > > > > comes to the callers.
> > > > > 
> > > > I am in favor of approach 2 which you suggested, but lets wait for other
> > > > reviewers feedback on this. Based up on the response i will make
> > > > necessary changes in v5.
> > > > 
> > > 
> > > I would go for (1). It is always going to be a problem to change a legacy
> > > API
> > > like this. We might end up causing regressions. So it is safe to rename to
> > > reflect the purpose and try to come up with a new API that does what you
> > > want.
> > > If callers want to migrate to the new API, they can also do it in the
> > > future.
> > 
> > That's my recommendation as well.
> I will take this as learning, we will go with approach (1) as both of
> you are having valid points.
> 
> llpo,
> In the previous patch you said you have some patches on can you send me
> those or shall I proceed with patches from myside.

Sure, attached 3 patches.

Those were based on 6.11 and I've booted them along with my ASPM rework 
series (refactoring custom ASPM code in drivers to use the ASPM driver 
instaed). I quickly checked aspm.c fixes after that point and nothing 
seemed to require adaptations AFAICT.

-- 
 i.

View attachment "0001-PCI-ASPM-Rename-pci_enable_link_state-to-pci_set_def.patch" of type "text/x-diff" (7342 bytes)

View attachment "0002-PCI-ASPM-Improve-pci_set_default_link_state-_locked-.patch" of type "text/x-diff" (3769 bytes)

View attachment "0003-PCI-ASPM-Add-pci_enable_link_state.patch" of type "text/x-diff" (3428 bytes)