All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Volker Rümelin" <vr_qemu@t-online.de>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Zack Marvel <zpmarvel@gmail.com>, qemu-devel@nongnu.org
Subject: [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles
Date: Sun, 18 Jul 2021 09:47:56 +0200	[thread overview]
Message-ID: <20210718074757.22489-1-vr_qemu@t-online.de> (raw)
In-Reply-To: <9e436e5c-ed11-69ec-3cb9-a19cbf96cb08@t-online.de>

Since commit 8eb13bbbac ("ui/gtk: vte: fix sending multiple
characeters") it's very easy to lock up QEMU with the gtk ui.
If you configure a guest with a serial device and the guest
doesn't listen on this device, QEMU will lock up after
entering two characters in the serial console.

To fix this problem copy the function kbd_send_chars() and
related code from ui/console.c to ui/gtk.c. kbd_send_chars()
doesn't lock up because it uses a timer instead of a busy loop
for the write retries.

Fixes: 8eb13bbbac ("ui/gtk: vte: fix sending multiple characeters")
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 include/ui/gtk.h |  5 +++++
 ui/gtk.c         | 53 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 9516670ebc..4714218376 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -25,6 +25,9 @@
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
 #endif
+#ifdef CONFIG_VTE
+#include "qemu/fifo8.h"
+#endif
 
 #define MAX_VCS 10
 
@@ -62,6 +65,8 @@ typedef struct VirtualVteConsole {
     GtkWidget *scrollbar;
     GtkWidget *terminal;
     Chardev *chr;
+    QEMUTimer *kbd_timer;
+    Fifo8 out_fifo;
     bool echo;
 } VirtualVteConsole;
 #endif
diff --git a/ui/gtk.c b/ui/gtk.c
index 376b4d528d..b95b077b65 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -48,6 +48,7 @@
 #include <locale.h>
 #if defined(CONFIG_VTE)
 #include <vte/vte.h>
+#include "chardev/char-fe.h"
 #endif
 #include <math.h>
 
@@ -1710,10 +1711,46 @@ static const TypeInfo char_gd_vc_type_info = {
     .class_init = char_gd_vc_class_init,
 };
 
+static void gd_vc_send_chars(VirtualConsole *vc)
+{
+    uint32_t len, avail;
+    const uint8_t *buf;
+
+    len = qemu_chr_be_can_write(vc->vte.chr);
+    avail = fifo8_num_used(&vc->vte.out_fifo);
+    if (len > avail) {
+        len = avail;
+    }
+    while (len > 0) {
+        uint32_t size;
+
+        buf = fifo8_pop_buf(&vc->vte.out_fifo, len, &size);
+        qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size);
+        len -= size;
+        avail -= size;
+    }
+    /*
+     * characters are pending: we send them a bit later (XXX:
+     * horrible, should change char device API)
+     */
+    if (avail > 0) {
+        timer_mod(vc->vte.kbd_timer,
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1);
+    }
+}
+
+static void gd_vc_timer_send_chars(void *opaque)
+{
+    VirtualConsole *vc = opaque;
+
+    gd_vc_send_chars(vc);
+}
+
 static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
                          gpointer user_data)
 {
     VirtualConsole *vc = user_data;
+    CharBackend *be = vc->vte.chr->be;
 
     if (vc->vte.echo) {
         VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
@@ -1733,16 +1770,13 @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
         }
     }
 
-    int remaining = size;
-    uint8_t* p = (uint8_t *)text;
-    while (remaining > 0) {
-        int can_write = qemu_chr_be_can_write(vc->vte.chr);
-        int written = MIN(remaining, can_write);
-        qemu_chr_be_write(vc->vte.chr, p, written);
+    if (be && be->chr_read) {
+        uint32_t free = fifo8_num_free(&vc->vte.out_fifo);
 
-        remaining -= written;
-        p += written;
+        fifo8_push_all(&vc->vte.out_fifo, (uint8_t *)text, MIN(free, size));
+        gd_vc_send_chars(vc);
     }
+
     return TRUE;
 }
 
@@ -1759,6 +1793,9 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, VirtualConsole *vc,
     vc->s = s;
     vc->vte.echo = vcd->echo;
     vc->vte.chr = chr;
+    fifo8_create(&vc->vte.out_fifo, 16);
+    vc->vte.kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+                                     gd_vc_timer_send_chars, vc);
     vcd->console = vc;
 
     snprintf(buffer, sizeof(buffer), "vc%d", idx);
-- 
2.26.2



  reply	other threads:[~2021-07-18  7:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-18  7:46 [PATCH 0/2] ui/gtk: prevent QEMU lock up Volker Rümelin
2021-07-18  7:47 ` Volker Rümelin [this message]
2021-07-18 13:00   ` [PATCH for 6.1 1/2] ui/gtk: add a keyboard fifo to the VTE consoles Peter Maydell
2021-07-19  9:20     ` Daniel P. Berrangé
2021-07-19 18:22     ` Volker Rümelin
2021-07-19 18:37       ` Peter Maydell
2021-07-19 18:41         ` Daniel P. Berrangé
2021-07-19 18:52           ` Peter Maydell
2021-07-21 12:39   ` Gerd Hoffmann
2021-07-21 21:11     ` Volker Rümelin
2021-07-18  7:47 ` [PATCH for 6.2 2/2] ui/gtk: drop chars if the chardev frontend makes no progress Volker Rümelin

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=20210718074757.22489-1-vr_qemu@t-online.de \
    --to=vr_qemu@t-online.de \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zpmarvel@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.