linux-kernel - Re: [PATCH v15 0/6] Refactor capability search into common macros

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] [day] [month] [year] [list]
Message-ID: <ea1e512f-55fb-4e20-a13c-1c5ebb0d14e1@163.com>
Date: Fri, 15 Aug 2025 06:21:11 +0800
From: Hans Zhang <18255117159@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: lpieralisi@...nel.org, kwilczynski@...nel.org, bhelgaas@...gle.com,
 jingoohan1@...il.com, mani@...nel.org, robh@...nel.org,
 ilpo.jarvinen@...ux.intel.com, schnelle@...ux.ibm.com, gbayer@...ux.ibm.com,
 lukas@...ner.de, arnd@...nel.org, geert@...ux-m68k.org,
 linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v15 0/6] Refactor capability search into common macros



On 2025/8/15 04:25, Bjorn Helgaas wrote:
> On Wed, Aug 13, 2025 at 10:45:23PM +0800, Hans Zhang wrote:
>> Dear Maintainers,
>>
>> This patch series addresses long-standing code duplication in PCI
>> capability discovery logic across the PCI core and controller drivers.
>> The existing implementation ties capability search to fully initialized
>> PCI device structures, limiting its usability during early controller
>> initialization phases where device/bus structures may not yet be
>> available.
>>
>> The primary goal is to decouple capability discovery from PCI device
>> dependencies by introducing a unified framework using config space
>> accessor-based macros. This enables:
>>
>> 1. Early Capability Discovery: Host controllers (e.g., Cadence, DWC)
>> can now perform capability searches during pre-initialization stages
>> using their native config accessors.
>>
>> 2. Code Consolidation: Common logic for standard and extended capability
>> searches is refactored into shared macros (`PCI_FIND_NEXT_CAP` and
>> `PCI_FIND_NEXT_EXT_CAP`), eliminating redundant implementations.
>>
>> 3. Safety and Maintainability: TTL checks are centralized within the
>> macros to prevent infinite loops, while hardcoded offsets in drivers
>> are replaced with dynamic discovery, reducing fragility.
>>
>> Key improvements include:
>> - Driver Conversions: DesignWare and Cadence drivers are migrated to
>>    use the new macros, removing device-specific assumptions and ensuring
>>    consistent error handling.
>>
>> - Enhanced Readability: Magic numbers are replaced with symbolic
>>    constants, and config space accessors are standardized for clarity.
>>
>> - Backward Compatibility: Existing PCI core behavior remains unchanged.
>>
>> ---
>> Dear Niklas and Gerd,
>>
>> Can you test this series of patches on the s390?
>>
>> Thank you very much.
>> ---
>>
>> ---
>> Changes since v14:
>> - Delete the first patch in the v14 series.
>> - The functions that can obtain the values of the registers u8/u16/u32 are
>>    directly called in PCI_FIND_NEXT_CAP() and PCI_FIND_NEXT_EXT_CAP().
>>    Use the pci_bus_read_config_byte/word/dword function directly.
>> - Delete dw_pcie_read_cfg and redefine three functions: dw_pcie_read_cfg_byte/word/dword.
>> - Delete cdns_pcie_read_cfg and redefine three functions: cdns_pcie_read_cfg_byte/word/dword.
>>
>> Changes since v13:
>> - Split patch 3/6 into two patches for searching standard and extended capability. (Bjorn)
>> - Optimize the code based on the review comments from Bjorn.
>> - Patch 5/7 and 6/7 use simplified macro definitions: PCI_FIND_NEXT_CAP(), PCI_FIND_NEXT_EXT_CAP().
>> - The other patches have not been modified.
>>
>> Changes since v12:
>> - Modify some commit messages, code format issues, and optimize the function return values.
>>
>> Changes since v11:
>> - Resolved some compilation warning.
>> - Add some include.
>> - Add the *** BLURB HERE *** description(Corrected by Mani and Krzysztof).
>>
>> Changes since v10:
>> - The patch [v10 2/6] remove #include <uapi/linux/pci_regs.h> and add macro definition comments.
>> - The patch [v10 3/6] remove #include <uapi/linux/pci_regs.h> and commit message were modified.
>> - The other patches have not been modified.
>>
>> Changes since v9:
>> - Resolved [v9 4/6] compilation error.
>>    The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses
>>    dw_pcie_find_next_ext_capability.
>> - The other patches have not been modified.
>>
>> Changes since v8:
>> - Split patch.
>> - The patch commit message were modified.
>> - Other patches(4/6, 5/6, 6/6) are unchanged.
>>
>> Changes since v7:
>> - Patch 2/5 and 3/5 compilation error resolved.
>> - Other patches are unchanged.
>>
>> Changes since v6:
>> - Refactor capability search into common macros.
>> - Delete pci-host-helpers.c and MAINTAINERS.
>>
>> Changes since v5:
>> - If you put the helpers in drivers/pci/pci.c, they unnecessarily enlarge
>>    the kernel's .text section even if it's known already at compile time
>>    that they're never going to be used (e.g. on x86).
>> - Move the API for find capabilitys to a new file called
>>    pci-host-helpers.c.
>> - Add new patch for MAINTAINERS.
>>
>> Changes since v4:
>> - Resolved [v4 1/4] compilation warning.
>> - The patch subject and commit message were modified.
>>
>> Changes since v3:
>> - Resolved [v3 1/4] compilation error.
>> - Other patches are not modified.
>>
>> Changes since v2:
>> - Add and split into a series of patches.
>> ---
>>
>> Hans Zhang (6):
>>    PCI: Clean up __pci_find_next_cap_ttl() readability
>>    PCI: Refactor capability search into PCI_FIND_NEXT_CAP()
>>    PCI: Refactor extended capability search into PCI_FIND_NEXT_EXT_CAP()
>>    PCI: dwc: Use PCI core APIs to find capabilities
>>    PCI: cadence: Use PCI core APIs to find capabilities
>>    PCI: cadence: Use cdns_pcie_find_*capability() to avoid hardcoding
>>      offsets
>>
>>   .../pci/controller/cadence/pcie-cadence-ep.c  | 38 +++++----
>>   drivers/pci/controller/cadence/pcie-cadence.c | 14 +++
>>   drivers/pci/controller/cadence/pcie-cadence.h | 39 +++++++--
>>   drivers/pci/controller/dwc/pcie-designware.c  | 77 ++---------------
>>   drivers/pci/controller/dwc/pcie-designware.h  | 21 +++++
>>   drivers/pci/pci.c                             | 76 +++--------------
>>   drivers/pci/pci.h                             | 85 +++++++++++++++++++
>>   include/uapi/linux/pci_regs.h                 |  3 +
>>   8 files changed, 194 insertions(+), 159 deletions(-)
> 
> I applied this on pci/capability-search for v6.18, thanks!
> 
> Niklas, I added your Tested-by, omitting the dwc and cadence patches
> because I think you tested s390 and probably didn't exercise dwc or
> cadence.  Thanks very much to you and Gerd for finding the issue and
> testing the resolution!

Dear Bjorn,

I have tested it on our company's own EVB board and it works normally. 
(Cadence PCIe 4.0 IP)

And it was tested on the RK3588 and worked normally. (Synopsys PCIe 3.0 IP)

Of course, it would be great if others were willing to give it a try.

Best regards,
Hans