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

[Samiullah Khawaja <skhawaja@google.com>]

On Wed, Aug 26, 2026 at 01:27:20PM -0700, David Matlack wrote:
>On Tue, Aug 25, 2026 at 12:06 PM David Matlack <dmatlack@google.com> 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.

The group->mutex is needed here also since it sets up the dma_ops on the
default_domain that is currently attached to the device. And those
attachments are protected with group->mutex.
>>
>> 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@google.com>
>> 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.

This might not work as reset_iommu_prepare() changes the domain of the
device being reset and those things protected by the 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.

Thanks,
Sami