From: sashiko-bot@kernel.org
To: "Thomas Zimmermann" <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH 3/5] vgacon: Use vt_font_cursor_{start,end}()
Date: Fri, 11 Sep 2026 09:58:16 +0000 [thread overview]
Message-ID: <20260911095817.D3BBC1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260911094518.78093-4-tzimmermann@suse.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Cursor bounds are calculated using `vc_font.height` instead of `vc_cell_height`, breaking hardware cursors when the text cell height is overridden.
--
commit 25f4b9b96dc26e69ace4c42e72c99970c3c5fcf1
Author: Thomas Zimmermann <tzimmermann@suse.de>
vgacon: Use vt_font_cursor_{start,end}()
This commit replaces vgacon's custom cursor size calculations with the shared
helpers vc_font_cursor_start() and vc_font_cursor_end(). It subtracts one
from the end value to account for VGA hardware expectations.
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 536e7fe4d1426..b16ddb1b09a70 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
[ ... ]
> @@ -524,33 +525,8 @@ static void vgacon_cursor(struct vc_data *c, bool enable)
> return;
> }
>
> - switch (CUR_SIZE(c->vc_cursor_type)) {
[ ... ]
> - default:
> - vgacon_set_cursor_size(1, c_height);
> - break;
> - }
> + vgacon_set_cursor_size(vc_font_cursor_start(&c->vc_font, c_type),
> + vc_font_cursor_end(&c->vc_font, c_type) - 1);
[Severity: High]
Does this code break the hardware cursor when the character cell height is
dynamically overridden?
If a user space utility like SVGATextMode calls the VT_RESIZEX ioctl with a
smaller v_clin, c->vc_cell_height is updated while c->vc_font.height remains
unchanged.
By computing the cursor bounds using the static font height via the shared
helpers instead of the active character cell height, it seems the programmed
start scanline could exceed the maximum scanline of the character cell
(VGA_CRTC_MAX_SCAN).
Since the hardware scanline counter resets before reaching the programmed cursor
start line, would this cause the hardware cursor to become completely invisible?
> }
>
> static void vgacon_doresize(struct vc_data *c,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911094518.78093-1-tzimmermann@suse.de?part=3
next prev parent reply other threads:[~2026-09-11 9:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-11 9:25 [PATCH 0/5] fbcon,vgacon,vt: Share helpers for text cursors Thomas Zimmermann
2026-09-11 9:25 ` [PATCH 1/5] vt: Add cursor-size helpers Thomas Zimmermann
2026-09-11 9:55 ` sashiko-bot
2026-09-12 19:01 ` Helge Deller
2026-09-11 9:25 ` [PATCH 2/5] vgacon: Remove trailing whitespaces Thomas Zimmermann
2026-09-11 9:46 ` sashiko-bot
2026-09-11 9:25 ` [PATCH 3/5] vgacon: Use vt_font_cursor_{start,end}() Thomas Zimmermann
2026-09-11 9:58 ` sashiko-bot [this message]
2026-09-11 9:25 ` [PATCH 4/5] lib/fonts: Add font_glyph_cursor() helper Thomas Zimmermann
2026-09-11 9:52 ` sashiko-bot
2026-09-11 9:25 ` [PATCH 5/5] fbcon: Replace fbcon_fill_cursor_mask() with fbcon_cursor_glyph() Thomas Zimmermann
2026-09-11 9:50 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260911095817.D3BBC1F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-serial@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox