Re: [PATCH v10 3/5] PCI/TPH: Add pcie_tph_enabled_mode() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The pcie_tph_enabled_mode() helper returns 0 both when TPH is disabled and when it is actively enabled in No ST Mode, causing an unresolvable API ambiguity.
- [Medium] Lockless multi-variable read of TPH state without memory barriers leads to inconsistent state and TOCTOU vulnerabilities.
--

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

PCI/TPH: Add pcie_tph_enabled_mode() helper

Add a helper to query enabled TPH mode on a PCI device. This is useful for
drivers like VFIO-PCI that need to validate TPH state before allowing
access to steering tag tables.

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index ba31b010f67ad..91c1e83410a3e 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
[ ... ]
> @@ -451,6 +451,18 @@ int pcie_enable_tph(struct pci_dev *pdev, int mode)
>  }
>  EXPORT_SYMBOL(pcie_enable_tph);
>  
> +/**
> + * pcie_tph_enabled_mode - Get current enabled TPH mode
> + * @pdev: PCI device
> + *
> + * Return the enabled TPH mode (IV/DS) or 0 if disabled.
> + */
> +int pcie_tph_enabled_mode(struct pci_dev *pdev)
> +{
> +	return pdev->tph_enabled ? pdev->tph_mode : 0;

Will this return value be ambiguous when TPH is enabled in No ST Mode?

Since No ST Mode (PCI_TPH_ST_NS_MODE) is defined as 0x0, returning 0 when
disabled makes it impossible for a caller to distinguish between TPH being
completely disabled and TPH being actively enabled in No ST Mode.

Could this return a negative error code (such as -ENODATA or -EINVAL) when
TPH is disabled, to avoid collisions with valid mode values?

Also, can this lockless read of pdev->tph_enabled and pdev->tph_mode see
an inconsistent state if pcie_enable_tph() runs concurrently?

For example, if VFIO allows userspace to trigger pcie_enable_tph()
concurrently with querying the enabled mode, could the CPU or compiler
reorder the load of pdev->tph_mode before pdev->tph_enabled?

This might result in seeing tph_enabled == 1 but an old or uninitialized
value for tph_mode.

> +}
> +EXPORT_SYMBOL(pcie_tph_enabled_mode);

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