[PATCH 26/60] ui/console-vc: unify the write path

Marc-André Lureau posted 60 patches 2 weeks, 6 days ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, John Snow <jsnow@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Mauro Carvalho Chehab <mchehab+huawei@kernel.org>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Jan Kiszka <jan.kiszka@web.de>, Phil Dennis-Jordan <phil@philjordan.eu>, Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Gerd Hoffmann <kraxel@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Samuel Tardieu <sam@rfc1149.net>, Igor Mitsyanko <i.mitsyanko@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <arikalo@gmail.com>, Laurent Vivier <laurent@vivier.eu>, Thomas Huth <th.huth+qemu@posteo.eu>, BALATON Zoltan <balaton@eik.bme.hu>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Fabiano Rosas <farosas@suse.de>
[PATCH 26/60] ui/console-vc: unify the write path
Posted by Marc-André Lureau 2 weeks, 6 days ago
VT100 escape responses (DSR) used qemu_chr_be_write() to write directly
to the chardev backend, bypassing the output FIFO, while keyboard input
went through the FIFO and flush path. This inconsistency could lead to
out-of-order delivery when both paths are active.

Introduce qemu_text_console_write() that pushes data into the output
FIFO and flushes it, and use it for both keyboard input and VT100
responses. Remove the now-unnecessary vc_respond_str() helper. Rename
kbd_send_chars() to qemu_text_console_flush() to better reflect its
purpose.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console-vc.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/ui/console-vc.c b/ui/console-vc.c
index 4ea9f88f55a..8d4178f8cab 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -299,7 +299,7 @@ static void vt100_scroll(QemuVT100 *vt, int ydelta)
     vt100_refresh(vt);
 }
 
-static void kbd_send_chars(QemuTextConsole *s)
+static void qemu_text_console_flush(QemuTextConsole *s)
 {
     uint32_t len, avail;
 
@@ -316,12 +316,20 @@ static void kbd_send_chars(QemuTextConsole *s)
     }
 }
 
+static void qemu_text_console_write(QemuTextConsole *s, const void *buf, size_t len)
+{
+    uint32_t num_free;
+
+    num_free = fifo8_num_free(&s->out_fifo);
+    fifo8_push_all(&s->out_fifo, buf, MIN(num_free, len));
+    qemu_text_console_flush(s);
+}
+
 /* called when an ascii key is pressed */
 void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym)
 {
     uint8_t buf[16], *q;
     int c;
-    uint32_t num_free;
 
     switch(keysym) {
     case QEMU_KEY_CTRL_UP:
@@ -360,9 +368,7 @@ void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym)
         if (s->vt.echo) {
             qemu_chr_write(s->chr, buf, q - buf, true);
         }
-        num_free = fifo8_num_free(&s->out_fifo);
-        fifo8_push_all(&s->out_fifo, buf, MIN(num_free, q - buf));
-        kbd_send_chars(s);
+        qemu_text_console_write(s, buf, q - buf);
         break;
     }
 }
@@ -673,13 +679,6 @@ static void vc_put_one(VCChardev *vc, int ch)
     s->vt.x++;
 }
 
