Re: [RFC PATCH 0/1] vfio: circular locking dependency in pci_dev_reset_iommu_prepare()

[David Matlack]

On Wed, Aug 26, 2026 at 1:27 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
>
> On Tue, Aug 25, 2026 at 12:06 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
> >
> > On 2026-08-21 12:35 PM, Vipin Sharma wrote:
> >
> > > ================================================================================
> > > Potential Solutions Suggested by AI
> > > ================================================================================
> > >
> > > 1. Decouple iommu_setup_dma_ops() from group->mutex in drivers/iommu/iommu.c:
> > >    iommu_setup_dma_ops() only requires struct device * and the domain pointer
> > >    (group->default_domain); it does not mutate any fields in struct iommu_group.
> > >    Moving the iommu_setup_dma_ops() calls after mutex_unlock(&group->mutex) in
> > >    iommu_probe_device(), bus_iommu_probe(), and iommu_group_store_type() breaks
> > >    the initial &group->mutex -> cpu_hotplug_lock dependency.
> >
> > Are there any other code paths that rely on group->mutex -->
> > mm->mmap_lock ordering? If so fixing this one case wouldn't help.
> >
> > > 2. Avoid holding down_write(&vdev->memory_lock) across pci_try_reset_function()
> > >    in VFIO:
> > >    vfio-pci could zap active BAR mappings under memory_lock and set a state
> > >    flag / disable memory decoding, drop memory_lock before calling
> > >    pci_try_reset_function(), and then re-acquire memory_lock to re-enable
> > >    memory. While resetting, any concurrent user fault will see the memory
> > >    disabled condition and return VM_FAULT_SIGBUS safely.
> >
> > This would change the userspace-visible behavior of faulting on a VFIO
> > device BAR from "block until reset is done and the succeed" to "fail
> > with SIGBUS". And it would allow VFIO to access VFIO device BARs during
> > the reset through vfio_pci_core_iowrite*().
> >
> > But I think we can extend this idea to solve those problems by
> > introducing a wait queue for tasks to sit on while a device is being
> > reset.
> >
> > e.g. Something like this (completely untested and partially written by AI):
> >
> > From: David Matlack <dmatlack@xxxxxxxxxx>
> > Date: Tue, 25 Aug 2026 18:37:52 +0000
> > Subject: [PATCH] vfio/pci: Avoid circular locking dependency during device reset
> >
> > Avoid a circular locking dependency during VFIO device reset by dropping
> > vdev->memory_lock prior to calling PCI reset functions
> > (pci_try_reset_function() and pci_reset_bus()). Introduce an explicit reset
> > state flag (vdev->resetting) and wait queue (vdev->reset_done_wq) to stall
> > concurrent BAR page faults and MMIO accesses during reset without holding
> > vdev->memory_lock across PCI reset operations.
> ...
>
> This approach does not look ideal. The implementation has a bug where
> concurrent resets can lead to vdev->resetting being cleared too early.
> And from a maintainability perspective, there are more call sites that
> currently take memory_lock that would probably also have to be updated
> to wait for vdev->resetting to become false.
>
> > >
> > > 3. Refine synchronization in pci_dev_reset_iommu_prepare():
> > >    Evaluate if attaching to the blocking domain and pausing ATS during device
> > >    reset can be protected using more fine-grained locking or atomic state
> > >    flags without holding the coarse &group->mutex.
> >
> > I don't know enough about this part of the kernel to say, but this would
> > directly address the new lock ordering dependency vdev->memory_lock -->
> > group->mutex introduced by commit f5b16b802174 ("PCI: Suspend iommu function
> > prior to resetting a device"), which is what led to this lockdep error.

I sent a proposed fix that takes this approach here:

  https://lore.kernel.org/linux-iommu/20260827173511.2322549-1-dmatlack@xxxxxxxxxx/