Re: [PATCH v7 2/5] PCI/TPH: Add requester/completer type helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pcie_tph_completer_type() does not check for PCI_ERROR_RESPONSE (~0U), leading to a false positive Extended TPH capability if the device is disconnected or in a low power state.
--

commit f9aeb32b2542c39fb3d9466efa934427ab4660a5
Author: Zhiping Zhang <zhipingz@meta.com>
PCI/TPH: Add requester/completer type helpers

This commit adds pcie_tph_enabled_req_type() and pcie_tph_completer_type()
helpers to allow drivers to query TPH enabled requester modes and completer
support without accessing pci_dev internals directly.

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index 91145e8d9d951..4fe076bba953a 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
[ ... ]
> @@ -174,6 +174,49 @@ u32 pcie_tph_get_st_table_loc(struct pci_dev *pdev)
[ ... ]
> +u8 pcie_tph_completer_type(struct pci_dev *pdev)
> +{
> +	u32 reg;
> +
> +	if (pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP2, &reg))
> +		return PCI_EXP_DEVCAP2_TPH_COMP_NONE;
> +
> +	switch (FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg)) {
> +	case PCI_EXP_DEVCAP2_TPH_COMP_TPH_ONLY:
> +		return PCI_EXP_DEVCAP2_TPH_COMP_TPH_ONLY;
> +	case PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH:
> +		return PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH;

[Severity: Medium]
Will this return a false positive if the device is disconnected or in a low
power state?

When reading the PCI config space of an unplugged or unresponsive device,
the host bridge typically returns all 1s (0xFFFFFFFF) and the config read
returns 0 (success).

If pcie_capability_read_dword() succeeds but sets reg to 0xFFFFFFFF,
FIELD_GET() will extract 0b11 (3) from bits 13:12. This matches
PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH, causing the switch statement to
incorrectly return that the device supports Extended TPH.

Could this trick drivers querying capabilities during teardown or error
recovery into believing a removed device supports Extended TPH? Using
PCI_POSSIBLE_ERROR() to check the register value before evaluating the bits
might prevent this.

> +	default:
> +		return PCI_EXP_DEVCAP2_TPH_COMP_NONE;
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611161546.4075580-1-zhipingz@meta.com?part=2