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

["Michael S. Tsirkin" <mst@redhat.com>]

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.

> +
>  	if (callback) {
>  		spin_lock_irqsave(&vp_dev->lock, flags);
>  		if (!vp_is_slow_path_vector(msix_vec))
> @@ -571,7 +581,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, const struct cpumask *cpu_mask)
>  
>  	if (vp_dev->msix_enabled) {
>  		mask = vp_dev->msix_affinity_masks[info->msix_vector];
> -		irq = pci_irq_vector(vp_dev->pci_dev, info->msix_vector);
> +		irq = info->virq;
>  		if (!cpu_mask)
>  			irq_update_affinity_hint(irq, NULL);
>  		else {
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index 8cd01de27baf..18b15a5f84ae 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -40,6 +40,9 @@ struct virtio_pci_vq_info {
>  
>  	/* MSI-X vector (or none) */
>  	unsigned int msix_vector;
> +
> +	/* Linux IRQ number of msix_vector, or 0 if not mapped */
> +	unsigned int virq;
>  };
>  
>  struct virtio_pci_admin_vq {
> -- 
> 2.56.0