Re: [PATCH RESEND v18 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:
- [Medium] Unchecked return value from `pcie_tph_set_st_entry()` causes state desynchronization between the virtual config space and physical hardware.
--

commit 94cd790796325d732c4348b7bef18a12078f330c
Author: Chengwen Feng <fengchengwen@huawei.com>

vfio/pci: Virtualize PCIe TPH capability registers

This commit virtualizes the TPH extended capability config space registers.
It splits permissions, blocks direct ST-table programming via config space,
and syncs the cached shadow ST table to physical hardware when the TPH
requester is enabled.

> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index 5c6ab172df6c0..10f4e9fabea7c 100644
> --- 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: Medium]
Should this loop check the return value of pcie_tph_set_st_entry()?

If syncing the Steering Tag (ST) table fails (for example, due to MSI-X
table access errors), pcie_tph_set_st_entry() disables TPH on the physical
hardware and returns an error code. 

Because the return value is ignored here, it skips the rollback of the
virtual config space that would happen if ret was updated.

Could this result in the guest OS incorrectly believing TPH is enabled,
leading to an inconsistent state between the physical hardware and the
virtual config space?

> +		}
> +		if (ret)
> +			vfio_default_config_write(vdev, pos, count, perm,
> +						  offset, org_val);
> +	} else if (req == PCI_TPH_REQ_DISABLE) {
> +		pcie_disable_tph(vdev->pdev);
> +	} else {
> +		vfio_default_config_write(vdev, pos, count, perm, offset,
> +					  org_val);
> +	}
> +
> +	up_write(&vdev->memory_lock);
> +
> +	return count;
> +}

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