LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Rob Landley <rob@landley.net>
Cc: Ingo Molnar <mingo@elte.hu>, linux-kernel@vger.kernel.org
Subject: Re: 2.6.24 says "serial8250: too much work for irq4" a lot.
Date: Thu, 07 Feb 2008 13:19:39 -0800	[thread overview]
Message-ID: <47AB75EB.3040405@zytor.com> (raw)
In-Reply-To: <200802071413.45085.rob@landley.net>

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

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

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 2376 bytes --]

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) {

  parent reply	other threads:[~2008-02-07 21:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-05 20:55 Rob Landley
2008-02-05 21:07 ` H. Peter Anvin
2008-02-07 12:39   ` Ingo Molnar
2008-02-07 17:37     ` H. Peter Anvin
2008-02-07 20:13       ` Rob Landley
2008-02-07 20:53         ` H. Peter Anvin
2008-02-07 21:19         ` H. Peter Anvin [this message]
2008-02-08  7:04           ` Rob Landley

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=47AB75EB.3040405@zytor.com \
    --to=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rob@landley.net \
    --subject='Re: 2.6.24 says "serial8250: too much work for irq4" a lot.' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).