[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH] hw/char/escc: Lower irq when transmit buffer

From: Paolo Bonzini
Subject: Re: [Qemu-trivial] [PATCH] hw/char/escc: Lower irq when transmit buffer is filled
Date: Wed, 6 Mar 2019 12:01:50 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

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 <address@hidden>

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



> ---
>  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)) {

reply via email to

[Prev in Thread] Current Thread [Next in Thread]