Re: [PATCH v20 06/16] PCI/TPH: Cache negotiated max requester type at probe time

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] get_rp_completer_type() does not validate the Root Port's TPH Completer Supported field, allowing a reserved value (2) to be propagated to the endpoint's TPH Requester Enable control register.
--

--- Patch [6]: [PATCH v20 06/16] PCI/TPH: Cache negotiated max requester type at probe time ---
commit 29ae72049d4aa847803163ffb3618441007a362d
Author: Chengwen Feng <fengchengwen@huawei.com>

PCI/TPH: Cache negotiated max requester type at probe time

pcie_enable_tph() recalculates the TPH requester capability and queries
root port completer type on every invocation, causing redundant config
space reads. Calculate the negotiated maximum requester type once during
pci_tph_init() and cache it in a new pdev->tph_max_type bitfield. Simplify
the enable path by reusing the cached value to eliminate repeated hardware
access.

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index 41084a45b0bbc..d123f91775c65 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
[ ... ]
> @@ -551,13 +536,31 @@ void pci_no_tph(void)
>  
>  void pci_tph_init(struct pci_dev *pdev)
>  {
> +	u8 tph_req_type, rp_req_type;
>  	int num_entries;
>  	u32 save_size;
> +	u32 reg = 0;
>  
>  	pdev->tph_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_TPH);
>  	if (!pdev->tph_cap)
>  		return;
>  
> +	/* Get req_type supported by device and its Root Port */
> +	pci_read_config_dword(pdev, pdev->tph_cap + PCI_TPH_CAP, &reg);
> +	if (FIELD_GET(PCI_TPH_CAP_EXT_TPH, reg))
> +		tph_req_type = PCI_TPH_REQ_EXT_TPH;
> +	else
> +		tph_req_type = PCI_TPH_REQ_TPH_ONLY;
> +
> +	/* Check if the device is behind a Root Port */
> +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_RC_END) {
> +		rp_req_type = get_rp_completer_type(pdev);
> +		/* Final req_type is the smallest value of two */
> +		tph_req_type = min(tph_req_type, rp_req_type);
> +	}
> +
> +	pdev->tph_max_type = tph_req_type;

[Severity: Medium]
This isn't a bug introduced by this patch, but this caching logic embeds a
pre-existing issue where get_rp_completer_type() does not validate the Root
Port's TPH Completer Supported field.

If a non-compliant Root Port returns the reserved value 10b (2), could this
unvalidated value be propagated into pdev->tph_max_type and later written
to the Endpoint's TPH Requester Enable control register by
pcie_enable_tph()?

Would writing this reserved value to the endpoint's TPH control register
cause unpredictable hardware behavior or PCIe bus errors?

>  
>  	num_entries = pcie_tph_get_st_table_size(pdev);
>  	save_size = sizeof(u32) + num_entries * sizeof(u16);
>  	pci_add_ext_cap_save_buffer(pdev, PCI_EXT_CAP_ID_TPH, save_size);

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