Re: [PATCH v5 5/5] vfio: selftests: igb: Recover after DMA-read faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Test flakiness due to fixed heuristic delay instead of polling on EAGAIN during VFIO_DEVICE_RESET
--

--- Patch [5]: [PATCH v5 5/5] vfio: selftests: igb: Recover after DMA-read faults ---
commit a631c99ce1be37dbbdda0314b730b2c6bd947b50
Author: Alex Williamson <alex.williamson@nvidia.com>

vfio: selftests: igb: Recover after DMA-read faults

This commit adds recovery logic to the igb selftest when an intentional DMA
read fault causes the device to hang. It introduces a reset and re-init
sequence using VFIO_DEVICE_RESET, including a 10ms delay to allow kernel
error handlers to release the device lock.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c

[ ... ]

> @@ -500,7 +522,28 @@ static int igb_memcpy_wait(struct vfio_pci_device *device)
>  
>  	igb_irq_enable(igb);
>  
> -	return (status & 1) ? 0 : -ETIMEDOUT;
> +	if (status & 1)
> +		return 0;
> +
> +	/*
> +	 * The descriptor never completed.  On real 82576 hardware this
> +	 * typically follows a DMA-read fault from one of the intentional
> +	 * unmapped-IOVA tests; the fault leaves the descriptor engine
> +	 * unable to service subsequent valid descriptors.  CTRL.RST alone
> +	 * reinitializes the queue registers but leaves the engine wedged
> +	 * for the current process, so a broader VFIO_DEVICE_RESET (FLR)
> +	 * is required.
> +	 *
> +	 * Delay before requesting reset so PCIe/IOMMU/AER error handling
> +	 * triggered by the just-observed DMA fault can release the device
> +	 * lock VFIO_DEVICE_RESET contends for.  The 10 ms value is
> +	 * heuristic.  The current memcpy still fails with -ETIMEDOUT;
> +	 * recovery only ensures the next memcpy starts from a usable state.
> +	 */
> +	usleep(10000);
> +	igb_error_reset_and_reinit(device);

[Severity: Medium]
Will this fixed heuristic delay cause test flakiness under heavy load?

If the kernel's PCIe/IOMMU/AER error handling takes longer than 10ms to
complete (for example, when KASAN or other heavy debugging options are
enabled), the device lock might still be held.

Looking at the kernel side in drivers/vfio/pci/vfio_pci_core.c:
vfio_pci_ioctl_reset(), lock contention causes pci_try_reset_function() to
return -EAGAIN:

	ret = pci_try_reset_function(vdev->pdev);
	if (__vfio_pci_memory_enabled(vdev))
		vfio_pci_dma_buf_move(vdev, false);
	up_write(&vdev->memory_lock);

	return ret;

Since igb_error_reset_and_reinit() calls vfio_pci_device_reset(), which
asserts that the ioctl returns 0, would this lock contention cause an
immediate test suite failure?

Should this code explicitly poll for -EAGAIN from the ioctl instead of
relying entirely on a fixed sleep?

> +	return -ETIMEDOUT;
>  }
>  
>  static void igb_send_msi(struct vfio_pci_device *device)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-igb_v3_b4-v5-0-91503eddb98b@google.com?part=5