[Qemu-devel] Re: [PATCH] 8250: more realistic TX-done IRQ rate

[Jan Kiszka]

Jan Kiszka wrote:
> The 8250 UART emulation currently raises a TX-done IRQ immediately when 
> the guest writes out some character. This is problematic for guests like 
> Linux which may flush its output buffer in a loop from IRQ context, 
> because they may then enter a tight loop with IRQs disabled. In fact, 
> Linux breaks out of this loop after some iterations and issue the 
> well-known [1] "too much work for irq..." warning. And in case the 
> console output is on the very same serial port, the console output is 
> utterly corrupted.
>
> Patch below addresses the issue by delaying the TX-done IRQ more 
> realistically, ie. according to the currently set baudrate.
>
> Jan
>
> [1] http://lkml.org/lkml/2008/1/12/135
>
> Signed-off-by: Jan Kiszka <address@hidden>

Sorry, my TB obviously "gained" some regression, now wrecking inline 
patches even in do-not-wrap mode. That used to work for ages. Dreck.

Clean patch attached.

Jan

---
 hw/serial.c |   39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

Index: b/hw/serial.c
===================================================================
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -25,6 +25,7 @@
 #include "qemu-char.h"
 #include "isa.h"
 #include "pc.h"
+#include "qemu-timer.h"
 
 //#define DEBUG_SERIAL
 
@@ -91,6 +92,8 @@ struct SerialState {
     int last_break_enable;
     target_phys_addr_t base;
     int it_shift;
+    QEMUTimer *tx_timer;
+    char tx_buf;
 };
 
 static void serial_receive_byte(SerialState *s, int ch);
@@ -111,6 +114,20 @@ static void serial_update_irq(SerialStat
     }
 }
 
+static void serial_tx_done(void *opaque)
+{
+    SerialState *s = opaque;
+
+    s->thr_ipending = 1;
+    s->lsr |= UART_LSR_THRE;
+    s->lsr |= UART_LSR_TEMT;
+    serial_update_irq(s);
+    if (s->mcr & UART_MCR_LOOP) {
+        /* in loopback mode, say that we just received a char */
+        serial_receive_byte(s, s->tx_buf);
+    }
+}
+
 static void serial_update_parameters(SerialState *s)
 {
     int speed, parity, data_bits, stop_bits;
@@ -146,7 +163,6 @@ static void serial_update_parameters(Ser
 static void serial_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     SerialState *s = opaque;
-    unsigned char ch;
 
     addr &= 7;
 #ifdef DEBUG_SERIAL
@@ -162,19 +178,12 @@ static void serial_ioport_write(void *op
             s->thr_ipending = 0;
             s->lsr &= ~UART_LSR_THRE;
             serial_update_irq(s);
-            ch = val;
+            s->tx_buf = val;
             if (!(s->mcr & UART_MCR_LOOP)) {
                 /* when not in loopback mode, send the char */
-                qemu_chr_write(s->chr, &ch, 1);
-            }
-            s->thr_ipending = 1;
-            s->lsr |= UART_LSR_THRE;
-            s->lsr |= UART_LSR_TEMT;
-            serial_update_irq(s);
-            if (s->mcr & UART_MCR_LOOP) {
-                /* in loopback mode, say that we just received a char */
-                serial_receive_byte(s, ch);
+                qemu_chr_write(s->chr, &s->tx_buf, 1);
             }
+            qemu_mod_timer(s->tx_timer, 1000 / (11520 / s->divider));
         }
         break;
     case 1:
@@ -387,6 +396,10 @@ SerialState *serial_init(int base, qemu_
         return NULL;
     s->irq = irq;
 
+    s->tx_timer = qemu_new_timer(vm_clock, serial_tx_done, s);
+    if (!s->tx_timer)
+        return NULL;
+
     qemu_register_reset(serial_reset, s);
     serial_reset(s);
 
@@ -486,6 +499,10 @@ SerialState *serial_mm_init (target_phys
     s->base = base;
     s->it_shift = it_shift;
 
+    s->tx_timer = qemu_new_timer(vm_clock, serial_tx_done, s);
+    if (!s->tx_timer)
+        return NULL;
+
     qemu_register_reset(serial_reset, s);
     serial_reset(s);