The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker
@ 2026-07-13 13:01 Ryosuke Yasuoka
  2026-07-13 16:31 ` Dmitry Osipenko
  0 siblings, 1 reply; 3+ messages in thread
From: Ryosuke Yasuoka @ 2026-07-13 13:01 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
	Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas
  Cc: dri-devel, virtualization, linux-kernel, Ryosuke Yasuoka

A probe-time deadlock can occur between the dequeue worker and
drm_client_register(). During probe, drm_client_register() holds
clientlist_mutex and calls the fbdev hotplug callback, which triggers an
atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
waiting for virtqueue space. The dequeue worker that would free that
space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
to acquire the same clientlist_mutex. Since wake_up() is only called
after the resp_cb loop, the probe thread is never woken and both threads
deadlock.

Fix this by removing the hotplug notification from
virtio_gpu_cmd_get_display_info_cb(). The display data (outputs[i].info)
is still updated synchronously in the callback.

For the init path, drm_client_register() already fires an initial
hotplug when the client is registered, which picks up the connector
state updated by display_info_cb.

For the runtime config_changed path, add a wait_event_timeout() in
config_changed_work_func() so that display_info_cb updates the connector
data before the hotplug notification is sent. Also replace
drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event() since
virtio-gpu never calls drm_kms_helper_poll_init() and thus
drm_helper_hpd_irq_event() always returns false without doing anything.

Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
I checked whether drm_helper_hpd_irq_event() is needed in
virtio_gpu_init(), as Dmitry suggested. AFAIS, it is not needed because:

1. drm_helper_hpd_irq_event() is always a no-op in virtio-gpu.
   It returns false immediately probe_helper.c:1088 because
   dev->mode_config.poll_enabled is false — virtio-gpu never calls
   drm_kms_helper_poll_init(). Even if it passed that gate, no
   virtio-gpu connectors set DRM_CONNECTOR_POLL_HPD.

1082 bool drm_helper_hpd_irq_event(struct drm_device *dev)
1083 {
...
1088         if (!dev->mode_config.poll_enabled)
1089                 return false;

2. virtio_gpu_init() runs before drm_dev_register() and
   drm_client_setup(), so no DRM clients are registered yet.
   drm_kms_helper_hotplug_event() would iterate an empty client list.
   The initial hotplug is handled by drm_client_register(), which fires
   a hotplug callback to the newly registered client. By that time,
   display_info_cb has already updated the connector data.

For the same reason, drm_helper_hpd_irq_event() in
config_changed_work_func() was also a no-op. The actual runtime hotplug
notification was always delivered by display_info_cb's call to
drm_kms_helper_hotplug_event(). This patch replaces it with a direct
drm_kms_helper_hotplug_event() call after waiting for the display info
response.
---
Changes in v2:
- Dropped the work_struct approach from v1.
- Instead, removed the hotplug calls from display_info_cb entirely, as
suggested by Dmitry.
- Added wait_event_timeout() in config_changed_work_func() so that the
display info response is received before sending the hotplug
notification.
- Replaced drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event()
in config_changed_work_func() since drm_helper_hpd_irq_event() is
always a no-op in virtio-gpu (poll_enabled is never set).
- No changes to virtio_gpu_init() — drm_client_register() already
handles the initial hotplug and hotplug event does nothing before DRM
device/client has been registered.
- Link to v1: https://lore.kernel.org/r/20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 5 ++++-
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 3 ---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index cfde9f573df6..b4329f28e976 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -49,7 +49,10 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
 				virtio_gpu_cmd_get_edids(vgdev);
 			virtio_gpu_cmd_get_display_info(vgdev);
 			virtio_gpu_notify(vgdev);
-			drm_helper_hpd_irq_event(vgdev->ddev);
+			wait_event_timeout(vgdev->resp_wq,
+					   !vgdev->display_info_pending,
+					   5 * HZ);
+			drm_kms_helper_hotplug_event(vgdev->ddev);
 		}
 		events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
 	}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index c8b9475a7472..e5e1af8b8e8a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -840,9 +840,6 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
 	vgdev->display_info_pending = false;
 	spin_unlock(&vgdev->display_info_lock);
 	wake_up(&vgdev->resp_wq);
-
-	if (!drm_helper_hpd_irq_event(vgdev->ddev))
-		drm_kms_helper_hotplug_event(vgdev->ddev);
 }
 
 static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,

---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260619-virtiogpu_syzbot-bdab508ffcd5

Best regards,
-- 
Ryosuke Yasuoka <ryasuoka@redhat.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker
  2026-07-13 13:01 [PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker Ryosuke Yasuoka
@ 2026-07-13 16:31 ` Dmitry Osipenko
  2026-07-23  7:14   ` Mikko Rapeli
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Osipenko @ 2026-07-13 16:31 UTC (permalink / raw)
  To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
	Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas
  Cc: dri-devel, virtualization, linux-kernel

On 7/13/26 16:01, Ryosuke Yasuoka wrote:
> A probe-time deadlock can occur between the dequeue worker and
> drm_client_register(). During probe, drm_client_register() holds
> clientlist_mutex and calls the fbdev hotplug callback, which triggers an
> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
> waiting for virtqueue space. The dequeue worker that would free that
> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
> to acquire the same clientlist_mutex. Since wake_up() is only called
> after the resp_cb loop, the probe thread is never woken and both threads
> deadlock.
> 
> Fix this by removing the hotplug notification from
> virtio_gpu_cmd_get_display_info_cb(). The display data (outputs[i].info)
> is still updated synchronously in the callback.
> 
> For the init path, drm_client_register() already fires an initial
> hotplug when the client is registered, which picks up the connector
> state updated by display_info_cb.
> 
> For the runtime config_changed path, add a wait_event_timeout() in
> config_changed_work_func() so that display_info_cb updates the connector
> data before the hotplug notification is sent. Also replace
> drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event() since
> virtio-gpu never calls drm_kms_helper_poll_init() and thus
> drm_helper_hpd_irq_event() always returns false without doing anything.
> 
> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> I checked whether drm_helper_hpd_irq_event() is needed in
> virtio_gpu_init(), as Dmitry suggested. AFAIS, it is not needed because:
> 
> 1. drm_helper_hpd_irq_event() is always a no-op in virtio-gpu.
>    It returns false immediately probe_helper.c:1088 because
>    dev->mode_config.poll_enabled is false — virtio-gpu never calls
>    drm_kms_helper_poll_init(). Even if it passed that gate, no
>    virtio-gpu connectors set DRM_CONNECTOR_POLL_HPD.
> 
> 1082 bool drm_helper_hpd_irq_event(struct drm_device *dev)
> 1083 {
> ...
> 1088         if (!dev->mode_config.poll_enabled)
> 1089                 return false;
> 
> 2. virtio_gpu_init() runs before drm_dev_register() and
>    drm_client_setup(), so no DRM clients are registered yet.
>    drm_kms_helper_hotplug_event() would iterate an empty client list.
>    The initial hotplug is handled by drm_client_register(), which fires
>    a hotplug callback to the newly registered client. By that time,
>    display_info_cb has already updated the connector data.
> 
> For the same reason, drm_helper_hpd_irq_event() in
> config_changed_work_func() was also a no-op. The actual runtime hotplug
> notification was always delivered by display_info_cb's call to
> drm_kms_helper_hotplug_event(). This patch replaces it with a direct
> drm_kms_helper_hotplug_event() call after waiting for the display info
> response.
> ---
> Changes in v2:
> - Dropped the work_struct approach from v1.
> - Instead, removed the hotplug calls from display_info_cb entirely, as
> suggested by Dmitry.
> - Added wait_event_timeout() in config_changed_work_func() so that the
> display info response is received before sending the hotplug
> notification.
> - Replaced drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event()
> in config_changed_work_func() since drm_helper_hpd_irq_event() is
> always a no-op in virtio-gpu (poll_enabled is never set).
> - No changes to virtio_gpu_init() — drm_client_register() already
> handles the initial hotplug and hotplug event does nothing before DRM
> device/client has been registered.
> - Link to v1: https://lore.kernel.org/r/20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com
> ---
>  drivers/gpu/drm/virtio/virtgpu_kms.c | 5 ++++-
>  drivers/gpu/drm/virtio/virtgpu_vq.c  | 3 ---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index cfde9f573df6..b4329f28e976 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -49,7 +49,10 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
>  				virtio_gpu_cmd_get_edids(vgdev);
>  			virtio_gpu_cmd_get_display_info(vgdev);
>  			virtio_gpu_notify(vgdev);
> -			drm_helper_hpd_irq_event(vgdev->ddev);
> +			wait_event_timeout(vgdev->resp_wq,
> +					   !vgdev->display_info_pending,
> +					   5 * HZ);
> +			drm_kms_helper_hotplug_event(vgdev->ddev);
>  		}
>  		events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
>  	}
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index c8b9475a7472..e5e1af8b8e8a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -840,9 +840,6 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>  	vgdev->display_info_pending = false;
>  	spin_unlock(&vgdev->display_info_lock);
>  	wake_up(&vgdev->resp_wq);
> -
> -	if (!drm_helper_hpd_irq_event(vgdev->ddev))
> -		drm_kms_helper_hotplug_event(vgdev->ddev);
>  }
>  
>  static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
> 
> ---
> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
> change-id: 20260619-virtiogpu_syzbot-bdab508ffcd5
> 
> Best regards,

Appled to misc-fixes, thanks!

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker
  2026-07-13 16:31 ` Dmitry Osipenko
