* [PATCH] virtio_pci: don't take the MSI mutex under cpu_hotplug_lock
@ 2026-10-29 8:40 Zenghui Yu
2026-10-29 10:15 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Zenghui Yu @ 2026-10-29 8:40 UTC (permalink / raw)
To: virtualization, linux-kernel
Cc: mst, jasowangio, eperezma, xuanzhuo, nicolinc, jgg, Zenghui Yu (Huawei)
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().
+ */
+ info->virq = 0;
+ if (msix_vec != VIRTIO_MSI_NO_VECTOR)
+ info->virq = pci_irq_vector(vp_dev->pci_dev, msix_vec);
+
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio_pci: don't take the MSI mutex under cpu_hotplug_lock
2026-10-29 8:40 [PATCH] virtio_pci: don't take the MSI mutex under cpu_hotplug_lock Zenghui Yu
@ 2026-10-29 10:15 ` Michael S. Tsirkin
2026-10-29 11:49 ` Zenghui Yu
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2026-10-29 10:15 UTC (permalink / raw)
To: Zenghui Yu
Cc: virtualization, linux-kernel, jasowangio, eperezma, xuanzhuo,
nicolinc, jgg
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio_pci: don't take the MSI mutex under cpu_hotplug_lock
2026-10-29 10:15 ` Michael S. Tsirkin
@ 2026-10-29 11:49 ` Zenghui Yu
0 siblings, 0 replies; 3+ messages in thread
From: Zenghui Yu @ 2026-10-29 11:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtualization, linux-kernel, jasowangio, eperezma, xuanzhuo,
nicolinc, jgg
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-29 11:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-29 8:40 [PATCH] virtio_pci: don't take the MSI mutex under cpu_hotplug_lock Zenghui Yu
2026-10-29 10:15 ` Michael S. Tsirkin
2026-10-29 11:49 ` Zenghui Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®