Re: [PATCH v20 09/16] PCI/TPH: Add macros to check standard and extended TPH support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The macro `pcie_std_tph_supported()` erroneously returns true if `tph_max_type` is set to the reserved TPH requester type value 2.
- [Low] The helper macros `pcie_std_tph_supported` and `pcie_ext_tph_supported` are implemented as macros rather than static inline functions, causing the !CONFIG_PCIE_TPH stubs to silently drop the pdev argument.
--

--- Patch [9]: [PATCH v20 09/16] PCI/TPH: Add macros to check standard and extended TPH support ---
commit 62018f833ad520cae3d9509aa5138e780c6df62a
Author: Chengwen Feng <fengchengwen@huawei.com>

PCI/TPH: Add macros to check standard and extended TPH support

This commit adds helper macros pcie_std_tph_supported() and
pcie_ext_tph_supported() for checking TPH capabilities. It relies on the
cached tph_max_type value and provides stub definitions for when
CONFIG_PCIE_TPH is disabled.

> diff --git a/include/linux/pci-tph.h b/include/linux/pci-tph.h
> index 6901f6e2f8c7a..04b521b7c2c7c 100644
> --- a/include/linux/pci-tph.h
> +++ b/include/linux/pci-tph.h

[ ... ]

> @@ -23,6 +23,18 @@ enum tph_mem_type {
>  };
>  
>  #ifdef CONFIG_PCIE_TPH
> +/**
> + * pcie_std_tph_supported - check standard TPH requester support
> + * @pdev: PCI device
> + */
> +#define pcie_std_tph_supported(pdev) \
> +	((pdev)->tph_max_type >= PCI_TPH_REQ_TPH_ONLY)

[Severity: Medium]
Will this macro falsely return true if the hardware advertises the reserved
value 2? 

If a PCIe Root Port incorrectly advertises the reserved value 10b (2) for its
TPH Completer Supported field, pci_tph_init() caches 2 into pdev->tph_max_type.
Since 2 >= PCI_TPH_REQ_TPH_ONLY (which is 1), this evaluates to true. However,
core functions like pcie_enable_tph_ext() explicitly reject req_type == 2 with
-EINVAL.

> +/**
> + * pcie_ext_tph_supported - check extended TPH requester support
> + * @pdev: PCI device
> + */
> +#define pcie_ext_tph_supported(pdev) \
> +	((pdev)->tph_max_type == PCI_TPH_REQ_EXT_TPH)

[ ... ]

> @@ -37,6 +49,8 @@ u32 pcie_tph_get_st_table_loc(struct pci_dev *pdev);
>  u8 pcie_tph_enabled_req_type(struct pci_dev *pdev);
>  u8 pcie_tph_completer_type(struct pci_dev *pdev);
>  #else
> +#define pcie_std_tph_supported(pdev) false
> +#define pcie_ext_tph_supported(pdev) false

[Severity: Low]
Could these stub macros cause unused variable build warnings when
CONFIG_PCIE_TPH is disabled, since they silently drop the pdev argument?

Would it be better to implement these as static inline functions to guarantee
consistent type checking and evaluation semantics across all configurations?

>  static inline int pcie_tph_set_st_entry(struct pci_dev *pdev,
>  					unsigned int index, u16 tag)
>  { return -EINVAL; }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714104325.54444-1-fengchengwen@huawei.com?part=9