From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mqr3D-0008GO-R4 for qemu-devel@nongnu.org; Thu, 24 Sep 2009 12:20:11 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mqr3C-0008F8-Sh for qemu-devel@nongnu.org; Thu, 24 Sep 2009 12:20:11 -0400 Received: from [199.232.76.173] (port=50726 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mqr3C-0008Ew-Oj for qemu-devel@nongnu.org; Thu, 24 Sep 2009 12:20:10 -0400 Received: from hall.aurel32.net ([88.191.82.174]:46589) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Mqr3C-00006K-8k for qemu-devel@nongnu.org; Thu, 24 Sep 2009 12:20:10 -0400 Date: Thu, 24 Sep 2009 18:20:00 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) Message-ID: <20090924162000.GG770@volta.aurel32.net> References: <20090911213508.GA97446@triton8.kn-bremen.de> <4AAB938B.9080004@web.de> <20090912165222.GA38048@triton8.kn-bremen.de> <200909231847.n8NIlNij097002@triton8.kn-bremen.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <200909231847.n8NIlNij097002@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 Wed, Sep 23, 2009 at 08:47:23PM +0200, Juergen Lock wrote: > In article <20090916190142.GC770@volta.aurel32.net> you write: > >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? > > Alright, if thats is what you guys prefer... (I just didn't want to > break the thread.) Don't need to break the thread, just use | --- a/hw/serial.c | +++ a/hw/serial.c instead of simply | Index: qemu/hw/serial.c ie the output of diff -u > Anyway, I guess this is also material for the stable branch(es)? > (I just saw 0.11.0 has already been tagged but not announced yet, and > another patch merged to the same branch after that, maybe the tag can > still be slided if this is possible with git?) > Pushed to the branch. For the details about the release, I let Anthony handling that. Worst case scenario, it will be in 0.11.1. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net