Re: [PATCH v5 10/10] vfio: selftests: mlx5 driver - add send_msi support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Invalid hardware initialization and teardown ordering with MSI-X vectors causes firmware command failures and IOMMU faults.
--

commit a10ddcc560d82fed1bf7785eb0d238998106b1d7
Author: Jason Gunthorpe <jgg@ziepe.ca>

vfio: selftests: mlx5 driver - add send_msi support

Wire an MSI-X vector to a dedicated EQ so the mlx5 driver supports
send_msi(). Each EQ can be linked to an MSI-X vector, and the CQ can be
set up to deliver an event to the EQ.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/mlx5/mlx5.c b/tools/testing/selftests/vfio/lib/drivers/mlx5/mlx5.c
> index e5e75adb25316..c8388aabb8c67 100644
> --- a/tools/testing/selftests/vfio/lib/drivers/mlx5/mlx5.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/mlx5/mlx5.c
[ ... ]
> @@ -1721,8 +1869,13 @@ static void mlx5st_init(struct vfio_pci_device *device)
>  	mlx5st_alloc_pd(dev);
>  	mlx5st_create_mkey(dev);
>  
> +	/* MSI EQ must be created before CQ so CQ can reference its eqn */
> +	mlx5st_create_msi_eq(dev);
>  	mlx5st_setup_datapath(dev);
>  
> +	vfio_pci_msix_enable(device, MSI_VECTOR, 1);
> +	device->driver.msi = MSI_VECTOR;
> +

[Severity: High]
Does this sequence result in the firmware rejecting the CREATE_EQ command?

In mlx5st_init(), mlx5st_create_msi_eq() sends the CREATE_EQ command with
intr = MSI_VECTOR. Because vfio_pci_msix_enable() hasn't been called yet,
the MSI-X vectors aren't allocated on the device via VFIO. Will the Mellanox
firmware validate the interrupt vector index against currently allocated
MSI-X vectors and reject this command?

[ ... ]
> @@ -1733,8 +1886,14 @@ static void mlx5st_remove(struct vfio_pci_device *device)
>  {
>  	struct mlx5st_device *dev = to_mlx5st(device);
>  
> +	vfio_pci_msix_disable(device);
>  	mlx5st_teardown_datapath(dev);
>  
> +	if (dev->have_msi_eq) {
> +		mlx5st_destroy_msi_eq(dev);
> +		dev->have_msi_eq = false;
> +	}
> +

[Severity: High]
Can disabling MSI-X before tearing down the datapath and MSI EQ lead to host
IOMMU faults?

In mlx5st_remove(), vfio_pci_msix_disable() is called while the queues are
still active in hardware. If delayed CQE or EQE completions attempt MSI-X
memory writes after the host IRTE is torn down, will this result in DMAR
faults on the host system? Should the datapath and MSI EQ be destroyed
before disabling MSI-X?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0-v5-4cacb99249f6+1b4-mlx5st_jgg@nvidia.com?part=10