Re: [PATCH 06/10] drm/i915: Implement vga_switcheroo_client_ops.pre_switch

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 07.07.26 um 16:17 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] AB-BA deadlock between `vgasr_mutex` and `dev->clientlist_mutex` during VGA switcheroo and DRM client hotplug/registration.
> --
>
> --- Patch [6]: [PATCH 06/10] drm/i915: Implement vga_switcheroo_client_ops.pre_switch ---
> commit ca43d8e034e1ee27918b22e1b7a19b8f6a5b4a67
> Author: Thomas Zimmermann <tzimmermann@suse.de>
>
> drm/i915: Implement vga_switcheroo_client_ops.pre_switch
>
> This commit implements the pre_switch callback for vga_switcheroo in the
> i915 driver. It calls drm_client_dev_acquire_outputs() to push fbcon
> updates from vga_switcheroo into DRM's fbdev emulation.
>
>> diff --git a/drivers/gpu/drm/i915/i915_switcheroo.c b/drivers/gpu/drm/i915/i915_switcheroo.c
>> index 7e0791024282e..6b306ece0556a 100644
>> --- a/drivers/gpu/drm/i915/i915_switcheroo.c
>> +++ b/drivers/gpu/drm/i915/i915_switcheroo.c
> [ ... ]
>> @@ -58,10 +59,19 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
>>   		atomic_read(&i915->drm.open_count) == 0;
>>   }
>>   
>> +static void i915_switcheroo_pre_switch(struct pci_dev *pdev)
>> +{
>> +	struct drm_i915_private *i915 = pdev_to_i915(pdev);
>> +
>> +	if (i915 && intel_display_device_present(i915->display))
>> +		drm_client_dev_acquire_outputs(&i915->drm);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> [Severity: High]
> Does this code introduce an AB-BA deadlock between vgasr_mutex and
> dev->clientlist_mutex?
>
> A GPU switch triggered via userspace will acquire vgasr_mutex and eventually
> call this pre_switch callback:
>
> vga_switcheroo_debugfs_write()  [acquires vgasr_mutex]
>    vga_switchto_stage2()
>      i915_switcheroo_pre_switch()
>        drm_client_dev_acquire_outputs()  [acquires dev->clientlist_mutex]
>
> However, a DRM client hotplug event can acquire these locks in the reverse
> order:
>
> drm_client_dev_hotplug()  [acquires dev->clientlist_mutex]
>    drm_fbdev_client_hotplug()
>      register_framebuffer()
>        fbcon_fb_registered()
>          do_fb_registered()
>            vga_switcheroo_client_fb_set()  [acquires vgasr_mutex]
>
> Could this lead to a system hang when a GPU switch races with a display
> hotplug event?

Patch 10 removes calls to vga_switcheroo_client_fb_set() entirely. I 
could try to reorganize the series to avoid the vulnerable intermediate 
states.

>
>> +}
>> +
>>   static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>>   	.set_gpu_state = i915_switcheroo_set_state,
>>   	.reprobe = NULL,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)