Re: [PATCH] drm/vmwgfx: uninstall the IRQ before freeing the command buffer manager on unload

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

On 7/14/26 2:45 AM, Fan Wu wrote:
> vmw_driver_unload() frees the command buffer manager before uninstalling
> the device IRQ. vmw_release_device_late() -> vmw_cmdbuf_man_destroy()
> frees the manager, while the threaded handler vmw_thread_fn(), released
> only later by vmw_irq_uninstall() -> free_irq(), dereferences
> dev_priv->cman without a NULL guard and is the sole producer of
> schedule_work(&man->work), whose worker vmw_cmdbuf_work_func() recovers
> the manager via container_of(). dev_priv->cman is never NULLed on the
> unload path, so a handler woken in the window between kfree(man) and
> free_irq() runs against freed memory and can re-arm man->work after the
> manager's cancel_work_sync() has already returned.
> 
> Reorder the unload path to drain pending fences, then uninstall the IRQ,
> and only then free the manager: free_irq() guarantees the threaded
> handler has exited before kfree(man). vmw_fence_fifo_down() is called
> explicitly before the IRQ uninstall so its dma_fence waits are signalled
> by the still-live threaded handler (vmw_fences_update() in
> vmw_thread_fn()); uninstalling the IRQ first could force pending fence
> waits to time out at VMW_FENCE_WAIT_TIMEOUT. vmw_release_device_late()
> is left unchanged because it is shared with the hibernation path; its
> existing call to vmw_fence_fifo_down() runs against an empty fence list
> after the explicit drain above.
> 
> Fixes: ef369904aaf7 ("drm/vmwgfx: Move irq bottom half processing to threads")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 599052d07ae88af2f5775f968fe1e7046a3efe4e..dbe02fe8d8571587733d6f002436bf18dac1887a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1181,10 +1181,11 @@
>  	vmw_devcaps_destroy(dev_priv);
>  	vmw_vram_manager_fini(dev_priv);
>  	ttm_device_fini(&dev_priv->bdev);
> -	vmw_release_device_late(dev_priv);
> -	vmw_fence_manager_takedown(dev_priv->fman);
> +	vmw_fence_fifo_down(dev_priv->fman);
>  	if (dev_priv->capabilities & SVGA_CAP_IRQMASK)
>  		vmw_irq_uninstall(&dev_priv->drm);
> +	vmw_release_device_late(dev_priv);
> +	vmw_fence_manager_takedown(dev_priv->fman);
> 
>  	ttm_object_device_release(&dev_priv->tdev);
> 
> --
> 2.39.5
> 

With this patch we would miss any command buffer processing done on the svga
device in the window between irq_uninstall and vmw_cmdbuf_man_destroy(), so it's
better to leave the takedown order as is.
If you want to address the case where a thread_fn awoken by an earlier irq is
scheduled before irq_uinstall but after vmw_cmdbuf_man_destroy(), then just add checks
to vmw_thread_fn(), though I'm not sure if that case is even possible since the
command buffer contexts should already be taken down at that point.


-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com>