Re: [PATCH v2 4/7] vga_switcheroo: Add post_switch callback to client ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware failure during mux switch leaves vga_switcheroo in an unrecoverable state with no active client.
--

--- Patch [4]: [PATCH v2 4/7] vga_switcheroo: Add post_switch callback to client ops ---
commit e87434614b85f1234ff64f6a0aeb8bd282739a69
Author: Thomas Zimmermann <tzimmermann@suse.de>

vga_switcheroo: Add post_switch callback to client ops

This patch adds a new post_switch callback to struct
vga_switcheroo_client_ops to notify clients about a completed output switch.
This callback is intended to replace the deprecated reprobe client operation.

> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index bdf1e56ae8910..7b0af4a8aa7d0 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -746,7 +746,9 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)

[ ... ]

>  	mutex_lock(&vgasr_priv.mux_hw_lock);
>  	ret = vgasr_priv.handler->switchto(new_client->id);
>  	mutex_unlock(&vgasr_priv.mux_hw_lock);
>  	if (ret)
>  		return ret;

[Severity: High]
This is a pre-existing issue, but if the hardware switch fails and returns an
error code here, does this leave the state machine unrecoverable?

Earlier in vga_switchto_stage2(), the active client is found and its active
flag is cleared:

drivers/gpu/vga/vga_switcheroo.c:vga_switchto_stage2() {
	active = find_active_client(&vgasr_priv.clients);
	if (!active)
		return 0;

	active->active = false;
	...

If vgasr_priv.handler->switchto() returns an error, the early return happens
without restoring active->active to true. This appears to leave the subsystem
with no active client, causing subsequent calls to find_active_client() to
fail and preventing any future GPU switches until a module reload or reboot.
Should the active flag be restored on this error path?

>  
> -	if (new_client->ops->reprobe)
> +	if (new_client->ops->post_switch)
> +		new_client->ops->post_switch(new_client->pdev);
> +	else if (new_client->ops->reprobe)
>  		new_client->ops->reprobe(new_client->pdev);
>  
>  	if (vga_switcheroo_pwr_state(active) == VGA_SWITCHEROO_ON)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709092215.168172-1-tzimmermann@suse.de?part=4