Re: [PATCH v2 7/7] vt: Remove old-style con_blank

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] vt: loss of text mode on unblanking if VT was blanked prior to KD_GRAPHICS
- [Medium] vt: display corruption due to premature vga_is_gfx assignment
--

commit 397a971ae2e535d87f46ac40e90cdd6df9d84c84
Author: Thomas Zimmermann <tzimmermann@suse.de>
Subject: vt: Remove old-style con_blank

This commit removes the mode_switch parameter from con_blank and con_unblank
callbacks. Consoles are now updated to detect mode switches internally from
the console state and vc_mode.

> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4674,10 +4674,7 @@ void do_blank_screen(int entering_gfx)
>  	if (console_blanked) {
>  		if (blank_state == blank_vesa_wait) {
>  			blank_state = blank_off;
> -			if (vc->vc_sw->con_unblank)
> -				vc->vc_sw->con_blank(vc, vesa_blank_mode, 0);
> -			else
> -				vc->vc_sw->con_blank(vc, vesa_blank_mode + 1, 0);
> +			vc->vc_sw->con_blank(vc, vesa_blank_mode);
>  		}
>  		return;
>  	}

[Severity: High]
If the console is already blanked and not in the vesa_wait state,
do_blank_screen() returns early and skips the con_blank callback entirely.

Because this patch removes the leaving_gfx parameter from the unblank
callback, fbcon_unblank() must now deduce the mode switch internally by
checking if par->graphics is true.

Since par->graphics is only set to true inside fbcon_blank(), skipping
fbcon_blank() leaves it as false:

do_blank_screen()
    if (console_blanked) {
        return; /* fbcon_blank is skipped, par->graphics remains false */
    }

When fbcon_unblank() is later called to switch back to KD_TEXT:

fbcon_unblank()
    bool mode_switch = par->graphics && vc->vc_mode == KD_TEXT;
    if (mode_switch) {
        /* restore text mode */
    }

Does this cause the hardware text mode restoration to be bypassed when
mode_switch evaluates to false, leaving the VT frozen?

> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -797,15 +797,16 @@ static void vga_pal_blank(struct vgastate *state)
>  	}
>  }
>  
> -static bool vgacon_blank(struct vc_data *c, enum vesa_blank_mode blank,
> -			 bool mode_switch)
> +static bool vgacon_blank(struct vc_data *c, enum vesa_blank_mode blank)
>  {
> +	bool mode_switch = !vga_is_gfx && c->vc_mode == KD_GRAPHICS;
> +
> +	if (mode_switch)
> +		vga_is_gfx = true;
> +

[Severity: Medium]
Does setting vga_is_gfx = true here cause vgacon_set_origin() to abort
early?

vgacon_set_origin() is called later in this function during normal blanking:

vgacon_blank()
    if (mode_switch)
        vga_is_gfx = true;

    switch (blank) {
    case VESA_NO_BLANKING:
        vgacon_set_origin(c);

However, vgacon_set_origin() includes an early return if vga_is_gfx is true:

vgacon_set_origin()
    if (vga_is_gfx || ... )
        return;

If vgacon_set_origin() is skipped, the hardware CRTC offset won't be
reset to vga_vram_base. Even though scr_memsetw() clears the memory
at vga_vram_base, will the unreset CRTC offset cause the display hardware
to continue showing old memory, resulting in display corruption?

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