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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aQ2EXqDvnxjyXq_7@wunner.de>
Date: Fri, 7 Nov 2025 06:32:14 +0100
From: Lukas Wunner <lukas@...ner.de>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: linux-pci@...r.kernel.org, Christian Zigotzky <chzigotzky@...osoft.de>,
	Manivannan Sadhasivam <mani@...nel.org>,
	mad skateman <madskateman@...il.com>,
	"R . T . Dickinson" <rtd2@...a.co.nz>,
	Darren Stevens <darren@...vens-zone.net>,
	John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
	luigi burdo <intermediadc@...mail.com>, Al <al@...azap.net>,
	Roland <rol7and@....com>, Hongxing Zhu <hongxing.zhu@....com>,
	hypexed@...oo.com.au, linuxppc-dev@...ts.ozlabs.org,
	debian-powerpc@...ts.debian.org, linux-kernel@...r.kernel.org,
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH 1/2] PCI/ASPM: Cache Link Capabilities so quirks can
 override them

On Thu, Nov 06, 2025 at 12:36:38PM -0600, Bjorn Helgaas wrote:
> Cache the PCIe Link Capabilities register in struct pci_dev so quirks can
> remove features to avoid hardware defects.  The idea is:
> 
>   - set_pcie_port_type() reads PCIe Link Capabilities and caches it in
>     dev->lnkcap
> 
>   - HEADER quirks can update the cached dev->lnkcap to remove advertised
>     features that don't work correctly
> 
>   - pcie_aspm_cap_init() relies on dev->lnkcap and ignores any features not
>     advertised there

I realize that memory is cheap, but it still feels a bit wasteful
to cache the entire 32-bit register wholesale.  It contains
reserved bits as of PCIe r7.0, various uninteresting bits and
portions of it are already cached elsewhere and thus now duplicated.
I'm wondering if it would make sense to instead only cache the ASPM bits
that are relevant here?  That's the approach we've followed so far.

You're initializing the link_active_reporting bit from the newly
cached lnkcap register, I'd prefer having a static inline instead
which extracts the bit from the cached register on demand,
thus obviating the need to have a duplicate cached copy of the bit.

pci_set_bus_speed() caches bus->max_bus_speed from the Link
Capabilities register and isn't converted by this patch to use
the cached register.  There are various others, e.g.
get_port_device_capability() in drivers/pci/pcie/portdrv.c
could also get PCI_EXP_LNKCAP_LBNC from the cached lnkcap
register.  Same for pcie_get_supported_speeds().  If the
intention is to convert these in a separate step in v6.19,
it would be good to mention that in the changelog.

Thanks,

Lukas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