From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mqw1v-0004wX-Rd for qemu-devel@nongnu.org; Thu, 24 Sep 2009 17:39:11 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mqw1u-0004vK-VA for qemu-devel@nongnu.org; Thu, 24 Sep 2009 17:39:11 -0400 Received: from [199.232.76.173] (port=57987 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mqw1u-0004v5-Hu for qemu-devel@nongnu.org; Thu, 24 Sep 2009 17:39:10 -0400 Received: from gelbbaer.kn-bremen.de ([78.46.108.116]:46698 helo=smtp.kn-bremen.de) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mqw1t-0000HS-Rv for qemu-devel@nongnu.org; Thu, 24 Sep 2009 17:39:10 -0400 From: Juergen Lock Date: Thu, 24 Sep 2009 23:26:59 +0200 Subject: Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) Message-ID: <20090924212659.GA3248@triton8.kn-bremen.de> References: <20090911213508.GA97446@triton8.kn-bremen.de> <4AAB938B.9080004@web.de> <20090912165222.GA38048@triton8.kn-bremen.de> <200909231847.n8NIlNij097002@triton8.kn-bremen.de> <20090924162000.GG770@volta.aurel32.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090924162000.GG770@volta.aurel32.net> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: Olivier =?iso-8859-1?Q?Cochard-Labb=E9?= , freebsd-current@FreeBSD.org, Jan Kiszka , Juergen Lock , qemu-devel@nongnu.org On Thu, Sep 24, 2009 at 06:20:00PM +0200, Aurelien Jarno wrote: > 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 > Oh ok, then I completely misunderstood. (And I had no idea this makes a difference... :) Anyway, I'll try to remember. > > 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. > Yeah, looks like it... Thanx, Juergen