From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:34088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h1UJE-0002IZ-1Y for qemu-devel@nongnu.org; Wed, 06 Mar 2019 06:02:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h1UJD-0005Gb-2v for qemu-devel@nongnu.org; Wed, 06 Mar 2019 06:01:59 -0500 References: <20190305051007.56009-1-stephen.checkoway@oberlin.edu> From: Paolo Bonzini Message-ID: <52a82d3a-3b79-c61d-93fa-f087af45f6fb@redhat.com> Date: Wed, 6 Mar 2019 12:01:50 +0100 MIME-Version: 1.0 In-Reply-To: <20190305051007.56009-1-stephen.checkoway@oberlin.edu> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/char/escc: Lower irq when transmit buffer is filled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stephen Checkoway , qemu-devel@nongnu.org Cc: qemu-trivial@nongnu.org, =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , Mark Cave-Ayland On 05/03/19 06:10, Stephen Checkoway wrote: > The SCC/ESCC will briefly stop asserting an interrupt when the > transmit FIFO is filled. > > This code doesn't model the transmit FIFO/shift register so the > pending transmit interrupt is never deasserted which means that an > edge-triggered interrupt controller will never see the low-to-high > transition it needs to raise another interrupt. The practical > consequence of this is that guest firmware with an interrupt service > routine for the ESCC that does not send all of the data it has > immediately will stop sending data if the following sequence of > events occurs: > 1. Disable processor interrupts > 2. Write a character to the ESCC > 3. Add additional characters to a buffer which is drained by the ISR > 4. Enable processor interrupts > > In this case, the first character will be sent, the interrupt will > fire and the ISR will output the second character. Since the pending > transmit interrupt remains asserted, no additional interrupts will > ever fire. > > This fixes that situation by explicitly lowering the IRQ when a > character is written to the buffer. > > Signed-off-by: Stephen Checkoway Looks good but I would like Mark to give his ack as well. Mark, could you also add hw/char/escc.c to both SPARC and Mac sections of MAINTAINERS? Thanks, Paolo > --- > hw/char/escc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/char/escc.c b/hw/char/escc.c > index 628f5f81f7..bea55ad8da 100644 > --- a/hw/char/escc.c > +++ b/hw/char/escc.c > @@ -509,6 +509,7 @@ static void escc_mem_write(void *opaque, hwaddr addr, > break; > case SERIAL_DATA: > trace_escc_mem_writeb_data(CHN_C(s), val); > + qemu_irq_lower(s->irq); > s->tx = val; > if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { // tx enabled > if (qemu_chr_fe_backend_connected(&s->chr)) { >