Re: [PATCH v7 3/6] vfio/pci: Add PCIe TPH interface with capability query

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The ST table size (`st_table_sz`) is incorrectly reported as 0 when the Steering Tag (ST) table is located in the MSI-X table.
--

commit 84cffb4ace52d740319be3c3a2f634b68268963c
Author: Chengwen Feng <fengchengwen@huawei.com>

vfio/pci: Add PCIe TPH interface with capability query

This commit adds the VFIO_DEVICE_PCI_TPH IOCTL to allow userspace to query
device TPH capabilities, supported modes, and steering tag table information.
It also introduces a module parameter to restrict unsafe device-specific TPH
mode to trusted userspace only.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 3f8d093aacf8a..e7efa8f230bec 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
[ ... ]
> @@ -1461,6 +1463,55 @@ static int vfio_pci_ioctl_ioeventfd(struct vfio_pci_core_device *vdev,
>  				  ioeventfd.fd);
>  }
>  
> +static int vfio_pci_tph_get_cap(struct vfio_pci_core_device *vdev,
> +				struct vfio_device_pci_tph_op *op,
> +				void __user *uarg)
> +{
> +	struct pci_dev *pdev = vdev->pdev;
> +	struct vfio_pci_tph_cap cap = {0};
> +	u8 mode;
> +
> +	if (op->argsz < offsetofend(struct vfio_device_pci_tph_op, cap))
> +		return -EINVAL;
> +
> +	mode = pcie_tph_get_st_modes(pdev);
> +	if (!enable_unsafe_tph_ds_mode)
> +		mode &= ~PCI_TPH_CAP_ST_DS;
> +	if (mode == 0 || mode == PCI_TPH_CAP_ST_NS)
> +		return -EOPNOTSUPP;
> +
> +	if (mode & PCI_TPH_CAP_ST_IV)
> +		cap.supported_modes |= VFIO_PCI_TPH_MODE_IV;
> +	if (mode & PCI_TPH_CAP_ST_DS)
> +		cap.supported_modes |= VFIO_PCI_TPH_MODE_DS;
> +
> +	if (pcie_tph_get_st_table_loc(pdev) != PCI_TPH_LOC_NONE)
> +		cap.st_table_sz = pcie_tph_get_st_table_size(pdev);

Is it possible that st_table_sz is incorrectly reported as 0 here when the
Steering Tag (ST) table is located in the MSI-X table?

The kernel function pcie_tph_get_st_table_size() returns 0 if the ST table
location is anything other than PCI_TPH_LOC_CAP, meaning it returns 0 for
PCI_TPH_LOC_MSIX.

As a result, userspace would receive st_table_sz == 0, which the UAPI
documentation explicitly defines as 'no ST table'.

Furthermore, because the location is not PCI_TPH_LOC_NONE, any subsequent
userspace attempt to use VFIO_PCI_TPH_GET_ST would be rejected by the kernel
with -EOPNOTSUPP.

This would leave userspace completely unable to properly program the ST table
using SET_ST because it falsely believes the table size is 0 and cannot query
the CPU tags.

Should the MSI-X table size (e.g., via pci_msix_vec_count(pdev)) be used to
populate st_table_sz when the location is PCI_TPH_LOC_MSIX?

> +
> +	if (copy_to_user(uarg, &cap, sizeof(cap)))
> +		return -EFAULT;
> +
> +	return 0;
> +}

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