From mboxrd@z Thu Jan 1 00:00:00 1970 From: Trolle Selander Subject: Re: [PATCH] iommu-remote: Fix lost serial TX interrupts. Report receive overruns. Date: Wed, 3 Mar 2010 13:52:37 -0500 Message-ID: <515922b51003031052v422b98eauf82c4552b95d8dd7@mail.gmail.com> References: <4B621D41.5030805@scsiguy.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <4B621D41.5030805@scsiguy.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: gibbs@scsiguy.com Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On Thu, Jan 28, 2010 at 6:26 PM, Justin T. Gibbs wrote: > This patch corrects emulation errors in QEMU's 16550 uart emulation, > which cause compatibility issues with FreeBSD's uart(9) driver. > > =A0o Implement receive overrun status. =A0The FreeBSD uart(9) driver > =A0 =A0relies on this status in it's probe routine to determine the size > =A0 =A0of the FIFO supported. > =A0o As per the 16550 spec, do not overwrite the RX FIFO on an RX overrun= . > =A0o Do not allow TX or RX FIFO overruns to increment the data valid coun= t > =A0 =A0beyond the size of the FIFO. > =A0o For reads of the IIR register, only clear the "TX holding register > =A0 =A0empty" (THRE) interrupt if the read reports this interrupt. =A0Thi= s > =A0 =A0is required by the specification and avoids losing TX interrupts > =A0 =A0when other, higher priority interrupts (usually RX) are reported f= irst. > > This patch also includes a fix for a second cause of lost TX interrupts, > which was submitted by Jergen Lock, and is already in the latest QEMU. > > =A0o If a receive interrupt is suppressed due to the FIFO not yet filling > =A0 =A0to its interrupt threshold, do not also supress any pending THRE > =A0 =A0interrupt. > > A version of this patch, against the latest QEMU, has also been submitted > to the qemu-devel mailing list. > > Signed-off-by: Justin T. Gibbs > > Index: xen-4.0.0-testing/tools/ioemu-remote/hw/serial.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- xen-4.0.0-testing.orig/tools/ioemu-remote/hw/serial.c > +++ xen-4.0.0-testing/tools/ioemu-remote/hw/serial.c > @@ -159,11 +159,19 @@ > =A0{ > =A0 =A0 SerialFIFO *f =3D (fifo) ? &s->recv_fifo : &s->xmit_fifo; > > - =A0 =A0f->data[f->head++] =3D chr; > + =A0 =A0/* Receive overruns do not overwrite FIFO contents. */ > + =A0 =A0if (fifo =3D=3D XMIT_FIFO || f->count < UART_FIFO_LENGTH) { > > - =A0 =A0if (f->head =3D=3D UART_FIFO_LENGTH) > - =A0 =A0 =A0 =A0f->head =3D 0; > - =A0 =A0f->count++; > + =A0 =A0 =A0 =A0f->data[f->head++] =3D chr; > + > + =A0 =A0 =A0 =A0if (f->head =3D=3D UART_FIFO_LENGTH) > + =A0 =A0 =A0 =A0 =A0 =A0f->head =3D 0; > + =A0 =A0} > + > + =A0 =A0if (f->count < UART_FIFO_LENGTH) > + =A0 =A0 =A0 =A0f->count++; > + =A0 =A0else if (fifo =3D=3D RECV_FIFO) > + =A0 =A0 =A0 =A0s->lsr |=3D UART_LSR_OE; > > =A0 =A0 return 1; > =A0} > @@ -195,12 +203,10 @@ > =A0 =A0 =A0 =A0 =A0* this is not in the specification but is observed on = existing > =A0 =A0 =A0 =A0 =A0* hardware. =A0*/ > =A0 =A0 =A0 =A0 tmp_iir =3D UART_IIR_CTI; > - =A0 =A0} else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) { > - =A0 =A0 =A0 =A0if (!(s->fcr & UART_FCR_FE)) { > - =A0 =A0 =A0 =A0 =A0 tmp_iir =3D UART_IIR_RDI; > - =A0 =A0 =A0 =A0} else if (s->recv_fifo.count >=3D s->recv_fifo.itl) { > - =A0 =A0 =A0 =A0 =A0 tmp_iir =3D UART_IIR_RDI; > - =A0 =A0 =A0 =A0} > + =A0 =A0} else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) && > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (!(s->fcr & UART_FCR_FE) || > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0s->recv_fifo.count >=3D s->recv_fifo.itl= )) { > + =A0 =A0 =A0 =A0tmp_iir =3D UART_IIR_RDI; > =A0 =A0 } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > =A0 =A0 =A0 =A0 tmp_iir =3D UART_IIR_THRI; > =A0 =A0 } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELT= A)) { > @@ -523,8 +529,10 @@ > =A0 =A0 =A0 =A0 break; > =A0 =A0 case 2: > =A0 =A0 =A0 =A0 ret =3D s->iir; > - =A0 =A0 =A0 =A0s->thr_ipending =3D 0; > - =A0 =A0 =A0 =A0serial_update_irq(s); > + =A0 =A0 =A0 =A0if (ret & UART_IIR_THRI) { > + =A0 =A0 =A0 =A0 =A0 =A0s->thr_ipending =3D 0; > + =A0 =A0 =A0 =A0 =A0 =A0serial_update_irq(s); > + =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 break; > =A0 =A0 case 3: > =A0 =A0 =A0 =A0 ret =3D s->lcr; > @@ -534,9 +542,9 @@ > =A0 =A0 =A0 =A0 break; > =A0 =A0 case 5: > =A0 =A0 =A0 =A0 ret =3D s->lsr; > - =A0 =A0 =A0 =A0/* Clear break interrupt */ > - =A0 =A0 =A0 =A0if (s->lsr & UART_LSR_BI) { > - =A0 =A0 =A0 =A0 =A0 =A0s->lsr &=3D ~UART_LSR_BI; > + =A0 =A0 =A0 =A0/* Clear break and overrun interrupts */ > + =A0 =A0 =A0 =A0if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) { > + =A0 =A0 =A0 =A0 =A0 =A0s->lsr &=3D ~(UART_LSR_BI|UART_LSR_OE); > =A0 =A0 =A0 =A0 =A0 =A0 serial_update_irq(s); > =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 break; > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > I'm assuming the overrun bugs were only triggered when the uart was put in loopback mode? Receive overruns & receive FIFO overwrite should never be able to occur during normal operation,since unless there is "room" in either the FIFO or DR is set in the lsr, serial_can_receive will return 0, and no further data will be "fed" to the emulated device. Looking through the code, I noticed that serial_xmit calls serial_receive1 directly when in loopback, however. Good catch on the xmit_fifo overwrite check. Again, due to the way the uart emulation works, this was not a situation that should ever be able to occur, since serial_xmit will get called immediately after every port write, and if the reader can't "keep up", after 4 failed writes, any further writes from emulated uart -> backing device will be immediately discarded until one goes through. Thus xmit FIFO fill should in fact never be able to go above 4 in practice. The exception would be if the backing device is a real serial port, and it was set to a lower baudrate than the virtual device - but again, this should not occur since qemu changes the baudrate of the backing port to whatever the virtual device is set to. If, however, you were seeing receive data being lost in "real" operation with serial-backed-serial, then there might be something else going on. I'm mentioning this because have seen issues with that in the past, and it was in fact the reason that I wrote the 16450 -> 16550 upgrade patch in the first place, since more agressive reading from the physical serial port helped the issue to some degree. -- Trolle