-static void vc_respond_str(VCChardev *vc, const char *buf)
-{
-    QemuTextConsole *s = vc->console;
-
-    qemu_chr_be_write(s->chr, (const uint8_t *)buf, strlen(buf));
-}
-
 /* set cursor, checking bounds */
 static void vc_set_cursor(VCChardev *vc, int x, int y)
 {
@@ -1018,13 +1017,13 @@ static void vc_putchar(VCChardev *vc, int ch)
                 switch (vc->esc_params[0]) {
                 case 5:
                     /* report console status (always succeed)*/
-                    vc_respond_str(vc, "\033[0n");
+                    qemu_text_console_write(s, "\033[0n", 4);
                     break;
                 case 6:
                     /* report cursor position */
                     response = g_strdup_printf("\033[%d;%dR",
                                                s->vt.y + 1, s->vt.x + 1);
-                    vc_respond_str(vc, response);
+                    qemu_text_console_write(s, response, strlen(response));
                     break;
                 }
                 break;
@@ -1183,7 +1182,7 @@ static void vc_chr_accept_input(Chardev *chr)
 {
     VCChardev *drv = VC_CHARDEV(chr);
 
-    kbd_send_chars(drv->console);
+    qemu_text_console_flush(drv->console);
 }
 
 static void vc_chr_set_echo(Chardev *chr, bool echo)

-- 
2.53.0


Re: [PATCH 26/60] ui/console-vc: unify the write path
Posted by Philippe Mathieu-Daudé 5 days, 11 hours ago
On 17/3/26 09:50, Marc-André Lureau wrote:
> VT100 escape responses (DSR) used qemu_chr_be_write() to write directly
> to the chardev backend, bypassing the output FIFO, while keyboard input
> went through the FIFO and flush path. This inconsistency could lead to
> out-of-order delivery when both paths are active.
> 
> Introduce qemu_text_console_write() that pushes data into the output
> FIFO and flushes it, and use it for both keyboard input and VT100
> responses. Remove the now-unnecessary vc_respond_str() helper. Rename
> kbd_send_chars() to qemu_text_console_flush() to better reflect its
> purpose.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/console-vc.c | 29 ++++++++++++++---------------
>   1 file changed, 14 insertions(+), 15 deletions(-)


> +static void qemu_text_console_write(QemuTextConsole *s, const void *buf, size_t len)

Nitpicking, maybe take 'uint8_t *buf' (or char *?) argument for this
console method, letting the FIFO8 API deals with void *?

> +{
> +    uint32_t num_free;
> +
> +    num_free = fifo8_num_free(&s->out_fifo);
> +    fifo8_push_all(&s->out_fifo, buf, MIN(num_free, len));
> +    qemu_text_console_flush(s);
> +}


Re: [PATCH 26/60] ui/console-vc: unify the write path
Posted by Marc-André Lureau 4 days, 11 hours ago
Hi

On Wed, Apr 1, 2026 at 5:42 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 17/3/26 09:50, Marc-André Lureau wrote:
> > VT100 escape responses (DSR) used qemu_chr_be_write() to write directly
> > to the chardev backend, bypassing the output FIFO, while keyboard input
> > went through the FIFO and flush path. This inconsistency could lead to
> > out-of-order delivery when both paths are active.
> >
> > Introduce qemu_text_console_write() that pushes data into the output
> > FIFO and flushes it, and use it for both keyboard input and VT100
> > responses. Remove the now-unnecessary vc_respond_str() helper. Rename
> > kbd_send_chars() to qemu_text_console_flush() to better reflect its
> > purpose.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   ui/console-vc.c | 29 ++++++++++++++---------------
> >   1 file changed, 14 insertions(+), 15 deletions(-)
>
>
> > +static void qemu_text_console_write(QemuTextConsole *s, const void *buf, size_t len)
>
> Nitpicking, maybe take 'uint8_t *buf' (or char *?) argument for this
> console method, letting the FIFO8 API deals with void *?

The function is called with a variety of arguments: char*, const
char*, uint8_t[16].. So this would add some extra casts, I am not sure
it's worth it.

>
> > +{
> > +    uint32_t num_free;
> > +
> > +    num_free = fifo8_num_free(&s->out_fifo);
> > +    fifo8_push_all(&s->out_fifo, buf, MIN(num_free, len));
> > +    qemu_text_console_flush(s);
> > +}
>
Re: [PATCH 26/60] ui/console-vc: unify the write path
Posted by Philippe Mathieu-Daudé 5 days, 11 hours ago
On 17/3/26 09:50, Marc-André Lureau wrote:
> VT100 escape responses (DSR) used qemu_chr_be_write() to write directly
> to the chardev backend, bypassing the output FIFO, while keyboard input
> went through the FIFO and flush path. This inconsistency could lead to
> out-of-order delivery when both paths are active.
> 
> Introduce qemu_text_console_write() that pushes data into the output
> FIFO and flushes it, and use it for both keyboard input and VT100
> responses. Remove the now-unnecessary vc_respond_str() helper. Rename
> kbd_send_chars() to qemu_text_console_flush() to better reflect its
> purpose.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/console-vc.c | 29 ++++++++++++++---------------
>   1 file changed, 14 insertions(+), 15 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>