[Qemu-devel] Re: 2.6.24 says "serial8250: too much work for irq4" a lot.

[Rob Landley]

Here's a patch Peter Anvin wrote so the serial I/O doesn't flood the kernel.

Here's the thread on linux-kernel aboout it:
http://lkml.org/lkml/2008/2/5/401

Rob

On Thursday 07 February 2008 15:19:39 you wrote:
> Rob Landley wrote:
> > Specifically, qemu isn't paravirtualized, it's fully virtualized.  The
> > same kernel can run on real hardware just fine.  (Sort of the point of
> > the project...)
> >
> > I can yank the warning for the kernels I build (or set PASS_LIMIT to
> > 9999999), but I'd rather not carry any more patches than I can avoid...
>
> Patch attached, completely untested beyond compilation.
>
> In particular:
>
>       - we should probably clear the burst counter when the interrupt
>         line goes from inactive to active.
>       - there probably should be a timer which clears the burst
>         counter.
>
> However, I think I've covered most of the bases...
>
>       -hpa

diff --git a/hw/serial.c b/hw/serial.c
index b1bd0ff..c902792 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -73,6 +73,15 @@
 #define UART_LSR_OE    0x02    /* Overrun error indicator */
 #define UART_LSR_DR    0x01    /* Receiver data ready */
 
+/*
+ * It's common for an IRQ handler to keep reading the RBR until
+ * the LSR indicates that the FIFO is empty, expecting that the
+ * CPU is vastly faster than the serial line.  This can cause
+ * overruns or error indications if the FIFO never empties, so
+ * give the target OS a breather every so often.
+ */
+#define MAX_BURST      512
+
 struct SerialState {
     uint16_t divider;
     uint8_t rbr; /* receive register */
@@ -91,8 +100,14 @@ struct SerialState {
     int last_break_enable;
     target_phys_addr_t base;
     int it_shift;
+    int burst_len;
 };
 
+static void serial_clear_burst(SerialState *s)
+{
+    s->burst_len = 0;
+}
+
 static void serial_update_irq(SerialState *s)
 {
     if ((s->lsr & UART_LSR_DR) && (s->ier & UART_IER_RDI)) {
@@ -114,6 +129,8 @@ static void serial_update_parameters(SerialState *s)
     int speed, parity, data_bits, stop_bits;
     QEMUSerialSetParams ssp;
 
+    serial_clear_burst(s);
+
     if (s->lcr & 0x08) {
         if (s->lcr & 0x10)
             parity = 'E';
@@ -221,9 +238,12 @@ static uint32_t serial_ioport_read(void *opaque, uint32_t 
addr)
             ret = s->divider & 0xff;
         } else {
             ret = s->rbr;
-            s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
-            serial_update_irq(s);
-            qemu_chr_accept_input(s->chr);
+           if (s->burst_len < MAX_BURST) {
+               s->burst_len++;
+               s->lsr &= ~(UART_LSR_DR | UART_LSR_BI);
+               serial_update_irq(s);
+               qemu_chr_accept_input(s->chr);
+           }
         }
         break;
     case 1:
@@ -235,6 +255,7 @@ static uint32_t serial_ioport_read(void *opaque, uint32_t 
addr)
         break;
     case 2:
         ret = s->iir;
+       serial_clear_burst(s);
         /* reset THR pending bit */
         if ((ret & 0x7) == UART_IIR_THRI)
             s->thr_ipending = 0;
@@ -248,6 +269,10 @@ static uint32_t serial_ioport_read(void *opaque, uint32_t 
addr)
         break;
     case 5:
         ret = s->lsr;
+       if (s->burst_len >= MAX_BURST)
+           ret &= ~(UART_LSR_DR|UART_LSR_BI);
+       if (!(ret & UART_LSR_DR))
+           serial_clear_burst(s);
         break;
     case 6:
         if (s->mcr & UART_MCR_LOOP) {


-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.