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: <649c2bb2-d9a3-66ce-8bc5-2735195aaa5e@linux.intel.com>
Date: Mon, 19 May 2025 16:21:32 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
cc: Bjorn Helgaas <bhelgaas@...gle.com>, Jingoo Han <jingoohan1@...il.com>, 
    Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, 
    Lorenzo Pieralisi <lpieralisi@...nel.org>, 
    Krzysztof Wilczyński <kw@...ux.com>, 
    Rob Herring <robh@...nel.org>, Johannes Berg <johannes@...solutions.net>, 
    Jeff Johnson <jjohnson@...nel.org>, Bartosz Golaszewski <brgl@...ev.pl>, 
    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 v3 04/11] PCI/ASPM: Clear aspm_disable as part of
 __pci_enable_link_state()

On Mon, 19 May 2025, Krishna Chaitanya Chundru wrote:

> If a driver wants to enable ASPM back after disabling ASPM for some
> usecase, it is not being enabled properly because of the aspm_disable
> flag is not getting cleared. This flag is being properly when aspm
> is controlled by sysfs.

This sentence has broken grammar/is missing something?

aspm -> ASPM

> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> ---
>  drivers/pci/pcie/aspm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 94324fc0d3e650cd3ca2c0bb8c1895ca7e647b9d..0f858ef86111b43328bc7db01e6493ce67178458 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -1453,6 +1453,7 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
>  		down_read(&pci_bus_sem);
>  	mutex_lock(&aspm_lock);
>  	link->aspm_default = pci_calc_aspm_enable_mask(state);
> +	link->aspm_disable &= ~state;
>  	pcie_config_aspm_link(link, policy_to_aspm_state(link));
>  
>  	link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;

I disagree with this change.

The problem currently with ASPM driver is that pci_disable_link_state() 
and pci_enable_link_state() are not symmetric pairs despite their 
misleading names. pci_enable_link_state() should be renamed to 
pci_set_default_link_state() and if the symmetric pair is needed for 
pci_disable_link_state(), it would have to be added separately.

I've some (rotting) patches which try to do that, in case you want to try 
to solve this inconsistency in the ASPM driver (I can send them to you)?

-- 
 i.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