| 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
| ||
|
Message-ID: <6075b776-d2be-49d3-8321-e6af66781709@163.com>
Date: Wed, 2 Apr 2025 23:31:10 +0800
From: Hans Zhang <18255117159@....com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: lpieralisi@...nel.org, bhelgaas@...gle.com, kw@...ux.com,
manivannan.sadhasivam@...aro.org, robh@...nel.org, jingoohan1@...il.com,
thomas.richard@...tlin.com, linux-pci@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [v7 1/5] PCI: Refactor capability search into common macros
On 2025/4/2 20:42, Ilpo Järvinen wrote:
> On Wed, 2 Apr 2025, Hans Zhang wrote:
>
>> Introduce PCI_FIND_NEXT_CAP_TTL and PCI_FIND_NEXT_EXT_CAPABILITY macros
>> to consolidate duplicate PCI capability search logic found throughout the
>> driver tree. This refactoring:
>>
>> 1. Eliminates code duplication in capability scanning routines
>> 2. Provides a standardized, maintainable implementation
>> 3. Reduces error-prone copy-paste implementations
>> 4. Maintains identical functionality to existing code
>>
>> The macros abstract the low-level capability register scanning while
>> preserving the existing PCI configuration space access patterns. They will
>> enable future conversions of multiple capability search implementations
>> across various drivers (e.g., PCI core, controller drivers) to use
>> this centralized logic.
>>
>> Signed-off-by: Hans Zhang <18255117159@....com>
>> ---
>> drivers/pci/pci.h | 81 +++++++++++++++++++++++++++++++++++
>> include/uapi/linux/pci_regs.h | 2 +
>> 2 files changed, 83 insertions(+)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 2e9cf26a9ee9..f705b8bd3084 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -89,6 +89,87 @@ bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
>> bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
>> bool pcie_cap_has_rtctl(const struct pci_dev *dev);
>>
>> +/* Standard Capability finder */
>> +/**
>> + * PCI_FIND_NEXT_CAP_TTL - Find a PCI standard capability
>> + * @read_cfg: Function pointer for reading PCI config space
>> + * @start: Starting position to begin search
>> + * @cap: Capability ID to find
>> + * @args: Arguments to pass to read_cfg function
>> + *
>> + * Iterates through the capability list in PCI config space to find
>> + * the specified capability. Implements TTL (time-to-live) protection
>> + * against infinite loops.
>> + *
>> + * Returns: Position of the capability if found, 0 otherwise.
>> + */
>> +#define PCI_FIND_NEXT_CAP_TTL(read_cfg, start, cap, args...) \
>> +({ \
>> + u8 __pos = (start); \
>> + int __ttl = PCI_FIND_CAP_TTL; \
>> + u16 __ent; \
>> + u8 __found_pos = 0; \
>> + u8 __id; \
>> + \
>> + read_cfg(args, __pos, 1, (u32 *)&__pos); \
>> + \
>> + while (__ttl--) { \
>> + if (__pos < PCI_STD_HEADER_SIZEOF) \
>> + break; \
>> + __pos = ALIGN_DOWN(__pos, 4); \
>> + read_cfg(args, __pos, 2, (u32 *)&__ent); \
>> + __id = FIELD_GET(PCI_CAP_ID_MASK, __ent); \
>> + if (__id == 0xff) \
>> + break; \
>> + if (__id == (cap)) { \
>> + __found_pos = __pos; \
>> + break; \
>> + } \
>> + __pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, __ent); \
>
> Could you please separate the coding style cleanups into own patch that
> is before the actual move patch. IMO, all those cleanups can be in the
> same patch.
>
Hi Ilpo,
Thanks your for reply. I don't understand. Is it like this?
#define PCI_FIND_NEXT_CAP_TTL(read_cfg, start, cap, args...) \
({ \
int __ttl = PCI_FIND_CAP_TTL; \
u8 __id, __found_pos = 0; \
u8 __pos = (start); \
u16 __ent; \
\
read_cfg(args, __pos, 1, (u32 *)&__pos); \
\
while (__ttl--) { \
if (__pos < PCI_STD_HEADER_SIZEOF) \
break; \
\
__pos = ALIGN_DOWN(__pos, 4); \
read_cfg(args, __pos, 2, (u32 *)&__ent); \
\
__id = FIELD_GET(PCI_CAP_ID_MASK, __ent); \
if (__id == 0xff) \
break; \
\
if (__id == (cap)) { \
__found_pos = __pos; \
break; \
} \
\
__pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, __ent); \
} \
__found_pos; \
})
> You also need to add #includes for the defines you now started to use.
>
Is that what you mean?
+#include <linux/bitfield.h>
+#include <linux/align.h>
+#include <uapi/linux/pci_regs.h>
Best regards,
Hans
>> + } \
>> + __found_pos; \
>> +})
>> +
>> +/* Extended Capability finder */
>> +/**
>> + * PCI_FIND_NEXT_EXT_CAPABILITY - Find a PCI extended capability
>> + * @read_cfg: Function pointer for reading PCI config space
>> + * @start: Starting position to begin search (0 for initial search)
>> + * @cap: Extended capability ID to find
>> + * @args: Arguments to pass to read_cfg function
>> + *
>> + * Searches the extended capability space in PCI config registers
>> + * for the specified capability. Implements TTL protection against
>> + * infinite loops using a calculated maximum search count.
>> + *
>> + * Returns: Position of the capability if found, 0 otherwise.
>> + */
>> +#define PCI_FIND_NEXT_EXT_CAPABILITY(read_cfg, start, cap, args...) \
>> +({ \
>> + u16 __pos = (start) ?: PCI_CFG_SPACE_SIZE; \
>> + u16 __found_pos = 0; \
>> + int __ttl, __ret; \
>> + u32 __header; \
>> + \
>> + __ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8; \
>> + while (__ttl-- > 0 && __pos >= PCI_CFG_SPACE_SIZE) { \
>> + __ret = read_cfg(args, __pos, 4, &__header); \
>> + if (__ret != PCIBIOS_SUCCESSFUL) \
>> + break; \
>> + \
>> + if (__header == 0) \
>> + break; \
>> + \
>> + if (PCI_EXT_CAP_ID(__header) == (cap) && __pos != start) { \
>> + __found_pos = __pos; \
>> + break; \
>> + } \
>> + \
>> + __pos = PCI_EXT_CAP_NEXT(__header); \
>> + } \
>> + __found_pos; \
>> +})
>> +
>> /* Functions internal to the PCI core code */
>>
>> #ifdef CONFIG_DMI
>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>> index 3445c4970e4d..a11ebbab99fc 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -206,6 +206,8 @@
>> /* 0x48-0x7f reserved */
>>
>> /* Capability lists */
>> +#define PCI_CAP_ID_MASK 0x00ff
>> +#define PCI_CAP_LIST_NEXT_MASK 0xff00
>>
>> #define PCI_CAP_LIST_ID 0 /* Capability ID */
>> #define PCI_CAP_ID_PM 0x01 /* Power Management */
>>
>
Powered by blists - more mailing lists