Re: [PATCH v6 2/6] PCI/TPH: Export pcie_tph_get_st_modes() for external use

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing error handling for pci_read_config_dword() in the newly exported pcie_tph_get_st_modes() leads to falsely reporting all TPH modes as supported when a device is disconnected.
- [Low] The !CONFIG_PCI_TPH stub for pcie_tph_get_st_table_loc() returns an inconsistent out-of-bounds value (0x7FF) instead of PCI_TPH_LOC_NONE.
--

commit d23fc69219a490dd1458be7c3d6f50ae929f70cd
Author: Chengwen Feng <fengchengwen@huawei.com>

PCI/TPH: Export pcie_tph_get_st_modes() for external use

This commit exports the pcie_tph_get_st_modes() helper so drivers like VFIO
can query supported PCIe TPH steering tag modes. It also adds stub functions
for when !CONFIG_PCI_TPH and tph_cap validation.

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index f17b74b5fb1e3..ba31b010f67ad 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
[ ... ]
> @@ -145,15 +145,27 @@ static void set_ctrl_reg_req_en(struct pci_dev *pdev, u8 req_type)
> -static u8 get_st_modes(struct pci_dev *pdev)
> +/**
> + * pcie_tph_get_st_modes - Get supported Steering Tag modes
> + * @pdev: PCI device to query
> + *
> + * Return:
> + *  Bitmask of supported ST modes (PCI_TPH_CAP_ST_NS, PCI_TPH_CAP_ST_IV,
> + *                                 PCI_TPH_CAP_ST_DS)
> + */
> +u8 pcie_tph_get_st_modes(struct pci_dev *pdev)
>  {
>  	u32 reg;
>  
> +	if (!pdev->tph_cap)
> +		return 0;
> +
>  	pci_read_config_dword(pdev, pdev->tph_cap + PCI_TPH_CAP, &reg);

If pci_read_config_dword() fails (e.g., when a device is disconnected), it
populates the output variable with an error response (~0 or 0xFFFFFFFF).

Since the return value is ignored here, will this masking operation:

>  	reg &= PCI_TPH_CAP_ST_NS | PCI_TPH_CAP_ST_IV | PCI_TPH_CAP_ST_DS;

evaluate to 0x07 and incorrectly report all three Steering Tag modes as
supported during a device disconnection?

>  
>  	return reg;
>  }
> +EXPORT_SYMBOL(pcie_tph_get_st_modes);

[ ... ]

> diff --git a/include/linux/pci-tph.h b/include/linux/pci-tph.h
> index be68cd17f2f83..586c75b19e019 100644
> --- a/include/linux/pci-tph.h
> +++ b/include/linux/pci-tph.h
[ ... ]
> @@ -41,6 +42,12 @@ static inline int pcie_tph_get_cpu_st(struct pci_dev *dev,
>  static inline void pcie_disable_tph(struct pci_dev *pdev) { }
>  static inline int pcie_enable_tph(struct pci_dev *pdev, int mode)
>  { return -EINVAL; }
> +static inline u16 pcie_tph_get_st_table_size(struct pci_dev *pdev)
> +{ return 0; }
> +static inline u32 pcie_tph_get_st_table_loc(struct pci_dev *pdev)
> +{ return 0x7FF; /* Values that do not appear in normal case */ }

When TPH is enabled but a device lacks TPH capabilities, the live function
safely returns PCI_TPH_LOC_NONE.

Does returning 0x7FF in this stub introduce an inconsistency in the API that
forces callers to handle arbitrary magic numbers instead of relying on
PCI_TPH_LOC_NONE?

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