Re: [Patch 5/9]: vt: Amend three Kconfig files

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Enabling CONFIG_FB_GLYPH_21BIT changes GLYPH_SZ to 4 globally, breaking hardware console drivers (like vgacon) and rotated fbcon drivers that still hardcode 16-bit screen buffer accesses.
- [Medium] FB_GLYPH_21BIT is a generic VT/fbdev feature but is inappropriately placed in the DRM clients menu, unnecessarily restricting its use.
--

commit 7b9fb605add2fa8c6f62e8ba79cee762cbcbd24c
Author: Alan Mackenzie <[email protected]>

vt: Amend three Kconfig files

Until it becomes clear how to integrate CONFIG_A11Y_BRAILLE_CONSOLE and
CONFIG_SPEAKUP, they will remain disabled when CONFIG_FB_GLYPH_21BIT is
#defined.

[ ... ]

> diff --git a/drivers/gpu/drm/clients/Kconfig b/drivers/gpu/drm/clients/Kconfig
> index 6096c623d9d5b..0ce8c1506eaad 100644
> --- a/drivers/gpu/drm/clients/Kconfig
> +++ b/drivers/gpu/drm/clients/Kconfig
> @@ -42,6 +42,15 @@ config DRM_FBDEV_EMULATION
>  
>  	  If in doubt, say "Y".
>  
> +config FB_GLYPH_21BIT
> +	bool "Enable 21-bit glyphs in the frame buffer device"
> +	depends on (DRM_FBDEV_EMULATION = y)

[Severity: Medium]
Could placing this generic VT/fbdev feature in the DRM clients menu
inappropriately restrict its use?

Native framebuffer drivers (e.g., vesafb, efifb, simplefb) might be prevented
from utilizing 21-bit glyph support, as this strictly requires
DRM_FBDEV_EMULATION. The feature modifies the generic Virtual Terminal and
fbcon subsystems, which are independent of DRM.

[Severity: High]
Can enabling CONFIG_FB_GLYPH_21BIT corrupt the screen buffer for hardware
console drivers and rotated fbcon drivers?

When this is enabled, GLYPH_SZ becomes 4 globally and VT uses 32-bit screen
buffer entries. However, the consw interface still passes pointers typed as
const unsigned short *, and some drivers still hardcode 16-bit accesses.

For example, rotated fbcon implementations iterate the buffer using scr_readw()
which advances the u16 pointer by 2 bytes instead of 4, reading half-characters:

drivers/video/fbdev/core/fbcon_cw.c:cw_putcs_aligned() {
    ...
    src = par->rotated.buf + (scr_readw(s++) & charmask) * cellsize;
    ...
}

Similarly, vgacon maps physical VGA VRAM assuming exactly 16 bits per character,
meaning 32-bit writes from scr_writeg in vt.c will overwrite adjacent cells
in hardware memory:

drivers/video/console/vgacon.c:vgacon_invert_region() {
    ...
    u16 a = scr_readw(p);
    ...
    scr_writew(a, p++);
    ...
}

> +	help
> +	  This option enables 21-bit glyphs in the fbdev framebuffer,
> +	  surmounting the traditional restriction to 256/512 glyphs dating back
> +	  to the VGA hardware of the 1980s and 1990s.  To use this effectively
> +	  you will need a suitable font and possibly a suitable keyboard layout.
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5