@ 2026-07-23  7:14   ` Mikko Rapeli
  0 siblings, 0 replies; 3+ messages in thread
From: Mikko Rapeli @ 2026-07-23  7:14 UTC (permalink / raw)
  To: Dmitry Osipenko, stable
  Cc: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
	Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas,
	dri-devel, virtualization, linux-kernel

Hi, adding stable@vger.kernel.org

In Yocto distro, this change released in v7.2-rc4 seems to fix quite severe qemu
boot hangs seen with 6.18 stable kernels. Details in
https://bugzilla.yoctoproject.org/show_bug.cgi?id=16217

Please apply this to to 6.18 and other stable trees.

Cheers,

-Mikko

On Mon, Jul 13, 2026 at 07:31:14PM +0300, Dmitry Osipenko wrote:
> On 7/13/26 16:01, Ryosuke Yasuoka wrote:
> > A probe-time deadlock can occur between the dequeue worker and
> > drm_client_register(). During probe, drm_client_register() holds
> > clientlist_mutex and calls the fbdev hotplug callback, which triggers an
> > atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
> > waiting for virtqueue space. The dequeue worker that would free that
> > space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
> > drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
> > to acquire the same clientlist_mutex. Since wake_up() is only called
> > after the resp_cb loop, the probe thread is never woken and both threads
> > deadlock.
> > 
> > Fix this by removing the hotplug notification from
> > virtio_gpu_cmd_get_display_info_cb(). The display data (outputs[i].info)
> > is still updated synchronously in the callback.
> > 
> > For the init path, drm_client_register() already fires an initial
> > hotplug when the client is registered, which picks up the connector
> > state updated by display_info_cb.
> > 
> > For the runtime config_changed path, add a wait_event_timeout() in
> > config_changed_work_func() so that display_info_cb updates the connector
> > data before the hotplug notification is sent. Also replace
> > drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event() since
> > virtio-gpu never calls drm_kms_helper_poll_init() and thus
> > drm_helper_hpd_irq_event() always returns false without doing anything.
> > 
> > Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
> > Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> > Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> > Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> > ---
> > I checked whether drm_helper_hpd_irq_event() is needed in
> > virtio_gpu_init(), as Dmitry suggested. AFAIS, it is not needed because:
> > 
> > 1. drm_helper_hpd_irq_event() is always a no-op in virtio-gpu.
> >    It returns false immediately probe_helper.c:1088 because
> >    dev->mode_config.poll_enabled is false — virtio-gpu never calls
> >    drm_kms_helper_poll_init(). Even if it passed that gate, no
> >    virtio-gpu connectors set DRM_CONNECTOR_POLL_HPD.
> > 
> > 1082 bool drm_helper_hpd_irq_event(struct drm_device *dev)
> > 1083 {
> > ...
> > 1088         if (!dev->mode_config.poll_enabled)
> > 1089                 return false;
> > 
> > 2. virtio_gpu_init() runs before drm_dev_register() and
> >    drm_client_setup(), so no DRM clients are registered yet.
> >    drm_kms_helper_hotplug_event() would iterate an empty client list.
> >    The initial hotplug is handled by drm_client_register(), which fires
> >    a hotplug callback to the newly registered client. By that time,
> >    display_info_cb has already updated the connector data.
> > 
> > For the same reason, drm_helper_hpd_irq_event() in
> > config_changed_work_func() was also a no-op. The actual runtime hotplug
> > notification was always delivered by display_info_cb's call to
> > drm_kms_helper_hotplug_event(). This patch replaces it with a direct
> > drm_kms_helper_hotplug_event() call after waiting for the display info
> > response.
> > ---
> > Changes in v2:
> > - Dropped the work_struct approach from v1.
> > - Instead, removed the hotplug calls from display_info_cb entirely, as
> > suggested by Dmitry.
> > - Added wait_event_timeout() in config_changed_work_func() so that the
> > display info response is received before sending the hotplug
> > notification.
> > - Replaced drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event()
> > in config_changed_work_func() since drm_helper_hpd_irq_event() is
> > always a no-op in virtio-gpu (poll_enabled is never set).
> > - No changes to virtio_gpu_init() — drm_client_register() already
> > handles the initial hotplug and hotplug event does nothing before DRM
> > device/client has been registered.
> > - Link to v1: https://lore.kernel.org/r/20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_kms.c | 5 ++++-
> >  drivers/gpu/drm/virtio/virtgpu_vq.c  | 3 ---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > index cfde9f573df6..b4329f28e976 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > @@ -49,7 +49,10 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
> >  				virtio_gpu_cmd_get_edids(vgdev);
> >  			virtio_gpu_cmd_get_display_info(vgdev);
> >  			virtio_gpu_notify(vgdev);
> > -			drm_helper_hpd_irq_event(vgdev->ddev);
> > +			wait_event_timeout(vgdev->resp_wq,
> > +					   !vgdev->display_info_pending,
> > +					   5 * HZ);
> > +			drm_kms_helper_hotplug_event(vgdev->ddev);
> >  		}
> >  		events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
> >  	}
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> > index c8b9475a7472..e5e1af8b8e8a 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> > @@ -840,9 +840,6 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
> >  	vgdev->display_info_pending = false;
> >  	spin_unlock(&vgdev->display_info_lock);
> >  	wake_up(&vgdev->resp_wq);
> > -
> > -	if (!drm_helper_hpd_irq_event(vgdev->ddev))
> > -		drm_kms_helper_hotplug_event(vgdev->ddev);
> >  }
> >  
> >  static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
> > 
> > ---
> > base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
> > change-id: 20260619-virtiogpu_syzbot-bdab508ffcd5
> > 
> > Best regards,
> 
> Appled to misc-fixes, thanks!
> 
> -- 
> Best regards,
> Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-23  7:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 13:01 [PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker Ryosuke Yasuoka
2026-07-13 16:31 ` Dmitry Osipenko
2026-07-23  7:14   ` Mikko Rapeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox