Re: [PATCH v17 12/12] vfio/pci: Virtualize PCIe TPH capability registers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Silent state desynchronization between hardware TPH and virtual config due to missing error handling for ST entry updates.
--

commit 22993c676c19c404abf1528141d0dd0e78a51f99
Author: Chengwen Feng <fengchengwen@huawei.com>

vfio/pci: Virtualize PCIe TPH capability registers

This commit virtualizes TPH extended capability config space registers.
It blocks direct ST-table programming via config space, adds vconfig
masking to hide EXT_TPH, and syncs the shadow ST table to physical hardware
when the requester is enabled.

> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c

[ ... ]

> +static int vfio_tph_config_write(struct vfio_pci_core_device *vdev, int pos,
> +				 int count, struct perm_bits *perm,
> +				 int offset, __le32 val)
> +{

[ ... ]

> +	if (req == PCI_TPH_REQ_TPH_ONLY || req == PCI_TPH_REQ_EXT_TPH) {
> +		extended = !!(req == PCI_TPH_REQ_EXT_TPH);
> +		ret = pcie_enable_tph_explicit(pdev, mode, extended);
> +		if (!ret && vdev->tph_st_shadow) {
> +			for (i = 0; i < vdev->tph_st_entries; i++)
> +				pcie_tph_set_st_entry(pdev, i,
> +						      vdev->tph_st_shadow[i]);
> +		}

[Severity: High]
If pcie_tph_set_st_entry() fails (for instance, due to missing MSI-X
allocation), does it leave the hardware and virtual configuration out of
sync?

The loop ignores the return value from pcie_tph_set_st_entry(), meaning
ret will remain 0 even if the ST entry update fails. Since a failure here
can forcefully disable TPH in the physical hardware via pcie_disable_tph(),
the subsequent if (ret) config rollback will be skipped. This would leave
the guest OS believing TPH is enabled while the physical hardware is actually
disabled.

Additionally, in the VFIO_DEVICE_FEATURE_TPH_ST_CONFIG ioctl handler
(vfio_pci_core_feature_tph_st_config), the rollback loop tries to restore
entries using pcie_tph_set_st_entry(). If the initial failure unconditionally
disabled TPH on the physical device, will these rollback calls fail
immediately, leaving the recovery broken?

> +		if (ret)
> +			vfio_default_config_write(vdev, pos, count, perm,
> +						  offset, org_val);
> +	} else if (req == PCI_TPH_REQ_DISABLE) {

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