From owner-freebsd-current@FreeBSD.ORG Thu Sep 24 16:20:05 2009 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CE6D9106568F for ; Thu, 24 Sep 2009 16:20:05 +0000 (UTC) (envelope-from aurelien@aurel32.net) Received: from hall.aurel32.net (hall.aurel32.net [IPv6:2002:58bf:52ae::1]) by mx1.freebsd.org (Postfix) with ESMTP id 419158FC0A for ; Thu, 24 Sep 2009 16:20:05 +0000 (UTC) Received: from [2002:52e8:2fb:1:21e:8cff:feb0:693b] (helo=volta.aurel32.net) by hall.aurel32.net with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1Mqr33-0003Tq-V3; Thu, 24 Sep 2009 18:20:02 +0200 Received: from aurel32 by volta.aurel32.net with local (Exim 4.69) (envelope-from ) id 1Mqr32-0001b6-Qs; Thu, 24 Sep 2009 18:20:00 +0200 Date: Thu, 24 Sep 2009 18:20:00 +0200 From: Aurelien Jarno To: Juergen Lock 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> X-Mailer: Mutt 1.5.18 (2008-05-17) User-Agent: Mutt/1.5.18 (2008-05-17) X-Mailman-Approved-At: Thu, 24 Sep 2009 16:24:35 +0000 Cc: Olivier =?iso-8859-15?Q?Cochard-Labb=E9?= , freebsd-current@FreeBSD.org, Jan Kiszka , qemu-devel@nongnu.org Subject: Re: [Qemu-devel] Re: qemu serial: lost tx irqs (affectig FreeBSD's new uart(4) driver) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Sep 2009 16:20:05 -0000 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