Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] pcie_tph_completer_type() incorrectly returns PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH when reading from a disconnected or hot-removed device. Pre-existing issues: - [High] The internal get_rp_completer_type() function was not updated to use the new pcie_tph_completer_type() helper, meaning it can still return the reserved 0b10 value, which can corrupt the device's TPH Requester Enable state. -- commit e3087f1c41da11f8e5258fd8b874372276270977 Author: Zhiping Zhang <[email protected]> PCI/TPH: Add requester/completer type helpers Add pcie_tph_enabled_req_type() and pcie_tph_completer_type() so drivers can query the enabled TPH requester mode and completer support safely. Fold the reserved 0b10 encoding into NONE so callers only see defined values. > 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 [ ... ] > +u8 pcie_tph_completer_type(struct pci_dev *pdev) > +{ > + u32 reg; > + > + if (pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP2, ®)) > + return PCI_EXP_DEVCAP2_TPH_COMP_NONE; > + > + switch (FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg)) { [Severity: Medium] If the device is hot-removed, pcie_capability_read_dword() returns success (0) but can leave reg as 0xFFFFFFFF (PCI_ERROR_RESPONSE due to master abort). Will FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg) then extract 0b11 (3), matching PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH, causing the helper to falsely advertise that a disconnected device supports Extended TPH? Should this logic check if reg is ~0? > + 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; > + default: > + return PCI_EXP_DEVCAP2_TPH_COMP_NONE; > + } > +} > +EXPORT_SYMBOL(pcie_tph_completer_type); [Severity: High] This is a pre-existing issue, but the internal get_rp_completer_type() function was not updated to use this new pcie_tph_completer_type() helper. Because it still returns the raw extracted Completer Supported field, it can return the reserved 0b10 value without applying the safety folding introduced here: drivers/pci/tph.c:get_rp_completer_type() { ... return FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg); } If the device supports EXT_TPH (3) and the Root Port returns the reserved value (2), could this incorrectly propagate the reserved value? drivers/pci/tph.c:pcie_enable_tph() { ... pdev->tph_req_type = min(pdev->tph_req_type, rp_req_type); ... } This would then write the reserved value (2) into the hardware's TPH Control register, misconfiguring the device: drivers/pci/tph.c:pcie_enable_tph() { ... reg &= ~PCI_TPH_CTRL_REQ_EN_MASK; reg |= FIELD_PREP(PCI_TPH_CTRL_REQ_EN_MASK, pdev->tph_req_type); pci_write_config_dword(pdev, pdev->tph_cap + PCI_TPH_CTRL, reg); ... } Should get_rp_completer_type() be updated to use the new helper? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
