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

[Alex Williamson <alex.williamson@nvidia.com>]

On Mon, 20 Jul 2026 14:27:12 -0700
Josh Hilke <jrhilke@google.com> wrote:

> On Tue, Jul 14, 2026 at 2:12 PM <sashiko-bot@kernel.org> wrote:
> >
> > 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;  
> 
> Alex, I'm guessing 10ms is what worked for you when testing with the
> physical hardware? If it's working fine, I don't think it needs to be
> changed.

Yes, the delay was found empirically and indeed we could make this more
robust if we retried on -EAGAIN.  I'd be in favor of doing that as a
follow-up.  Thanks,

Alex