Fix lost serial TX interrupts. Report receive overruns.

Submitted by Justin T. Gibbs on Jan. 28, 2010, 11:13 p.m.

Details

Message ID 4B621A1B.6090309@FreeBSD.org
State New
Headers show

Commit Message

Justin T. Gibbs Jan. 28, 2010, 11:13 p.m.
This patch compliments the patch submitted by Jergen Lock and further
improves the performance of QEMU's serial emulation with FreeBSD's
uart(9) driver.

  o Implement receive overrun status.  The FreeBSD uart driver
    relies on this status in it's probe routine to determine the size
    of the FIFO supported.
  o As per the 16550 spec, do not overwrite the RX FIFO on an RX overrun.
  o Do not allow TX or RX FIFO overruns to increment the data valid count
    beyond the size of the FIFO.
  o For reads of the IIR register, only clear the "TX holding register
    emtpy interrupt" if the read reports this interrupt.  This is required
    by the specification and avoids losing TX interrupts when other, higher
    priority interrupts (usually RX) are reported first.

Signed-off-by: Justin T. Gibbs <gibbs@FreeBSD.org>

      }

Comments

Anthony Liguori Feb. 10, 2010, 5:58 p.m.
On 01/28/2010 05:13 PM, Justin T. Gibbs wrote:
> This patch compliments the patch submitted by Jergen Lock and further
> improves the performance of QEMU's serial emulation with FreeBSD's
> uart(9) driver.
>
>  o Implement receive overrun status.  The FreeBSD uart driver
>    relies on this status in it's probe routine to determine the size
>    of the FIFO supported.
>  o As per the 16550 spec, do not overwrite the RX FIFO on an RX overrun.
>  o Do not allow TX or RX FIFO overruns to increment the data valid count
>    beyond the size of the FIFO.
>  o For reads of the IIR register, only clear the "TX holding register
>    emtpy interrupt" if the read reports this interrupt.  This is required
>    by the specification and avoids losing TX interrupts when other, 
> higher
>    priority interrupts (usually RX) are reported first.
>
> Signed-off-by: Justin T. Gibbs <gibbs@FreeBSD.org>

This patch is malformed.  Please export the patch from a git tree via 
git-format-patch.

Regards,

Anthony Liguori

>
> --- serial.c.orig    Thu Jan 14 15:18:00 2010
> +++ serial.c    Thu Jan 28 15:36:04 2010
> @@ -169,11 +169,19 @@
>  {
>      SerialFIFO *f = (fifo) ? &s->recv_fifo : &s->xmit_fifo;
>
> -    f->data[f->head++] = chr;
> +    /* Receive overruns do not overwrite FIFO contents. */
> +    if (fifo == XMIT_FIFO || f->count < UART_FIFO_LENGTH) {
>
> -    if (f->head == UART_FIFO_LENGTH)
> -        f->head = 0;
> -    f->count++;
> +        f->data[f->head++] = chr;
> +
> +        if (f->head == UART_FIFO_LENGTH)
> +            f->head = 0;
> +    }
> +
> +    if (f->count < UART_FIFO_LENGTH)
> +        f->count++;
> +    else if (fifo == RECV_FIFO)
> +        s->lsr |= UART_LSR_OE;
>
>      return 1;
>  }
> @@ -533,8 +541,10 @@
>          break;
>      case 2:
>          ret = s->iir;
> +        if (ret & UART_IIR_THRI) {
>              s->thr_ipending = 0;
> -        serial_update_irq(s);
> +            serial_update_irq(s);
> +        }
>          break;
>      case 3:
>          ret = s->lcr;
> @@ -544,9 +554,9 @@
>          break;
>      case 5:
>          ret = s->lsr;
> -        /* Clear break interrupt */
> -        if (s->lsr & UART_LSR_BI) {
> -            s->lsr &= ~UART_LSR_BI;
> +        /* Clear break and overrun interrupts */
> +        if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) {
> +            s->lsr &= ~(UART_LSR_BI|UART_LSR_OE);
>              serial_update_irq(s);
>          }
>          break;
> @@ -629,6 +639,8 @@
>          /* call the timeout receive callback in 4 char transmit time */
>          qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock 
> (vm_clock) + s->char_transmit_time * 4);
>      } else {
> +        if (s->lsr & UART_LSR_DR)
> +            s->lsr |= UART_LSR_OE;
>          s->rbr = buf[0];
>          s->lsr |= UART_LSR_DR;
>      }
>
>
>
>

Patch hide | download patch | download mbox

--- serial.c.orig    Thu Jan 14 15:18:00 2010
+++ serial.c    Thu Jan 28 15:36:04 2010
@@ -169,11 +169,19 @@ 
  {
      SerialFIFO *f = (fifo) ? &s->recv_fifo : &s->xmit_fifo;

-    f->data[f->head++] = chr;
+    /* Receive overruns do not overwrite FIFO contents. */
+    if (fifo == XMIT_FIFO || f->count < UART_FIFO_LENGTH) {

-    if (f->head == UART_FIFO_LENGTH)
-        f->head = 0;
-    f->count++;
+        f->data[f->head++] = chr;
+
+        if (f->head == UART_FIFO_LENGTH)
+            f->head = 0;
+    }
+
+    if (f->count < UART_FIFO_LENGTH)
+        f->count++;
+    else if (fifo == RECV_FIFO)
+        s->lsr |= UART_LSR_OE;

      return 1;
  }
@@ -533,8 +541,10 @@ 
          break;
      case 2:
          ret = s->iir;
+        if (ret & UART_IIR_THRI) {
              s->thr_ipending = 0;
-        serial_update_irq(s);
+            serial_update_irq(s);
+        }
          break;
      case 3:
          ret = s->lcr;
@@ -544,9 +554,9 @@ 
          break;
      case 5:
          ret = s->lsr;
-        /* Clear break interrupt */
-        if (s->lsr & UART_LSR_BI) {
-            s->lsr &= ~UART_LSR_BI;
+        /* Clear break and overrun interrupts */
+        if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) {
+            s->lsr &= ~(UART_LSR_BI|UART_LSR_OE);
              serial_update_irq(s);
          }
          break;
@@ -629,6 +639,8 @@ 
          /* call the timeout receive callback in 4 char transmit time */
          qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock 
(vm_clock) + s->char_transmit_time * 4);
      } else {
+        if (s->lsr & UART_LSR_DR)
+            s->lsr |= UART_LSR_OE;
          s->rbr = buf[0];
          s->lsr |= UART_LSR_DR;