Re: [PATCH v4 9/9] 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] Using a fixed heuristic delay before `vfio_pci_device_reset()` causes test flakes due to `ioctl_assert` failing on `-EAGAIN`.
--

--- Patch [9]: [PATCH v4 9/9] vfio: selftests: igb: Recover after DMA-read faults ---
commit 774200982f984c4b603b9d07688586c25dcf8c58
Author: Alex Williamson <alex.williamson@nvidia.com>

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

This commit adds a mechanism to recover from DMA-read faults in the igb
selftest by issuing a VFIO_DEVICE_RESET. It introduces a 10ms heuristic
delay before the reset to allow PCIe/IOMMU/AER error handling to complete
and 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
[ ... ]
> @@ -490,7 +512,28 @@ static int igb_memcpy_wait(struct vfio_pci_device *device)
[ ... ]
> +	/*
> +	 * 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]
Since the 10ms delay is a heuristic, what happens if asynchronous PCIe AER
recovery (which can involve link resets and training) takes longer than
10ms?

If the device lock is still held when igb_error_reset_and_reinit() calls
vfio_pci_device_reset(), the underlying ioctl will return -EAGAIN. 

Because vfio_pci_device_reset() internally uses ioctl_assert() which expects
a return value of 0, returning -EAGAIN will immediately abort the test
program, resulting in a flaky test if the timing is missed.

Would it be more robust to explicitly retry the VFIO_DEVICE_RESET ioctl on 
-EAGAIN instead of relying on a fixed sleep and an unconditional assert?

> +
> +	return -ETIMEDOUT;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-igb_v3_b4-v4-0-56e7e2576cc1@google.com?part=9