From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MnzlJ-0005iM-04 for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:01:53 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MnzlH-0005hP-DS for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:01:51 -0400 Received: from [199.232.76.173] (port=54980 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MnzlH-0005hM-7K for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:01:51 -0400 Received: from hall.aurel32.net ([88.191.82.174]:47119) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MnzlE-0004Fc-I8 for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:01:49 -0400 Date: Wed, 16 Sep 2009 21:01:42 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) Message-ID: <20090916190142.GC770@volta.aurel32.net> References: <20090911213508.GA97446@triton8.kn-bremen.de> <4AAB938B.9080004@web.de> <20090912165222.GA38048@triton8.kn-bremen.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20090912165222.GA38048@triton8.kn-bremen.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juergen Lock Cc: Olivier =?iso-8859-15?Q?Cochard-Labb=E9?= , freebsd-current@FreeBSD.org, Jan Kiszka , qemu-devel@nongnu.org On Sat, Sep 12, 2009 at 06:52:22PM +0200, Juergen Lock wrote: > On Sat, Sep 12, 2009 at 02:26:51PM +0200, Jan Kiszka wrote: > > Juergen Lock wrote: > > > Hi! > > > > > > I got a report of FreeBSD guest's new uart(4) driver misbehaving in > > > qemu again(?) (output stopping for no apparent reason), and now found > > > out the problem is tx irqs (UART_IIR_THRI) are getting lost because > > > serial_update_irq() checks for the rx condtion, > > > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > > > first before checking for the tx irq condition, > > > ... if ((s->ier & UART_IER_THRI) && s->thr_ipending) > > > which at least in this case (FreeBSD 8 guest after doing > > > set console="comconsole" > > > at the loader prompt or when simply echo'ing text to /dev/ttyu0 > > > or typing to the serial port from cu(1) on a `regular' vga console) > > > causes the second condition (.. && s->thr_ipending) to be never > > > reached anymore, or only after a very long delay. Moving that > > > condition up so it is checked first like this, > > > > > > Index: qemu/hw/serial.c > > > @@ -189,7 +188,9 @@ static void serial_update_irq(SerialStat > > > { > > > uint8_t tmp_iir = UART_IIR_NO_INT; > > > > > > - if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > > + if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > > + tmp_iir = UART_IIR_THRI; > > > + } else if ((s->ier & UART_IER_RLSI) && (s->lsr & UART_LSR_INT_ANY)) { > > > tmp_iir = UART_IIR_RLSI; > > > } else if ((s->ier & UART_IER_RDI) && s->timeout_ipending) { > > > /* Note that(s->ier & UART_IER_RDI) can mask this interrupt, > > > @@ -202,8 +203,6 @@ static void serial_update_irq(SerialStat > > > } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > > > tmp_iir = UART_IIR_RDI; > > > } > > > - } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > > > - tmp_iir = UART_IIR_THRI; > > > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > > tmp_iir = UART_IIR_MSI; > > > } > > > > > > ...fixes the issue for me, but I'm not 100% sure if this might cause > > > rx irqs to come (too?) late when a guest keeps sending while its > > > receiving at the same time. Anyone care to comment? :) > > > > The reordering violates the 16550A spec in that RX event overrules TX in > > the IRQ status register. Maybe something else is wrong but it's not the > > ordering in serial_update_irq. > > Well one problem seems to be the rx condition, > ... if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) > is not enough to trigger an irq, yet still causes the following > conditions not to be checked anymore at all. And ideed, fixing that > seems to get my FreeBSD 8 guest back to working order as well: Applied. In the future, could you please make sure to send patches with a correct unified headers? > Index: qemu/hw/serial.c > @@ -196,12 +195,10 @@ static void serial_update_irq(SerialStat > * this is not in the specification but is observed on existing > * hardware. */ > tmp_iir = UART_IIR_CTI; > - } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR)) { > - if (!(s->fcr & UART_FCR_FE)) { > - tmp_iir = UART_IIR_RDI; > - } else if (s->recv_fifo.count >= s->recv_fifo.itl) { > - tmp_iir = UART_IIR_RDI; > - } > + } else if ((s->ier & UART_IER_RDI) && (s->lsr & UART_LSR_DR) && > + (!(s->fcr & UART_FCR_FE) || > + s->recv_fifo.count >= s->recv_fifo.itl)) { > + tmp_iir = UART_IIR_RDI; > } else if ((s->ier & UART_IER_THRI) && s->thr_ipending) { > tmp_iir = UART_IIR_THRI; > } else if ((s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA)) { > > Signed-off-by: Juergen Lock > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net