Re: [PATCH v11 1/4] 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:

Pre-existing issues:
- [High] The newly added `pcie_tph_enabled_req_type()` can return an invalid reserved value (2 / 0b10) because a pre-existing internal function, `get_rp_completer_type()`, lacks the safety checks introduced in this patch and was not refactored.
--

commit e4c05b2d02aee077c988b64f210719a661f03806
Author: Zhiping Zhang <zhipingz@meta.com>

PCI/TPH: Add requester/completer type helpers

This commit adds pcie_tph_enabled_req_type() to query the enabled TPH
requester mode and pcie_tph_completer_type() to check TPH completer support.
The new completer helper safely folds the reserved 0b10 encoding into "not
supported".

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index 655ffd60e62f6..e7693fd9d6763 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c

[ ... ]

> @@ -173,6 +173,51 @@ u32 pcie_tph_get_st_table_loc(struct pci_dev *pdev)
>  }
>  EXPORT_SYMBOL(pcie_tph_get_st_table_loc);
>  
> +/**
> + * pcie_tph_enabled_req_type - Return the device's enabled TPH requester type
> + * @pdev: PCI device to query
> + *
> + * Return: PCI_TPH_REQ_DISABLE, PCI_TPH_REQ_TPH_ONLY or PCI_TPH_REQ_EXT_TPH.
> + */
> +u8 pcie_tph_enabled_req_type(struct pci_dev *pdev)
> +{
> +	return pdev->tph_req_type;

[Severity: High]
This is a pre-existing issue, but could pdev->tph_req_type hold the reserved
value 2 (0b10) here?

While this patch safely decodes the completer field via
pcie_tph_completer_type(), the pre-existing get_rp_completer_type() function
accesses the same capability on the Root Port but lacks this bounds checking.

If a Root Port presents the reserved value 0b10, get_rp_completer_type()
propagates it:

drivers/pci/tph.c:get_rp_completer_type() {
    ...
    return FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg);
}

This unvalidated value can then be assigned to pdev->tph_req_type during
setup:

drivers/pci/tph.c:pcie_enable_tph() {
    ...
    pdev->tph_req_type = min(pdev->tph_req_type, rp_req_type);
    ...
}

Since the newly added pcie_tph_enabled_req_type() API is documented to return
only 0, 1, or 3, returning the reserved value 2 violates this contract and
could risk logic errors in callers. 

Writing this undocumented value to the TPH Control register could also cause
undefined hardware behavior or AER panics.

Should get_rp_completer_type() be refactored to fold the reserved value
identically to how pcie_tph_completer_type() handles it?

> +}
> +EXPORT_SYMBOL(pcie_tph_enabled_req_type);

[ ... ]

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