Re: [PATCH] virtio_pci: don't take the MSI mutex under cpu_hotplug_lock

[Zenghui Yu <zenghui.yu@linux.dev>]

On 10/29/26 6:15 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 29, 2026 at 04:40:43PM +0800, Zenghui Yu wrote:
> > From: "Zenghui Yu (Huawei)" <zenghui.yu@linux.dev>
> >
> > Lockdep reports the following circular locking dependency when a virtio-net
> > device is probed with lockdep enabled on a system whose devices sit behind
> > an IOMMU:
> >
> >     ======================================================
> >     WARNING: possible circular locking dependency detected
> >     7.3.0-rc1+ #84 Not tainted
> >     ------------------------------------------------------
> >     kworker/u34:0/57 is trying to acquire lock:
> >     ffff800104f83ee8 (&md->mutex){+.+.}-{4:4}, at: msi_domain_get_virq+0xf0/0x118
> >
> >     but task is already holding lock:
> >     ffffc00081edd930 (cpu_hotplug_lock){++++}-{0:0}, at: cpus_read_lock+0x10/0x1c
> >
> >     which lock already depends on the new lock.
> >
> >     Chain exists of:
> >       &md->mutex --> &group->mutex --> cpu_hotplug_lock
> >
> > The cycle is composed of three dependencies:
> >
> > * cpu_hotplug_lock -> &md->mutex (which triggers the splat). virtio_net's
> >   init_vqs() calls cpus_read_lock() around virtnet_set_affinity(), which
> >   reaches vp_set_vq_affinity(). That calls pci_irq_vector(), which takes
> >   the per-device MSI mutex (&dev->msi.data->mutex) via
> >   msi_domain_get_virq().
> >
> > * &md->mutex -> &domain->mutex -> &group->mutex.  MSI-X allocation of any
> >   device behind the IOMMU holds the MSI mutex while allocating interrupts
> >   through the IRQ domain hierarchy. The MSI parent domain (GICv2m, or ITS
> >   likewise) calls iommu_dma_prepare_msi() from its .alloc() to map the MSI
> >   doorbell, which takes the IOMMU group mutex.
> >
> > * &group->mutex -> cpu_hotplug_lock. bus_iommu_probe() calls
> >   iommu_setup_dma_ops() under the group mutex; initializing the IOVA
> >   rcaches registers a cpuhp instance, which takes cpu_hotplug_lock.
> >
> > The cycle only became complete since commit 288683c92b1a ("iommu: Make
> > iommu_dma_prepare_msi() into a generic operation"), where
> > iommu_dma_prepare_msi() switched to use the group mutex.
> >
> > pci_irq_vector() is a pure lookup of a vector that is bound to the
> > queue for the whole lifetime of the queue, so there is no reason to
> > take the MSI mutex for it from cpu hotplug locked contexts.
> >
> > Cache the IRQ number in virtio_pci_vq_info {} at vector binding time and
> > make vp_set_vq_affinity() use the cached value.
> >
> > This also covers the CPU hotplug callbacks
> > (virtnet_cpu_online/virtnet_cpu_dead), which call virtnet_set_affinity()
> > with cpu_hotplug_lock already held.
> >
> > Signed-off-by: Zenghui Yu (Huawei) <zenghui.yu@linux.dev>
> > ---
> >  drivers/virtio/virtio_pci_common.c | 12 +++++++++++-
> >  drivers/virtio/virtio_pci_common.h |  3 +++
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index b90c174450b2..2f42300a8a84 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -224,6 +224,16 @@ static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int in
> >  		goto out_info;
> >  
> >  	info->vq = vq;
> > +
> > +	/*
> > +	 * Cache the Linux IRQ number so that later users (e.g.
> > +	 * vp_set_vq_affinity() from cpu hotplug locked contexts)
> > +	 * don't have to take the MSI mutex via pci_irq_vector().
> > +	 */
> 
> I don't think this comment is helpful. Commit log is enough.
> 
> > +	info->virq = 0;
> > +	if (msix_vec != VIRTIO_MSI_NO_VECTOR)
> > +		info->virq = pci_irq_vector(vp_dev->pci_dev, msix_vec);
> 
> 	else
> 		info->virq = 0;
> 
> 	Please.

Thanks. I'll send an updated version shortly.

Zenghui